diff --git a/docs/policy-reports.md b/docs/policy-reports.md index 52e0ad2fee..4162e0befb 100644 --- a/docs/policy-reports.md +++ b/docs/policy-reports.md @@ -6,26 +6,31 @@ that property for the Policy Reporter compliance dashboard. ## Exemptions must produce a current result -For report-backed validation policies, prefer a false `preconditions` entry for -an exemption instead of `rule.exclude` when the exempt resource may already have -a result: - -- a false precondition emits `result: skip` for the same policy, rule and - resource; -- `exclude` or a changed `match` emits no replacement result. - -The second shape can leave an older result visible even after hourly background -scans. Issue #2573 reproduced this with `validate-replica-floor`: eleven June -failures remained while unrelated results in the same reports kept refreshing. +For report-backed validation policies, express an exemption so that the exempt +resource still gets a current result when it may already have one: + +- in a `ClusterPolicy`, a false `preconditions` entry emits `result: skip` for + the same policy, rule and resource; +- in a `ValidatingPolicy`, fold the exemption into the validation expression + (`variables.exempt || `), which emits `result: pass`. A false + `matchConditions` entry writes no result to the report; +- `exclude`, a changed `match` or a narrower `matchConstraints` emits no + replacement result. + +A shape that writes no result can leave an older result visible even after +hourly background scans. Issue #2573 reproduced this with +`validate-replica-floor`: eleven June failures remained while unrelated +results in the same reports kept refreshing. The policy's committed Kyverno test pins the supported exemption shape: ```bash kyverno test tests/validate-replica-floor --require-tests ``` -The test requires a normal singleton to fail, two replicas to pass, and every -supported exemption form (workload label, pod-template label, namespace, exact -name and wildcard primary name) to emit `skip`. +The test requires a normal singleton, an unset replica count, a scaled-to-zero +workload and a single-replica StatefulSet to fail, two or more replicas to pass, +and every supported exemption form (workload label, pod-template label, +namespace, exact name and wildcard primary name) to emit `pass`. `kyverno test` counts a fixture row as passing when its rule never evaluated the resource (`REASON=Excluded`), whatever the row declared, and it drops a @@ -43,8 +48,9 @@ which a static rule-evaluation test cannot establish. ## Safe rollout and verification -1. Express a new replica-floor exemption as a precondition in - `validate-replica-floor.yaml`. Do not add a new `exclude` entry. +1. Express a new replica-floor exemption in the `exempt` variables of + `validate-replica-floor.yaml`. Do not add a `matchConditions` entry or + narrow its match scope. 2. Run the Kyverno test above and the normal local/prod static validation. 3. Let Flux deploy the policy change. Do not patch or delete PolicyReports by hand and do not restart the reports controller. @@ -58,7 +64,7 @@ which a static rule-evaluation test cannot establish. | map({result: .[0].result, count: length})' ``` -The precondition changes only the target policy/rule result. Other results in +The exemption changes only the target policy result. Other results in the same per-resource report remain controller-owned and untouched. ## Automatic pruning of stale failures @@ -86,8 +92,8 @@ What operators should expect: two hours old; in those first two hours a report can still be deleted, and it stays missing until the controller recovers and rescans. - A result left behind by an `exclude` on a resource that no other rule still - evaluates is not pruned either. Use a precondition for the exemption (see - above) so the scan rewrites it as a current `skip`. + evaluates is not pruned either. Express the exemption as described above so the + scan rewrites it as a current result. - Cluster-scoped `ClusterPolicyReport` objects are not covered. ## Authorization review dispatch diff --git a/k8s/bases/infrastructure/cluster-policies/best-practices/validate-replica-floor.yaml b/k8s/bases/infrastructure/cluster-policies/best-practices/validate-replica-floor.yaml index ad310c78c7..0781761618 100644 --- a/k8s/bases/infrastructure/cluster-policies/best-practices/validate-replica-floor.yaml +++ b/k8s/bases/infrastructure/cluster-policies/best-practices/validate-replica-floor.yaml @@ -13,12 +13,11 @@ # workloads in Kyverno's PolicyReports for a human to raise (or exempt) # deliberately. # -# Exemptions (a replica floor is *wrong*, not missing, for these) are expressed -# as preconditions rather than rule excludes. A false precondition emits a -# current `skip` PolicyReport result; an exclude emits no replacement result and -# can leave an older `fail` result behind after the exemption is added (#2573). -# This keeps Policy Reporter current without deleting or patching shared -# reports. See docs/policy-reports.md for the regression test and rollout check. +# Exemptions (a replica floor is *wrong*, not missing, for these) are part of the +# validation expression, so an exempt workload gets a current `pass` result. +# Neither a narrower match scope nor a false matchCondition writes a result, +# so either could leave an older `fail` in place after the exemption is added +# (#2573). See docs/policy-reports.md for the regression test and rollout check. # # Exemption categories: # * the opt-out label platform.devantler.tech/replica-floor: exempt on the @@ -44,8 +43,8 @@ # The other leader-election controllers that USED to be exempt (cloudnative-pg, # reloader, trust-manager, hcloud CCM/CSI, snapshot-controller, kyverno # background, flagger) opt in to 2+ replicas for HA instead of opting out. -apiVersion: kyverno.io/v1 -kind: ClusterPolicy +apiVersion: policies.kyverno.io/v1 +kind: ValidatingPolicy metadata: name: validate-replica-floor annotations: @@ -53,106 +52,86 @@ metadata: policies.kyverno.io/category: Best Practices, High Availability policies.kyverno.io/severity: medium policies.kyverno.io/subject: Deployment, StatefulSet - policies.kyverno.io/minversion: 1.13.0 + policies.kyverno.io/minversion: 1.17.0 policies.kyverno.io/description: >- Flags Deployments and StatefulSets running fewer than 2 replicas so their drain-safe PDBs actually preserve availability. Workloads that are intentionally singleton, scale-to-zero or controller-managed opt out with the platform.devantler.tech/replica-floor: exempt label. spec: - background: true - rules: - - name: require-replica-floor - match: - any: - - resources: - kinds: - - Deployment - - StatefulSet - preconditions: - all: - # Durable opt-out on the workload itself. Keep exemptions here, not in - # rule.exclude, so a background scan records a fresh `skip` result. - - key: >- - {{ request.object.metadata.labels."platform.devantler.tech/replica-floor" - || '' }} - operator: NotEquals - value: exempt - # Durable opt-out on the pod template (charts expose podLabels more - # widely than top-level workload labels). - - key: >- - {{ request.object.spec.template.metadata.labels."platform.devantler.tech/replica-floor" - || '' }} - operator: NotEquals - value: exempt - # Whole namespaces whose controllers are intentionally singleton or - # controller-managed: Velero has no leader election; Flux, Coroot and - # Kubescape controllers sit off serving paths; Crossplane reconcilers - # are external-state single writers. Crossplane is exempt by namespace - # because provider Deployment names include a changing revision hash. - - key: "{{ request.object.metadata.namespace || '' }}" - operator: AnyNotIn - value: - - velero - - flux-system - - observability - - kubescape - - crossplane-system - # Flagger owns replica counts for generated *-primary Deployments. - - key: "{{ request.object.metadata.name || '' }}" - operator: NotEquals - value: "?*-primary" - # Stable-name exemptions documented above. A false precondition emits - # `skip`, which supersedes any older failure for this policy and rule. - - key: "{{ request.object.metadata.name || '' }}" - operator: AnyNotIn - value: - # Single-replica apps/UIs and paused Flagger canary sources. - - actual-budget-actualbudget - - fleet - - headlamp - - whoami - - hubble-ui - - longhorn-ui - - homepage - - umami-umami - # Single-writer or ephemeral controllers with no useful second - # replica: DNS, disabled issuer and the canary load generator. - - external-dns - - origin-ca-issuer - - flagger-loadtester - # Lean leader-election controllers whose outage delays only an - # idempotent control loop. Production may still opt them into HA. - - vertical-pod-autoscaler-vpa-recommender - - vertical-pod-autoscaler-vpa-updater - - kyverno-reports-controller - - kyverno-cleanup-controller - # Controller-owned or single-by-design components triaged in the - # June 2026 HA sweep. SPIRE needs an external datastore for real - # HA. - - cluster-autoscaler-hetzner-cluster-autoscaler - - keda-operator-metrics-apiserver - - longhorn-driver-deployer - - spire-server - # CNPG-I exposes its gRPC readiness endpoint only on the elected - # leader, so a second replica can never become Ready. - - plugin-barman-cloud - # Crossview and its ephemeral bundled Postgres intentionally form - # one non-HA unit until the datastore is replaced. - - crossview - - crossview-postgres - validate: - failureAction: Audit - message: >- - {{ request.object.kind }} '{{ request.object.metadata.name }}' runs - {{ request.object.spec.replicas || `1` }} replica(s); the platform HA - floor is 2 so its maxUnavailable: 1 PDB keeps it available through a - node drain (see #1882). If it is intentionally singleton, scale-to-zero - or controller-managed, add the label - 'platform.devantler.tech/replica-floor: exempt'. - deny: - conditions: - all: - - key: "{{ request.object.spec.replicas || `1` }}" - operator: LessThan - value: 2 + validationActions: + - Audit + evaluation: + background: + enabled: true + matchConstraints: + resourceRules: + - apiGroups: ["apps"] + apiVersions: ["v1"] + operations: ["CREATE", "UPDATE"] + resources: ["deployments", "statefulsets"] + variables: + # Durable opt-out on the workload itself. + - name: workloadExempt + expression: >- + object.metadata.?labels[?'platform.devantler.tech/replica-floor'].orValue('') == 'exempt' + # Durable opt-out on the pod template (charts expose podLabels more + # widely than top-level workload labels). + - name: templateExempt + expression: >- + object.spec.?template.?metadata.?labels[?'platform.devantler.tech/replica-floor'].orValue('') == 'exempt' + # Whole namespaces whose controllers are intentionally singleton or + # controller-managed: Velero has no leader election; Flux, Coroot and + # Kubescape controllers sit off serving paths; Crossplane reconcilers + # are external-state single writers. Crossplane is exempt by namespace + # because provider Deployment names include a changing revision hash. + - name: namespaceExempt + expression: >- + object.metadata.?namespace.orValue('') in [ + 'velero', 'flux-system', 'observability', 'kubescape', + 'crossplane-system'] + # Flagger owns replica counts for generated *-primary Deployments. + - name: flaggerPrimary + expression: >- + object.metadata.name.matches('^.+-primary$') + # Named exemptions, one list per reason: single-replica apps/UIs and paused + # Flagger canary sources; single-writer or ephemeral controllers (DNS, + # disabled issuer, canary load generator); lean leader-election controllers + # whose outage delays only an idempotent loop (production may still opt + # them into HA); and components single by design from the June 2026 HA + # sweep -- SPIRE needs an external datastore for real HA, CNPG-I exposes + # its gRPC readiness endpoint only on the elected leader, and Crossview with + # its bundled Postgres is one non-HA unit until the datastore is replaced. + - name: namedExempt + expression: >- + object.metadata.name in [ + 'actual-budget-actualbudget', 'fleet', 'headlamp', 'whoami', + 'hubble-ui', 'longhorn-ui', 'homepage', 'umami-umami'] || + object.metadata.name in [ + 'external-dns', 'origin-ca-issuer', 'flagger-loadtester'] || + object.metadata.name in [ + 'vertical-pod-autoscaler-vpa-recommender', + 'vertical-pod-autoscaler-vpa-updater', + 'kyverno-reports-controller', 'kyverno-cleanup-controller'] || + object.metadata.name in [ + 'cluster-autoscaler-hetzner-cluster-autoscaler', + 'keda-operator-metrics-apiserver', 'longhorn-driver-deployer', + 'spire-server', 'plugin-barman-cloud', 'crossview', + 'crossview-postgres'] + - name: exempt + expression: >- + variables.workloadExempt || variables.templateExempt || + variables.namespaceExempt || variables.flaggerPrimary || + variables.namedExempt + # An unset replica count defaults to 1; an explicit 0 stays 0. + - name: replicas + expression: object.spec.?replicas.orValue(1) + validations: + - expression: "variables.exempt || variables.replicas >= 2" + messageExpression: >- + object.kind + " '" + object.metadata.name + "' runs " + + string(variables.replicas) + " replica(s); the platform HA floor is 2 + so its maxUnavailable: 1 PDB keeps it available through a node drain + (see #1882). If it is intentionally singleton, scale-to-zero or + controller-managed, add the label + 'platform.devantler.tech/replica-floor: exempt'." diff --git a/scripts/tests/test-coroot-availability-remediation.sh b/scripts/tests/test-coroot-availability-remediation.sh index 568b741bf6..b726089c3e 100755 --- a/scripts/tests/test-coroot-availability-remediation.sh +++ b/scripts/tests/test-coroot-availability-remediation.sh @@ -140,7 +140,13 @@ SNAPSHOTTER_SUBSTITUTION="${snapshotter_substitution}" yq e -e \ "${longhorn_release}" >/dev/null || fail 'Longhorn must continue sourcing the snapshotter replica count from the production contract' -if grep -Fxq ' - csi-snapshotter' "${replica_floor}"; then +# The exemptions are CEL expressions; read them structurally so a format change +# fails here instead of leaving a text match that can never fire. +replica_floor_exemptions="$(yq e -e '.spec.variables[] | select(.name == "namedExempt") | .expression' "${replica_floor}")" || + fail 'the replica floor must keep its named exemptions in the namedExempt variable' +[[ "${replica_floor_exemptions}" == *"object.metadata.name in ["* ]] || + fail 'the replica floor named exemptions must stay a name list this guard can read' +if [[ "${replica_floor_exemptions}" == *csi-snapshotter* ]]; then fail 'the HA CSI snapshotter must not remain exempt from the replica floor' fi diff --git a/tests/policy-failure-actions.json b/tests/policy-failure-actions.json index c3715885fe..d3ea8126e6 100644 --- a/tests/policy-failure-actions.json +++ b/tests/policy-failure-actions.json @@ -40,7 +40,7 @@ "validate-pod-security/require-seccomp-profile": "Enforce", "validate-pod-security/deny-container-seccomp-unconfined": "Enforce", "validate-replica-ceiling": "Audit", - "validate-replica-floor/require-replica-floor": "Audit", + "validate-replica-floor": "Audit", "enforce-flux-best-practices/kustomization-recommended-settings": "Enforce", "enforce-flux-best-practices/helmrelease-recommended-settings": "Enforce", "restrict-arc-retirement/fence-pool-retirement": "Enforce", diff --git a/tests/validate-replica-floor/kyverno-test.yaml b/tests/validate-replica-floor/kyverno-test.yaml index 2689590b3f..099458bc34 100644 --- a/tests/validate-replica-floor/kyverno-test.yaml +++ b/tests/validate-replica-floor/kyverno-test.yaml @@ -10,19 +10,28 @@ resources: - resources.yaml results: - policy: validate-replica-floor - rule: require-replica-floor resources: - normal-singleton + - replicas-unset + - scaled-to-zero kind: Deployment result: fail - policy: validate-replica-floor - rule: require-replica-floor + resources: + - statefulset-singleton + kind: StatefulSet + result: fail + - policy: validate-replica-floor resources: - normal-ha kind: Deployment result: pass - policy: validate-replica-floor - rule: require-replica-floor + resources: + - statefulset-ha + kind: StatefulSet + result: pass + - policy: validate-replica-floor resources: - workload-label-exempt - pod-label-exempt @@ -30,4 +39,4 @@ results: - keda-operator-metrics-apiserver - demo-primary kind: Deployment - result: skip + result: pass diff --git a/tests/validate-replica-floor/resources.yaml b/tests/validate-replica-floor/resources.yaml index 3869a2f226..c97f248aea 100644 --- a/tests/validate-replica-floor/resources.yaml +++ b/tests/validate-replica-floor/resources.yaml @@ -134,3 +134,80 @@ spec: containers: - name: app image: nginx:1.29.0 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: replicas-unset + namespace: test +spec: + selector: + matchLabels: + app: replicas-unset + template: + metadata: + labels: + app: replicas-unset + spec: + containers: + - name: app + image: nginx:1.29.0 +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: scaled-to-zero + namespace: test +spec: + replicas: 0 + selector: + matchLabels: + app: scaled-to-zero + template: + metadata: + labels: + app: scaled-to-zero + spec: + containers: + - name: app + image: nginx:1.29.0 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: statefulset-singleton + namespace: test +spec: + replicas: 1 + serviceName: statefulset-singleton + selector: + matchLabels: + app: statefulset-singleton + template: + metadata: + labels: + app: statefulset-singleton + spec: + containers: + - name: app + image: nginx:1.29.0 +--- +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: statefulset-ha + namespace: test +spec: + replicas: 3 + serviceName: statefulset-ha + selector: + matchLabels: + app: statefulset-ha + template: + metadata: + labels: + app: statefulset-ha + spec: + containers: + - name: app + image: nginx:1.29.0