🐛 fix(webhook): validate spec-derived GVKs when the profile is missing - #308
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Follow-up to #304, which is the parent commit of this branch. #304 defers the whole GVK check when profileRef does not resolve, but only the profile targetGVK actually depends on the profile. `CollectRuleGVKs` returns the `spec.resourceRules[].gvk` list and ignores the profile GVK entirely whenever resourceRules is non-empty, so those GVKs are knowable at admission and were being waved through. Not an exploit path — a target cannot collect while its profile is missing, and reconcile now catches it once the profile appears — but admission should reject a spec it can fully evaluate. The NotFound branch also duplicated the happy path's two namespace checks. That is the divergence shape that caused the bug #304 fixes: a third check added to the tail later would silently not run for missing profiles. Resolve is now a switch that yields `profileGVK` plus `profileResolved`, and the whole tail runs once for both paths. GVK validation is skipped only when the profile is missing *and* resourceRules is empty, where the alternative is comparing a zero GVK against allowedGVKs and rejecting a legal create-before-profile. Tests: - `missingProfileSpecDerivedChecks` — denied resourceRules GVK and denied profileRef namespace, both with no profile object; red on the first before this change. It also covers the `ValidateClusterScopeStaticRefNamespace` call #304 added, which its own fixture could not exercise: allowedStaticRefNamespaces was unset there, so the call returned nil unconditionally. - `deniedRuleGVKDegrades` — reconcile side, permitted profile targetGVK plus a denied resourceRules GVK. Green before this commit; it locks the rules path through CollectRuleGVKs that #304's test did not reach. - ScopeCeiling's doc comment claimed "namespace and GVK checks" while the struct only carries namespaces, which points readers at the wrong enforcement point. Gates: lint (golangci v2 + arch-lint) clean, verify ok, scrub ok, webhook and controller unit tests green. envtest suites are CI-only on this host; a GVK-denied spec in cluster_scope_enforce_envtest_test.go is left as follow-up rather than added unverified.
konih
force-pushed
the
fix/clustertarget-admission-gvk-rules
branch
from
August 18, 2026 09:52
25b0677 to
c13d30c
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #304. That PR made the reconcile backstop check every rule GVK, and made
admission skip the GVK check entirely when the
KollectProfileis absent. This closes theremaining admission gap: the GVKs that come from the spec are knowable without the
profile, so they stay enforced.
Problem
scope.CollectRuleGVKsreturns theresourceRulesGVKs and ignoresprofileGVKentirely once
resourceRulesis non-empty. Only the profiletargetGVKis unknowablewhen the profile has not been created yet — the rule GVKs are right there in the spec.
Skipping the whole loop on
IsNotFoundtherefore admitted aKollectClusterTargetwhoseresourceRulesname a GVK outsideallowedGVKs. It was caught later by the reconcilebackstop (
ScopeGVKDenied), but afailurePolicy=failwebhook that already has the datashould not defer a decision it can make.
Change
switch, keepingprofileResolvedalongsideprofileGVK.profileResolved || len(spec.ResourceRules) > 0.NotFoundbranch, so thetwo paths cannot drift apart.
ScopeCeilingdoc comment: it carries namespaces only; GVKs are enforced atadmission and, as backstop, at reconcile.
Tests
TestKollectClusterTargetValidator_missingProfileSpecDerivedChecks— no profile object;asserts an in-scope target still admits, a denied
resourceRulesGVK rejects, and aprofileRef.namespaceoutsideallowedStaticRefNamespacesrejects. That last assertioncovers a line 🐛 fix(webhook): enforce cluster-scope when the profile is missing #304 added but left unexercised, because its fixture never set
allowedStaticRefNamespaces.TestKollectClusterTargetReconciler_deniedRuleGVKDegrades— a permitted profiletargetGVKmust not launder a denied rule GVK. Green before this change; it locks thereconcile half of the same invariant.
Red before the fix on the
resourceRulesassertion.Validation
go build ./..., the webhook and controller packages,task verify, andhack/test/cluster_scope_rbac_test.shall pass on top of currentmain.