From 706c734eceff46fab45a3933ed1bf8a150d2ffd5 Mon Sep 17 00:00:00 2001 From: Henry Sachs Date: Tue, 26 May 2026 15:16:19 +0200 Subject: [PATCH 1/3] fix: bound service account baseline permission checks Signed-off-by: Henry Sachs --- pkg/cluster/clients/groups/fake/zz_fake.go | 6 +- .../instance/serviceaccounts/zz_controller.go | 2 +- .../serviceaccounts/zz_controller_helpers.go | 107 ++++++------------ .../zz_controller_helpers_test.go | 57 ++++++++-- .../serviceaccounts/zz_controller_test.go | 48 ++++++++ pkg/namespaced/clients/groups/fake/fake.go | 6 +- .../instance/serviceaccounts/controller.go | 2 +- .../serviceaccounts/controller_helpers.go | 107 ++++++------------ .../controller_helpers_test.go | 57 ++++++++-- .../serviceaccounts/controller_test.go | 48 ++++++++ 10 files changed, 270 insertions(+), 170 deletions(-) diff --git a/pkg/cluster/clients/groups/fake/zz_fake.go b/pkg/cluster/clients/groups/fake/zz_fake.go index 7e0206fe..23ed2d04 100644 --- a/pkg/cluster/clients/groups/fake/zz_fake.go +++ b/pkg/cluster/clients/groups/fake/zz_fake.go @@ -120,17 +120,17 @@ func (c *MockClient) ListGroups(opt *gitlab.ListGroupsOptions, options ...gitlab // GetGroupMember calls the underlying MockGetMember method. func (c *MockClient) GetGroupMember(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { - return c.MockGetMember(gid, user) + return c.MockGetMember(gid, user, options...) } // AddGroupMember calls the underlying MockAddMember method. func (c *MockClient) AddGroupMember(gid interface{}, opt *gitlab.AddGroupMemberOptions, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { - return c.MockAddMember(gid, opt) + return c.MockAddMember(gid, opt, options...) } // EditGroupMember calls the underlying MockEditMember method. func (c *MockClient) EditGroupMember(gid interface{}, user int64, opt *gitlab.EditGroupMemberOptions, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { - return c.MockEditMember(gid, user, opt) + return c.MockEditMember(gid, user, opt, options...) } // RemoveGroupMember calls the underlying MockRemoveMember method. diff --git a/pkg/cluster/controller/instance/serviceaccounts/zz_controller.go b/pkg/cluster/controller/instance/serviceaccounts/zz_controller.go index c2b26d78..91320ff3 100644 --- a/pkg/cluster/controller/instance/serviceaccounts/zz_controller.go +++ b/pkg/cluster/controller/instance/serviceaccounts/zz_controller.go @@ -183,7 +183,7 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex if minPermission != accessLevelNoAccess { minPermissionValue := getAccessLevelValue(minPermission) - notInGroups, wrongPermsGroups, err := e.fetchTopLevelGroupsMissingPermissions(minPermissionValue, serviceAccount.ID) + notInGroups, wrongPermsGroups, err := e.fetchTopLevelGroupsMissingPermissions(ctx, minPermissionValue, serviceAccount.ID) if err != nil { return managed.ExternalObservation{}, errors.Wrap(err, "cannot fetch Gitlab groups missing permissions for service account") } diff --git a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers.go b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers.go index 314a8617..ad2cc6ab 100644 --- a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers.go +++ b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers.go @@ -20,37 +20,26 @@ package serviceaccounts import ( "context" + "sort" "sync" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" gitlab "gitlab.com/gitlab-org/api/client-go/v2" + "golang.org/x/sync/errgroup" "k8s.io/utils/ptr" "github.com/crossplane-contrib/provider-gitlab/pkg/cluster/clients" ) +const baselinePermissionsMaxConcurrentRequests = 16 + // addServiceAccountToGroups is a helper function that adds the service account to the specified groups with the specified access level. This is used to satisfy the BaselinePermissions field by adding the service account to all groups that have at least the specified access level. func (e *external) addServiceAccountToGroups(ctx context.Context, serviceAccountID int64, groupIDs []int64, accessLevel int) error { - var addToGroupsWaitGroup sync.WaitGroup - var requestErr error - var requestErrMu sync.Mutex - - setRequestErr := func(err error) { - if err == nil { - return - } - requestErrMu.Lock() - if requestErr == nil { - requestErr = err - } - requestErrMu.Unlock() - } + g, ctx := errgroup.WithContext(ctx) + g.SetLimit(baselinePermissionsMaxConcurrentRequests) for _, groupID := range groupIDs { - addToGroupsWaitGroup.Add(1) - go func(groupID int64) { - defer addToGroupsWaitGroup.Done() - + g.Go(func() error { _, _, err := e.groupMemberClient.AddGroupMember( groupID, &gitlab.AddGroupMemberOptions{ @@ -60,38 +49,22 @@ func (e *external) addServiceAccountToGroups(ctx context.Context, serviceAccount gitlab.WithContext(ctx), ) if err != nil { - setRequestErr(errors.Wrapf(err, "cannot add service account to Gitlab group with ID %d", groupID)) - return + return errors.Wrapf(err, "cannot add service account to Gitlab group with ID %d", groupID) } - }(groupID) + return nil + }) } - addToGroupsWaitGroup.Wait() - return requestErr + return g.Wait() } // updateServiceAccountGroupPermissions is a helper function that updates the service account's permissions for the specified groups to the specified access level. This is used to satisfy the BaselinePermissions field by updating the service account's permissions for all groups that have at least the specified access level. func (e *external) updateServiceAccountGroupPermissions(ctx context.Context, serviceAccountID int64, groupIDs []int64, accessLevel int) error { - var updateGroupsWaitGroup sync.WaitGroup - var requestErr error - var requestErrMu sync.Mutex - - setRequestErr := func(err error) { - if err == nil { - return - } - requestErrMu.Lock() - if requestErr == nil { - requestErr = err - } - requestErrMu.Unlock() - } + g, ctx := errgroup.WithContext(ctx) + g.SetLimit(baselinePermissionsMaxConcurrentRequests) for _, groupID := range groupIDs { - updateGroupsWaitGroup.Add(1) - go func(groupID int64) { - defer updateGroupsWaitGroup.Done() - + g.Go(func() error { _, _, err := e.groupMemberClient.EditGroupMember( groupID, serviceAccountID, @@ -101,37 +74,25 @@ func (e *external) updateServiceAccountGroupPermissions(ctx context.Context, ser gitlab.WithContext(ctx), ) if err != nil { - setRequestErr(errors.Wrapf(err, "cannot update service account permissions for Gitlab group with ID %d", groupID)) - return + return errors.Wrapf(err, "cannot update service account permissions for Gitlab group with ID %d", groupID) } - }(groupID) + return nil + }) } - updateGroupsWaitGroup.Wait() - return requestErr + return g.Wait() } // fetchTopLevelGroupsMissingPermissions is a helper function that returns a list of top level group IDs that the service account is missing permissions for, based on the specified minimum permission level. This is used to determine which groups the service account needs to be added to in order to satisfy the BaselinePermissions field. // //nolint:gocyclo -func (e *external) fetchTopLevelGroupsMissingPermissions(minPermission int, serviceAccountID int64) (notInGroups []int64, wrongPermsGroups []int64, err error) { +func (e *external) fetchTopLevelGroupsMissingPermissions(ctx context.Context, minPermission int, serviceAccountID int64) (notInGroups []int64, wrongPermsGroups []int64, err error) { var ( - permissionsCheckWaitGroup sync.WaitGroup groupsMissingPermissionsMu sync.Mutex - requestErr error - requestErrMu sync.Mutex ) - setRequestErr := func(err error) { - if err == nil { - return - } - requestErrMu.Lock() - if requestErr == nil { - requestErr = err - } - requestErrMu.Unlock() - } + g, ctx := errgroup.WithContext(ctx) + g.SetLimit(baselinePermissionsMaxConcurrentRequests) for page := int64(1); ; { groups, resp, err := e.fetchTopLevelGroupsPage(page) @@ -140,14 +101,11 @@ func (e *external) fetchTopLevelGroupsMissingPermissions(minPermission int, serv } for _, group := range groups { - permissionsCheckWaitGroup.Add(1) - go func(groupID int64) { - defer permissionsCheckWaitGroup.Done() - - isMissingMembership, hasInsufficientPermissions, err := e.getGroupPermissionStatus(groupID, serviceAccountID, minPermission) + groupID := group.ID + g.Go(func() error { + isMissingMembership, hasInsufficientPermissions, err := e.getGroupPermissionStatus(ctx, groupID, serviceAccountID, minPermission) if err != nil { - setRequestErr(err) - return + return err } groupsMissingPermissionsMu.Lock() @@ -158,7 +116,8 @@ func (e *external) fetchTopLevelGroupsMissingPermissions(minPermission int, serv wrongPermsGroups = append(wrongPermsGroups, groupID) } groupsMissingPermissionsMu.Unlock() - }(group.ID) + return nil + }) } if resp.NextPage == 0 { @@ -167,11 +126,13 @@ func (e *external) fetchTopLevelGroupsMissingPermissions(minPermission int, serv page = resp.NextPage } - permissionsCheckWaitGroup.Wait() - if requestErr != nil { - return nil, nil, requestErr + if err := g.Wait(); err != nil { + return nil, nil, err } + sort.Slice(notInGroups, func(i, j int) bool { return notInGroups[i] < notInGroups[j] }) + sort.Slice(wrongPermsGroups, func(i, j int) bool { return wrongPermsGroups[i] < wrongPermsGroups[j] }) + return notInGroups, wrongPermsGroups, nil } @@ -193,8 +154,8 @@ func (e *external) fetchTopLevelGroupsPage(page int64) ([]*gitlab.Group, *gitlab return groups, resp, nil } -func (e *external) getGroupPermissionStatus(groupID int64, serviceAccountID int64, minPermission int) (isMissingMembership bool, hasInsufficientPermissions bool, err error) { - groupMember, resp, err := e.groupMemberClient.GetGroupMember(groupID, serviceAccountID) +func (e *external) getGroupPermissionStatus(ctx context.Context, groupID int64, serviceAccountID int64, minPermission int) (isMissingMembership bool, hasInsufficientPermissions bool, err error) { + groupMember, resp, err := e.groupMemberClient.GetGroupMember(groupID, serviceAccountID, gitlab.WithContext(ctx)) if err != nil { if clients.IsResponseNotFound(resp) { return true, false, nil diff --git a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go index cab9f707..1e14ad68 100644 --- a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go +++ b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go @@ -28,6 +28,7 @@ import ( xerrors "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/go-retryablehttp" gitlab "gitlab.com/gitlab-org/api/client-go/v2" groupsfake "github.com/crossplane-contrib/provider-gitlab/pkg/cluster/clients/groups/fake" @@ -60,6 +61,23 @@ func TestGetAccessLevelValue(t *testing.T) { } } +func requestContext(t *testing.T, options []gitlab.RequestOptionFunc) context.Context { + t.Helper() + if len(options) == 0 { + t.Fatal("expected request option") + } + req, err := retryablehttp.NewRequest(http.MethodGet, "https://example.com", nil) + if err != nil { + t.Fatalf("NewRequest() error: %v", err) + } + for _, option := range options { + if err := option(req); err != nil { + t.Fatalf("request option error: %v", err) + } + } + return req.Context() +} + func TestFetchTopLevelGroupsPage(t *testing.T) { client := &groupsfake.MockClient{ MockListGroups: func(opt *gitlab.ListGroupsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Group, *gitlab.Response, error) { @@ -134,7 +152,7 @@ func TestGetGroupPermissionStatus(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { e := &external{groupMemberClient: tc.memberClient} - missing, wrong, err := e.getGroupPermissionStatus(42, 99, accessLevelDeveloperValue) + missing, wrong, err := e.getGroupPermissionStatus(context.Background(), 42, 99, accessLevelDeveloperValue) if tc.wantErr == nil { if err != nil { t.Fatalf("getGroupPermissionStatus() unexpected error: %v", err) @@ -176,12 +194,12 @@ func TestFetchTopLevelGroupsMissingPermissions(t *testing.T) { }, MockGetMember: func(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { switch gid.(int64) { - case 1: - return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, errors.New("not found") - case 2: - return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil case 3: return &gitlab.GroupMember{AccessLevel: gitlab.AccessLevelValue(accessLevelReporterValue)}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + case 2: + return &gitlab.GroupMember{AccessLevel: gitlab.AccessLevelValue(accessLevelReporterValue)}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + case 1: + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, errors.New("not found") default: t.Fatalf("unexpected group id %v", gid) return nil, nil, nil @@ -191,14 +209,14 @@ func TestFetchTopLevelGroupsMissingPermissions(t *testing.T) { e := &external{groupsClient: client, groupMemberClient: client} - notIn, wrong, err := e.fetchTopLevelGroupsMissingPermissions(accessLevelDeveloperValue, 99) + notIn, wrong, err := e.fetchTopLevelGroupsMissingPermissions(context.Background(), accessLevelDeveloperValue, 99) if err != nil { t.Fatalf("fetchTopLevelGroupsMissingPermissions() unexpected error: %v", err) } - if diff := cmp.Diff([]int64{1}, notIn, cmpopts.SortSlices(func(a, b int64) bool { return a < b })); diff != "" { + if diff := cmp.Diff([]int64{1}, notIn); diff != "" { t.Fatalf("fetchTopLevelGroupsMissingPermissions() notInGroups mismatch (-want +got):\n%s", diff) } - if diff := cmp.Diff([]int64{2, 3}, wrong, cmpopts.SortSlices(func(a, b int64) bool { return a < b })); diff != "" { + if diff := cmp.Diff([]int64{2, 3}, wrong); diff != "" { t.Fatalf("fetchTopLevelGroupsMissingPermissions() wrongPermsGroups mismatch (-want +got):\n%s", diff) } @@ -209,6 +227,29 @@ func TestFetchTopLevelGroupsMissingPermissions(t *testing.T) { } } +func TestGetGroupPermissionStatusUsesContext(t *testing.T) { + ctxKey := struct{}{} + ctxValue := "observe-context" + client := &groupsfake.MockClient{ + MockGetMember: func(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { + if got := requestContext(t, options).Value(ctxKey); got != ctxValue { + t.Fatalf("getGroupPermissionStatus() context value = %v, want %v", got, ctxValue) + } + return &gitlab.GroupMember{AccessLevel: gitlab.AccessLevelValue(accessLevelMaintainerValue)}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + }, + } + e := &external{groupMemberClient: client} + ctx := context.WithValue(context.Background(), ctxKey, ctxValue) + + missing, wrong, err := e.getGroupPermissionStatus(ctx, 42, 99, accessLevelDeveloperValue) + if err != nil { + t.Fatalf("getGroupPermissionStatus() unexpected error: %v", err) + } + if missing || wrong { + t.Fatalf("getGroupPermissionStatus() = (%v, %v), want (false, false)", missing, wrong) + } +} + func TestAddServiceAccountToGroups(t *testing.T) { var ( callsMu sync.Mutex diff --git a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_test.go b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_test.go index a279427f..a2314a00 100644 --- a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_test.go +++ b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_test.go @@ -34,10 +34,12 @@ import ( pkgerrors "github.com/pkg/errors" gitlab "gitlab.com/gitlab-org/api/client-go/v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane-contrib/provider-gitlab/apis/cluster/instance/v1alpha1" commonv1alpha1 "github.com/crossplane-contrib/provider-gitlab/apis/common/v1alpha1" + groupsfake "github.com/crossplane-contrib/provider-gitlab/pkg/cluster/clients/groups/fake" "github.com/crossplane-contrib/provider-gitlab/pkg/cluster/clients/instance" ) @@ -228,11 +230,57 @@ func TestObserve(t *testing.T) { result: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: false, ResourceLateInitialized: false}, }, }, + "SuccessfulBaselinePermissionsSortsStatus": { + args: args{ + client: &MockClient{MockGetUser: func(user int64, opt gitlab.GetUsersOptions, options ...gitlab.RequestOptionFunc) (*gitlab.User, *gitlab.Response, error) { + return &gitlab.User{ID: 123, Name: testServiceAccountName, Username: testServiceAccountUsername, Email: testServiceAccountEmail}, &gitlab.Response{Response: &http.Response{StatusCode: 200}}, nil + }}, + cr: serviceAccount(withExternalName("123"), withSpec(v1alpha1.ServiceAccountParameters{ + CommonServiceAccountParameters: commonv1alpha1.CommonServiceAccountParameters{Name: sPtr(testServiceAccountName), Username: sPtr(testServiceAccountUsername), Email: sPtr(testServiceAccountEmail)}, + BaselinePermissions: ptr.To(accessLevelDeveloper), + })), + }, + want: want{ + cr: serviceAccount( + withExternalName("123"), + withSpec(v1alpha1.ServiceAccountParameters{ + CommonServiceAccountParameters: commonv1alpha1.CommonServiceAccountParameters{Name: sPtr(testServiceAccountName), Username: sPtr(testServiceAccountUsername), Email: sPtr(testServiceAccountEmail)}, + BaselinePermissions: ptr.To(accessLevelDeveloper), + }), + withConditions(xpv1.Available()), + func(r *v1alpha1.ServiceAccount) { + r.Status.AtProvider = instance.GenerateServiceAccountObservation(&gitlab.User{ID: 123, Name: testServiceAccountName, Username: testServiceAccountUsername, Email: testServiceAccountEmail}) + r.Status.AtProvider.MissingMemberShipGroups = []int64{1, 3} + r.Status.AtProvider.WrongPermissionsGroups = []int64{2, 4} + }, + ), + result: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: false, ResourceLateInitialized: false}, + }, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { e := &external{kube: tc.args.kube, client: tc.args.client} + if name == "SuccessfulBaselinePermissionsSortsStatus" { + e.groupsClient = &groupsfake.MockClient{MockListGroups: func(opt *gitlab.ListGroupsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Group, *gitlab.Response, error) { + return []*gitlab.Group{{ID: 4}, {ID: 1}, {ID: 2}, {ID: 3}}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}, NextPage: 0}, nil + }} + e.groupMemberClient = &groupsfake.MockClient{MockGetMember: func(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { + switch gid.(int64) { + case 4: + return &gitlab.GroupMember{AccessLevel: gitlab.AccessLevelValue(accessLevelReporterValue)}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + case 1: + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, pkgerrors.New("not found") + case 2: + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + case 3: + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, pkgerrors.New("not found") + default: + return nil, nil, pkgerrors.New("unexpected group") + } + }} + } got, err := e.Observe(context.Background(), tc.args.cr) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("Observe(): -want error, +got error:\n%s", diff) diff --git a/pkg/namespaced/clients/groups/fake/fake.go b/pkg/namespaced/clients/groups/fake/fake.go index 5c6856cd..4b7a24c3 100644 --- a/pkg/namespaced/clients/groups/fake/fake.go +++ b/pkg/namespaced/clients/groups/fake/fake.go @@ -118,17 +118,17 @@ func (c *MockClient) ListGroups(opt *gitlab.ListGroupsOptions, options ...gitlab // GetGroupMember calls the underlying MockGetMember method. func (c *MockClient) GetGroupMember(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { - return c.MockGetMember(gid, user) + return c.MockGetMember(gid, user, options...) } // AddGroupMember calls the underlying MockAddMember method. func (c *MockClient) AddGroupMember(gid interface{}, opt *gitlab.AddGroupMemberOptions, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { - return c.MockAddMember(gid, opt) + return c.MockAddMember(gid, opt, options...) } // EditGroupMember calls the underlying MockEditMember method. func (c *MockClient) EditGroupMember(gid interface{}, user int64, opt *gitlab.EditGroupMemberOptions, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { - return c.MockEditMember(gid, user, opt) + return c.MockEditMember(gid, user, opt, options...) } // RemoveGroupMember calls the underlying MockRemoveMember method. diff --git a/pkg/namespaced/controller/instance/serviceaccounts/controller.go b/pkg/namespaced/controller/instance/serviceaccounts/controller.go index 3b2bc224..77a39ab9 100644 --- a/pkg/namespaced/controller/instance/serviceaccounts/controller.go +++ b/pkg/namespaced/controller/instance/serviceaccounts/controller.go @@ -181,7 +181,7 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex if minPermission != accessLevelNoAccess { minPermissionValue := getAccessLevelValue(minPermission) - notInGroups, wrongPermsGroups, err := e.fetchTopLevelGroupsMissingPermissions(minPermissionValue, serviceAccount.ID) + notInGroups, wrongPermsGroups, err := e.fetchTopLevelGroupsMissingPermissions(ctx, minPermissionValue, serviceAccount.ID) if err != nil { return managed.ExternalObservation{}, errors.Wrap(err, "cannot fetch Gitlab groups missing permissions for service account") } diff --git a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers.go b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers.go index a68b2996..434cc3fb 100644 --- a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers.go +++ b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers.go @@ -18,37 +18,26 @@ package serviceaccounts import ( "context" + "sort" "sync" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" gitlab "gitlab.com/gitlab-org/api/client-go/v2" + "golang.org/x/sync/errgroup" "k8s.io/utils/ptr" "github.com/crossplane-contrib/provider-gitlab/pkg/namespaced/clients" ) +const baselinePermissionsMaxConcurrentRequests = 16 + // addServiceAccountToGroups is a helper function that adds the service account to the specified groups with the specified access level. This is used to satisfy the BaselinePermissions field by adding the service account to all groups that have at least the specified access level. func (e *external) addServiceAccountToGroups(ctx context.Context, serviceAccountID int64, groupIDs []int64, accessLevel int) error { - var addToGroupsWaitGroup sync.WaitGroup - var requestErr error - var requestErrMu sync.Mutex - - setRequestErr := func(err error) { - if err == nil { - return - } - requestErrMu.Lock() - if requestErr == nil { - requestErr = err - } - requestErrMu.Unlock() - } + g, ctx := errgroup.WithContext(ctx) + g.SetLimit(baselinePermissionsMaxConcurrentRequests) for _, groupID := range groupIDs { - addToGroupsWaitGroup.Add(1) - go func(groupID int64) { - defer addToGroupsWaitGroup.Done() - + g.Go(func() error { _, _, err := e.groupMemberClient.AddGroupMember( groupID, &gitlab.AddGroupMemberOptions{ @@ -58,38 +47,22 @@ func (e *external) addServiceAccountToGroups(ctx context.Context, serviceAccount gitlab.WithContext(ctx), ) if err != nil { - setRequestErr(errors.Wrapf(err, "cannot add service account to Gitlab group with ID %d", groupID)) - return + return errors.Wrapf(err, "cannot add service account to Gitlab group with ID %d", groupID) } - }(groupID) + return nil + }) } - addToGroupsWaitGroup.Wait() - return requestErr + return g.Wait() } // updateServiceAccountGroupPermissions is a helper function that updates the service account's permissions for the specified groups to the specified access level. This is used to satisfy the BaselinePermissions field by updating the service account's permissions for all groups that have at least the specified access level. func (e *external) updateServiceAccountGroupPermissions(ctx context.Context, serviceAccountID int64, groupIDs []int64, accessLevel int) error { - var updateGroupsWaitGroup sync.WaitGroup - var requestErr error - var requestErrMu sync.Mutex - - setRequestErr := func(err error) { - if err == nil { - return - } - requestErrMu.Lock() - if requestErr == nil { - requestErr = err - } - requestErrMu.Unlock() - } + g, ctx := errgroup.WithContext(ctx) + g.SetLimit(baselinePermissionsMaxConcurrentRequests) for _, groupID := range groupIDs { - updateGroupsWaitGroup.Add(1) - go func(groupID int64) { - defer updateGroupsWaitGroup.Done() - + g.Go(func() error { _, _, err := e.groupMemberClient.EditGroupMember( groupID, serviceAccountID, @@ -99,37 +72,25 @@ func (e *external) updateServiceAccountGroupPermissions(ctx context.Context, ser gitlab.WithContext(ctx), ) if err != nil { - setRequestErr(errors.Wrapf(err, "cannot update service account permissions for Gitlab group with ID %d", groupID)) - return + return errors.Wrapf(err, "cannot update service account permissions for Gitlab group with ID %d", groupID) } - }(groupID) + return nil + }) } - updateGroupsWaitGroup.Wait() - return requestErr + return g.Wait() } // fetchTopLevelGroupsMissingPermissions is a helper function that returns a list of top level group IDs that the service account is missing permissions for, based on the specified minimum permission level. This is used to determine which groups the service account needs to be added to in order to satisfy the BaselinePermissions field. // //nolint:gocyclo -func (e *external) fetchTopLevelGroupsMissingPermissions(minPermission int, serviceAccountID int64) (notInGroups []int64, wrongPermsGroups []int64, err error) { +func (e *external) fetchTopLevelGroupsMissingPermissions(ctx context.Context, minPermission int, serviceAccountID int64) (notInGroups []int64, wrongPermsGroups []int64, err error) { var ( - permissionsCheckWaitGroup sync.WaitGroup groupsMissingPermissionsMu sync.Mutex - requestErr error - requestErrMu sync.Mutex ) - setRequestErr := func(err error) { - if err == nil { - return - } - requestErrMu.Lock() - if requestErr == nil { - requestErr = err - } - requestErrMu.Unlock() - } + g, ctx := errgroup.WithContext(ctx) + g.SetLimit(baselinePermissionsMaxConcurrentRequests) for page := int64(1); ; { groups, resp, err := e.fetchTopLevelGroupsPage(page) @@ -138,14 +99,11 @@ func (e *external) fetchTopLevelGroupsMissingPermissions(minPermission int, serv } for _, group := range groups { - permissionsCheckWaitGroup.Add(1) - go func(groupID int64) { - defer permissionsCheckWaitGroup.Done() - - isMissingMembership, hasInsufficientPermissions, err := e.getGroupPermissionStatus(groupID, serviceAccountID, minPermission) + groupID := group.ID + g.Go(func() error { + isMissingMembership, hasInsufficientPermissions, err := e.getGroupPermissionStatus(ctx, groupID, serviceAccountID, minPermission) if err != nil { - setRequestErr(err) - return + return err } groupsMissingPermissionsMu.Lock() @@ -156,7 +114,8 @@ func (e *external) fetchTopLevelGroupsMissingPermissions(minPermission int, serv wrongPermsGroups = append(wrongPermsGroups, groupID) } groupsMissingPermissionsMu.Unlock() - }(group.ID) + return nil + }) } if resp.NextPage == 0 { @@ -165,11 +124,13 @@ func (e *external) fetchTopLevelGroupsMissingPermissions(minPermission int, serv page = resp.NextPage } - permissionsCheckWaitGroup.Wait() - if requestErr != nil { - return nil, nil, requestErr + if err := g.Wait(); err != nil { + return nil, nil, err } + sort.Slice(notInGroups, func(i, j int) bool { return notInGroups[i] < notInGroups[j] }) + sort.Slice(wrongPermsGroups, func(i, j int) bool { return wrongPermsGroups[i] < wrongPermsGroups[j] }) + return notInGroups, wrongPermsGroups, nil } @@ -191,8 +152,8 @@ func (e *external) fetchTopLevelGroupsPage(page int64) ([]*gitlab.Group, *gitlab return groups, resp, nil } -func (e *external) getGroupPermissionStatus(groupID int64, serviceAccountID int64, minPermission int) (isMissingMembership bool, hasInsufficientPermissions bool, err error) { - groupMember, resp, err := e.groupMemberClient.GetGroupMember(groupID, serviceAccountID) +func (e *external) getGroupPermissionStatus(ctx context.Context, groupID int64, serviceAccountID int64, minPermission int) (isMissingMembership bool, hasInsufficientPermissions bool, err error) { + groupMember, resp, err := e.groupMemberClient.GetGroupMember(groupID, serviceAccountID, gitlab.WithContext(ctx)) if err != nil { if clients.IsResponseNotFound(resp) { return true, false, nil diff --git a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go index 7f776766..68b79da8 100644 --- a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go +++ b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go @@ -26,6 +26,7 @@ import ( xerrors "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/hashicorp/go-retryablehttp" gitlab "gitlab.com/gitlab-org/api/client-go/v2" groupsfake "github.com/crossplane-contrib/provider-gitlab/pkg/namespaced/clients/groups/fake" @@ -58,6 +59,23 @@ func TestGetAccessLevelValue(t *testing.T) { } } +func requestContext(t *testing.T, options []gitlab.RequestOptionFunc) context.Context { + t.Helper() + if len(options) == 0 { + t.Fatal("expected request option") + } + req, err := retryablehttp.NewRequest(http.MethodGet, "https://example.com", nil) + if err != nil { + t.Fatalf("NewRequest() error: %v", err) + } + for _, option := range options { + if err := option(req); err != nil { + t.Fatalf("request option error: %v", err) + } + } + return req.Context() +} + func TestFetchTopLevelGroupsPage(t *testing.T) { client := &groupsfake.MockClient{ MockListGroups: func(opt *gitlab.ListGroupsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Group, *gitlab.Response, error) { @@ -132,7 +150,7 @@ func TestGetGroupPermissionStatus(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { e := &external{groupMemberClient: tc.memberClient} - missing, wrong, err := e.getGroupPermissionStatus(42, 99, accessLevelDeveloperValue) + missing, wrong, err := e.getGroupPermissionStatus(context.Background(), 42, 99, accessLevelDeveloperValue) if tc.wantErr == nil { if err != nil { t.Fatalf("getGroupPermissionStatus() unexpected error: %v", err) @@ -174,12 +192,12 @@ func TestFetchTopLevelGroupsMissingPermissions(t *testing.T) { }, MockGetMember: func(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { switch gid.(int64) { - case 1: - return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, errors.New("not found") - case 2: - return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil case 3: return &gitlab.GroupMember{AccessLevel: gitlab.AccessLevelValue(accessLevelReporterValue)}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + case 2: + return &gitlab.GroupMember{AccessLevel: gitlab.AccessLevelValue(accessLevelReporterValue)}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + case 1: + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, errors.New("not found") default: t.Fatalf("unexpected group id %v", gid) return nil, nil, nil @@ -189,14 +207,14 @@ func TestFetchTopLevelGroupsMissingPermissions(t *testing.T) { e := &external{groupsClient: client, groupMemberClient: client} - notIn, wrong, err := e.fetchTopLevelGroupsMissingPermissions(accessLevelDeveloperValue, 99) + notIn, wrong, err := e.fetchTopLevelGroupsMissingPermissions(context.Background(), accessLevelDeveloperValue, 99) if err != nil { t.Fatalf("fetchTopLevelGroupsMissingPermissions() unexpected error: %v", err) } - if diff := cmp.Diff([]int64{1}, notIn, cmpopts.SortSlices(func(a, b int64) bool { return a < b })); diff != "" { + if diff := cmp.Diff([]int64{1}, notIn); diff != "" { t.Fatalf("fetchTopLevelGroupsMissingPermissions() notInGroups mismatch (-want +got):\n%s", diff) } - if diff := cmp.Diff([]int64{2, 3}, wrong, cmpopts.SortSlices(func(a, b int64) bool { return a < b })); diff != "" { + if diff := cmp.Diff([]int64{2, 3}, wrong); diff != "" { t.Fatalf("fetchTopLevelGroupsMissingPermissions() wrongPermsGroups mismatch (-want +got):\n%s", diff) } @@ -207,6 +225,29 @@ func TestFetchTopLevelGroupsMissingPermissions(t *testing.T) { } } +func TestGetGroupPermissionStatusUsesContext(t *testing.T) { + ctxKey := struct{}{} + ctxValue := "observe-context" + client := &groupsfake.MockClient{ + MockGetMember: func(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { + if got := requestContext(t, options).Value(ctxKey); got != ctxValue { + t.Fatalf("getGroupPermissionStatus() context value = %v, want %v", got, ctxValue) + } + return &gitlab.GroupMember{AccessLevel: gitlab.AccessLevelValue(accessLevelMaintainerValue)}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + }, + } + e := &external{groupMemberClient: client} + ctx := context.WithValue(context.Background(), ctxKey, ctxValue) + + missing, wrong, err := e.getGroupPermissionStatus(ctx, 42, 99, accessLevelDeveloperValue) + if err != nil { + t.Fatalf("getGroupPermissionStatus() unexpected error: %v", err) + } + if missing || wrong { + t.Fatalf("getGroupPermissionStatus() = (%v, %v), want (false, false)", missing, wrong) + } +} + func TestAddServiceAccountToGroups(t *testing.T) { var ( callsMu sync.Mutex diff --git a/pkg/namespaced/controller/instance/serviceaccounts/controller_test.go b/pkg/namespaced/controller/instance/serviceaccounts/controller_test.go index 7d594d83..2af8258b 100644 --- a/pkg/namespaced/controller/instance/serviceaccounts/controller_test.go +++ b/pkg/namespaced/controller/instance/serviceaccounts/controller_test.go @@ -32,10 +32,12 @@ import ( pkgerrors "github.com/pkg/errors" gitlab "gitlab.com/gitlab-org/api/client-go/v2" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" commonv1alpha1 "github.com/crossplane-contrib/provider-gitlab/apis/common/v1alpha1" "github.com/crossplane-contrib/provider-gitlab/apis/namespaced/instance/v1alpha1" + groupsfake "github.com/crossplane-contrib/provider-gitlab/pkg/namespaced/clients/groups/fake" "github.com/crossplane-contrib/provider-gitlab/pkg/namespaced/clients/instance" ) @@ -226,11 +228,57 @@ func TestObserve(t *testing.T) { result: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: false, ResourceLateInitialized: false}, }, }, + "SuccessfulBaselinePermissionsSortsStatus": { + args: args{ + client: &MockClient{MockGetUser: func(user int64, opt gitlab.GetUsersOptions, options ...gitlab.RequestOptionFunc) (*gitlab.User, *gitlab.Response, error) { + return &gitlab.User{ID: 123, Name: testServiceAccountName, Username: testServiceAccountUsername, Email: testServiceAccountEmail}, &gitlab.Response{Response: &http.Response{StatusCode: 200}}, nil + }}, + cr: serviceAccount(withExternalName("123"), withSpec(v1alpha1.ServiceAccountParameters{ + CommonServiceAccountParameters: commonv1alpha1.CommonServiceAccountParameters{Name: sPtr(testServiceAccountName), Username: sPtr(testServiceAccountUsername), Email: sPtr(testServiceAccountEmail)}, + BaselinePermissions: ptr.To(accessLevelDeveloper), + })), + }, + want: want{ + cr: serviceAccount( + withExternalName("123"), + withSpec(v1alpha1.ServiceAccountParameters{ + CommonServiceAccountParameters: commonv1alpha1.CommonServiceAccountParameters{Name: sPtr(testServiceAccountName), Username: sPtr(testServiceAccountUsername), Email: sPtr(testServiceAccountEmail)}, + BaselinePermissions: ptr.To(accessLevelDeveloper), + }), + withConditions(xpv1.Available()), + func(r *v1alpha1.ServiceAccount) { + r.Status.AtProvider = instance.GenerateServiceAccountObservation(&gitlab.User{ID: 123, Name: testServiceAccountName, Username: testServiceAccountUsername, Email: testServiceAccountEmail}) + r.Status.AtProvider.MissingMemberShipGroups = []int64{1, 3} + r.Status.AtProvider.WrongPermissionsGroups = []int64{2, 4} + }, + ), + result: managed.ExternalObservation{ResourceExists: true, ResourceUpToDate: false, ResourceLateInitialized: false}, + }, + }, } for name, tc := range cases { t.Run(name, func(t *testing.T) { e := &external{kube: tc.args.kube, client: tc.args.client} + if name == "SuccessfulBaselinePermissionsSortsStatus" { + e.groupsClient = &groupsfake.MockClient{MockListGroups: func(opt *gitlab.ListGroupsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Group, *gitlab.Response, error) { + return []*gitlab.Group{{ID: 4}, {ID: 1}, {ID: 2}, {ID: 3}}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}, NextPage: 0}, nil + }} + e.groupMemberClient = &groupsfake.MockClient{MockGetMember: func(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { + switch gid.(int64) { + case 4: + return &gitlab.GroupMember{AccessLevel: gitlab.AccessLevelValue(accessLevelReporterValue)}, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + case 1: + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, pkgerrors.New("not found") + case 2: + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusOK}}, nil + case 3: + return nil, &gitlab.Response{Response: &http.Response{StatusCode: http.StatusNotFound}}, pkgerrors.New("not found") + default: + return nil, nil, pkgerrors.New("unexpected group") + } + }} + } got, err := e.Observe(context.Background(), tc.args.cr) if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" { t.Errorf("Observe(): -want error, +got error:\n%s", diff) From 16ca26db2e4bfc0ec4861d0f1f410c50f6097860 Mon Sep 17 00:00:00 2001 From: Henry Sachs Date: Tue, 26 May 2026 15:28:25 +0200 Subject: [PATCH 2/3] fix: tidy service account baseline permission deps Signed-off-by: Henry Sachs --- go.mod | 2 +- .../instance/serviceaccounts/zz_controller_helpers.go | 2 -- .../instance/serviceaccounts/zz_controller_helpers_test.go | 4 +++- .../controller/instance/serviceaccounts/controller_helpers.go | 2 -- .../instance/serviceaccounts/controller_helpers_test.go | 4 +++- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index 0edc970c..0e3f6c4f 100644 --- a/go.mod +++ b/go.mod @@ -60,7 +60,7 @@ require ( github.com/google/gnostic-models v0.7.1 // indirect github.com/google/go-querystring v1.2.0 // indirect github.com/google/uuid v1.6.0 // indirect - github.com/hashicorp/go-retryablehttp v0.7.8 // indirect + github.com/hashicorp/go-retryablehttp v0.7.8 github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/mattn/go-colorable v0.1.14 // indirect diff --git a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers.go b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers.go index ad2cc6ab..9dd010de 100644 --- a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers.go +++ b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers.go @@ -84,8 +84,6 @@ func (e *external) updateServiceAccountGroupPermissions(ctx context.Context, ser } // fetchTopLevelGroupsMissingPermissions is a helper function that returns a list of top level group IDs that the service account is missing permissions for, based on the specified minimum permission level. This is used to determine which groups the service account needs to be added to in order to satisfy the BaselinePermissions field. -// -//nolint:gocyclo func (e *external) fetchTopLevelGroupsMissingPermissions(ctx context.Context, minPermission int, serviceAccountID int64) (notInGroups []int64, wrongPermsGroups []int64, err error) { var ( groupsMissingPermissionsMu sync.Mutex diff --git a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go index 1e14ad68..8c1d07e3 100644 --- a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go +++ b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go @@ -78,6 +78,8 @@ func requestContext(t *testing.T, options []gitlab.RequestOptionFunc) context.Co return req.Context() } +type observeContextKey struct{} + func TestFetchTopLevelGroupsPage(t *testing.T) { client := &groupsfake.MockClient{ MockListGroups: func(opt *gitlab.ListGroupsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Group, *gitlab.Response, error) { @@ -228,7 +230,7 @@ func TestFetchTopLevelGroupsMissingPermissions(t *testing.T) { } func TestGetGroupPermissionStatusUsesContext(t *testing.T) { - ctxKey := struct{}{} + ctxKey := observeContextKey{} ctxValue := "observe-context" client := &groupsfake.MockClient{ MockGetMember: func(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { diff --git a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers.go b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers.go index 434cc3fb..acbcdf3b 100644 --- a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers.go +++ b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers.go @@ -82,8 +82,6 @@ func (e *external) updateServiceAccountGroupPermissions(ctx context.Context, ser } // fetchTopLevelGroupsMissingPermissions is a helper function that returns a list of top level group IDs that the service account is missing permissions for, based on the specified minimum permission level. This is used to determine which groups the service account needs to be added to in order to satisfy the BaselinePermissions field. -// -//nolint:gocyclo func (e *external) fetchTopLevelGroupsMissingPermissions(ctx context.Context, minPermission int, serviceAccountID int64) (notInGroups []int64, wrongPermsGroups []int64, err error) { var ( groupsMissingPermissionsMu sync.Mutex diff --git a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go index 68b79da8..d617f71a 100644 --- a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go +++ b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go @@ -76,6 +76,8 @@ func requestContext(t *testing.T, options []gitlab.RequestOptionFunc) context.Co return req.Context() } +type observeContextKey struct{} + func TestFetchTopLevelGroupsPage(t *testing.T) { client := &groupsfake.MockClient{ MockListGroups: func(opt *gitlab.ListGroupsOptions, options ...gitlab.RequestOptionFunc) ([]*gitlab.Group, *gitlab.Response, error) { @@ -226,7 +228,7 @@ func TestFetchTopLevelGroupsMissingPermissions(t *testing.T) { } func TestGetGroupPermissionStatusUsesContext(t *testing.T) { - ctxKey := struct{}{} + ctxKey := observeContextKey{} ctxValue := "observe-context" client := &groupsfake.MockClient{ MockGetMember: func(gid interface{}, user int64, options ...gitlab.RequestOptionFunc) (*gitlab.GroupMember, *gitlab.Response, error) { From 1f653c2733f1b3bb30958dfc802c57b3af49e270 Mon Sep 17 00:00:00 2001 From: Markus Siebert Date: Tue, 7 Jul 2026 12:16:42 +0200 Subject: [PATCH 3/3] fix: resolve rebase conflicts and adapt to client-go v2 Rebase #313 onto current master: - Update gitlab client imports from client-go to client-go/v2 in the instance serviceaccounts helpers/tests (namespaced + regenerated cluster). - Fix the new baseline-permissions test to the v2 API: *gitlab.GetUserOptions (was gitlab.GetUsersOptions) and v2.Available() (was xpv1.Available()); drop the now-unused cmpopts import. - Keep golang.org/x/sync as a direct dependency (errgroup) and master's newer golang.org/x/* versions. - Regenerate the cluster zz_ mirror. Signed-off-by: Markus Siebert --- go.mod | 2 +- .../instance/serviceaccounts/zz_controller_helpers_test.go | 1 - .../controller/instance/serviceaccounts/zz_controller_test.go | 4 ++-- .../instance/serviceaccounts/controller_helpers_test.go | 1 - .../controller/instance/serviceaccounts/controller_test.go | 4 ++-- 5 files changed, 5 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index 0e3f6c4f..679dcd11 100644 --- a/go.mod +++ b/go.mod @@ -82,7 +82,7 @@ require ( go.uber.org/multierr v1.11.0 // indirect golang.org/x/mod v0.36.0 // indirect golang.org/x/net v0.55.0 // indirect - golang.org/x/sync v0.21.0 // indirect + golang.org/x/sync v0.21.0 golang.org/x/sys v0.45.0 // indirect golang.org/x/term v0.43.0 // indirect golang.org/x/text v0.38.0 // indirect diff --git a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go index 8c1d07e3..bc3351f2 100644 --- a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go +++ b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_helpers_test.go @@ -27,7 +27,6 @@ import ( xerrors "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/go-retryablehttp" gitlab "gitlab.com/gitlab-org/api/client-go/v2" diff --git a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_test.go b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_test.go index a2314a00..04342692 100644 --- a/pkg/cluster/controller/instance/serviceaccounts/zz_controller_test.go +++ b/pkg/cluster/controller/instance/serviceaccounts/zz_controller_test.go @@ -232,7 +232,7 @@ func TestObserve(t *testing.T) { }, "SuccessfulBaselinePermissionsSortsStatus": { args: args{ - client: &MockClient{MockGetUser: func(user int64, opt gitlab.GetUsersOptions, options ...gitlab.RequestOptionFunc) (*gitlab.User, *gitlab.Response, error) { + client: &MockClient{MockGetUser: func(user int64, opt *gitlab.GetUserOptions, options ...gitlab.RequestOptionFunc) (*gitlab.User, *gitlab.Response, error) { return &gitlab.User{ID: 123, Name: testServiceAccountName, Username: testServiceAccountUsername, Email: testServiceAccountEmail}, &gitlab.Response{Response: &http.Response{StatusCode: 200}}, nil }}, cr: serviceAccount(withExternalName("123"), withSpec(v1alpha1.ServiceAccountParameters{ @@ -247,7 +247,7 @@ func TestObserve(t *testing.T) { CommonServiceAccountParameters: commonv1alpha1.CommonServiceAccountParameters{Name: sPtr(testServiceAccountName), Username: sPtr(testServiceAccountUsername), Email: sPtr(testServiceAccountEmail)}, BaselinePermissions: ptr.To(accessLevelDeveloper), }), - withConditions(xpv1.Available()), + withConditions(v2.Available()), func(r *v1alpha1.ServiceAccount) { r.Status.AtProvider = instance.GenerateServiceAccountObservation(&gitlab.User{ID: 123, Name: testServiceAccountName, Username: testServiceAccountUsername, Email: testServiceAccountEmail}) r.Status.AtProvider.MissingMemberShipGroups = []int64{1, 3} diff --git a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go index d617f71a..7a5b3244 100644 --- a/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go +++ b/pkg/namespaced/controller/instance/serviceaccounts/controller_helpers_test.go @@ -25,7 +25,6 @@ import ( xerrors "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" "github.com/hashicorp/go-retryablehttp" gitlab "gitlab.com/gitlab-org/api/client-go/v2" diff --git a/pkg/namespaced/controller/instance/serviceaccounts/controller_test.go b/pkg/namespaced/controller/instance/serviceaccounts/controller_test.go index 2af8258b..b5363287 100644 --- a/pkg/namespaced/controller/instance/serviceaccounts/controller_test.go +++ b/pkg/namespaced/controller/instance/serviceaccounts/controller_test.go @@ -230,7 +230,7 @@ func TestObserve(t *testing.T) { }, "SuccessfulBaselinePermissionsSortsStatus": { args: args{ - client: &MockClient{MockGetUser: func(user int64, opt gitlab.GetUsersOptions, options ...gitlab.RequestOptionFunc) (*gitlab.User, *gitlab.Response, error) { + client: &MockClient{MockGetUser: func(user int64, opt *gitlab.GetUserOptions, options ...gitlab.RequestOptionFunc) (*gitlab.User, *gitlab.Response, error) { return &gitlab.User{ID: 123, Name: testServiceAccountName, Username: testServiceAccountUsername, Email: testServiceAccountEmail}, &gitlab.Response{Response: &http.Response{StatusCode: 200}}, nil }}, cr: serviceAccount(withExternalName("123"), withSpec(v1alpha1.ServiceAccountParameters{ @@ -245,7 +245,7 @@ func TestObserve(t *testing.T) { CommonServiceAccountParameters: commonv1alpha1.CommonServiceAccountParameters{Name: sPtr(testServiceAccountName), Username: sPtr(testServiceAccountUsername), Email: sPtr(testServiceAccountEmail)}, BaselinePermissions: ptr.To(accessLevelDeveloper), }), - withConditions(xpv1.Available()), + withConditions(v2.Available()), func(r *v1alpha1.ServiceAccount) { r.Status.AtProvider = instance.GenerateServiceAccountObservation(&gitlab.User{ID: 123, Name: testServiceAccountName, Username: testServiceAccountUsername, Email: testServiceAccountEmail}) r.Status.AtProvider.MissingMemberShipGroups = []int64{1, 3}