Add new tag conversion funcitons that preserve order in lists of Tags (#572)

Description of changes:
When converting between ACK tags (map[string]string) and 
resource tags(which could be either a list of Tag struct or map[string]*string)
can be tricky. One issue that we found was order preservation, especially when 
converting from `map[string]string` to list of `Tag`.  The changed order, when 
patched to the resource in the cluster, would trigger a reconciliation, for an
unpredictable amount of times (at least until we luckily convert to the list with
a preserved order.)

This change introduces two new functions, similar to the existing ones.

The `toACKTagsWithKeyOrder` has the same function as `ToACKTags`,
but besides returning the ACK tags, it also returns a list of keys in the same
order it is in the resource.

The `fromACKTagsWithKeyOrder`, also similar to `FromACKTags`, accepts
the keyOrder returned by `toACKTagsWithKeyOrder`, and rebuilds the resource
tags, if it's a list, in the same order as it was before. Any extra tags will be added
in random order.

These two new functions will be called before and after `mirrowAWSTags` and
`FilterSystemTags`. The extra tags mentioned earlier are the AWS tags that are 
mirrored from the latest resource (more explanation on what they are [here](aws-controllers-k8s/runtime#170)) and will be filtered before getting patched with [this](aws-controllers-k8s/runtime#172)
change.
This will ensure that the tags in desired will maintain their order and avoid triggering
unnecessary reconciliations.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: michaelhtm

templates/pkg/resource/manager.go.tpl

@@ -341,10 +341,10 @@ func (rm *resourceManager) FilterSystemTags(res acktypes.AWSResource) {
     }
 {{ end -}}
     existingTags = r.ko.Spec.{{ $tagField.Path }}
-	resourceTags := ToACKTags(existingTags)
+	resourceTags, tagKeyOrder := toACKTagsWithKeyOrder(existingTags)
 	ignoreSystemTags(resourceTags)
 {{ GoCodeInitializeNestedStructField .CRD "r.ko" $tagField "svcapitypes" 1 -}}
-	r.ko.Spec.{{ $tagField.Path }} = FromACKTags(resourceTags)
+	r.ko.Spec.{{ $tagField.Path }} = fromACKTagsWithKeyOrder(resourceTags, tagKeyOrder)
 {{- end }}
 {{- end }}
 }
@@ -393,11 +393,11 @@ func mirrorAWSTags(a *resource, b *resource) {
 	existingDesiredTags = a.ko.Spec.{{ $tagField.Path }}
 {{ end -}}
     existingLatestTags = b.ko.Spec.{{ $tagField.Path }}
-	desiredTags := ToACKTags(existingDesiredTags)
-	latestTags := ToACKTags(existingLatestTags)
+	desiredTags, desiredTagKeyOrder := toACKTagsWithKeyOrder(existingDesiredTags)
+	latestTags, _ := toACKTagsWithKeyOrder(existingLatestTags)
 	syncAWSTags(desiredTags, latestTags)
 {{ GoCodeInitializeNestedStructField .CRD "a.ko" $tagField "svcapitypes" 1 -}}
-	a.ko.Spec.{{ $tagField.Path }} = FromACKTags(desiredTags)
+	a.ko.Spec.{{ $tagField.Path }} = fromACKTagsWithKeyOrder(desiredTags, desiredTagKeyOrder)
 {{- end }}
 {{- end }}
 }

templates/pkg/resource/tags.go.tpl

@@ -62,6 +62,41 @@ func ToACKTags(tags {{ $tagFieldGoType }}) acktags.Tags {
     return result
 }
 
+// toACKTagsWithKeyOrder converts the tags parameter into 'acktags.Tags' shape.
+// This method helps in creating the hub(acktags.Tags) for merging
+// default controller tags with existing resource tags. It also returns a slice
+// of keys maintaining the original key Order when the tags are a list
+func toACKTagsWithKeyOrder(tags {{ $tagFieldGoType }}) (acktags.Tags, []string) {
+    result := acktags.NewTags()
+    keyOrder := []string{}
+{{- if $hookCode := Hook .CRD "pre_convert_to_ack_tags" }}
+{{ $hookCode }}
+{{ end }}
+    if tags == nil || len(tags) == 0 {
+        return result, keyOrder
+    }
+{{ if eq "map" $tagFieldShapeType }}
+    for k, v := range tags {
+        if v == nil {
+            result[k] = ""
+        } else {
+            result[k] = *v
+        }
+    }
+{{ else if eq "list" $tagFieldShapeType }}
+    for _, t := range tags {
+        if t.{{ $keyMemberName}} != nil {
+            keyOrder = append(keyOrder, *t.{{ $keyMemberName}})
+        }
+    }
+    result = ToACKTags(tags)
+{{ end }}
+{{- if $hookCode := Hook .CRD "post_convert_to_ack_tags" }}
+{{ $hookCode }}
+{{ end }}
+    return result, keyOrder
+}
+
 // FromACKTags converts the tags parameter into {{ $tagFieldGoType }} shape.
 // This method helps in setting the tags back inside AWSResource after merging
 // default controller tags with existing resource tags.
@@ -86,6 +121,43 @@ func FromACKTags(tags acktags.Tags) {{ $tagFieldGoType }} {
 {{ end }}
     return result
 }
+
+// fromACKTagsWithTagKeys converts the tags parameter into {{ $tagFieldGoType }} shape.
+// This method helps in setting the tags back inside AWSResource after merging
+// default controller tags with existing resource tags. When a list, 
+// it maintains the order from original 
+func fromACKTagsWithKeyOrder(tags acktags.Tags, keyOrder []string) {{ $tagFieldGoType }} {
+    result := {{ $tagFieldGoType }}{}
+{{- if $hookCode := Hook .CRD "pre_convert_from_ack_tags" }}
+{{ $hookCode }}
+{{ end }}
+
+{{- if eq "list" $tagFieldShapeType }}
+    for _, k := range keyOrder {
+		v, ok := tags[k]
+        if ok {
+            tag := svcapitypes.Tag{Key: &k, Value: &v}
+            result = append(result, &tag)
+            delete(tags, k)
+        }
+	}
+
+{{- else }}
+    _ = keyOrder
+{{- end }}
+{{- if eq "map" $tagFieldShapeType }}
+    for k, v := range tags {
+        vCopy := v
+        result[k] = &vCopy
+    }
+{{- else if eq "list" $tagFieldShapeType }}
+        result = append(result, FromACKTags(tags)...)
+{{- end }}
+{{- if $hookCode := Hook .CRD "post_convert_from_ack_tags" }}
+{{ $hookCode }}
+{{ end }}
+    return result
+}
 {{ end }}
 
 // ignoreSystemTags ignores tags that have keys that start with "aws:"