fix(kwctl): reject AdmissionPolicy scaffold for cluster-scoped targets#1743
fix(kwctl): reject AdmissionPolicy scaffold for cluster-scoped targets#1743officialasishkumar wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a static scope check to the kwctl scaffold flow so namespaced AdmissionPolicy manifests are not generated for targets that are known to be cluster-scoped, and warns when scope can’t be determined.
Changes:
- Introduces a built-in table-based classifier for Kubernetes resource scope (cluster-scoped vs unknown vs implicitly namespaced).
- Hooks the classifier into
AdmissionPolicymanifest generation to error on cluster-scoped targets and warn on unknown targets. - Adds unit tests covering the classifier and the new scaffold behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/kwctl/src/scaffold/resource_scope.rs | Adds static tables + classification logic (with unit tests) to categorize rule targets by scope. |
| crates/kwctl/src/scaffold/manifest.rs | Enforces scope validation when scaffolding AdmissionPolicy; errors or warns based on findings; adds tests. |
| crates/kwctl/src/scaffold.rs | Registers the new resource_scope module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// List of well-known cluster-scoped Kubernetes resources from the standard | ||
| /// API groups shipped with kube-apiserver (v1.33). Subresources (`pods/log`, | ||
| /// `nodes/proxy`, ...) inherit the scope of their parent resource and are not | ||
| /// listed here. | ||
| const CLUSTER_SCOPED_RESOURCES: &[ResourceKey] = &[ | ||
| // core / "" | ||
| ResourceKey { | ||
| api_group: "", | ||
| resource: "componentstatuses", | ||
| }, |
There was a problem hiding this comment.
Added the missing authn/authz review resources and tests in dfaae61.
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Findings are now deduped and sorted before returning in dfaae61.
| // Cannot statically check the test logger output here, but the function | ||
| // must return Ok and the warning is exercised by the integration tests. | ||
| let scaffold_data = | ||
| admission_policy_scaffold_data_with_rules(vec![rule(&["example.com"], &["widgets"])]); | ||
| let result = generate_yaml_resource(scaffold_data, ManifestType::AdmissionPolicy, false); | ||
| assert!(result.is_ok()); |
There was a problem hiding this comment.
The unknown-scope scaffold test now captures tracing output and asserts the warning in dfaae61.
`kwctl scaffold manifest --type AdmissionPolicy` previously generated a manifest regardless of which resources the policy's rules targeted. However, `AdmissionPolicy` is itself a namespaced CRD: the cluster only invokes it for namespaced admission requests, so a policy that targets a cluster-scoped resource such as `Namespace`, `Node`, `PersistentVolume`, `ClusterRole`, `StorageClass`, etc., is scaffolded into a manifest that will simply never be evaluated by the cluster. The user gets no error, no warning, no log line; the policy just silently does nothing. This commit makes `kwctl scaffold manifest --type AdmissionPolicy` classify each `(apiGroup, resource)` pair in the policy's rules into one of three buckets: 1. A known cluster-scoped Kubernetes built-in. Scaffolding fails with a message naming each offending resource and pointing the user at `--type ClusterAdmissionPolicy`. This addresses the first acceptance criterion of kubewarden#1305. 2. An unknown resource (an apiGroup outside the kube-apiserver core set, most commonly a Custom Resource Definition, or a rule that uses wildcards we cannot disambiguate statically). A `tracing::warn` message asks the user to verify the resource scope with `kubectl api-resources` and consider a `ClusterAdmissionPolicy` instead, but scaffolding still succeeds. This addresses the second acceptance criterion of kubewarden#1305. 3. A known namespaced built-in. Nothing to flag; scaffolding proceeds. `ClusterAdmissionPolicy` scaffolding is unaffected: it is allowed to target both namespaced and cluster-scoped resources. The classifier lives in the new `crates/kwctl/src/scaffold/resource_scope.rs` module and is driven by a static table of cluster-scoped resources from the Kubernetes 1.33 API surface (`api_groups` shipped by kube-apiserver, plus the cluster-scoped resources within them). The table is small, self-contained, and easy to update when the upstream API surface evolves; it intentionally lists subresources as inheriting the scope of their parent resource (so `nodes/proxy` is treated as cluster-scoped, `pods/status` as namespaced), matching how the Kubernetes API treats them. Unit tests cover the classifier (nine cases: namespaced built-in, core cluster-scoped, named-group cluster-scoped, CRD, wildcards on each side, subresource inheritance, mixed rules, empty input) and the scaffold integration (five cases: AdmissionPolicy with namespaced target succeeds, with core cluster-scoped target fails, with named- group cluster-scoped target fails, with CRD target succeeds with warning, and ClusterAdmissionPolicy with cluster-scoped target is unaffected). Closes kubewarden#1305 Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
329d1f2 to
dfaae61
Compare
Summary
Closes #1305.
kwctl scaffold manifest --type AdmissionPolicycurrently produces a manifest no matter whatresources the underlying policy targets. But
AdmissionPolicyis a namespaced CRD: the clusteronly routes namespaced admission requests to it, so a manifest that targets a cluster-scoped
resource (e.g.
Namespace,Node,PersistentVolume,ClusterRole,StorageClass, ...)is scaffolded just fine and then silently does nothing once applied. There is no warning,
no error, no log line; the user has to discover the misconfiguration the hard way.
This PR closes that gap. After the change, scaffolding an
AdmissionPolicyfor a policy whoserules include a cluster-scoped Kubernetes built-in fails with a clear message:
If the policy targets a resource we cannot classify statically (any apiGroup outside the
kube-apiserver core set, including most CRDs, or a rule that uses wildcards), the scaffold
prints a warning and proceeds:
ClusterAdmissionPolicyscaffolding is unaffected.This matches both acceptance criteria spelled out in the issue.
What changes
crates/kwctl/src/scaffold/resource_scope.rs(new)classify_admission_policy_ruleshelper that splits a policy's rules intocluster_scoped/unknownfindings. Subresources inherit the scope of their parent (nodes/proxyis cluster-scoped,pods/statusis namespaced).crates/kwctl/src/scaffold.rscrates/kwctl/src/scaffold/manifest.rsAdmissionPolicy, callsclassify_admission_policy_rulesand either errors out oncluster_scopedfindings or warns onunknownfindings before delegating to the existing serialization path.Resource scope table
The table covers the cluster-scoped resources in the kube-apiserver core distribution for
Kubernetes 1.33: the core group (
namespaces,nodes,persistentvolumes,componentstatuses) plus the admission registration, apiextensions, apiregistration,certificates, flowcontrol, networking (
ingressclasses,ipaddresses,servicecidrs),node, RBAC, resource.k8s.io, scheduling, storage and storagemigration groups. Any apiGroup
outside that built-in set (the canonical user-facing CRD case) is treated as
unknownandtriggers the warning, not the error.
Test plan
cargo build -p kwctlbuilds cleanlycargo clippy -p kwctl --tests --no-depsproduces no new warningscargo fmt --all -- --checkis cleancargo test -p kwctl --bin kwctlpasses (96 tests total). The new module adds:resource_scope::tests(namespaced built-in, core cluster-scoped,named-group cluster-scoped, CRD, wildcard apiGroup, wildcard resource, subresource,
mixed rules, empty rules)
scaffold::manifest::tests(AdmissionPolicy targetingnamespaced resource succeeds, AdmissionPolicy targeting core cluster-scoped resource
errors, AdmissionPolicy targeting named-group cluster-scoped resource errors,
AdmissionPolicy targeting CRD succeeds with warning, ClusterAdmissionPolicy targeting
cluster-scoped resource is unaffected)
test_scaffold_manifeste2e test (which uses--type ClusterAdmissionPolicy)is unaffected because the check only fires for
AdmissionPolicy