From 8031f7583e3b9bf8d9292a7c5d8cade4f2025650 Mon Sep 17 00:00:00 2001 From: Ben McDonie Date: Thu, 20 Apr 2023 14:15:28 -0400 Subject: [PATCH 1/7] observe and update IAM user permissions boundary Signed-off-by: Ben McDonie --- build | 2 +- pkg/clients/iam/fake/user.go | 20 +++++--- pkg/clients/iam/user.go | 17 +++++++ pkg/controller/iam/user/controller.go | 56 +++++++++++++++++----- pkg/controller/iam/user/controller_test.go | 46 +++++++++++++++++- 5 files changed, 119 insertions(+), 22 deletions(-) diff --git a/build b/build index 85713f83bd..292f958d2d 160000 --- a/build +++ b/build @@ -1 +1 @@ -Subproject commit 85713f83bd601a0d415386f12a2adff27348bd4b +Subproject commit 292f958d2d97f26b450723998f82f7fc1767920c diff --git a/pkg/clients/iam/fake/user.go b/pkg/clients/iam/fake/user.go index 299fbcda8a..55376b0d95 100644 --- a/pkg/clients/iam/fake/user.go +++ b/pkg/clients/iam/fake/user.go @@ -35,13 +35,14 @@ type MockUserInput struct { // MockUserClient is a type that implements all the methods for RoleClient interface type MockUserClient struct { - MockUserInput MockUserInput - MockGetUser func(ctx context.Context, input *iam.GetUserInput, opts []func(*iam.Options)) (*iam.GetUserOutput, error) - MockCreateUser func(ctx context.Context, input *iam.CreateUserInput, opts []func(*iam.Options)) (*iam.CreateUserOutput, error) - MockDeleteUser func(ctx context.Context, input *iam.DeleteUserInput, opts []func(*iam.Options)) (*iam.DeleteUserOutput, error) - MockUpdateUser func(ctx context.Context, input *iam.UpdateUserInput, opts []func(*iam.Options)) (*iam.UpdateUserOutput, error) - MockTagUser func(ctx context.Context, input *iam.TagUserInput, opt []func(*iam.Options)) (*iam.TagUserOutput, error) - MockUntagUser func(ctx context.Context, input *iam.UntagUserInput, opts []func(*iam.Options)) (*iam.UntagUserOutput, error) + MockUserInput MockUserInput + MockGetUser func(ctx context.Context, input *iam.GetUserInput, opts []func(*iam.Options)) (*iam.GetUserOutput, error) + MockCreateUser func(ctx context.Context, input *iam.CreateUserInput, opts []func(*iam.Options)) (*iam.CreateUserOutput, error) + MockDeleteUser func(ctx context.Context, input *iam.DeleteUserInput, opts []func(*iam.Options)) (*iam.DeleteUserOutput, error) + MockUpdateUser func(ctx context.Context, input *iam.UpdateUserInput, opts []func(*iam.Options)) (*iam.UpdateUserOutput, error) + MockPutUserPermissionsBoundary func(ctx context.Context, input *iam.PutUserPermissionsBoundaryInput, opts []func(*iam.Options)) (*iam.PutUserPermissionsBoundaryOutput, error) + MockTagUser func(ctx context.Context, input *iam.TagUserInput, opt []func(*iam.Options)) (*iam.TagUserOutput, error) + MockUntagUser func(ctx context.Context, input *iam.UntagUserInput, opts []func(*iam.Options)) (*iam.UntagUserOutput, error) } // GetUser mocks GetUser method @@ -64,6 +65,11 @@ func (m *MockUserClient) UpdateUser(ctx context.Context, input *iam.UpdateUserIn return m.MockUpdateUser(ctx, input, opts) } +// PutUserPermissionsBoundary mocks PutUserPermissionsBoundary method +func (m *MockUserClient) PutUserPermissionsBoundary(ctx context.Context, input *iam.PutUserPermissionsBoundaryInput, opts ...func(*iam.Options)) (*iam.PutUserPermissionsBoundaryOutput, error) { + return m.MockPutUserPermissionsBoundary(ctx, input, opts) +} + // TagUser mocks TagUser method func (m *MockUserClient) TagUser(ctx context.Context, input *iam.TagUserInput, opts ...func(*iam.Options)) (*iam.TagUserOutput, error) { m.MockUserInput.TagUserInput = input diff --git a/pkg/clients/iam/user.go b/pkg/clients/iam/user.go index 93eadc46a4..e8845b2e39 100644 --- a/pkg/clients/iam/user.go +++ b/pkg/clients/iam/user.go @@ -17,6 +17,7 @@ type UserClient interface { CreateUser(ctx context.Context, input *iam.CreateUserInput, opts ...func(*iam.Options)) (*iam.CreateUserOutput, error) DeleteUser(ctx context.Context, input *iam.DeleteUserInput, opts ...func(*iam.Options)) (*iam.DeleteUserOutput, error) UpdateUser(ctx context.Context, input *iam.UpdateUserInput, opts ...func(*iam.Options)) (*iam.UpdateUserOutput, error) + PutUserPermissionsBoundary(ctx context.Context, params *iam.PutUserPermissionsBoundaryInput, optFns ...func(*iam.Options)) (*iam.PutUserPermissionsBoundaryOutput, error) TagUser(ctx context.Context, params *iam.TagUserInput, opts ...func(*iam.Options)) (*iam.TagUserOutput, error) UntagUser(ctx context.Context, params *iam.UntagUserInput, opts ...func(*iam.Options)) (*iam.UntagUserOutput, error) } @@ -44,3 +45,19 @@ func LateInitializeUser(in *v1beta1.UserParameters, user *iamtypes.User) { } } } + +// IsUserUpToDate checks whether there is a change in any of the modifiable fields in user. +func IsUserUpToDate(in v1beta1.UserParameters, observed iamtypes.User) (bool, string, error) { + if aws.ToString(in.Path) != aws.ToString(observed.Path) { + return false, "paths differ", nil + } + + if observed.PermissionsBoundary == nil { + if aws.ToString(in.PermissionsBoundary) != "" { + return false, "permission boundary needs set", nil + } + return true, "", nil + } + + return aws.ToString(in.PermissionsBoundary) == aws.ToString(observed.PermissionsBoundary.PermissionsBoundaryArn), "permission boundary", nil +} diff --git a/pkg/controller/iam/user/controller.go b/pkg/controller/iam/user/controller.go index f1c5898b72..6b52c3362b 100644 --- a/pkg/controller/iam/user/controller.go +++ b/pkg/controller/iam/user/controller.go @@ -135,16 +135,27 @@ func (e *external) Observe(ctx context.Context, mgd resource.Managed) (managed.E UserID: aws.ToString(user.UserId), } + // check path + isPathUpdated := aws.ToString(cr.Spec.ForProvider.Path) == aws.ToString(user.Path) + + // check tags crTagMap := make(map[string]string, len(cr.Spec.ForProvider.Tags)) for _, v := range cr.Spec.ForProvider.Tags { crTagMap[v.Key] = v.Value } _, _, areTagsUpdated := iam.DiffIAMTags(crTagMap, observed.User.Tags) + // check permissions boundary + boundaryArn := "" + if observed.User.PermissionsBoundary != nil { + boundaryArn = *observed.User.PermissionsBoundary.PermissionsBoundaryArn + } + isBoundaryUpdated := + aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) == aws.ToString(&boundaryArn) + return managed.ExternalObservation{ - ResourceExists: true, - ResourceUpToDate: aws.ToString(cr.Spec.ForProvider.Path) == aws.ToString(user.Path) && - areTagsUpdated, + ResourceExists: true, + ResourceUpToDate: isPathUpdated && areTagsUpdated && isBoundaryUpdated, }, nil } @@ -171,23 +182,44 @@ func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.Ex return managed.ExternalUpdate{}, errors.New(errUnexpectedObject) } - _, err := e.client.UpdateUser(ctx, &awsiam.UpdateUserInput{ - NewPath: cr.Spec.ForProvider.Path, - UserName: aws.String(meta.GetExternalName(cr)), + username := aws.String(meta.GetExternalName(cr)) + observed, err := e.client.GetUser(ctx, &awsiam.GetUserInput{ + UserName: username, }) if err != nil { - return managed.ExternalUpdate{}, awsclient.Wrap(err, errUpdate) + return managed.ExternalUpdate{}, awsclient.Wrap(resource.Ignore(iam.IsErrorNotFound, err), errGet) } - observed, err := e.client.GetUser(ctx, &awsiam.GetUserInput{ - UserName: aws.String(meta.GetExternalName(cr)), - }) + // take care of changes to path (only call if necessary) + if aws.ToString(observed.User.Path) != aws.ToString(cr.Spec.ForProvider.Path) { + _, err = e.client.UpdateUser(ctx, &awsiam.UpdateUserInput{ + NewPath: cr.Spec.ForProvider.Path, + UserName: username, + }) - if err != nil { - return managed.ExternalUpdate{}, awsclient.Wrap(err, errGet) + if err != nil { + return managed.ExternalUpdate{}, awsclient.Wrap(err, errUpdate) + } + } + + // take care of changes to PermissionBoundary (only call if necessary) + boundaryArn := "" + if observed.User.PermissionsBoundary != nil { + boundaryArn = *observed.User.PermissionsBoundary.PermissionsBoundaryArn + } + if aws.ToString(&boundaryArn) != aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) { + _, err = e.client.PutUserPermissionsBoundary(ctx, &awsiam.PutUserPermissionsBoundaryInput{ + PermissionsBoundary: cr.Spec.ForProvider.PermissionsBoundary, + UserName: username, + }) + + if err != nil { + return managed.ExternalUpdate{}, awsclient.Wrap(err, errUpdate) + } } + // take care of changes to Tags add, remove, _ := iam.DiffIAMTagsWithUpdates(cr.Spec.ForProvider.Tags, observed.User.Tags) if len(add) > 0 { diff --git a/pkg/controller/iam/user/controller_test.go b/pkg/controller/iam/user/controller_test.go index a807360799..64f2708d5f 100644 --- a/pkg/controller/iam/user/controller_test.go +++ b/pkg/controller/iam/user/controller_test.go @@ -108,6 +108,18 @@ func withGroupVersionKind() userModifier { } } +func withPath(path *string) userModifier { + return func(r *v1beta1.User) { + r.Spec.ForProvider.Path = path + } +} + +func withBoundary(boundary *string) userModifier { + return func(r *v1beta1.User) { + r.Spec.ForProvider.PermissionsBoundary = boundary + } +} + func user(m ...userModifier) *v1beta1.User { cr := &v1beta1.User{} for _, f := range m { @@ -196,6 +208,31 @@ func TestObserve(t *testing.T) { }, }, }, + "DifferentBoundary": { + args: args{ + iam: &fake.MockUserClient{ + MockGetUser: func(ctx context.Context, input *awsiam.GetUserInput, opts []func(*awsiam.Options)) (*awsiam.GetUserOutput, error) { + return &awsiam.GetUserOutput{ + User: &awsiamtypes.User{ + PermissionsBoundary: &awsiamtypes.AttachedPermissionsBoundary{ + PermissionsBoundaryArn: aws.String("old"), + }, + }, + }, nil + }, + }, + cr: user(withExternalName(userName), withBoundary(aws.String("new"))), + }, + want: want{ + cr: user(withExternalName(userName), + withConditions(xpv1.Available()), + withBoundary(aws.String("new"))), + result: managed.ExternalObservation{ + ResourceExists: true, + ResourceUpToDate: false, + }, + }, + }, } for name, tc := range cases { @@ -331,11 +368,16 @@ func TestUpdate(t *testing.T) { MockUpdateUser: func(ctx context.Context, input *awsiam.UpdateUserInput, opts []func(*awsiam.Options)) (*awsiam.UpdateUserOutput, error) { return nil, errBoom }, + MockGetUser: func(ctx context.Context, input *awsiam.GetUserInput, opts []func(*awsiam.Options)) (*awsiam.GetUserOutput, error) { + return &awsiam.GetUserOutput{ + User: &awsiamtypes.User{}, + }, nil + }, }, - cr: user(withExternalName(userName)), + cr: user(withExternalName(userName), withPath(aws.String("foo"))), }, want: want{ - cr: user(withExternalName(userName)), + cr: user(withExternalName(userName), withPath(aws.String("foo"))), err: awsclient.Wrap(errBoom, errUpdate), }, }, From a756fb59cf9a2c7d6c0a67e7f9aafb320dd26ee3 Mon Sep 17 00:00:00 2001 From: Ben McDonie Date: Thu, 20 Apr 2023 17:37:15 -0400 Subject: [PATCH 2/7] formatting and cyclo fix Signed-off-by: Ben McDonie --- pkg/controller/iam/user/controller.go | 112 +++++++++++++++----------- 1 file changed, 65 insertions(+), 47 deletions(-) diff --git a/pkg/controller/iam/user/controller.go b/pkg/controller/iam/user/controller.go index 6b52c3362b..43a3e000fc 100644 --- a/pkg/controller/iam/user/controller.go +++ b/pkg/controller/iam/user/controller.go @@ -148,9 +148,9 @@ func (e *external) Observe(ctx context.Context, mgd resource.Managed) (managed.E // check permissions boundary boundaryArn := "" if observed.User.PermissionsBoundary != nil { - boundaryArn = *observed.User.PermissionsBoundary.PermissionsBoundaryArn + boundaryArn = *observed.User.PermissionsBoundary.PermissionsBoundaryArn } - isBoundaryUpdated := + isBoundaryUpdated := aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) == aws.ToString(&boundaryArn) return managed.ExternalObservation{ @@ -182,9 +182,8 @@ func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.Ex return managed.ExternalUpdate{}, errors.New(errUnexpectedObject) } - username := aws.String(meta.GetExternalName(cr)) observed, err := e.client.GetUser(ctx, &awsiam.GetUserInput{ - UserName: username, + UserName: aws.String(meta.GetExternalName(cr)), }) if err != nil { @@ -192,55 +191,20 @@ func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.Ex } // take care of changes to path (only call if necessary) - if aws.ToString(observed.User.Path) != aws.ToString(cr.Spec.ForProvider.Path) { - _, err = e.client.UpdateUser(ctx, &awsiam.UpdateUserInput{ - NewPath: cr.Spec.ForProvider.Path, - UserName: username, - }) - - if err != nil { - return managed.ExternalUpdate{}, awsclient.Wrap(err, errUpdate) - } + msg, err := e.updateUser(ctx, observed, cr) + if err != nil { + return managed.ExternalUpdate{}, awsclient.Wrap(err, msg) } // take care of changes to PermissionBoundary (only call if necessary) - boundaryArn := "" - if observed.User.PermissionsBoundary != nil { - boundaryArn = *observed.User.PermissionsBoundary.PermissionsBoundaryArn - } - if aws.ToString(&boundaryArn) != aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) { - _, err = e.client.PutUserPermissionsBoundary(ctx, &awsiam.PutUserPermissionsBoundaryInput{ - PermissionsBoundary: cr.Spec.ForProvider.PermissionsBoundary, - UserName: username, - }) - - if err != nil { - return managed.ExternalUpdate{}, awsclient.Wrap(err, errUpdate) - } + msg, err = e.updatePermissionsBoundary(ctx, observed, cr) + if err != nil { + return managed.ExternalUpdate{}, awsclient.Wrap(err, msg) } // take care of changes to Tags - add, remove, _ := iam.DiffIAMTagsWithUpdates(cr.Spec.ForProvider.Tags, observed.User.Tags) - - if len(add) > 0 { - if _, err := e.client.TagUser(ctx, &awsiam.TagUserInput{ - UserName: aws.String(meta.GetExternalName(cr)), - Tags: add, - }); err != nil { - return managed.ExternalUpdate{}, awsclient.Wrap(err, errTag) - } - } - - if len(remove) > 0 { - if _, err := e.client.UntagUser(ctx, &awsiam.UntagUserInput{ - TagKeys: remove, - UserName: aws.String(meta.GetExternalName(cr)), - }); err != nil { - return managed.ExternalUpdate{}, awsclient.Wrap(err, errUntag) - } - } - - return managed.ExternalUpdate{}, awsclient.Wrap(err, errUpdate) + msg, err = e.updateTags(ctx, observed, cr) + return managed.ExternalUpdate{}, awsclient.Wrap(err, msg) } func (e *external) Delete(ctx context.Context, mgd resource.Managed) error { @@ -290,3 +254,57 @@ func (t *tagger) Initialize(ctx context.Context, mgd resource.Managed) error { } return errors.Wrap(t.kube.Update(ctx, cr), errKubeUpdateFailed) } + +func (e *external) updateUser(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) (string, error) { + if aws.ToString(observed.User.Path) != aws.ToString(cr.Spec.ForProvider.Path) { + _, err := e.client.UpdateUser(ctx, &awsiam.UpdateUserInput{ + NewPath: cr.Spec.ForProvider.Path, + UserName: aws.String(meta.GetExternalName(cr)), + }) + + return errUpdate, err + } + + return "", nil +} + +func (e *external) updatePermissionsBoundary(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) (string, error) { + boundaryArn := "" + if observed.User.PermissionsBoundary != nil { + boundaryArn = *observed.User.PermissionsBoundary.PermissionsBoundaryArn + } + if aws.ToString(&boundaryArn) != aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) { + _, err := e.client.PutUserPermissionsBoundary(ctx, &awsiam.PutUserPermissionsBoundaryInput{ + PermissionsBoundary: cr.Spec.ForProvider.PermissionsBoundary, + UserName: aws.String(meta.GetExternalName(cr)), + }) + + return errUpdate, err + } + + return "", nil +} + +func (e *external) updateTags(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) (string, error) { + add, remove, _ := iam.DiffIAMTagsWithUpdates(cr.Spec.ForProvider.Tags, observed.User.Tags) + + if len(add) > 0 { + if _, err := e.client.TagUser(ctx, &awsiam.TagUserInput{ + UserName: aws.String(meta.GetExternalName(cr)), + Tags: add, + }); err != nil { + return errTag, err + } + } + + if len(remove) > 0 { + if _, err := e.client.UntagUser(ctx, &awsiam.UntagUserInput{ + TagKeys: remove, + UserName: aws.String(meta.GetExternalName(cr)), + }); err != nil { + return errUntag, err + } + } + + return "", nil +} From 834110465fecd3f66c1b9d8002947f3a3a4a187c Mon Sep 17 00:00:00 2001 From: Ben McDonie Date: Thu, 20 Apr 2023 17:42:07 -0400 Subject: [PATCH 3/7] up CLI upgrade for build Signed-off-by: Ben McDonie --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ea01e0d69c..253f59c3d2 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,7 @@ GO111MODULE = on # ==================================================================================== # Setup Kubernetes tools -UP_VERSION = v0.13.0 +UP_VERSION = v0.16.1 UP_CHANNEL = stable UPTEST_VERSION = v0.5.0 From 30af53300ee24919137a5a2e5b0ed81189d2af93 Mon Sep 17 00:00:00 2001 From: Ben McDonie Date: Sun, 23 Apr 2023 09:25:24 -0400 Subject: [PATCH 4/7] delete boundaries when removed from spec Signed-off-by: Ben McDonie --- pkg/clients/iam/fake/role.go | 26 +++++++++++++++++++------- pkg/clients/iam/fake/user.go | 22 ++++++++++++++-------- pkg/clients/iam/role.go | 7 +++++++ pkg/clients/iam/user.go | 17 +---------------- pkg/controller/iam/role/controller.go | 21 +++++++++++++++++++++ pkg/controller/iam/user/controller.go | 16 ++++++++++++---- 6 files changed, 74 insertions(+), 35 deletions(-) diff --git a/pkg/clients/iam/fake/role.go b/pkg/clients/iam/fake/role.go index d51c656909..968baf04f2 100644 --- a/pkg/clients/iam/fake/role.go +++ b/pkg/clients/iam/fake/role.go @@ -29,13 +29,15 @@ var _ clientset.RoleClient = (*MockRoleClient)(nil) // MockRoleClient is a type that implements all the methods for RoleClient interface type MockRoleClient struct { - MockGetRole func(ctx context.Context, input *iam.GetRoleInput, opts []func(*iam.Options)) (*iam.GetRoleOutput, error) - MockCreateRole func(ctx context.Context, input *iam.CreateRoleInput, opts []func(*iam.Options)) (*iam.CreateRoleOutput, error) - MockDeleteRole func(ctx context.Context, input *iam.DeleteRoleInput, opts []func(*iam.Options)) (*iam.DeleteRoleOutput, error) - MockUpdateRole func(ctx context.Context, input *iam.UpdateRoleInput, opts []func(*iam.Options)) (*iam.UpdateRoleOutput, error) - MockUpdateAssumeRolePolicy func(ctx context.Context, input *iam.UpdateAssumeRolePolicyInput, opts []func(*iam.Options)) (*iam.UpdateAssumeRolePolicyOutput, error) - MockTagRole func(ctx context.Context, input *iam.TagRoleInput, opts []func(*iam.Options)) (*iam.TagRoleOutput, error) - MockUntagRole func(ctx context.Context, input *iam.UntagRoleInput, opts []func(*iam.Options)) (*iam.UntagRoleOutput, error) + MockGetRole func(ctx context.Context, input *iam.GetRoleInput, opts []func(*iam.Options)) (*iam.GetRoleOutput, error) + MockCreateRole func(ctx context.Context, input *iam.CreateRoleInput, opts []func(*iam.Options)) (*iam.CreateRoleOutput, error) + MockDeleteRole func(ctx context.Context, input *iam.DeleteRoleInput, opts []func(*iam.Options)) (*iam.DeleteRoleOutput, error) + MockUpdateRole func(ctx context.Context, input *iam.UpdateRoleInput, opts []func(*iam.Options)) (*iam.UpdateRoleOutput, error) + MockPutRolePermissionsBoundary func(ctx context.Context, input *iam.PutRolePermissionsBoundaryInput, opts []func(*iam.Options)) (*iam.PutRolePermissionsBoundaryOutput, error) + MockDeleteRolePermissionsBoundary func(ctx context.Context, input *iam.DeleteRolePermissionsBoundaryInput, opts []func(*iam.Options)) (*iam.DeleteRolePermissionsBoundaryOutput, error) + MockUpdateAssumeRolePolicy func(ctx context.Context, input *iam.UpdateAssumeRolePolicyInput, opts []func(*iam.Options)) (*iam.UpdateAssumeRolePolicyOutput, error) + MockTagRole func(ctx context.Context, input *iam.TagRoleInput, opts []func(*iam.Options)) (*iam.TagRoleOutput, error) + MockUntagRole func(ctx context.Context, input *iam.UntagRoleInput, opts []func(*iam.Options)) (*iam.UntagRoleOutput, error) } // GetRole mocks GetRole method @@ -58,6 +60,16 @@ func (m *MockRoleClient) UpdateRole(ctx context.Context, input *iam.UpdateRoleIn return m.MockUpdateRole(ctx, input, opts) } +// PutRolePermissionsBoundary mocks PutRolePermissionsBoundary method +func (m *MockRoleClient) PutRolePermissionsBoundary(ctx context.Context, input *iam.PutRolePermissionsBoundaryInput, opts ...func(*iam.Options)) (*iam.PutRolePermissionsBoundaryOutput, error) { + return m.MockPutRolePermissionsBoundary(ctx, input, opts) +} + +// DeleteRolePermissionsBoundary mocks DeleteRolePermissionsBoundary method +func (m *MockRoleClient) DeleteRolePermissionsBoundary(ctx context.Context, input *iam.DeleteRolePermissionsBoundaryInput, opts ...func(*iam.Options)) (*iam.DeleteRolePermissionsBoundaryOutput, error) { + return m.MockDeleteRolePermissionsBoundary(ctx, input, opts) +} + // UpdateAssumeRolePolicy mocks UpdateAssumeRolePolicy method func (m *MockRoleClient) UpdateAssumeRolePolicy(ctx context.Context, input *iam.UpdateAssumeRolePolicyInput, opts ...func(*iam.Options)) (*iam.UpdateAssumeRolePolicyOutput, error) { return m.MockUpdateAssumeRolePolicy(ctx, input, opts) diff --git a/pkg/clients/iam/fake/user.go b/pkg/clients/iam/fake/user.go index 55376b0d95..129ad8f0cf 100644 --- a/pkg/clients/iam/fake/user.go +++ b/pkg/clients/iam/fake/user.go @@ -35,14 +35,15 @@ type MockUserInput struct { // MockUserClient is a type that implements all the methods for RoleClient interface type MockUserClient struct { - MockUserInput MockUserInput - MockGetUser func(ctx context.Context, input *iam.GetUserInput, opts []func(*iam.Options)) (*iam.GetUserOutput, error) - MockCreateUser func(ctx context.Context, input *iam.CreateUserInput, opts []func(*iam.Options)) (*iam.CreateUserOutput, error) - MockDeleteUser func(ctx context.Context, input *iam.DeleteUserInput, opts []func(*iam.Options)) (*iam.DeleteUserOutput, error) - MockUpdateUser func(ctx context.Context, input *iam.UpdateUserInput, opts []func(*iam.Options)) (*iam.UpdateUserOutput, error) - MockPutUserPermissionsBoundary func(ctx context.Context, input *iam.PutUserPermissionsBoundaryInput, opts []func(*iam.Options)) (*iam.PutUserPermissionsBoundaryOutput, error) - MockTagUser func(ctx context.Context, input *iam.TagUserInput, opt []func(*iam.Options)) (*iam.TagUserOutput, error) - MockUntagUser func(ctx context.Context, input *iam.UntagUserInput, opts []func(*iam.Options)) (*iam.UntagUserOutput, error) + MockUserInput MockUserInput + MockGetUser func(ctx context.Context, input *iam.GetUserInput, opts []func(*iam.Options)) (*iam.GetUserOutput, error) + MockCreateUser func(ctx context.Context, input *iam.CreateUserInput, opts []func(*iam.Options)) (*iam.CreateUserOutput, error) + MockDeleteUser func(ctx context.Context, input *iam.DeleteUserInput, opts []func(*iam.Options)) (*iam.DeleteUserOutput, error) + MockUpdateUser func(ctx context.Context, input *iam.UpdateUserInput, opts []func(*iam.Options)) (*iam.UpdateUserOutput, error) + MockPutUserPermissionsBoundary func(ctx context.Context, input *iam.PutUserPermissionsBoundaryInput, opts []func(*iam.Options)) (*iam.PutUserPermissionsBoundaryOutput, error) + MockDeleteUserPermissionsBoundary func(ctx context.Context, input *iam.DeleteUserPermissionsBoundaryInput, opts []func(*iam.Options)) (*iam.DeleteUserPermissionsBoundaryOutput, error) + MockTagUser func(ctx context.Context, input *iam.TagUserInput, opt []func(*iam.Options)) (*iam.TagUserOutput, error) + MockUntagUser func(ctx context.Context, input *iam.UntagUserInput, opts []func(*iam.Options)) (*iam.UntagUserOutput, error) } // GetUser mocks GetUser method @@ -70,6 +71,11 @@ func (m *MockUserClient) PutUserPermissionsBoundary(ctx context.Context, input * return m.MockPutUserPermissionsBoundary(ctx, input, opts) } +// DeleteUserPermissionsBoundary mocks DeleteUserPermissionsBoundary method +func (m *MockUserClient) DeleteUserPermissionsBoundary(ctx context.Context, input *iam.DeleteUserPermissionsBoundaryInput, opts ...func(*iam.Options)) (*iam.DeleteUserPermissionsBoundaryOutput, error) { + return m.MockDeleteUserPermissionsBoundary(ctx, input, opts) +} + // TagUser mocks TagUser method func (m *MockUserClient) TagUser(ctx context.Context, input *iam.TagUserInput, opts ...func(*iam.Options)) (*iam.TagUserOutput, error) { m.MockUserInput.TagUserInput = input diff --git a/pkg/clients/iam/role.go b/pkg/clients/iam/role.go index 2943045dbf..d759eabb4d 100644 --- a/pkg/clients/iam/role.go +++ b/pkg/clients/iam/role.go @@ -33,6 +33,8 @@ type RoleClient interface { CreateRole(ctx context.Context, input *iam.CreateRoleInput, opts ...func(*iam.Options)) (*iam.CreateRoleOutput, error) DeleteRole(ctx context.Context, input *iam.DeleteRoleInput, opts ...func(*iam.Options)) (*iam.DeleteRoleOutput, error) UpdateRole(ctx context.Context, input *iam.UpdateRoleInput, opts ...func(*iam.Options)) (*iam.UpdateRoleOutput, error) + PutRolePermissionsBoundary(ctx context.Context, params *iam.PutRolePermissionsBoundaryInput, optFns ...func(*iam.Options)) (*iam.PutRolePermissionsBoundaryOutput, error) + DeleteRolePermissionsBoundary(ctx context.Context, params *iam.DeleteRolePermissionsBoundaryInput, optFns ...func(*iam.Options)) (*iam.DeleteRolePermissionsBoundaryOutput, error) UpdateAssumeRolePolicy(ctx context.Context, input *iam.UpdateAssumeRolePolicyInput, opts ...func(*iam.Options)) (*iam.UpdateAssumeRolePolicyOutput, error) TagRole(ctx context.Context, input *iam.TagRoleInput, opts ...func(*iam.Options)) (*iam.TagRoleOutput, error) UntagRole(ctx context.Context, input *iam.UntagRoleInput, opts ...func(*iam.Options)) (*iam.UntagRoleOutput, error) @@ -89,6 +91,11 @@ func GenerateRole(in v1beta1.RoleParameters, role *iamtypes.Role) error { role.Description = in.Description role.MaxSessionDuration = in.MaxSessionDuration role.Path = in.Path + if in.PermissionsBoundary != nil { + role.PermissionsBoundary = &iamtypes.AttachedPermissionsBoundary{ + PermissionsBoundaryArn: in.PermissionsBoundary, + } + } if len(in.Tags) != 0 { role.Tags = make([]iamtypes.Tag, len(in.Tags)) diff --git a/pkg/clients/iam/user.go b/pkg/clients/iam/user.go index e8845b2e39..cc0b938a8a 100644 --- a/pkg/clients/iam/user.go +++ b/pkg/clients/iam/user.go @@ -18,6 +18,7 @@ type UserClient interface { DeleteUser(ctx context.Context, input *iam.DeleteUserInput, opts ...func(*iam.Options)) (*iam.DeleteUserOutput, error) UpdateUser(ctx context.Context, input *iam.UpdateUserInput, opts ...func(*iam.Options)) (*iam.UpdateUserOutput, error) PutUserPermissionsBoundary(ctx context.Context, params *iam.PutUserPermissionsBoundaryInput, optFns ...func(*iam.Options)) (*iam.PutUserPermissionsBoundaryOutput, error) + DeleteUserPermissionsBoundary(ctx context.Context, params *iam.DeleteUserPermissionsBoundaryInput, optFns ...func(*iam.Options)) (*iam.DeleteUserPermissionsBoundaryOutput, error) TagUser(ctx context.Context, params *iam.TagUserInput, opts ...func(*iam.Options)) (*iam.TagUserOutput, error) UntagUser(ctx context.Context, params *iam.UntagUserInput, opts ...func(*iam.Options)) (*iam.UntagUserOutput, error) } @@ -45,19 +46,3 @@ func LateInitializeUser(in *v1beta1.UserParameters, user *iamtypes.User) { } } } - -// IsUserUpToDate checks whether there is a change in any of the modifiable fields in user. -func IsUserUpToDate(in v1beta1.UserParameters, observed iamtypes.User) (bool, string, error) { - if aws.ToString(in.Path) != aws.ToString(observed.Path) { - return false, "paths differ", nil - } - - if observed.PermissionsBoundary == nil { - if aws.ToString(in.PermissionsBoundary) != "" { - return false, "permission boundary needs set", nil - } - return true, "", nil - } - - return aws.ToString(in.PermissionsBoundary) == aws.ToString(observed.PermissionsBoundary.PermissionsBoundaryArn), "permission boundary", nil -} diff --git a/pkg/controller/iam/role/controller.go b/pkg/controller/iam/role/controller.go index aadaa07bd1..fe4f34f2d3 100644 --- a/pkg/controller/iam/role/controller.go +++ b/pkg/controller/iam/role/controller.go @@ -210,6 +210,27 @@ func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.Ex } } + boundaryArn := "" + if observed.Role.PermissionsBoundary != nil { + boundaryArn = *observed.Role.PermissionsBoundary.PermissionsBoundaryArn + } + if aws.ToString(&boundaryArn) != aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) { + if aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) == "" { + _, err = e.client.DeleteRolePermissionsBoundary(ctx, &awsiam.DeleteRolePermissionsBoundaryInput{ + RoleName: aws.String(meta.GetExternalName(cr)), + }) + } else { + _, err = e.client.PutRolePermissionsBoundary(ctx, &awsiam.PutRolePermissionsBoundaryInput{ + PermissionsBoundary: cr.Spec.ForProvider.PermissionsBoundary, + RoleName: aws.String(meta.GetExternalName(cr)), + }) + } + + if err != nil { + return managed.ExternalUpdate{}, awsclient.Wrap(err, errUpdate) + } + } + if patch.AssumeRolePolicyDocument != "" { _, err = e.client.UpdateAssumeRolePolicy(ctx, &awsiam.UpdateAssumeRolePolicyInput{ PolicyDocument: &cr.Spec.ForProvider.AssumeRolePolicyDocument, diff --git a/pkg/controller/iam/user/controller.go b/pkg/controller/iam/user/controller.go index 43a3e000fc..4563f42e8a 100644 --- a/pkg/controller/iam/user/controller.go +++ b/pkg/controller/iam/user/controller.go @@ -270,14 +270,22 @@ func (e *external) updateUser(ctx context.Context, observed *awsiam.GetUserOutpu func (e *external) updatePermissionsBoundary(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) (string, error) { boundaryArn := "" + var err error + if observed.User.PermissionsBoundary != nil { boundaryArn = *observed.User.PermissionsBoundary.PermissionsBoundaryArn } if aws.ToString(&boundaryArn) != aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) { - _, err := e.client.PutUserPermissionsBoundary(ctx, &awsiam.PutUserPermissionsBoundaryInput{ - PermissionsBoundary: cr.Spec.ForProvider.PermissionsBoundary, - UserName: aws.String(meta.GetExternalName(cr)), - }) + if aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) == "" { + _, err = e.client.DeleteUserPermissionsBoundary(ctx, &awsiam.DeleteUserPermissionsBoundaryInput{ + UserName: aws.String(meta.GetExternalName(cr)), + }) + } else { + _, err = e.client.PutUserPermissionsBoundary(ctx, &awsiam.PutUserPermissionsBoundaryInput{ + PermissionsBoundary: cr.Spec.ForProvider.PermissionsBoundary, + UserName: aws.String(meta.GetExternalName(cr)), + }) + } return errUpdate, err } From a7670e47d0ab6dd6df074e878958263e9fd15fb3 Mon Sep 17 00:00:00 2001 From: Ben McDonie Date: Sun, 9 Jul 2023 09:03:14 -0400 Subject: [PATCH 5/7] rollback UP_VERSION change move error.Wrap calls Signed-off-by: Ben McDonie --- Makefile | 2 +- pkg/controller/iam/user/controller.go | 32 +++++++++++++-------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index 253f59c3d2..ea01e0d69c 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,7 @@ GO111MODULE = on # ==================================================================================== # Setup Kubernetes tools -UP_VERSION = v0.16.1 +UP_VERSION = v0.13.0 UP_CHANNEL = stable UPTEST_VERSION = v0.5.0 diff --git a/pkg/controller/iam/user/controller.go b/pkg/controller/iam/user/controller.go index 4563f42e8a..8b8ec8a089 100644 --- a/pkg/controller/iam/user/controller.go +++ b/pkg/controller/iam/user/controller.go @@ -191,20 +191,20 @@ func (e *external) Update(ctx context.Context, mgd resource.Managed) (managed.Ex } // take care of changes to path (only call if necessary) - msg, err := e.updateUser(ctx, observed, cr) + err = e.updateUser(ctx, observed, cr) if err != nil { - return managed.ExternalUpdate{}, awsclient.Wrap(err, msg) + return managed.ExternalUpdate{}, err } // take care of changes to PermissionBoundary (only call if necessary) - msg, err = e.updatePermissionsBoundary(ctx, observed, cr) + err = e.updatePermissionsBoundary(ctx, observed, cr) if err != nil { - return managed.ExternalUpdate{}, awsclient.Wrap(err, msg) + return managed.ExternalUpdate{}, err } // take care of changes to Tags - msg, err = e.updateTags(ctx, observed, cr) - return managed.ExternalUpdate{}, awsclient.Wrap(err, msg) + err = e.updateTags(ctx, observed, cr) + return managed.ExternalUpdate{}, err } func (e *external) Delete(ctx context.Context, mgd resource.Managed) error { @@ -255,20 +255,20 @@ func (t *tagger) Initialize(ctx context.Context, mgd resource.Managed) error { return errors.Wrap(t.kube.Update(ctx, cr), errKubeUpdateFailed) } -func (e *external) updateUser(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) (string, error) { +func (e *external) updateUser(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) error { if aws.ToString(observed.User.Path) != aws.ToString(cr.Spec.ForProvider.Path) { _, err := e.client.UpdateUser(ctx, &awsiam.UpdateUserInput{ NewPath: cr.Spec.ForProvider.Path, UserName: aws.String(meta.GetExternalName(cr)), }) - return errUpdate, err + return awsclient.Wrap(err, errUpdate) } - return "", nil + return nil } -func (e *external) updatePermissionsBoundary(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) (string, error) { +func (e *external) updatePermissionsBoundary(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) error { boundaryArn := "" var err error @@ -287,13 +287,13 @@ func (e *external) updatePermissionsBoundary(ctx context.Context, observed *awsi }) } - return errUpdate, err + return awsclient.Wrap(err, errUpdate) } - return "", nil + return nil } -func (e *external) updateTags(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) (string, error) { +func (e *external) updateTags(ctx context.Context, observed *awsiam.GetUserOutput, cr *v1beta1.User) error { add, remove, _ := iam.DiffIAMTagsWithUpdates(cr.Spec.ForProvider.Tags, observed.User.Tags) if len(add) > 0 { @@ -301,7 +301,7 @@ func (e *external) updateTags(ctx context.Context, observed *awsiam.GetUserOutpu UserName: aws.String(meta.GetExternalName(cr)), Tags: add, }); err != nil { - return errTag, err + return awsclient.Wrap(err, errTag) } } @@ -310,9 +310,9 @@ func (e *external) updateTags(ctx context.Context, observed *awsiam.GetUserOutpu TagKeys: remove, UserName: aws.String(meta.GetExternalName(cr)), }); err != nil { - return errUntag, err + return awsclient.Wrap(err, errUntag) } } - return "", nil + return nil } From 6455fe89e1d6025ee7db2e239e9bd4aed996328f Mon Sep 17 00:00:00 2001 From: Ben McDonie Date: Tue, 11 Jul 2023 21:42:53 -0400 Subject: [PATCH 6/7] update build submodule to latest Signed-off-by: Ben McDonie --- build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build b/build index 292f958d2d..85713f83bd 160000 --- a/build +++ b/build @@ -1 +1 @@ -Subproject commit 292f958d2d97f26b450723998f82f7fc1767920c +Subproject commit 85713f83bd601a0d415386f12a2adff27348bd4b From af8e1987d0152841ecffe55d08183553a910426c Mon Sep 17 00:00:00 2001 From: Ben McDonie Date: Wed, 19 Jul 2023 07:24:11 -0400 Subject: [PATCH 7/7] split isUpToDate better error messages Signed-off-by: Ben McDonie --- pkg/controller/iam/user/controller.go | 77 ++++++++++++---------- pkg/controller/iam/user/controller_test.go | 2 +- 2 files changed, 45 insertions(+), 34 deletions(-) diff --git a/pkg/controller/iam/user/controller.go b/pkg/controller/iam/user/controller.go index 8b8ec8a089..23c6323402 100644 --- a/pkg/controller/iam/user/controller.go +++ b/pkg/controller/iam/user/controller.go @@ -21,6 +21,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" awsiam "github.com/aws/aws-sdk-go-v2/service/iam" + "github.com/aws/aws-sdk-go-v2/service/iam/types" "github.com/google/go-cmp/cmp" "github.com/pkg/errors" @@ -45,13 +46,15 @@ import ( const ( errUnexpectedObject = "The managed resource is not an IAM User resource" - errGet = "cannot get IAM User" - errCreate = "cannot create the IAM User resource" - errDelete = "cannot delete the IAM User resource" - errUpdate = "cannot update the IAM User resource" - errSDK = "empty IAM User received from IAM API" - errTag = "cannot tag the IAM User resource" - errUntag = "cannot remove tags from the IAM User resource" + errGet = "cannot get IAM User" + errCreate = "cannot create the IAM User resource" + errDelete = "cannot delete the IAM User resource" + errUpdateUser = "cannot update the IAM User resource" + errPutUserPermissionsBoundary = "cannot update the IAM User permission boundary" + errDeleteUserPermissionsBoundary = "cannot delete the IAM User permission boundary" + errSDK = "empty IAM User received from IAM API" + errTag = "cannot tag the IAM User resource" + errUntag = "cannot remove tags from the IAM User resource" errKubeUpdateFailed = "cannot late initialize IAM User" ) @@ -135,27 +138,9 @@ func (e *external) Observe(ctx context.Context, mgd resource.Managed) (managed.E UserID: aws.ToString(user.UserId), } - // check path - isPathUpdated := aws.ToString(cr.Spec.ForProvider.Path) == aws.ToString(user.Path) - - // check tags - crTagMap := make(map[string]string, len(cr.Spec.ForProvider.Tags)) - for _, v := range cr.Spec.ForProvider.Tags { - crTagMap[v.Key] = v.Value - } - _, _, areTagsUpdated := iam.DiffIAMTags(crTagMap, observed.User.Tags) - - // check permissions boundary - boundaryArn := "" - if observed.User.PermissionsBoundary != nil { - boundaryArn = *observed.User.PermissionsBoundary.PermissionsBoundaryArn - } - isBoundaryUpdated := - aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) == aws.ToString(&boundaryArn) - return managed.ExternalObservation{ ResourceExists: true, - ResourceUpToDate: isPathUpdated && areTagsUpdated && isBoundaryUpdated, + ResourceUpToDate: isUpToDate(cr, &user), }, nil } @@ -262,7 +247,7 @@ func (e *external) updateUser(ctx context.Context, observed *awsiam.GetUserOutpu UserName: aws.String(meta.GetExternalName(cr)), }) - return awsclient.Wrap(err, errUpdate) + return awsclient.Wrap(err, errUpdateUser) } return nil @@ -276,18 +261,22 @@ func (e *external) updatePermissionsBoundary(ctx context.Context, observed *awsi boundaryArn = *observed.User.PermissionsBoundary.PermissionsBoundaryArn } if aws.ToString(&boundaryArn) != aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) { + // is this a delete? if aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) == "" { _, err = e.client.DeleteUserPermissionsBoundary(ctx, &awsiam.DeleteUserPermissionsBoundaryInput{ UserName: aws.String(meta.GetExternalName(cr)), }) - } else { - _, err = e.client.PutUserPermissionsBoundary(ctx, &awsiam.PutUserPermissionsBoundaryInput{ - PermissionsBoundary: cr.Spec.ForProvider.PermissionsBoundary, - UserName: aws.String(meta.GetExternalName(cr)), - }) + + return awsclient.Wrap(err, errDeleteUserPermissionsBoundary) } - return awsclient.Wrap(err, errUpdate) + // must be an update + _, err = e.client.PutUserPermissionsBoundary(ctx, &awsiam.PutUserPermissionsBoundaryInput{ + PermissionsBoundary: cr.Spec.ForProvider.PermissionsBoundary, + UserName: aws.String(meta.GetExternalName(cr)), + }) + + return awsclient.Wrap(err, errPutUserPermissionsBoundary) } return nil @@ -316,3 +305,25 @@ func (e *external) updateTags(ctx context.Context, observed *awsiam.GetUserOutpu return nil } + +func isUpToDate(cr *v1beta1.User, user *types.User) bool { + // check path + isPathUpdated := aws.ToString(cr.Spec.ForProvider.Path) == aws.ToString(user.Path) + + // check tags + crTagMap := make(map[string]string, len(cr.Spec.ForProvider.Tags)) + for _, v := range cr.Spec.ForProvider.Tags { + crTagMap[v.Key] = v.Value + } + _, _, areTagsUpdated := iam.DiffIAMTags(crTagMap, user.Tags) + + // check permissions boundary + boundaryArn := "" + if user.PermissionsBoundary != nil { + boundaryArn = *user.PermissionsBoundary.PermissionsBoundaryArn + } + isBoundaryUpdated := + aws.ToString(cr.Spec.ForProvider.PermissionsBoundary) == aws.ToString(&boundaryArn) + + return isPathUpdated && areTagsUpdated && isBoundaryUpdated +} diff --git a/pkg/controller/iam/user/controller_test.go b/pkg/controller/iam/user/controller_test.go index 64f2708d5f..6b0b389be7 100644 --- a/pkg/controller/iam/user/controller_test.go +++ b/pkg/controller/iam/user/controller_test.go @@ -378,7 +378,7 @@ func TestUpdate(t *testing.T) { }, want: want{ cr: user(withExternalName(userName), withPath(aws.String("foo"))), - err: awsclient.Wrap(errBoom, errUpdate), + err: awsclient.Wrap(errBoom, errUpdateUser), }, }, "GetUserError": {