fix(rbac): enumerate the github-config tenant Role's managed-resource kinds - #3004
Conversation
Replace the resources wildcard on the tenant Role with the kinds the ManagedResourceActivationPolicy activates. Part of #2787
Add a parity check asserting the tenant Role grants exactly the managed-resource kinds the ManagedResourceActivationPolicy activates, in both directions, and wire it into the k8s-gated CI job. Part of #2787
✅
|
The tenant Role narrowing moves the aggregate rendered-surface fingerprint. Re-measured per the constant's own re-approval ritual: 515 documents both sides, membership set difference zero in both directions, 133 grant-bearing documents unchanged, every aws-bearing line byte-identical. Part of #2787
Verification recordRED → GREEN
The guard was ablated, not just run greenEach arm changed exactly one thing and fired its own named assertion; the restored tree returned
The glob is end-anchored and verified per-case: Exercised as a user, against live prodThe tenant's Flux Kustomization inventory holds 10 distinct group/kind combinations — every one Authorization pin re-approvalThe surface fingerprint moved, as designed. Re-measured to the standard the constant's own note The consuming CI job ran rather than skipped: DevExThe Role and the activation policy already had to change together — the policy's comments assert the |
CI red here is a GitHub Actions incident, not this change. All four failing jobs at head githubstatus.com confirms an active partial outage of GitHub Actions (opened 15:22Z; at 15:45Z I re-ran the failed jobs once, then stopped — re-running into an active outage is futile and the Next tick: confirm the incident is resolved, re-run whatever is still red, and only then request a Self-review of the diff is done and found no defect. One thing I checked explicitly, because it |
The 9 red checks at
|
Self-reviewed at this head, and the central invariant is verified behaviourally against the live
The No P0/P1 from my own pass. Note that every check on this head is @coderabbitai review |
|
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughThe GitHub Config Role replaces wildcard GitHub API resources with explicit managed-resource kinds and retains explicit ProviderConfig access. A Bash test compares Role permissions with the managed-resource activation policy and fails on invalid or mismatched sets. CI runs ShellCheck and the parity test for Kubernetes changes. The approved authorization-surface fingerprint is updated. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Readiness recordTried and evaluated against the live cluster. Re-ran the parity claim directly against returns 10 activated GitHub managed-resource kinds. The Role in this PR enumerates 10, and
No activated kind is ungranted, so the wildcard removal cannot cost the tenant an apply. The one red check is infrastructure, not this change. Review: CodeRabbit completed at this head with no actionable findings and 5/5 pre-merge checks. |
@coderabbitai review |
|
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
CodeRabbit and Codex are both quota-limited at this head. Requesting Cursor Bugbot next, per lane priority. The bare trigger follows in the next comment (Bugbot exact-matches the whole comment body). |
|
@cursor review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_619f58af-79a9-4437-99e1-33ec34dc9cbe) |
devantler
left a comment
devantler
left a comment
There was a problem hiding this comment.
🤖 Generated by the Agentic Engineer
Self-review (fallback — CodeRabbit, Codex and Cursor Bugbot unavailable)
Reviewed commit: 769f4021e35480216e132fd9009640c430c91e09
All three external lanes were requested at this head, in priority order, and none delivered a review.
Each is a provider quota state, not a verdict on this change, and none states a usable retry window:
- CodeRabbit —
Review rate limited; its comment reads "you've reached your PR review limit, so we couldn't start this review" (00:38Z). - Codex — "You have reached your Codex usage limits for code reviews" (00:44Z).
- Cursor Bugbot — check-run
neutralwithoutput.title: Error, comment "Bugbot couldn't run - usage limit reached" (00:45Z). TheErrortitle means the run never started, as distinct from a review that found nothing.
What I checked, behaviourally
Parity against the live cluster. kubectl --context admin@prod reports 10 activated GitHub
managed-resource kinds; the Role enumerates 10, matching one-for-one across all four API groups.
No activated kind is left ungranted, so removing the wildcard cannot cost the tenant an apply.
The new parity test actually enforces, in both directions. Run at this head it reports
grants exactly the 10 activated GitHub managed-resource kinds. Three ablations, each naming the
assertion expected to fire:
| Ablation | Expected | Result |
|---|---|---|
Drop a granted kind (issuelabels) |
"activated but the Role does not grant it" | fired, naming the kind |
| Grant a kind nothing activates | "which no ManagedResourceActivationPolicy activates" | fired, naming the kind |
Restore resources: ['*'] |
the explicit wildcard guard | fired |
The file restores clean and the test passes again afterwards, so the ablations were reversible and
the pass is not stale. The script's fail-closed floors (< 5 on either side) mean a vacuous read
from a yq change or a path typo errors rather than comparing two empty sets.
Direction of the change. Strictly a tightening: every resource the new rules admit, the wildcard
already admitted. providerconfigs is added under the bare github.m.upbound.io group for the
ProviderConfig the tenant applies; that group is deliberately outside the parity check because it
carries no managed resource, and ProviderConfigUsage is correctly left out as controller-written.
CI at this head: 0 failing, 0 pending, mergeStateStatus: CLEAN, 0 unresolved threads. The
earlier reds on this branch were inherited from main (#3009) and cleared when #3008 merged.
Verdict: no P0/P1 findings
Parked — do not re-queue. This PR is pentad-clear (CLEAN, green checks, 0 unresolved threads, The blocker is repository-wide, not PR-specific: the root Re-queuing before that clears just repeats the eviction and burns a prod deploy plus a heal deploy |
1 participant
Why
The
github-configtenant holds authority over the org's real repositories, and its Rolegranted a blanket wildcard over five GitHub API groups — so it automatically gained every new
resource kind the moment one was activated, reviewed by nobody.
What
The Role now names the kinds it needs, matching the activation policy that decides which kinds
exist at all. A CI check fails — naming the offending kind — if the two ever drift apart, so the
extra maintenance step can't be silently forgotten.
Verified against live prod: every kind the tenant currently has deployed is still granted, so this
cannot break the running app.
Fixes #3003
Part of #2787
Important
Merge-order gate: #2985 and #3004 both re-approve the same
expectedRenderedSurfaceSHAfrom the same base value. Whichever merges second will conflict on that line and must re-measure and re-approve against the newmain— the second PR's hash is not valid once the first lands.