Skip to content

fix: bound service account baseline permission checks#313

Merged
markussiebert merged 3 commits into
masterfrom
fix/service-account-baseline-permissions-reconcile
Jul 7, 2026
Merged

fix: bound service account baseline permission checks#313
markussiebert merged 3 commits into
masterfrom
fix/service-account-baseline-permissions-reconcile

Conversation

@henrysachs

Copy link
Copy Markdown
Collaborator

Summary

Follow-up for the baselinePermissions reconcile path added in #302.

This change makes the instance service account baseline permission flow safer and more stable under larger GitLab installations.

  • bound per-group baseline permission API work with errgroup and a concurrency limit of 16
  • propagate reconcile ctx into observe-side group membership checks
  • sort missing and wrong-permission group IDs before persisting status
  • forward request options in group fake clients so helper tests can assert ctx propagation
  • add focused tests for deterministic status ordering and ctx-aware observe calls in namespaced + cluster controllers

Closes #310

Testing

  • go test ./pkg/namespaced/controller/instance/serviceaccounts ./pkg/cluster/controller/instance/serviceaccounts

henrysachs and others added 3 commits July 7, 2026 12:06
Signed-off-by: Henry Sachs <henry.sachs@deutschebahn.com>
Signed-off-by: Henry Sachs <henry.sachs@deutschebahn.com>
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 <markus.siebert@deutschebahn.com>
@markussiebert markussiebert force-pushed the fix/service-account-baseline-permissions-reconcile branch from c4f6089 to 1f653c2 Compare July 7, 2026 10:16
@markussiebert

Copy link
Copy Markdown
Collaborator

Rebased onto current master and resolved the conflicts (the branch predated the client-go/v2 migration and the Go 1.26 bump):

  • gitlab imports client-goclient-go/v2 in the helpers/tests (namespaced + regenerated cluster mirror)
  • test fixes to the v2 API: *gitlab.GetUserOptions (was gitlab.GetUsersOptions), v2.Available() (was xpv1.Available()), removed a now-unused cmpopts import
  • golang.org/x/sync kept as a direct dep (errgroup), master's newer golang.org/x/* versions retained
  • go.mod tidied, cluster zz_ files regenerated

go build ./..., full go test ./..., and golangci-lint v2.12.2 all pass locally on Go 1.26.4.

Review: the change looks good — bounded concurrency via errgroup (limit 16), mutex-guarded aggregation, g.Wait() before sort.Slice for deterministic status ordering, and ctx propagated into the per-group membership checks. One minor, non-blocking note: fetchTopLevelGroupsPage (ListGroups) doesn't propagate ctx, so a cancelled reconcile won't interrupt the group-listing pagination — could be a small follow-up.

@markussiebert markussiebert merged commit 00e146a into master Jul 7, 2026
8 checks passed
@markussiebert markussiebert deleted the fix/service-account-baseline-permissions-reconcile branch July 7, 2026 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: bound baselinePermissions reconcile concurrency for instance service accounts

3 participants