🐛 fix(webhook): keep KollectScope checks when the profile is missing - #307
Open
konih wants to merge 1 commit into
Open
🐛 fix(webhook): keep KollectScope checks when the profile is missing#307konih wants to merge 1 commit into
konih wants to merge 1 commit into
Conversation
`validateScope` returned nil as soon as the referenced KollectProfile was absent, so a KollectTarget applied before its profile skipped the namespace allow/deny checks and the resourceRules GVK check entirely. PR #304 fixes the same skip on the cluster-scoped kind; this is the namespaced half. Severity is lower here than on the cluster kind: `scopeCheck.enforceTarget` already re-checks GVKs and namespaces at reconcile, so a target admitted this way degrades instead of collecting. What was wrong is that admission accepted a spec it can fully evaluate — `deniedNamespaces`, `allowedNamespaces` and `spec.resourceRules[].gvk` do not depend on the profile at all. Only the profile `targetGVK` does, and `CollectRuleGVKs` ignores it whenever resourceRules is non-empty. The resolve now runs as a switch: found resolves the GVK, NotFound continues with `profileResolved = false`, any other error still fails closed. GVK validation is skipped only in the one case where it would compare against a zero GVK — profile missing *and* no resourceRules — which would otherwise reject a legal create-before-profile against a non-empty allowedGVKs. Test first: `TestKollectTargetValidator_scopeAdmissionMissingProfile` covers denied namespace, out-of-allowlist namespace, and out-of-scope resourceRules GVK with no profile object present, plus the in-scope case that must stay admissible. Red before the change on the denied-namespace assertion. Gates: lint (golangci v2 + arch-lint) clean, verify ok, scrub ok, webhook unit tests green. envtest suites (TestWebhookEnvtest, TestSetupWithManager_*) do not run on this host — bin/k8s ships linux-amd64 assets only and setup-envtest is rate-limited — so they are CI-only, and they fail identically on an unmodified main checkout here.
|
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.
The namespaced half of #304. Independent of it — different file, no overlap, mergeable in any order.
validateScopereturned nil as soon as the referenced KollectProfile was absent, so a KollectTarget applied before its profile skipped the namespace allow/deny checks and theresourceRulesGVK check entirely.Severity is lower here than on the cluster kind, because
scopeCheck.enforceTargetalready re-checks GVKs and namespaces at reconcile — a target admitted this way degrades rather than collecting. What was wrong is that admission accepted a spec it can fully evaluate:deniedNamespaces,allowedNamespacesandspec.resourceRules[].gvkdo not depend on the profile at all. Only the profiletargetGVKdoes, andCollectRuleGVKsignores it wheneverresourceRulesis non-empty.The resolve is now a switch: found resolves the GVK, NotFound continues with
profileResolved = false, any other error still fails closed. GVK validation is skipped only where it would otherwise compare a zero GVK against a non-emptyallowedGVKsand reject a legal create-before-profile.Test first —
TestKollectTargetValidator_scopeAdmissionMissingProfilecovers denied namespace, out-of-allowlist namespace, and out-of-scoperesourceRulesGVK with no profile object present, plus the in-scope case that must stay admissible. Red before the change on the denied-namespace assertion.Gates:
task lint(golangci v2 + arch-lint) clean,task verifyok,task scrubok, webhook unit tests green. envtest suites are CI-only on this host and fail identically on unmodified main here.