Merge pull request crossplane-contrib#1362 from crossplane-contrib/ba…

…ckport-1355-to-release-0.28 [Backport release-0.28] fixed infinite default tags adding in iam user
AlexMichaelJonesNC · Jun 17, 2022 · c08ed6a · c08ed6a
2 parents 9e1ce16 + f2e3f72
commit c08ed6a
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 13 deletions.
diff --git a/pkg/controller/iam/openidconnectprovider/controller_test.go b/pkg/controller/iam/openidconnectprovider/controller_test.go
@@ -774,6 +774,19 @@ func TestInitialize(t *testing.T) {
 				cr: oidcProvider(withTags(resource.GetExternalTags(oidcProvider(withGroupVersionKind())), map[string]string{"foo": "bar"}), withGroupVersionKind()),
 			},
 		},
+		"NoChanges": {
+			args: args{
+				cr: oidcProvider(
+					withTags(map[string]string{"foo": "bar"}),
+					withGroupVersionKind()),
+				kube: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
+			},
+			want: want{
+				cr: oidcProvider(
+					withTags(resource.GetExternalTags(oidcProvider(withGroupVersionKind())), map[string]string{"foo": "bar"}),
+					withGroupVersionKind()),
+			},
+		},
 		"UpdateFailed": {
 			args: args{
 				cr:   oidcProvider(),

diff --git a/pkg/controller/iam/policy/controller_test.go b/pkg/controller/iam/policy/controller_test.go
@@ -999,6 +999,19 @@ func TestInitialize(t *testing.T) {
 				cr: policy(withTags(resource.GetExternalTags(policy(withGroupVersionKind()))), withGroupVersionKind()),
 			},
 		},
+		"NoChanges": {
+			args: args{
+				cr: policy(
+					withTags(resource.GetExternalTags(policy(withGroupVersionKind())), map[string]string{"foo": "bar"}),
+					withGroupVersionKind()),
+				kube: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
+			},
+			want: want{
+				cr: policy(
+					withTags(resource.GetExternalTags(policy(withGroupVersionKind())), map[string]string{"foo": "bar"}),
+					withGroupVersionKind()),
+			},
+		},
 		"UpdateFailed": {
 			args: args{
 				cr:   policy(),

diff --git a/pkg/controller/iam/role/controller.go b/pkg/controller/iam/role/controller.go
@@ -245,17 +245,24 @@ func (t *tagger) Initialize(ctx context.Context, mgd resource.Managed) error {
 	if !ok {
 		return errors.New(errUnexpectedObject)
 	}
+
 	added := false
-	tagMap := map[string]string{}
-	for _, t := range cr.Spec.ForProvider.Tags {
-		tagMap[t.Key] = t.Value
-	}
-	for k, v := range resource.GetExternalTags(mgd) {
-		if p, ok := tagMap[k]; !ok || v != p {
-			cr.Spec.ForProvider.Tags = append(cr.Spec.ForProvider.Tags, v1beta1.Tag{Key: k, Value: v})
-			added = true
+	defaultTags := resource.GetExternalTags(mgd)
+
+	for i, t := range cr.Spec.ForProvider.Tags {
+		if v, ok := defaultTags[t.Key]; ok {
+			if v != t.Value {
+				cr.Spec.ForProvider.Tags[i].Value = v
+				added = true
+			}
+			delete(defaultTags, t.Key)
 		}
 	}
+
+	for k, v := range defaultTags {
+		cr.Spec.ForProvider.Tags = append(cr.Spec.ForProvider.Tags, v1beta1.Tag{Key: k, Value: v})
+		added = true
+	}
 	if !added {
 		return nil
 	}

diff --git a/pkg/controller/iam/role/controller_test.go b/pkg/controller/iam/role/controller_test.go
@@ -459,7 +459,7 @@ func TestDelete(t *testing.T) {
 
 func TestInitialize(t *testing.T) {
 	type args struct {
-		cr   *v1beta1.Role
+		cr   resource.Managed
 		kube client.Client
 	}
 	type want struct {
@@ -471,6 +471,15 @@ func TestInitialize(t *testing.T) {
 		args
 		want
 	}{
+		"Unexpected": {
+			args: args{
+				cr:   unexpectedItem,
+				kube: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
+			},
+			want: want{
+				err: errors.New(errUnexpectedObject),
+			},
+		},
 		"Successful": {
 			args: args{
 				cr:   role(withTags(map[string]string{"foo": "bar"})),
@@ -480,7 +489,7 @@ func TestInitialize(t *testing.T) {
 				cr: role(withTags(resource.GetExternalTags(role()), map[string]string{"foo": "bar"})),
 			},
 		},
-		"Check Tag values": {
+		"DefaultTags": {
 			args: args{
 				cr:   role(withTags(map[string]string{"foo": "bar"}), withGroupVersionKind()),
 				kube: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
@@ -489,6 +498,36 @@ func TestInitialize(t *testing.T) {
 				cr: role(withTags(resource.GetExternalTags(role(withGroupVersionKind())), map[string]string{"foo": "bar"}), withGroupVersionKind()),
 			},
 		},
+		"UpdateDefaultTags": {
+			args: args{
+				cr: role(
+					withRoleName(&roleName),
+					withGroupVersionKind(),
+					withTags(map[string]string{resource.ExternalResourceTagKeyKind: "bar"})),
+				kube: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
+			},
+			want: want{
+				cr: role(
+					withRoleName(&roleName),
+					withGroupVersionKind(),
+					withTags(resource.GetExternalTags(role(withRoleName(&roleName), withGroupVersionKind())))),
+			},
+		},
+		"NoChange": {
+			args: args{
+				cr: role(
+					withRoleName(&roleName),
+					withGroupVersionKind(),
+					withTags(resource.GetExternalTags(role(withRoleName(&roleName), withGroupVersionKind())))),
+				kube: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
+			},
+			want: want{
+				cr: role(
+					withRoleName(&roleName),
+					withGroupVersionKind(),
+					withTags(resource.GetExternalTags(role(withRoleName(&roleName), withGroupVersionKind())))),
+			},
+		},
 		"UpdateFailed": {
 			args: args{
 				cr:   role(),

diff --git a/pkg/controller/iam/user/controller.go b/pkg/controller/iam/user/controller.go
@@ -239,9 +239,11 @@ func (t *tagger) Initialize(ctx context.Context, mgd resource.Managed) error {
 	defaultTags := resource.GetExternalTags(mgd)
 
 	for i, t := range cr.Spec.ForProvider.Tags {
-		if v, ok := defaultTags[t.Key]; ok && v != t.Value {
-			cr.Spec.ForProvider.Tags[i].Value = v
-			added = true
+		if v, ok := defaultTags[t.Key]; ok {
+			if v != t.Value {
+				cr.Spec.ForProvider.Tags[i].Value = v
+				added = true
+			}
 			delete(defaultTags, t.Key)
 		}
 	}

diff --git a/pkg/controller/iam/user/controller_test.go b/pkg/controller/iam/user/controller_test.go
@@ -714,6 +714,17 @@ func TestTagger_Initialize(t *testing.T) {
 					withTags(resource.GetExternalTags(user(withGroupVersionKind()))), withGroupVersionKind()),
 			},
 		},
+		"NoChange": {
+			args: args{
+				cr: user(withExternalName(userName), withGroupVersionKind(),
+					withTags(map[string]string{"foo": "bar"}, resource.GetExternalTags(user(withExternalName(userName), withGroupVersionKind())))),
+				kube: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
+			},
+			want: want{
+				cr: user(withExternalName(userName), withGroupVersionKind(),
+					withTags(map[string]string{"foo": "bar"}, resource.GetExternalTags(user(withExternalName(userName), withGroupVersionKind())))),
+			},
+		},
 		"UpdateFailed": {
 			args: args{
 				cr:   user(),