Skip to content

feat(projects): support contributor registry for default project role rules#6034

Merged
EronWright merged 1 commit intomainfrom
EronWright/issue-460-project-role-contributors
Apr 24, 2026
Merged

feat(projects): support contributor registry for default project role rules#6034
EronWright merged 1 commit intomainfrom
EronWright/issue-460-project-role-contributors

Conversation

@EronWright
Copy link
Copy Markdown
Contributor

@EronWright EronWright commented Apr 2, 2026

Summary

  • Adds GetAll() to the PredicateBasedRegistry interface (and listBasedRegistry implementation) in pkg/component, enabling "apply all matching" semantics in addition to the existing "find first" semantics.
  • Introduces a RoleRulesContributorRegistration registry in pkg/controller/management/projects that allows callers to inject additional PolicyRules into the default project roles (kargo-admin, kargo-viewer, kargo-promoter) at startup time.
  • The ensureDefaultUserRoles function applies all registered contributors when creating default roles for a new project.

Companion EE PR: akuityio/kargo-enterprise#460

Test plan

  • Unit tests for GetAll() in pkg/component/list_based_registry_test.go
  • Unit tests for contributor mechanism in pkg/controller/management/projects/projects_test.go

… rules

Add a `RoleRulesContributorRegistry` to the project reconciler that allows
downstream distributions (e.g. Kargo EE) to inject additional PolicyRules
into the default project roles (kargo-admin, kargo-viewer, kargo-promoter)
at startup time, without needing to modify the core reconciler.

The registry uses the existing `pkg/component.PredicateBasedRegistry`
pattern. To support "apply all matching" semantics (vs. the existing
"find first" semantics), a `GetAll()` method is added to the
`PredicateBasedRegistry` interface and `listBasedRegistry` implementation.

Callers register a `RoleRulesContributorRegistration` before calling
`SetupReconcilerWithManager`. The reconciler applies all matching
contributors when creating default roles for a new project.

Part of: akuityio/kargo-enterprise#460

Signed-off-by: Eron Wright <eron.wright@akuity.io>
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 2, 2026

Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit fe40d89
🔍 Latest deploy log https://app.netlify.com/projects/docs-kargo-io/deploys/69ceb7f18b89ec000839a8af
😎 Deploy Preview https://deploy-preview-6034.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 91.30435% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.07%. Comparing base (8fdd7f4) to head (fe40d89).
⚠️ Report is 55 commits behind head on main.

Files with missing lines Patch % Lines
...ller/management/projects/role_rules_contributor.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6034      +/-   ##
==========================================
+ Coverage   57.03%   57.07%   +0.03%     
==========================================
  Files         463      464       +1     
  Lines       39112    39135      +23     
==========================================
+ Hits        22309    22335      +26     
+ Misses      15474    15472       -2     
+ Partials     1329     1328       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@EronWright EronWright marked this pull request as ready for review April 2, 2026 18:59
@EronWright EronWright requested a review from a team as a code owner April 2, 2026 18:59
@krancour krancour added this to the v1.10.0 milestone Apr 2, 2026
@krancour krancour added priority/normal This is the priority for most work area/management-controller Affects the controller that manages the Kargo control plane itself kind/enhancement An entirely new feature labels Apr 2, 2026
@EronWright EronWright requested a review from krancour April 2, 2026 19:30
@krancour krancour modified the milestones: v1.10.0, v1.11.0 Apr 2, 2026
Copy link
Copy Markdown
Member

@fuskovic fuskovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think GetMatches is a more suitable name than GetAll but not a hill I wanna die on.

@krancour
Copy link
Copy Markdown
Member

@EronWright can this replace ensureExtendedPermissions()?

@EronWright
Copy link
Copy Markdown
Contributor Author

Yes — confirmed effective, and I'd like to handle it in a follow-up PR to keep the blast radius small here.

The follow-up would introduce two additional registries modeled on the same PredicateBasedRegistry + GetAll() pattern, with *kargoapi.Project as the predicate arg (so a contributor can opt out for certain projects if needed):

ProjectSetupContributorRegistration  // called during reconciliation
ProjectCleanupContributorRegistration // called on deletion

ensureExtendedPermissions moves to the EE as a ProjectSetupContributorRegistration, and the Argo CD CRB deletion becomes a ProjectCleanupContributorRegistration. All the EE-specific fields currently in ReconcilerConfig go away from OSS.

One ordering note worth capturing: setup contributors run after the OSS base resources are created (OSS → EE), while cleanup contributors run before the OSS base resources are deleted (EE → OSS), so that EE resources referencing OSS ClusterRoles are removed first. That symmetry wasn't present in the original cleanupProject (which used a map, giving non-deterministic order).

@EronWright EronWright added this pull request to the merge queue Apr 24, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 24, 2026
]
)

var defaultRoleRulesContributorRegistry = component.MustNewPredicateBasedRegistry[
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that's a new registry, why use list with predicates and function with lookup by name instead of map registry?
Am I missing something?

Copy link
Copy Markdown
Contributor Author

@EronWright EronWright Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main thing is that there's numerous registered contributors for a given key, and you want all of the contributors.

@EronWright EronWright added this pull request to the merge queue Apr 24, 2026
Merged via the queue into main with commit ab35e66 Apr 24, 2026
29 checks passed
@EronWright EronWright deleted the EronWright/issue-460-project-role-contributors branch April 24, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/management-controller Affects the controller that manages the Kargo control plane itself kind/enhancement An entirely new feature priority/normal This is the priority for most work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants