From f2e3f72be4098350dd6673ef03b4e27ec2baebc6 Mon Sep 17 00:00:00 2001 From: Cecilia Bernardi Date: Tue, 7 Jun 2022 12:51:44 +0100 Subject: [PATCH] fixed infinite default tags adding in iam user added a test to detect infinite default tag adding to oidcProvider added a test to detect infinite defalt tag adding to oicdProvider added a test to detect infinite default tag addition to role Signed-off-by: Cecilia Bernardi (cherry picked from commit 81534cfdc8e15f5e95ae174d24a2621468a7eff5) --- .../openidconnectprovider/controller_test.go | 13 ++++++ pkg/controller/iam/policy/controller_test.go | 13 ++++++ pkg/controller/iam/role/controller.go | 23 ++++++---- pkg/controller/iam/role/controller_test.go | 43 ++++++++++++++++++- pkg/controller/iam/user/controller.go | 8 ++-- pkg/controller/iam/user/controller_test.go | 11 +++++ 6 files changed, 98 insertions(+), 13 deletions(-) diff --git a/pkg/controller/iam/openidconnectprovider/controller_test.go b/pkg/controller/iam/openidconnectprovider/controller_test.go index fa7762c812..d34bdc0985 100644 --- 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 index 16ed52c5d4..f3d59d54e0 100644 --- 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 index 761bd17f08..5beaf442bd 100644 --- 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 index 3ae2bc2bfc..0321358043 100644 --- 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 index f3ae255606..3fa64b5654 100644 --- 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 index 1edb39ff0a..a807360799 100644 --- 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(),