diff --git a/.changeset/operator-write-policy-selfcheck.md b/.changeset/operator-write-policy-selfcheck.md new file mode 100644 index 000000000..439dad7f4 --- /dev/null +++ b/.changeset/operator-write-policy-selfcheck.md @@ -0,0 +1,5 @@ +--- +"@getknext/core": patch +--- + +`knext doctor` now checks that the operator's Secret and ConfigMap write policy is in force. The policy, a `ValidatingAdmissionPolicy` shipped in the install bundle, is the only thing that limits the operator to its per-zone signing-key Secrets and JWKS ConfigMaps. It can be off with nothing saying so: a cluster older than Kubernetes 1.30 rejects it while the rest of the bundle installs, an install under a different namespace or ServiceAccount name than the bundle rendered leaves a policy that names a user the operator is not, and an admin or a GitOps prune can delete it. The new `write-policy` row reports a `WARN` that names which of these it is, and `PASS` only when the policy exists, covers create/update/delete on Secrets and ConfigMaps without narrowing, carries the validations this release ships, has a binding that denies, and names the ServiceAccount the operator runs as. A read it cannot classify or parse is a `WARN`, never a skip. The operator reports the same verdict as the `knext_operator_write_policy_enforced` gauge (re-checked every five minutes, warning log on change, still starts when it is `0`), with a `KnextOperatorWritePolicyNotEnforced` alert (a `0`) and a `KnextOperatorWritePolicyUnreported` alert (an operator pod that is scraped but exports no gauge). The operator's role gains `get` on exactly the policy and its binding, by name, and nothing else. diff --git a/apps/docs/content/docs/cli.mdx b/apps/docs/content/docs/cli.mdx index 20b00f3e5..c86ae67cb 100644 --- a/apps/docs/content/docs/cli.mdx +++ b/apps/docs/content/docs/cli.mdx @@ -489,6 +489,19 @@ whether your cluster's CNI can **enforce** the default-on `NetworkPolicy` the op on flannel it cannot (the policy is declarative only), and doctor says so plainly rather than guessing; "cannot determine" is reported as its own outcome, never as "enforced". +**Operator write policy.** The operator's Secret and ConfigMap writes are limited by a +`ValidatingAdmissionPolicy` that can be off with nothing failing: a server older than Kubernetes +1.30, an install under a different namespace or ServiceAccount name than the bundle rendered, or a +deleted policy or binding. Doctor reads the policy and its binding by name and reports `PASS` only +when the policy exists, covers create, update and delete on Secrets and ConfigMaps without narrowing +(no `resourceNames`, `Cluster` scope or `excludeResourceRules`), carries the validations this release +ships, has a binding that includes `Deny`, and names the ServiceAccount the operator Deployment runs +as. Anything else, including output it cannot read, is a `WARN` that says which part is wrong, and a server +older than 1.30 is a `WARN` naming the floor. A read that RBAC denies is a `WARN` ("cannot +determine"), never a pass. It never fails the run, because the missing safety net is not an +outage; the operator publishes the same verdict as the `knext_operator_write_policy_enforced` gauge +(see [Security](/docs/security#write-policy-enforced)). + **Redis key prefixes.** When any `NextApp` uses a Redis cache, doctor compares the apps' key prefixes: `cache.keyPrefix`, a `REDIS_KEY_PREFIX` in `env`, or the per-app default the operator derives. It `WARN`s, naming the apps, when: diff --git a/apps/docs/content/docs/security.mdx b/apps/docs/content/docs/security.mdx index 672172b91..e0c97beb8 100644 --- a/apps/docs/content/docs/security.mdx +++ b/apps/docs/content/docs/security.mdx @@ -474,6 +474,56 @@ Two limits. The policy checks name and type, not ownership: a compromised operat change an `Opaque` Secret or a ConfigMap that happens to carry the name of a zone's key or key set. And a cluster administrator can remove the policy. +#### Knowing whether the policy is in force [#write-policy-enforced] + +The policy can be off without anything failing. A cluster older than Kubernetes 1.30 rejects the two +objects while the rest of the bundle installs. An install under a different namespace or +ServiceAccount name than the bundle rendered leaves a policy that names a user the operator is not. +An administrator, or a GitOps tool pruning objects it does not own, can delete either object. In +each case the operator is simply unrestricted. + +So the operator checks for itself, when it starts and every five minutes after. It reads the policy +and its binding (a read-only `get` on those two names, nothing else) and compares the policy's +username with the one the API server reports for the operator. It publishes the result as a gauge: + +| Metric | Meaning | +|---|---| +| `knext_operator_write_policy_enforced` | `1` when the policy exists, its binding denies, and the policy names the identity the operator runs as. `0` when any of that is false, or when the objects could not be read. | + +It logs one warning each time the answer changes to "not in force", naming the reason. The operator +still starts: refusing to run is your decision, not the operator's. The `KnextOperatorWritePolicyNotEnforced` +alert (warning, 15 minutes) fires on a `0`; see [SLOs & alerting](/docs/slos-alerting). + +The gauge reads `0` for any of these: + +- the policy or the binding is missing; +- the binding does not include `Deny` (a binding that only audits or warns refuses nothing), or it + limits the policy to some namespaces; +- the policy names a different user than the operator runs as; +- the policy fails open (`failurePolicy: Ignore`); +- the policy does not cover create, update and delete on both Secrets and ConfigMaps in the core + `v1` API: a rule that names objects (`resourceNames`), is scoped `Cluster`, or is for another API + version does not count, a namespace or object selector narrows it, and any `excludeResourceRules` + entry is treated as removing something; +- the policy has a condition the check cannot read as an exact user; +- the policy declares parameters (`paramKind`) or its binding sets a `paramRef`. The shipped policy + takes none, and a parameter that cannot be found makes the API server skip every validation; +- the policy's validations are not the ones this release ships. The check compares the policy's + variables (the allow-lists and the Secret type) and each validation's expression and + `messageExpression` with the shipped copy, so a validation replaced by `true` is caught. The + `message` text and the `reason` are not compared. An operator newer than the policy installed in the + cluster reads `0` here until you reapply the bundle. + +Everything the check cannot read, it counts as not in force. + +If the operator exports no such gauge at all (a release older than the check, or a failing metrics +scrape), the `0` alert cannot fire, so a second alert, `KnextOperatorWritePolicyUnreported`, fires +for an operator pod that Prometheus has discovered but that exports no gauge. + +`knext doctor` reports the same verdict from your machine, plus the server version against the 1.30 +floor. A read it cannot classify or parse is a `WARN`, not a skip. It reads the policy name from the operator's `WRITE_POLICY_NAME` environment variable, which +the bundle sets, so a renamed install is checked under its own name. + The policy needs **Kubernetes 1.30 or later**, where `ValidatingAdmissionPolicy` became generally available. That is the same server floor as the rest of knext (see the [support matrix](/docs/support-matrix#kubernetes)). On an older cluster the API server does not diff --git a/apps/docs/content/docs/slos-alerting.mdx b/apps/docs/content/docs/slos-alerting.mdx index 2d3c04da6..2aafd3747 100644 --- a/apps/docs/content/docs/slos-alerting.mdx +++ b/apps/docs/content/docs/slos-alerting.mdx @@ -20,7 +20,7 @@ catalog and scrape setup see [Observability](/docs/observability); for logs see | `knext_bunexec_http_inflight_requests` | App runtime. Saturation | | `knext_bunexec_startup_duration_seconds` | App runtime. Set once per process, when the listener binds — the cold-start signal | | `knext_bunexec_process_uptime_seconds`, `knext_bunexec_process_resident_memory_bytes` | App runtime | -| `knext_nextapp_reconcile_total{result}`, `knext_nextapp_reconcile_errors_total`, `knext_nextapp_reconcile_duration_seconds_bucket`, `knext_nextapp_image_prewarm_errors_total` | Operator | +| `knext_nextapp_reconcile_total{result}`, `knext_nextapp_reconcile_errors_total`, `knext_nextapp_reconcile_duration_seconds_bucket`, `knext_nextapp_image_prewarm_errors_total`, `knext_operator_write_policy_enforced` | Operator | | `workqueue_depth{name="nextapp"}` | Operator (control-plane saturation) | | `knext_nextapp_condition{type,status,namespace,name}` | kube-state-metrics, reading `NextApp.status.conditions` ([setup below](#kube-state-metrics)) | | `up{job="knext-nextapp"}` | Prometheus itself — scrape health, and the anchor for the meta-alerts | @@ -122,6 +122,39 @@ histogram_quantile(0.95, Alert `KnextOperatorReconcileSlow` (p95 above 30 seconds for 15 minutes, warning). A related alert, `KnextImagePrewarmFailing` (warning), fires when image pre-warming errors persist for 15 minutes. +## Operator write policy (control plane) + +**Objective:** the policy that limits the operator's Secret and ConfigMap writes is in force on every +cluster that supports it. + +```text +knext_operator_write_policy_enforced +``` + +The operator sets this gauge to `1` only when all of this holds, and to `0` otherwise, including when +the objects cannot be read: + +- the policy exists and does not fail open; +- it covers create, update and delete on Secrets and ConfigMaps (core `v1`), with no `resourceNames`, + no `Cluster` scope, no `excludeResourceRules` and no namespace or object selector; +- its condition is an exact match on the user the operator runs as; +- its variables and validations (their expressions and `messageExpression`) are the ones the + operator release ships; +- neither the policy declares `paramKind` nor its binding sets a `paramRef` (a parameter that cannot + be found makes the API server skip every validation); +- its binding exists, names the policy, includes `Deny` and is not limited to some namespaces. + +Alert `KnextOperatorWritePolicyNotEnforced` (a `0` for 15 minutes, warning, one series per operator +pod). It is a warning because a missing safety net is not an outage, and a cluster older than +Kubernetes 1.30 cannot hold the policy at all. + +An alert on `== 0` says nothing when the series is missing, so a companion alert, +`KnextOperatorWritePolicyUnreported` (warning, 15 minutes), fires for an operator pod that Prometheus +scrapes (`up` exists) but that exports no such gauge: an operator older than the check, or a metrics +scrape that is failing. It is keyed on a discovered target rather than a bare `absent()`, so it stays +quiet on a cluster with no operator at all. `knext doctor` names the reason; see +[Security](/docs/security#write-policy-enforced). + ## NextApp health (control plane) **Objective:** no `NextApp` stays `Degraded=True`. diff --git a/apps/file-manager/src/app/api/_metrics/prometheusrule.test.ts b/apps/file-manager/src/app/api/_metrics/prometheusrule.test.ts index c51e2ca7d..e61ddd7b0 100644 --- a/apps/file-manager/src/app/api/_metrics/prometheusrule.test.ts +++ b/apps/file-manager/src/app/api/_metrics/prometheusrule.test.ts @@ -199,6 +199,74 @@ describe('PrometheusRule manifest', () => { expect(prewarm?.annotations?.runbook_url ?? prewarm?.annotations?.runbook).toBeTruthy(); }); + // The operator's Secret/ConfigMap write policy can be off with nothing saying + // so: a server older than 1.30 rejects it, a repackaged install leaves a + // matchCondition that matches nobody, a prune deletes it. The operator reports + // it as a 0/1 gauge; this alert is what makes a 0 visible. + it('alerts when the operator write policy is not in force', () => { + const { docs } = loadRule(); + const rule = docs.find((d) => d?.kind === 'PrometheusRule'); + const alerts: (Rule & { for?: string })[] = rule.spec.groups.flatMap( + (g: { rules: Rule[] }) => g.rules, + ); + const off = alerts.find((a) => a.alert === 'KnextOperatorWritePolicyNotEnforced'); + + expect(off, 'KnextOperatorWritePolicyNotEnforced alert must exist').toBeDefined(); + // Per pod, not aggregated: every replica reports, and one that cannot see the + // policy is itself a finding. + expect(off?.expr).toMatch(/knext_operator_write_policy_enforced\s*==\s*0/); + // Warning, not critical: it is a missing safety net, not an outage, and every + // pre-1.30 cluster would page forever otherwise. A "for" longer than the + // operator's re-check interval keeps a restart from flapping it. + expect(off?.labels?.severity).toBe('warning'); + expect(off?.for).toBeTruthy(); + expect(off?.annotations?.runbook_url ?? off?.annotations?.runbook).toBeTruthy(); + }); + + // The alert above reads `== 0`, and PromQL over a series nobody emits returns + // an empty vector: an operator that never publishes the gauge (an older + // operator over a newer rule file, the monitor not started, a scrape that + // fails) would make it silent forever. This companion fires on a DISCOVERED + // operator target that exports no gauge. It keys on `up`, never a bare + // absent(): with no operator running at all there is nothing to report on, + // and an always-on absent() would fire on every cluster before install. + it('alerts when a scraped operator pod exports no write-policy gauge', () => { + const { docs } = loadRule(); + const rule = docs.find((d) => d?.kind === 'PrometheusRule'); + const alerts: (Rule & { for?: string })[] = rule.spec.groups.flatMap( + (g: { rules: Rule[] }) => g.rules, + ); + const blind = alerts.find((a) => a.alert === 'KnextOperatorWritePolicyUnreported'); + + expect(blind, 'KnextOperatorWritePolicyUnreported alert must exist').toBeDefined(); + expect(blind?.expr).toMatch(/\bup\{/); + expect(blind?.expr).not.toMatch(/\babsent\(/); + expect(blind?.expr).toMatch( + /unless\s+on\s*\(\s*namespace\s*,\s*pod\s*\)\s*knext_operator_write_policy_enforced/, + ); + expect(blind?.labels?.severity).toBe('warning'); + expect(blind?.for).toBeTruthy(); + expect(blind?.annotations?.runbook_url ?? blind?.annotations?.runbook).toBeTruthy(); + + // The job selector must actually match the operator's metrics Service as the + // bundle renders it (name prefix + service name), or the alert selects + // nothing and is silent in exactly the way it exists to prevent. + const op = join(__dirname, '../../../../../../packages/kn-next-operator/config'); + const prefix = /^namePrefix:\s*(\S+)/m.exec( + readFileSync(join(op, 'default/kustomization.yaml'), 'utf8'), + )?.[1]; + const svc = /^kind: Service[\s\S]*?^ {2}name:\s*(\S+)/m.exec( + readFileSync(join(op, 'default/metrics_service.yaml'), 'utf8'), + )?.[1]; + expect(prefix, 'namePrefix must be readable').toBeTruthy(); + expect(svc, 'the metrics Service name must be readable').toBeTruthy(); + const job = /job=~"([^"]+)"/.exec(blind?.expr ?? '')?.[1]; + expect(job, 'the alert must select the operator by job=~"..."').toBeTruthy(); + expect(new RegExp(`^(?:${job})$`).test(`${prefix}${svc}`)).toBe(true); + // ...and must not select the app pods or anything else. + expect(new RegExp(`^(?:${job})$`).test('knext-nextapp')).toBe(false); + }); + // The reconcile-error page must stay CRITICAL — the decoupling narrowed what // reaches it, and if that alert were softened too, nothing would page on a // genuinely failing control loop. diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index a56854d47..004180386 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -340,6 +340,38 @@ to replace it, and a ServiceAccount with the operator's name in another namespac bundle is one artifact; there is no policy-free variant (jev: align the docs 1.00, ship a second bundle 0.00). On an older cluster the apply rejects the two policy objects and the rest of the bundle applies, leaving the operator unrestricted. +- *Silent-off detection (#2500).* Three ways leave the operator unrestricted with nothing reporting + it: the version floor above, an install under another namespace or ServiceAccount name than the + bundle rendered (a Helm or OLM repackage, a hand-edited overlay), and deletion of the policy or its + binding by an admin or a GitOps prune. `internal/writepolicy` is the operator's read-only + self-check: at start-up and every five minutes it `get`s the policy and binding, asks the API + server who it is (`SelfSubjectReview`, which every authenticated user may create), and publishes + `knext_operator_write_policy_enforced`. It is 1 only if ALL of this holds: the policy exists with + `failurePolicy` not `Ignore`; its `matchConstraints` cover create/update/delete on core `v1` + Secrets and ConfigMaps through a rule with no `resourceNames`, no `Cluster` scope and the `v1` (or + `*`) apiVersion, with no `excludeResourceRules` (any is treated as carving something out) and no + namespace/object selector; its matchCondition is an exact equality on the operator's own username; + **its variables and validations are the shipped CEL** (round-1 review finding: matchConstraints, + identity and binding can all be right while a validation was replaced by `true`; compared are each + variable's name and expression and each validation's expression and `messageExpression`, against a + copy in `canonical.go` / `write-policy.ts` that `TestCanonicalPolicyCELEqualsTheShippedPolicy` and + its TS twin fail on if `config/admission` changes without it; `message` and `reason` are cosmetic + and not compared; an operator newer than the installed policy reports `PolicyModified` until the + bundle is reapplied); the policy declares no `paramKind` and the binding sets no `paramRef` + (closing-review finding: a `paramKind` plus a `paramRef` that finds nothing, with + `parameterNotFoundAction: Allow`, makes the apiserver run no validation while everything else + checks out; the shipped bundle sets neither, so either is `PolicyModified` / `BindingScoped`); and + the binding exists, names the policy, includes `Deny` and is not scoped. + Unreadable or unknown is 0, never 1. The `== 0` alert is blind if the series is missing, so + `KnextOperatorWritePolicyUnreported` fires on a discovered operator target (`up`) with no gauge. It logs one warning per change and **does not + refuse to start** (that is a founder decision, not made here). The alert + `KnextOperatorWritePolicyNotEnforced` and `knext doctor`'s `write-policy` row report the same + verdict; both are held to one shared table (`internal/writepolicy/testdata/verdict-cases.json`). + The RBAC cost is `get` on exactly the two named objects (`config/rbac/write_policy_reader_role.yaml`, + pinned by `TestWritePolicyReaderRoleIsExactlyGetOnTheTwoNamedObjects`), a separate role because a + marker's `resourceNames` is not rewritten by the bundle's name prefix and the bundle rewrites it + from the policy's real name. **Residual:** a compromised operator can still read the two objects + it already could not change, and the check detects a missing control, it does not restore it. **Defence in depth: the function mounts the JWKS only if the zone controls it (#2464).** Before it renders a function, the BackendService reconciler reads the metadata of `-knext-jwks` diff --git a/packages/kn-next-operator/cmd/main.go b/packages/kn-next-operator/cmd/main.go index 7d9976122..6e97755ea 100644 --- a/packages/kn-next-operator/cmd/main.go +++ b/packages/kn-next-operator/cmd/main.go @@ -45,6 +45,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" + "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/metrics/filters" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -53,6 +54,7 @@ import ( appsv1alpha1 "github.com/AhmedElBanna80/knext/packages/kn-next-operator/api/v1alpha1" "github.com/AhmedElBanna80/knext/packages/kn-next-operator/internal/controller" webhookv1alpha1 "github.com/AhmedElBanna80/knext/packages/kn-next-operator/internal/webhook/v1alpha1" + "github.com/AhmedElBanna80/knext/packages/kn-next-operator/internal/writepolicy" servingv1 "knative.dev/serving/pkg/apis/serving/v1" servingv1beta1 "knative.dev/serving/pkg/apis/serving/v1beta1" // +kubebuilder:scaffold:imports @@ -83,6 +85,8 @@ func main() { var secureMetrics bool var enableHTTP2 bool var zoneKeyPropagationBound, zoneKeyRotationInterval time.Duration + var writePolicyName string + var writePolicyCheckInterval time.Duration var tlsOpts []func(*tls.Config) flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") @@ -108,6 +112,12 @@ func main() { flag.DurationVar(&zoneKeyRotationInterval, "zone-signing-key-rotation-interval", 0, "Rotate every zone signing key this long after it became current. 0 (the default) rotates only on "+ "request, through the apps.kn-next.dev/rotate-signing-key annotation on the zone's NextApp.") + flag.StringVar(&writePolicyName, "write-policy-name", writePolicyNameDefault(os.Getenv), + "Name of the ValidatingAdmissionPolicy (and of its binding) that limits this operator's Secret and "+ + "ConfigMap writes. The shipped bundle sets WRITE_POLICY_NAME to the prefixed name it renders; the "+ + "self-check reports knext_operator_write_policy_enforced from it.") + flag.DurationVar(&writePolicyCheckInterval, "write-policy-check-interval", writepolicy.DefaultInterval, + "How often the operator re-reads its write policy and binding to report whether the policy is in force.") opts := zap.Options{ Development: true, } @@ -217,6 +227,14 @@ func main() { } // +kubebuilder:scaffold:builder + // Report whether the policy that bounds this operator's Secret/ConfigMap + // writes is actually in force. It never blocks start-up (that is the + // owner's call, not the monitor's); it publishes a gauge and warns. + if err := addWritePolicyMonitor(mgr.Add, mgr.GetConfig(), writePolicyName, writePolicyCheckInterval); err != nil { + setupLog.Error(err, "Failed to set up the write-policy self-check") + os.Exit(1) + } + if err := setupHealthChecks(mgr, len(webhookCertPath) > 0); err != nil { setupLog.Error(err, "Failed to set up health/ready checks") os.Exit(1) @@ -314,3 +332,35 @@ func setupHealthChecks(mgr ctrl.Manager, webhookEnabled bool) error { } return nil } + +// defaultWritePolicyName is the policy's name in the unprefixed base manifests +// (config/admission). The rendered bundle prefixes it and says so in +// WRITE_POLICY_NAME. +const defaultWritePolicyName = "secret-configmap-writes" + +// writePolicyNameDefault is the --write-policy-name default: the name the +// bundle rendered into WRITE_POLICY_NAME, else the unprefixed base name. A +// name the bundle did not render is how a repackaged install reports the +// policy missing rather than silently checking nothing. +func writePolicyNameDefault(getenv func(string) string) string { + if v := getenv("WRITE_POLICY_NAME"); v != "" { + return v + } + return defaultWritePolicyName +} + +// addWritePolicyMonitor registers the write-policy self-check +// (internal/writepolicy) with the manager. It is its own function, taking the +// manager's Add, so the wiring is testable without standing up a manager. +func addWritePolicyMonitor(add func(manager.Runnable) error, cfg *rest.Config, name string, interval time.Duration) error { + src, err := writepolicy.NewClientSource(cfg) + if err != nil { + return fmt.Errorf("building the write-policy reader: %w", err) + } + return add(&writepolicy.Monitor{ + Source: src, + Name: name, + Interval: interval, + Log: ctrl.Log.WithName("write-policy"), + }) +} diff --git a/packages/kn-next-operator/cmd/write_policy_monitor_test.go b/packages/kn-next-operator/cmd/write_policy_monitor_test.go new file mode 100644 index 000000000..177ad3cb1 --- /dev/null +++ b/packages/kn-next-operator/cmd/write_policy_monitor_test.go @@ -0,0 +1,91 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "os" + "strings" + "testing" + "time" + + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/AhmedElBanna80/knext/packages/kn-next-operator/internal/writepolicy" +) + +// The write-policy self-check (internal/writepolicy) only reports anything if +// main wires it in. These tests pin the wiring: the monitor is registered, it +// runs on every replica, and it reads the policy name the bundle rendered. + +func TestAddWritePolicyMonitorRegistersAnEveryReplicaMonitor(t *testing.T) { + var added []manager.Runnable + add := func(r manager.Runnable) error { + added = append(added, r) + return nil + } + // Building a clientset does not dial; nothing is read here. + if err := addWritePolicyMonitor(add, &rest.Config{Host: "https://127.0.0.1:1"}, "kn-next-operator-secret-configmap-writes", 7*time.Minute); err != nil { + t.Fatal(err) + } + if len(added) != 1 { + t.Fatalf("want one runnable added, got %d", len(added)) + } + m, ok := added[0].(*writepolicy.Monitor) + if !ok { + t.Fatalf("the runnable is %T, want *writepolicy.Monitor", added[0]) + } + if m.Name != "kn-next-operator-secret-configmap-writes" || m.Interval != 7*time.Minute { + t.Errorf("monitor name/interval = %q/%v, not the configured ones", m.Name, m.Interval) + } + if le, ok := added[0].(manager.LeaderElectionRunnable); !ok || le.NeedLeaderElection() { + t.Error("the monitor must opt out of leader election: a standby replica must report the gauge too") + } +} + +func TestWritePolicyNameComesFromTheRenderedEnvironment(t *testing.T) { + if got := writePolicyNameDefault(func(string) string { return "" }); got != "secret-configmap-writes" { + t.Errorf("unset env must default to the unprefixed shipped name, got %q", got) + } + got := writePolicyNameDefault(func(k string) string { + if k == "WRITE_POLICY_NAME" { + return "acme-secret-configmap-writes" + } + return "" + }) + if got != "acme-secret-configmap-writes" { + t.Errorf("WRITE_POLICY_NAME must override the default, got %q", got) + } +} + +// main() cannot be run in a test, so scan it: the monitor must be added to the +// manager, exactly once, and be configurable. +func TestMainRegistersTheWritePolicyMonitor(t *testing.T) { + raw, err := os.ReadFile("main.go") + if err != nil { + t.Fatal(err) + } + src := string(raw) + if n := strings.Count(src, "addWritePolicyMonitor(mgr.Add"); n != 1 { + t.Errorf("main() must call addWritePolicyMonitor(mgr.Add, ...) exactly once, found %d", n) + } + for _, want := range []string{`"write-policy-name"`, `"write-policy-check-interval"`, "writePolicyNameDefault(os.Getenv)"} { + if !strings.Contains(src, want) { + t.Errorf("main.go lacks %s", want) + } + } +} diff --git a/packages/kn-next-operator/config/default/kustomization.yaml b/packages/kn-next-operator/config/default/kustomization.yaml index adddf790d..76c675d8b 100644 --- a/packages/kn-next-operator/config/default/kustomization.yaml +++ b/packages/kn-next-operator/config/default/kustomization.yaml @@ -195,6 +195,35 @@ replacements: options: delimiter: "'" index: 7 +# [ADMISSION] The operator's write-policy self-check reads the policy and its binding by NAME, and +# its role grants `get` on exactly that name. The name prefix is applied to the policy but reaches +# neither a Deployment env value nor a rule's resourceNames, so both are rewritten from the policy's +# real name. A base name left in either place makes the check read (or be forbidden from reading) an +# object that is not there. internal/install/write_policy_selfcheck_test.go proves the three agree. +# policy name -> manager env WRITE_POLICY_NAME +- source: + kind: ValidatingAdmissionPolicy + group: admissionregistration.k8s.io + name: secret-configmap-writes + fieldPath: .metadata.name + targets: + - select: + kind: Deployment + name: controller-manager + fieldPaths: + - .spec.template.spec.containers.[name=manager].env.[name=WRITE_POLICY_NAME].value +# policy name -> write-policy-reader role resourceNames +- source: + kind: ValidatingAdmissionPolicy + group: admissionregistration.k8s.io + name: secret-configmap-writes + fieldPath: .metadata.name + targets: + - select: + kind: ClusterRole + name: write-policy-reader + fieldPaths: + - .rules.0.resourceNames.0 # --- The original commented metrics/conversion replacement scaffold follows (left disabled) --- #replacements_disabled: # - source: # Uncomment the following block to enable certificates for metrics diff --git a/packages/kn-next-operator/config/manager/manager.yaml b/packages/kn-next-operator/config/manager/manager.yaml index 1f6a263f8..33f368e11 100644 --- a/packages/kn-next-operator/config/manager/manager.yaml +++ b/packages/kn-next-operator/config/manager/manager.yaml @@ -83,6 +83,14 @@ spec: # the bundle resolves to and the bootstrap/release notes for issue #76. image: controller name: manager + # The name of the ValidatingAdmissionPolicy (and binding) that limits this operator's + # Secret/ConfigMap writes. The operator reads it to report whether the policy is in + # force (knext_operator_write_policy_enforced). The value is the BASE name; config/default + # rewrites it from the policy's real, prefixed name (see replacements there), because + # kustomize's name prefix does not reach an env value. + env: + - name: WRITE_POLICY_NAME + value: secret-configmap-writes ports: [] securityContext: readOnlyRootFilesystem: true diff --git a/packages/kn-next-operator/config/observability/prometheusrule.yaml b/packages/kn-next-operator/config/observability/prometheusrule.yaml index 31c371224..de3d37ce3 100644 --- a/packages/kn-next-operator/config/observability/prometheusrule.yaml +++ b/packages/kn-next-operator/config/observability/prometheusrule.yaml @@ -84,6 +84,61 @@ spec: daemonsets create/update/delete. runbook_url: "https://github.com/getknext-dev/knext/blob/main/docs/runbooks/troubleshooting.md" + # The ValidatingAdmissionPolicy that limits the operator's Secret/ConfigMap + # writes is not in force, so the operator is unrestricted. The gauge is the + # operator's own read of the policy and its binding (internal/writepolicy): + # 0 when either is missing, the binding does not Deny, the policy names a + # different username than the operator runs as, or the objects could not be + # read. Warning, not critical: a missing safety net, not an outage, and a + # cluster older than Kubernetes 1.30 cannot have the policy at all. Per pod + # (no aggregation): each replica reports, and one that cannot read the + # policy is a finding too. `for` is longer than the operator's 5m re-check. + - alert: KnextOperatorWritePolicyNotEnforced + expr: knext_operator_write_policy_enforced == 0 + for: 15m + labels: + severity: warning + annotations: + summary: "the knext operator's Secret/ConfigMap write policy is not in force" + description: >- + Pod {{ $labels.pod }} reports that the ValidatingAdmissionPolicy that + limits the operator to its per-zone signing-key Secrets and JWKS + ConfigMaps is not in force, so a compromised operator could write any + Secret or ConfigMap its RBAC allows. Run `knext doctor` for the + reason (deleted policy or binding, a Kubernetes version older than + 1.30, a binding that only audits, or an operator running under a + different ServiceAccount than the policy names). + runbook_url: "https://github.com/getknext-dev/knext/blob/main/docs/security/threat-model.md" + + # The alert above reads `== 0`, so it is blind exactly when the operator + # never publishes the gauge: an operator older than the self-check, a + # monitor that did not start, or a scrape that fails. PromQL over a series + # nobody emits is an empty vector, which looks like "all enforced". This + # fires on a DISCOVERED operator target (`up` exists only for one) that + # exports no gauge. Keyed on `up`, not a bare absent(): with no operator + # at all there is nothing to report on, and absent() would fire on every + # cluster before install (the same reason the knext.app.staleness rules + # below key on `up`). The job selector matches the metrics Service as the + # bundle renders it (name prefix + controller-manager-metrics-service); + # prometheusrule.test.ts checks it against the manifests. + - alert: KnextOperatorWritePolicyUnreported + expr: >- + up{job=~".*controller-manager-metrics-service"} + unless on (namespace, pod) knext_operator_write_policy_enforced + for: 15m + labels: + severity: warning + annotations: + summary: "the knext operator pod {{ $labels.namespace }}/{{ $labels.pod }} reports no write-policy gauge" + description: >- + Prometheus has discovered this operator pod but it exports no + knext_operator_write_policy_enforced series, so + KnextOperatorWritePolicyNotEnforced cannot fire for it and nothing + says whether its Secret/ConfigMap write policy is in force. Check + that the operator is a release that ships the self-check, that its + metrics scrape succeeds, and run `knext doctor`. + runbook_url: "https://github.com/getknext-dev/knext/blob/main/docs/security/threat-model.md" + # SLO: reconcile latency. A slow control loop delays scale-up and # rollbacks. p95 over 30s sustained is degraded. - alert: KnextOperatorReconcileSlow diff --git a/packages/kn-next-operator/config/rbac/kustomization.yaml b/packages/kn-next-operator/config/rbac/kustomization.yaml index 842b5f862..495644891 100644 --- a/packages/kn-next-operator/config/rbac/kustomization.yaml +++ b/packages/kn-next-operator/config/rbac/kustomization.yaml @@ -7,6 +7,10 @@ resources: - service_account.yaml - role.yaml - role_binding.yaml +# Read-only `get` on the write policy and its binding, so the operator can report whether +# the policy that bounds its Secret/ConfigMap writes is in force (internal/writepolicy). +- write_policy_reader_role.yaml +- write_policy_reader_role_binding.yaml - leader_election_role.yaml - leader_election_role_binding.yaml # The following RBAC configurations are used to protect diff --git a/packages/kn-next-operator/config/rbac/write_policy_reader_role.yaml b/packages/kn-next-operator/config/rbac/write_policy_reader_role.yaml new file mode 100644 index 000000000..763510a87 --- /dev/null +++ b/packages/kn-next-operator/config/rbac/write_policy_reader_role.yaml @@ -0,0 +1,33 @@ +# Lets the operator READ, and only read, the two objects that bound its writes. +# +# The operator checks at start-up and every few minutes that its Secret/ConfigMap +# write policy is in force (internal/writepolicy). That is a `get` on the policy +# and its binding, by name, and nothing else: no list, no watch, no write. A +# role that could write them would let a compromised operator lift its own +# restriction, which internal/controller TestOperatorCannotEditItsOwnWritePolicy +# forbids and TestWritePolicyReaderRoleIsExactlyGetOnTheTwoNamedObjects pins. +# +# It is a separate hand-written role rather than a +kubebuilder:rbac marker on +# manager-role: a marker's resourceNames is a literal, and the bundle's name +# prefix does not reach it. config/default rewrites the name below from the +# policy's real (prefixed) name, so the grant follows a renamed install. +# +# The operator learns its own username with SelfSubjectReview, which every +# authenticated user may create (system:basic-user); that needs no rule here. +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: write-policy-reader + labels: + app.kubernetes.io/name: kn-next-operator + app.kubernetes.io/managed-by: kustomize +rules: +- apiGroups: + - admissionregistration.k8s.io + resources: + - validatingadmissionpolicies + - validatingadmissionpolicybindings + resourceNames: + - secret-configmap-writes + verbs: + - get diff --git a/packages/kn-next-operator/config/rbac/write_policy_reader_role_binding.yaml b/packages/kn-next-operator/config/rbac/write_policy_reader_role_binding.yaml new file mode 100644 index 000000000..b4997dea0 --- /dev/null +++ b/packages/kn-next-operator/config/rbac/write_policy_reader_role_binding.yaml @@ -0,0 +1,15 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: write-policy-reader-rolebinding + labels: + app.kubernetes.io/name: kn-next-operator + app.kubernetes.io/managed-by: kustomize +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: write-policy-reader +subjects: +- kind: ServiceAccount + name: controller-manager + namespace: system diff --git a/packages/kn-next-operator/internal/controller/operator_write_inventory_test.go b/packages/kn-next-operator/internal/controller/operator_write_inventory_test.go index 2cf33c92e..6058d0275 100644 --- a/packages/kn-next-operator/internal/controller/operator_write_inventory_test.go +++ b/packages/kn-next-operator/internal/controller/operator_write_inventory_test.go @@ -116,6 +116,9 @@ var dynamicWriteAllowlist = map[string]string{ var readOnlyKindUses = map[string]string{ "(*BackendServiceReconciler).secretMetadata": "metadata-only GET of a Secret a function references", "(*BackendServiceReconciler).observeZoneJWKS": "metadata-only GET of the zone's JWKS ConfigMap", + "github.com/AhmedElBanna80/knext/packages/kn-next-operator/internal/writepolicy.covers": "reads the write " + + "policy's own matchConstraints and spells the resource names \"secrets\" and \"configmaps\" to check the " + + "policy covers them; it touches no Secret or ConfigMap object", } var writeVerbs = map[string]bool{ diff --git a/packages/kn-next-operator/internal/controller/write_policy_reader_rbac_test.go b/packages/kn-next-operator/internal/controller/write_policy_reader_rbac_test.go new file mode 100644 index 000000000..5bd0d7018 --- /dev/null +++ b/packages/kn-next-operator/internal/controller/write_policy_reader_rbac_test.go @@ -0,0 +1,80 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "strings" + "testing" +) + +// The operator checks that its own Secret/ConfigMap write policy is in force +// (internal/writepolicy). The only RBAC that costs is a read of the policy and +// its binding. This pins it to exactly that: `get`, by name, on those two kinds, +// and nowhere else: not on manager-role, not by wildcard, not list/watch (which +// would expose every admission policy in the cluster), and not under any name +// but the shipped policy's. +// +// TestOperatorCannotEditItsOwnWritePolicy already forbids any WRITE verb on the +// policy kinds; this is the other half, that the read is not widened. +func TestWritePolicyReaderRoleIsExactlyGetOnTheTwoNamedObjects(t *testing.T) { + policy, binding := loadShippedWritePolicy(t) + if policy.Name != binding.Name { + t.Fatalf("the check reads the policy and its binding by one name; they are %q and %q", policy.Name, binding.Name) + } + + policyKinds := map[string]bool{"validatingadmissionpolicies": true, "validatingadmissionpolicybindings": true} + var grants []rbacRuleRef + for _, r := range loadRBACRules(t) { + hit := false + for _, g := range r.rule.APIGroups { + if g == "admissionregistration.k8s.io" || g == "*" { + for _, res := range r.rule.Resources { + hit = hit || policyKinds[res] || res == "*" + } + } + } + if hit { + grants = append(grants, r) + } + } + + // Both halves: a scan that finds no grant would pass the assertions below + // vacuously, and the operator could not read its own policy. + if len(grants) != 1 { + t.Fatalf("want exactly one RBAC rule touching the admission policy kinds, found %d: %+v", len(grants), grants) + } + g := grants[0] + if g.kind != "ClusterRole" || g.name != "write-policy-reader" { + t.Errorf("the grant must be the dedicated write-policy-reader ClusterRole, found %s/%s (%s)", g.kind, g.name, g.file) + } + if got := sortedVerbs(g.rule); got != "get" { + t.Errorf("the grant must be exactly [get], got [%s]", got) + } + if strings.Join(g.rule.ResourceNames, ",") != policy.Name { + t.Errorf("the grant must be limited to the shipped policy's name %q by resourceNames, got %v", policy.Name, g.rule.ResourceNames) + } + got := map[string]bool{} + for _, res := range g.rule.Resources { + got[res] = true + } + if len(got) != 2 || !got["validatingadmissionpolicies"] || !got["validatingadmissionpolicybindings"] { + t.Errorf("the grant must cover exactly the policy and its binding, got %v", g.rule.Resources) + } + if len(g.rule.APIGroups) != 1 || g.rule.APIGroups[0] != "admissionregistration.k8s.io" { + t.Errorf("the grant must name the admissionregistration.k8s.io group only, got %v", g.rule.APIGroups) + } +} diff --git a/packages/kn-next-operator/internal/install/write_policy_selfcheck_test.go b/packages/kn-next-operator/internal/install/write_policy_selfcheck_test.go new file mode 100644 index 000000000..1e4632ea2 --- /dev/null +++ b/packages/kn-next-operator/internal/install/write_policy_selfcheck_test.go @@ -0,0 +1,125 @@ +package install + +import ( + "os" + "strings" + "testing" + + appsv1 "k8s.io/api/apps/v1" + rbacv1 "k8s.io/api/rbac/v1" + "sigs.k8s.io/yaml" +) + +// The operator's write-policy self-check (internal/writepolicy) reads the policy +// and its binding BY NAME, and its role grants `get` on exactly that name. Both +// names are literals in the base manifests; the bundle prefixes every +// cluster-scoped name, and kustomize's name references do not reach a +// Deployment env value or a rule's resourceNames. If either is left as the base +// name, the check reads an object that does not exist and reports the policy +// off while it is on, or (with the role right and the env wrong) the reverse. +// These tests prove the three agree after the prefix is applied. + +func TestRenderedBundleGivesTheSelfCheckTheRenderedPolicyName(t *testing.T) { + raw, err := os.ReadFile(requireBundle(t)) + if err != nil { + t.Fatal(err) + } + o := decodeWritePolicyObjects(t, raw, "dist/install.yaml") + if o.policy == nil || o.binding == nil { + t.Fatal("the rendered bundle carries no write policy and binding") + } + if o.policy.Name != o.binding.Name { + t.Fatalf("the self-check reads the policy and its binding by one name; they render as %q and %q", o.policy.Name, o.binding.Name) + } + var manager *appsv1.Deployment + for i := range o.deployments { + for _, c := range o.deployments[i].Spec.Template.Spec.Containers { + if c.Name == "manager" { + manager = &o.deployments[i] + } + } + } + if manager == nil { + t.Fatal("no manager Deployment in the rendered bundle") + } + var envName string + for _, c := range manager.Spec.Template.Spec.Containers { + if c.Name != "manager" { + continue + } + for _, e := range c.Env { + if e.Name == "WRITE_POLICY_NAME" { + envName = e.Value + } + } + } + if envName != o.policy.Name { + t.Errorf("the manager's WRITE_POLICY_NAME is %q but the policy renders as %q: the self-check would read an object that is not there", + envName, o.policy.Name) + } + + var role *rbacv1.ClusterRole + var binding *rbacv1.ClusterRoleBinding + for _, doc := range decodeYAMLDocs(t, raw, "dist/install.yaml") { + var head struct { + Kind string `json:"kind"` + Metadata struct { + Name string `json:"name"` + } `json:"metadata"` + } + if err := yaml.Unmarshal(doc, &head); err != nil { + continue + } + switch { + case head.Kind == "ClusterRole" && strings.HasSuffix(head.Metadata.Name, "write-policy-reader"): + role = &rbacv1.ClusterRole{} + if err := yaml.Unmarshal(doc, role); err != nil { + t.Fatal(err) + } + case head.Kind == "ClusterRoleBinding" && strings.HasSuffix(head.Metadata.Name, "write-policy-reader-rolebinding"): + binding = &rbacv1.ClusterRoleBinding{} + if err := yaml.Unmarshal(doc, binding); err != nil { + t.Fatal(err) + } + } + } + if role == nil || binding == nil { + t.Fatalf("the rendered bundle must carry the write-policy-reader role and binding (role %v, binding %v)", role != nil, binding != nil) + } + if len(role.Rules) != 1 { + t.Fatalf("want one rule on the write-policy-reader role, got %d", len(role.Rules)) + } + rule := role.Rules[0] + if strings.Join(rule.ResourceNames, ",") != o.policy.Name { + t.Errorf("the role grants get on %v but the policy renders as %q: the check would be forbidden from reading it", rule.ResourceNames, o.policy.Name) + } + if strings.Join(rule.Verbs, ",") != "get" { + t.Errorf("the rendered role must grant exactly get, got %v", rule.Verbs) + } + if binding.RoleRef.Name != role.Name { + t.Errorf("the binding points at ClusterRole %q, the bundle names it %q", binding.RoleRef.Name, role.Name) + } + if len(binding.Subjects) != 1 || binding.Subjects[0].Kind != rbacv1.ServiceAccountKind || + binding.Subjects[0].Name != manager.Spec.Template.Spec.ServiceAccountName || + binding.Subjects[0].Namespace != manager.Namespace { + t.Errorf("the binding must grant the manager's own ServiceAccount (%s/%s), got %+v", + manager.Namespace, manager.Spec.Template.Spec.ServiceAccountName, binding.Subjects) + } +} + +// The policy's own name is what both of the above are rewritten from, so the +// base manifests must keep the policy and its binding under the one name the +// base role and the base env default name. +func TestBaseManifestsNameThePolicyConsistently(t *testing.T) { + o := shippedWritePolicy(t) + if o.policy.Name != "secret-configmap-writes" || o.binding.Name != o.policy.Name { + t.Fatalf("base policy/binding are %q/%q; the self-check's default name is secret-configmap-writes", o.policy.Name, o.binding.Name) + } + var role rbacv1.ClusterRole + if err := yaml.Unmarshal([]byte(repoFile(t, "config/rbac/write_policy_reader_role.yaml")), &role); err != nil { + t.Fatal(err) + } + if len(role.Rules) != 1 || strings.Join(role.Rules[0].ResourceNames, ",") != o.policy.Name { + t.Errorf("the base role must grant get on %q, got %+v", o.policy.Name, role.Rules) + } +} diff --git a/packages/kn-next-operator/internal/metriccontract/golden_test.go b/packages/kn-next-operator/internal/metriccontract/golden_test.go index d5e82fba2..74062300d 100644 --- a/packages/kn-next-operator/internal/metriccontract/golden_test.go +++ b/packages/kn-next-operator/internal/metriccontract/golden_test.go @@ -28,11 +28,12 @@ import ( "github.com/AhmedElBanna80/knext/packages/kn-next-operator/internal/metriccontract" - // The two packages that register on the operator's /metrics today. They are + // The packages that register on the operator's /metrics today. They are // imported ONLY for the live cross-check below; the scan itself discovers // packages from cmd/'s import graph and does not rely on this list. _ "github.com/AhmedElBanna80/knext/packages/kn-next-operator/internal/controller" _ "github.com/AhmedElBanna80/knext/packages/kn-next-operator/internal/webhook/v1alpha1" + _ "github.com/AhmedElBanna80/knext/packages/kn-next-operator/internal/writepolicy" ) // moduleRoot is the operator module, two levels up from this package. diff --git a/packages/kn-next-operator/internal/metriccontract/operator-metrics.golden.json b/packages/kn-next-operator/internal/metriccontract/operator-metrics.golden.json index a5bb836bc..3ba0d8742 100644 --- a/packages/kn-next-operator/internal/metriccontract/operator-metrics.golden.json +++ b/packages/kn-next-operator/internal/metriccontract/operator-metrics.golden.json @@ -7,6 +7,7 @@ "knext_nextapp_reconcile_duration_seconds": "histogram", "knext_nextapp_reconcile_errors_total": "counter", "knext_nextapp_reconcile_total": "counter", + "knext_operator_write_policy_enforced": "gauge", "knext_platform_apps_held": "gauge", "knext_platform_budget_fallback_total": "counter", "knext_platform_holds_total": "counter", diff --git a/packages/kn-next-operator/internal/writepolicy/canonical.go b/packages/kn-next-operator/internal/writepolicy/canonical.go new file mode 100644 index 000000000..8cd9091be --- /dev/null +++ b/packages/kn-next-operator/internal/writepolicy/canonical.go @@ -0,0 +1,85 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package writepolicy + +import ( + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" +) + +// celPair is the part of a policy variable or validation that decides what is +// admitted: its CEL. A variable is identified by Name; a validation is +// identified by its position. Cosmetic fields (message, reason) are not part of +// the comparison; messageExpression is, because it is CEL the apiserver +// evaluates on every refusal. +type celPair struct { + Name string + Expression string + MessageExpression string +} + +// The CEL config/admission/secret-configmap-writes.yaml ships, copied here +// verbatim. A policy whose matchConstraints, identity and binding are all right +// still enforces nothing if its validations were replaced by `true`, so the +// check compares them with this copy. +// +// KEEP IN LOCKSTEP with the shipped policy: +// TestCanonicalPolicyCELEqualsTheShippedPolicy fails if config/admission changes +// without this, and packages/kn-next/src/cli/doctor/checks/write-policy.ts holds +// the same copy (checked by its own test). +// +// A consequence worth stating: an operator that ships newer CEL than the policy +// installed in the cluster (an upgraded operator over a not-yet-reapplied +// bundle) reports PolicyModified until the bundle is reapplied. That is the +// honest answer; the installed policy is not the one this operator expects. +var canonicalVariables = []celPair{ + {Name: "objectName", Expression: "request.operation == 'DELETE' ? oldObject.metadata.name : object.metadata.name"}, + {Name: "allowedSecretSuffixes", Expression: "['-knext-signing-key']"}, + {Name: "allowedConfigMapSuffixes", Expression: "['-knext-jwks']"}, + {Name: "allowedSecretType", Expression: "'Opaque'"}, + {Name: "allowedSuffixes", Expression: "request.resource.resource == 'secrets' ? variables.allowedSecretSuffixes : variables.allowedConfigMapSuffixes"}, +} + +var canonicalValidations = []celPair{ + { + Expression: "variables.allowedSuffixes.exists(s, variables.objectName.endsWith(s) && size(variables.objectName) > size(s))", + MessageExpression: "'the knext operator may not ' + request.operation.lowerAscii() + ' the ' + request.resource.resource + ' object \"' + variables.objectName + '\": it writes only the per-zone signing key and JWKS'", + }, + { + Expression: "request.resource.resource != 'secrets' || request.operation == 'DELETE' || dyn(object).type == variables.allowedSecretType", + }, +} + +// policyCELSet is a policy's variables and validations, reduced to their CEL. +type policyCELSet struct { + Variables []celPair + Validations []celPair +} + +func canonicalCEL() policyCELSet { + return policyCELSet{Variables: canonicalVariables, Validations: canonicalValidations} +} + +func policyCEL(p *admissionregistrationv1.ValidatingAdmissionPolicy) policyCELSet { + var out policyCELSet + for _, v := range p.Spec.Variables { + out.Variables = append(out.Variables, celPair{Name: v.Name, Expression: v.Expression}) + } + for _, v := range p.Spec.Validations { + out.Validations = append(out.Validations, celPair{Expression: v.Expression, MessageExpression: v.MessageExpression}) + } + return out +} diff --git a/packages/kn-next-operator/internal/writepolicy/canonical_test.go b/packages/kn-next-operator/internal/writepolicy/canonical_test.go new file mode 100644 index 000000000..1a4df18ef --- /dev/null +++ b/packages/kn-next-operator/internal/writepolicy/canonical_test.go @@ -0,0 +1,70 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package writepolicy + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + "sigs.k8s.io/yaml" +) + +// The check compares a live policy's variables and validations with the ones +// this operator ships. Those are a copy in verdict.go (canonicalVariables, +// canonicalValidations); this is what keeps the copy from drifting. Change the +// CEL in config/admission without updating the copy (and the shared table's base +// policy) and this goes red, instead of the gauge reporting 0 on a correct, +// freshly installed policy, or 1 on one that no longer enforces what it should. + +func shippedPolicy(t *testing.T) *admissionregistrationv1.ValidatingAdmissionPolicy { + t.Helper() + raw, err := os.ReadFile(filepath.Join("..", "..", "config", "admission", "secret-configmap-writes.yaml")) + if err != nil { + t.Fatal(err) + } + var p admissionregistrationv1.ValidatingAdmissionPolicy + if err := yaml.UnmarshalStrict(raw, &p); err != nil { + t.Fatal(err) + } + // Both halves: an empty shipped policy would make the equality below vacuous. + if len(p.Spec.Variables) == 0 || len(p.Spec.Validations) == 0 { + t.Fatalf("the shipped policy parsed to %d variables and %d validations", len(p.Spec.Variables), len(p.Spec.Validations)) + } + return &p +} + +func TestCanonicalPolicyCELEqualsTheShippedPolicy(t *testing.T) { + p := shippedPolicy(t) + got, want := policyCEL(p), canonicalCEL() + if !reflect.DeepEqual(got, want) { + t.Errorf("config/admission/secret-configmap-writes.yaml differs from the canonical copy in verdict.go.\nshipped %#v\ncanonical %#v\n"+ + "Update canonicalVariables/canonicalValidations, the base policy in testdata/verdict-cases.json and the TS copy in "+ + "packages/kn-next/src/cli/doctor/checks/write-policy.ts together.", got, want) + } +} + +// "Enforced" in the shared table must be proved against the CEL that ships. +func TestSharedTableBasePolicyCarriesTheShippedCEL(t *testing.T) { + f := loadVerdictFixture(t) + p, _, _ := objectsFor(t, f, verdictCase{Name: "base"}) + if got, want := policyCEL(p), policyCEL(shippedPolicy(t)); !reflect.DeepEqual(got, want) { + t.Errorf("testdata/verdict-cases.json's base policy CEL differs from the shipped policy:\n%#v\n%#v", got, want) + } +} diff --git a/packages/kn-next-operator/internal/writepolicy/envtest_test.go b/packages/kn-next-operator/internal/writepolicy/envtest_test.go new file mode 100644 index 000000000..b047aaa5a --- /dev/null +++ b/packages/kn-next-operator/internal/writepolicy/envtest_test.go @@ -0,0 +1,242 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package writepolicy + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/go-logr/logr" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/yaml" +) + +// The self-check against a real kube-apiserver: the shipped policy and binding, +// the shipped read-only role, and SelfSubjectReview for the operator's identity. +// Each unenforced case is arranged on the live cluster the way it happens in +// production (a deleted object, a policy for another identity, an audit-only +// binding) and must drop the gauge to 0; the matching case must raise it to 1. + +const policyName = "secret-configmap-writes" + +func startEnvtest(t *testing.T) (*rest.Config, *envtest.Environment) { + t.Helper() + env := &envtest.Environment{} + if os.Getenv("KUBEBUILDER_ASSETS") == "" { + base := filepath.Join("..", "..", "bin", "k8s") + if entries, err := os.ReadDir(base); err == nil { + for _, e := range entries { + if e.IsDir() { + env.BinaryAssetsDirectory = filepath.Join(base, e.Name()) + break + } + } + } + } + cfg, err := env.Start() + if err != nil { + t.Fatalf("start envtest: %v", err) + } + t.Cleanup(func() { _ = env.Stop() }) + return cfg, env +} + +// shippedObjects loads the policy and binding exactly as config/admission ships +// them, with the policy's identity replaced by username (the shipped literals +// are the unprefixed bundle's). +func shippedObjects(t *testing.T, username string) (*admissionregistrationv1.ValidatingAdmissionPolicy, *admissionregistrationv1.ValidatingAdmissionPolicyBinding) { + t.Helper() + read := func(name string, into any) { + raw, err := os.ReadFile(filepath.Join("..", "..", "config", "admission", name)) + if err != nil { + t.Fatal(err) + } + if err := yaml.UnmarshalStrict(raw, into); err != nil { + t.Fatalf("%s: %v", name, err) + } + } + p := &admissionregistrationv1.ValidatingAdmissionPolicy{} + b := &admissionregistrationv1.ValidatingAdmissionPolicyBinding{} + read("secret-configmap-writes.yaml", p) + read("secret-configmap-writes-binding.yaml", b) + if len(p.Spec.MatchConditions) != 1 { + t.Fatalf("the shipped policy must carry one matchCondition, has %d", len(p.Spec.MatchConditions)) + } + p.Spec.MatchConditions[0].Expression = "request.userInfo.username == '" + username + "'" + return p, b +} + +func TestSelfCheckAgainstARealApiserver(t *testing.T) { + cfg, env := startEnvtest(t) + ctx := context.Background() + admin, err := kubernetes.NewForConfig(cfg) + if err != nil { + t.Fatal(err) + } + src, err := NewClientSource(cfg) + if err != nil { + t.Fatal(err) + } + // SelfSubjectReview is how the operator learns the username the apiserver + // sees; the policy below confines exactly that. + self, err := src.OperatorUsername(ctx) + if err != nil || self == "" { + t.Fatalf("SelfSubjectReview returned %q, %v", self, err) + } + + install := func(t *testing.T, username string, mutateBinding func(*admissionregistrationv1.ValidatingAdmissionPolicyBinding)) { + t.Helper() + p, b := shippedObjects(t, username) + if mutateBinding != nil { + mutateBinding(b) + } + if _, err := admin.AdmissionregistrationV1().ValidatingAdmissionPolicies().Create(ctx, p, metav1.CreateOptions{}); err != nil { + t.Fatalf("the shipped policy is not accepted: %v", err) + } + if _, err := admin.AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Create(ctx, b, metav1.CreateOptions{}); err != nil { + t.Fatalf("the shipped binding is not accepted: %v", err) + } + t.Cleanup(func() { + _ = admin.AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Delete(ctx, policyName, metav1.DeleteOptions{}) + _ = admin.AdmissionregistrationV1().ValidatingAdmissionPolicies().Delete(ctx, policyName, metav1.DeleteOptions{}) + }) + } + check := func(t *testing.T) Verdict { + t.Helper() + m := &Monitor{Source: src, Name: policyName, Log: logr.Discard()} + return m.CheckOnce(ctx) + } + + t.Run("the policy present and matching: gauge 1", func(t *testing.T) { + install(t, self, nil) + v := check(t) + if !v.Enforced || gaugeValue(t) != 1 { + t.Fatalf("verdict %+v, gauge %v: want enforced and 1", v, gaugeValue(t)) + } + }) + + t.Run("the policy deleted: gauge 0", func(t *testing.T) { + install(t, self, nil) + if err := admin.AdmissionregistrationV1().ValidatingAdmissionPolicies().Delete(ctx, policyName, metav1.DeleteOptions{}); err != nil { + t.Fatal(err) + } + v := check(t) + if v.Enforced || v.Reason != ReasonPolicyMissing || gaugeValue(t) != 0 { + t.Fatalf("verdict %+v, gauge %v: want PolicyMissing and 0", v, gaugeValue(t)) + } + }) + + t.Run("the binding deleted: gauge 0", func(t *testing.T) { + install(t, self, nil) + if err := admin.AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Delete(ctx, policyName, metav1.DeleteOptions{}); err != nil { + t.Fatal(err) + } + v := check(t) + if v.Enforced || v.Reason != ReasonBindingMissing || gaugeValue(t) != 0 { + t.Fatalf("verdict %+v, gauge %v: want BindingMissing and 0", v, gaugeValue(t)) + } + }) + + t.Run("the policy confines another username: gauge 0", func(t *testing.T) { + install(t, "system:serviceaccount:somewhere-else:not-the-operator", nil) + v := check(t) + if v.Enforced || v.Reason != ReasonIdentityMismatch || gaugeValue(t) != 0 { + t.Fatalf("verdict %+v, gauge %v: want IdentityMismatch and 0", v, gaugeValue(t)) + } + }) + + t.Run("the binding only audits: gauge 0", func(t *testing.T) { + install(t, self, func(b *admissionregistrationv1.ValidatingAdmissionPolicyBinding) { + b.Spec.ValidationActions = []admissionregistrationv1.ValidationAction{admissionregistrationv1.Audit} + }) + v := check(t) + if v.Enforced || v.Reason != ReasonBindingNotDeny || gaugeValue(t) != 0 { + t.Fatalf("verdict %+v, gauge %v: want BindingNotDeny and 0", v, gaugeValue(t)) + } + }) + + // The RBAC the operator is given for this is `get` on the two named + // objects. Prove it is sufficient, and no wider, on a real apiserver: an + // identity holding exactly the shipped role runs the whole check. + t.Run("the shipped read-only role is enough, and no more", func(t *testing.T) { + raw, err := os.ReadFile(filepath.Join("..", "..", "config", "rbac", "write_policy_reader_role.yaml")) + if err != nil { + t.Fatal(err) + } + var role rbacv1.ClusterRole + if err := yaml.Unmarshal(raw, &role); err != nil || len(role.Rules) == 0 { + t.Fatalf("write_policy_reader_role.yaml parsed to %d rules (err %v)", len(role.Rules), err) + } + if strings.Join(role.Rules[0].ResourceNames, ",") != policyName { + t.Fatalf("the shipped role names %v, this proof installs %q", role.Rules[0].ResourceNames, policyName) + } + role.Name = "knext-write-policy-reader-proof" + const reader = "system:serviceaccount:kn-next-operator-system:proof-operator" + if _, err := admin.RbacV1().ClusterRoles().Create(ctx, &role, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + crb := &rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: role.Name}, + RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "ClusterRole", Name: role.Name}, + Subjects: []rbacv1.Subject{{Kind: rbacv1.ServiceAccountKind, Namespace: "kn-next-operator-system", Name: "proof-operator"}}, + } + if _, err := admin.RbacV1().ClusterRoleBindings().Create(ctx, crb, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } + u, err := env.AddUser(envtest.User{Name: reader, Groups: []string{"system:serviceaccounts", "system:serviceaccounts:kn-next-operator-system"}}, nil) + if err != nil { + t.Fatal(err) + } + readerSrc, err := NewClientSource(u.Config()) + if err != nil { + t.Fatal(err) + } + install(t, reader, nil) + m := &Monitor{Source: readerSrc, Name: policyName, Log: logr.Discard()} + if v := m.CheckOnce(ctx); !v.Enforced || gaugeValue(t) != 1 { + t.Fatalf("with exactly the shipped role the check must succeed, got %+v (gauge %v)", v, gaugeValue(t)) + } + rc, err := kubernetes.NewForConfig(u.Config()) + if err != nil { + t.Fatal(err) + } + if _, err := rc.AdmissionregistrationV1().ValidatingAdmissionPolicies().List(ctx, metav1.ListOptions{}); !apierrors.IsForbidden(err) { + t.Errorf("the role must not allow listing policies, got %v", err) + } + if _, err := rc.AdmissionregistrationV1().ValidatingAdmissionPolicies().Get(ctx, "some-other-policy", metav1.GetOptions{}); !apierrors.IsForbidden(err) { + t.Errorf("the role must not allow reading any other policy, got %v", err) + } + if err := rc.AdmissionregistrationV1().ValidatingAdmissionPolicies().Delete(ctx, policyName, metav1.DeleteOptions{}); !apierrors.IsForbidden(err) { + t.Errorf("the role must not allow deleting the policy, got %v", err) + } + // A read the role does not permit is reported as not enforced, not as enforced. + other := &Monitor{Source: readerSrc, Name: "not-in-the-role", Log: logr.Discard()} + if v := other.CheckOnce(ctx); v.Enforced || v.Reason != ReasonUnverifiable || gaugeValue(t) != 0 { + t.Errorf("a forbidden read must be Unverifiable and 0, got %+v (gauge %v)", v, gaugeValue(t)) + } + }) + +} diff --git a/packages/kn-next-operator/internal/writepolicy/monitor.go b/packages/kn-next-operator/internal/writepolicy/monitor.go new file mode 100644 index 000000000..ed475c390 --- /dev/null +++ b/packages/kn-next-operator/internal/writepolicy/monitor.go @@ -0,0 +1,134 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package writepolicy + +import ( + "context" + "time" + + "github.com/go-logr/logr" + "github.com/prometheus/client_golang/prometheus" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +// DefaultInterval is how often the live objects are read again. A policy +// deleted after start-up is the case a one-shot check would miss; a get on two +// named objects every few minutes costs the apiserver nothing. +const DefaultInterval = 5 * time.Minute + +// enforcedGauge is 1 when the operator's Secret/ConfigMap write policy is in +// force for this operator and 0 otherwise. 0 also covers "could not verify": +// the control is only reported as on when it was seen to be. Unlabeled by +// design (one operator, one policy), so the series cannot multiply. +var enforcedGauge = prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "knext_operator_write_policy_enforced", + Help: "1 when the ValidatingAdmissionPolicy that limits the operator's Secret and ConfigMap writes " + + "exists, is bound with Deny, and names the identity the operator runs as; 0 when it is off or " + + "could not be verified (the operator is then unrestricted).", +}) + +func init() { + metrics.Registry.MustRegister(enforcedGauge) +} + +// Source reads the objects the verdict needs. Policy and Binding return a nil +// object and a nil error when it does not exist. +type Source interface { + Policy(ctx context.Context, name string) (*admissionregistrationv1.ValidatingAdmissionPolicy, error) + Binding(ctx context.Context, name string) (*admissionregistrationv1.ValidatingAdmissionPolicyBinding, error) + // OperatorUsername is the username the apiserver sees for this process. + OperatorUsername(ctx context.Context) (string, error) +} + +// Check reads the policy, its binding and the operator's own identity from src +// and decides. A read that fails is Unverifiable, which is not enforced. +func Check(ctx context.Context, src Source, name string) Verdict { + username, err := src.OperatorUsername(ctx) + if err != nil { + return no(ReasonUnverifiable, "the operator could not look up its own identity: %v", err) + } + policy, err := src.Policy(ctx, name) + if err != nil { + return no(ReasonUnverifiable, "reading ValidatingAdmissionPolicy %q failed: %v", name, err) + } + binding, err := src.Binding(ctx, name) + if err != nil { + return no(ReasonUnverifiable, "reading ValidatingAdmissionPolicyBinding %q failed: %v", name, err) + } + return Evaluate(policy, binding, username) +} + +// Monitor is a manager Runnable that keeps the gauge current. It runs on every +// replica, leader or not: the gauge describes the cluster, and a standby that +// reported nothing would leave a gap in the alert. +type Monitor struct { + Source Source + Name string + Interval time.Duration + Log logr.Logger + + last *Verdict +} + +// NeedLeaderElection is false: see Monitor. +func (m *Monitor) NeedLeaderElection() bool { return false } + +// CheckOnce runs one check, publishes the gauge and logs a change. +func (m *Monitor) CheckOnce(ctx context.Context) Verdict { + v := Check(ctx, m.Source, m.Name) + if v.Enforced { + enforcedGauge.Set(1) + } else { + enforcedGauge.Set(0) + } + changed := m.last == nil || m.last.Enforced != v.Enforced || m.last.Reason != v.Reason + m.last = &v + switch { + case !changed: + case v.Enforced: + m.Log.Info("the operator write policy is in force", "policy", m.Name) + default: + // One line per change, not per check: a policy that stays off is already + // on the gauge and the alert. + m.Log.Info("WARNING: the operator write policy is NOT in force; the operator can write any Secret or ConfigMap "+ + "its RBAC allows", "policy", m.Name, "reason", string(v.Reason), "detail", v.Detail) + } + return v +} + +// Start checks immediately, then every Interval until ctx is cancelled. It +// returns nil on cancellation and never an error for a failed check: not +// starting because a security control is off is a decision for the operator's +// owner, not for this monitor. +func (m *Monitor) Start(ctx context.Context) error { + interval := m.Interval + if interval <= 0 { + interval = DefaultInterval + } + m.CheckOnce(ctx) + t := time.NewTicker(interval) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return nil + case <-t.C: + m.CheckOnce(ctx) + } + } +} diff --git a/packages/kn-next-operator/internal/writepolicy/monitor_test.go b/packages/kn-next-operator/internal/writepolicy/monitor_test.go new file mode 100644 index 000000000..c64ee32a0 --- /dev/null +++ b/packages/kn-next-operator/internal/writepolicy/monitor_test.go @@ -0,0 +1,222 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package writepolicy + +import ( + "context" + "errors" + "strings" + "sync" + "testing" + "time" + + "github.com/go-logr/logr" + "github.com/go-logr/logr/funcr" + dto "github.com/prometheus/client_model/go" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" +) + +// fakeSource serves the fixture's objects, with each part replaceable per test. +type fakeSource struct { + mu sync.Mutex + policy *admissionregistrationv1.ValidatingAdmissionPolicy + binding *admissionregistrationv1.ValidatingAdmissionPolicyBinding + username string + policyErr, bindingErr, whoErr error + calls int +} + +func (s *fakeSource) Policy(context.Context, string) (*admissionregistrationv1.ValidatingAdmissionPolicy, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.calls++ + return s.policy, s.policyErr +} + +func (s *fakeSource) Binding(context.Context, string) (*admissionregistrationv1.ValidatingAdmissionPolicyBinding, error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.binding, s.bindingErr +} + +func (s *fakeSource) OperatorUsername(context.Context) (string, error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.username, s.whoErr +} + +func (s *fakeSource) set(f func(*fakeSource)) { + s.mu.Lock() + defer s.mu.Unlock() + f(s) +} + +func healthySource(t *testing.T) *fakeSource { + t.Helper() + f := loadVerdictFixture(t) + p, b, username := objectsFor(t, f, verdictCase{Name: "healthy"}) + return &fakeSource{policy: p, binding: b, username: username} +} + +func gaugeValue(t *testing.T) float64 { + t.Helper() + var m dto.Metric + if err := enforcedGauge.Write(&m); err != nil { + t.Fatal(err) + } + return m.GetGauge().GetValue() +} + +type logLines struct { + mu sync.Mutex + lines []string +} + +func (l *logLines) logger() logr.Logger { + return funcr.New(func(prefix, args string) { + l.mu.Lock() + defer l.mu.Unlock() + l.lines = append(l.lines, prefix+" "+args) + }, funcr.Options{}) +} + +func (l *logLines) warnings() []string { + l.mu.Lock() + defer l.mu.Unlock() + var out []string + for _, s := range l.lines { + if strings.Contains(s, "WARNING") { + out = append(out, s) + } + } + return out +} + +func TestGaugeIsOneWhenThePolicyIsEnforced(t *testing.T) { + src := healthySource(t) + logs := &logLines{} + m := &Monitor{Source: src, Name: "kn-next-operator-secret-configmap-writes", Log: logs.logger()} + if v := m.CheckOnce(context.Background()); !v.Enforced { + t.Fatalf("want enforced, got %+v", v) + } + if got := gaugeValue(t); got != 1 { + t.Fatalf("gauge = %v, want 1", got) + } + if w := logs.warnings(); len(w) != 0 { + t.Fatalf("an enforced policy must log no warning, got %v", w) + } +} + +func TestGaugeIsZeroAndOneWarningIsLoggedWhenNotEnforced(t *testing.T) { + for _, tc := range []struct { + name string + mut func(*fakeSource) + want Reason + }{ + {"policy deleted", func(s *fakeSource) { s.policy = nil }, ReasonPolicyMissing}, + {"binding deleted", func(s *fakeSource) { s.binding = nil }, ReasonBindingMissing}, + {"username mismatch", func(s *fakeSource) { s.username = "system:serviceaccount:elsewhere:op" }, ReasonIdentityMismatch}, + {"binding audits only", func(s *fakeSource) { + s.binding.Spec.ValidationActions = []admissionregistrationv1.ValidationAction{admissionregistrationv1.Audit} + }, ReasonBindingNotDeny}, + {"the identity lookup fails", func(s *fakeSource) { s.whoErr = errors.New("selfsubjectreviews is forbidden") }, ReasonUnverifiable}, + {"the policy read fails", func(s *fakeSource) { s.policyErr = errors.New("connection refused") }, ReasonUnverifiable}, + } { + t.Run(tc.name, func(t *testing.T) { + src := healthySource(t) + src.set(tc.mut) + logs := &logLines{} + m := &Monitor{Source: src, Name: "kn-next-operator-secret-configmap-writes", Log: logs.logger()} + // Repeated checks of the same broken state: one warning, not one per tick. + var v Verdict + for i := 0; i < 3; i++ { + v = m.CheckOnce(context.Background()) + } + if v.Enforced || v.Reason != tc.want { + t.Fatalf("verdict = %+v, want not enforced with reason %s", v, tc.want) + } + if got := gaugeValue(t); got != 0 { + t.Fatalf("gauge = %v, want 0", got) + } + if w := logs.warnings(); len(w) != 1 { + t.Fatalf("want exactly one warning across three identical checks, got %d: %v", len(w), w) + } + }) + } +} + +// A control that comes back is announced, and one that goes away again warns +// again: the log follows the state, not the clock. +func TestWarningFollowsTheStateAcrossRecoveryAndRegression(t *testing.T) { + src := healthySource(t) + logs := &logLines{} + m := &Monitor{Source: src, Name: "n", Log: logs.logger()} + ctx := context.Background() + savedPolicy := src.policy + + m.CheckOnce(ctx) + src.set(func(s *fakeSource) { s.policy = nil }) + m.CheckOnce(ctx) + if gaugeValue(t) != 0 || len(logs.warnings()) != 1 { + t.Fatalf("deleting the policy must drop the gauge to 0 and warn once (gauge %v, warnings %v)", gaugeValue(t), logs.warnings()) + } + src.set(func(s *fakeSource) { s.policy = savedPolicy }) + m.CheckOnce(ctx) + if gaugeValue(t) != 1 { + t.Fatalf("restoring the policy must raise the gauge to 1, got %v", gaugeValue(t)) + } + src.set(func(s *fakeSource) { s.policy = nil }) + m.CheckOnce(ctx) + if len(logs.warnings()) != 2 { + t.Fatalf("a second regression must warn again, got %v", logs.warnings()) + } +} + +func TestMonitorRunsOnEveryReplicaAndRechecksOnAnInterval(t *testing.T) { + src := healthySource(t) + m := &Monitor{Source: src, Name: "n", Interval: 10 * time.Millisecond, Log: logr.Discard()} + // The gauge describes the cluster, not leadership: a standby that never + // leads must still report it. + if m.NeedLeaderElection() { + t.Fatal("the self-check must run on every replica, leader or not") + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan error, 1) + go func() { done <- m.Start(ctx) }() + deadline := time.Now().Add(5 * time.Second) + for { + src.mu.Lock() + n := src.calls + src.mu.Unlock() + if n >= 3 { + break + } + if time.Now().After(deadline) { + t.Fatalf("the monitor checked %d times in 5s; a deletion after start-up would go unseen", n) + } + time.Sleep(5 * time.Millisecond) + } + cancel() + select { + case err := <-done: + if err != nil { + t.Fatalf("Start returned %v on cancellation", err) + } + case <-time.After(5 * time.Second): + t.Fatal("Start did not return after the context was cancelled") + } +} diff --git a/packages/kn-next-operator/internal/writepolicy/source.go b/packages/kn-next-operator/internal/writepolicy/source.go new file mode 100644 index 000000000..adf35109c --- /dev/null +++ b/packages/kn-next-operator/internal/writepolicy/source.go @@ -0,0 +1,80 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package writepolicy + +import ( + "context" + "fmt" + + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + authenticationv1 "k8s.io/api/authentication/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" +) + +// ClientSource reads the live objects with a plain clientset. +// +// It does NOT use the manager's cached client: that one lists and watches, and +// the operator's role grants only `get` on these two named objects +// (config/rbac/write_policy_reader_role.yaml). Nothing here writes. +type ClientSource struct { + cs kubernetes.Interface +} + +// NewClientSource builds a ClientSource from the operator's own rest config, so +// the identity it reports is the identity the operator's writes carry. +func NewClientSource(cfg *rest.Config) (*ClientSource, error) { + cs, err := kubernetes.NewForConfig(cfg) + if err != nil { + return nil, fmt.Errorf("building the write-policy clientset: %w", err) + } + return &ClientSource{cs: cs}, nil +} + +// Policy gets the named ValidatingAdmissionPolicy; (nil, nil) when it is absent. +func (s *ClientSource) Policy(ctx context.Context, name string) (*admissionregistrationv1.ValidatingAdmissionPolicy, error) { + p, err := s.cs.AdmissionregistrationV1().ValidatingAdmissionPolicies().Get(ctx, name, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + return nil, nil + } + return p, err +} + +// Binding gets the named ValidatingAdmissionPolicyBinding; (nil, nil) when absent. +func (s *ClientSource) Binding(ctx context.Context, name string) (*admissionregistrationv1.ValidatingAdmissionPolicyBinding, error) { + b, err := s.cs.AdmissionregistrationV1().ValidatingAdmissionPolicyBindings().Get(ctx, name, metav1.GetOptions{}) + if apierrors.IsNotFound(err) { + return nil, nil + } + return b, err +} + +// OperatorUsername asks the apiserver who it thinks this process is +// (SelfSubjectReview, GA in Kubernetes 1.28). That is the username an +// admission policy's matchCondition sees, so comparing against it catches a +// namespace or service-account name that differs from what the bundle rendered. +// Every authenticated user may create a SelfSubjectReview (system:basic-user), +// so this adds nothing to the operator's role. +func (s *ClientSource) OperatorUsername(ctx context.Context) (string, error) { + r, err := s.cs.AuthenticationV1().SelfSubjectReviews().Create(ctx, &authenticationv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + return "", err + } + return r.Status.UserInfo.Username, nil +} diff --git a/packages/kn-next-operator/internal/writepolicy/testdata/verdict-cases.json b/packages/kn-next-operator/internal/writepolicy/testdata/verdict-cases.json new file mode 100644 index 000000000..b0e393b79 --- /dev/null +++ b/packages/kn-next-operator/internal/writepolicy/testdata/verdict-cases.json @@ -0,0 +1,812 @@ +{ + "$comment": "Shared by the operator (internal/writepolicy) and by `knext doctor` (packages/kn-next/src/cli/doctor/checks/write-policy.ts): both must reach the same verdict on the same objects, so the self-check and the CLI cannot drift. Objects are shaped as the apiserver returns them for the rendered bundle (name prefix and namespace applied). Ops: remove-policy, remove-binding, set (target policy|binding, dotted path, value; arrays are replaced whole), identity (the operator's username; absent = the rendered one). The base policy carries the SHIPPED variables and validations verbatim (config/admission/secret-configmap-writes.yaml); a test in each language fails if the shipped CEL changes without this file and the check's canonical copy being updated.", + "operatorUsername": "system:serviceaccount:kn-next-operator-system:kn-next-operator-controller-manager", + "policy": { + "apiVersion": "admissionregistration.k8s.io/v1", + "kind": "ValidatingAdmissionPolicy", + "metadata": { + "name": "kn-next-operator-secret-configmap-writes" + }, + "spec": { + "failurePolicy": "Fail", + "matchConstraints": { + "resourceRules": [ + { + "apiGroups": [""], + "apiVersions": ["v1"], + "operations": ["CREATE", "UPDATE", "DELETE"], + "resources": ["secrets", "configmaps"] + } + ] + }, + "matchConditions": [ + { + "name": "request-is-from-the-operator", + "expression": "request.userInfo.username == 'system:serviceaccount:' + 'kn-next-operator-system' + ':' + 'kn-next-operator-controller-manager'" + } + ], + "variables": [ + { + "name": "objectName", + "expression": "request.operation == 'DELETE' ? oldObject.metadata.name : object.metadata.name" + }, + { + "name": "allowedSecretSuffixes", + "expression": "['-knext-signing-key']" + }, + { + "name": "allowedConfigMapSuffixes", + "expression": "['-knext-jwks']" + }, + { + "name": "allowedSecretType", + "expression": "'Opaque'" + }, + { + "name": "allowedSuffixes", + "expression": "request.resource.resource == 'secrets' ? variables.allowedSecretSuffixes : variables.allowedConfigMapSuffixes" + } + ], + "validations": [ + { + "expression": "variables.allowedSuffixes.exists(s, variables.objectName.endsWith(s) && size(variables.objectName) > size(s))", + "messageExpression": "'the knext operator may not ' + request.operation.lowerAscii() + ' the ' + request.resource.resource + ' object \"' + variables.objectName + '\": it writes only the per-zone signing key and JWKS'", + "reason": "Forbidden" + }, + { + "expression": "request.resource.resource != 'secrets' || request.operation == 'DELETE' || dyn(object).type == variables.allowedSecretType", + "message": "the knext operator may write only Opaque Secrets: a service-account-token or any other Secret type under an allowed name would let it mint a credential", + "reason": "Forbidden" + } + ] + } + }, + "binding": { + "apiVersion": "admissionregistration.k8s.io/v1", + "kind": "ValidatingAdmissionPolicyBinding", + "metadata": { + "name": "kn-next-operator-secret-configmap-writes" + }, + "spec": { + "policyName": "kn-next-operator-secret-configmap-writes", + "validationActions": ["Deny", "Audit"] + } + }, + "cases": [ + { + "name": "policy and binding present and matching", + "ops": [], + "enforced": true, + "reason": "Enforced" + }, + { + "name": "Deny alone is enough", + "ops": [ + { + "op": "set", + "target": "binding", + "path": "spec.validationActions", + "value": ["Deny"] + } + ], + "enforced": true, + "reason": "Enforced" + }, + { + "name": "policy deleted", + "ops": [ + { + "op": "remove-policy" + } + ], + "enforced": false, + "reason": "PolicyMissing" + }, + { + "name": "binding deleted", + "ops": [ + { + "op": "remove-binding" + } + ], + "enforced": false, + "reason": "BindingMissing" + }, + { + "name": "both deleted reports the policy first", + "ops": [ + { + "op": "remove-policy" + }, + { + "op": "remove-binding" + } + ], + "enforced": false, + "reason": "PolicyMissing" + }, + { + "name": "operator runs as another namespace", + "ops": [ + { + "op": "identity", + "value": "system:serviceaccount:platform:kn-next-operator-controller-manager" + } + ], + "enforced": false, + "reason": "IdentityMismatch" + }, + { + "name": "operator runs as another service account", + "ops": [ + { + "op": "identity", + "value": "system:serviceaccount:kn-next-operator-system:controller-manager" + } + ], + "enforced": false, + "reason": "IdentityMismatch" + }, + { + "name": "the operator's identity cannot be determined", + "ops": [ + { + "op": "identity", + "value": "" + } + ], + "enforced": false, + "reason": "IdentityUnknown" + }, + { + "name": "binding audits only", + "ops": [ + { + "op": "set", + "target": "binding", + "path": "spec.validationActions", + "value": ["Audit"] + } + ], + "enforced": false, + "reason": "BindingNotDeny" + }, + { + "name": "binding warns only", + "ops": [ + { + "op": "set", + "target": "binding", + "path": "spec.validationActions", + "value": ["Warn", "Audit"] + } + ], + "enforced": false, + "reason": "BindingNotDeny" + }, + { + "name": "binding names a different policy", + "ops": [ + { + "op": "set", + "target": "binding", + "path": "spec.policyName", + "value": "something-else" + } + ], + "enforced": false, + "reason": "BindingPolicyMismatch" + }, + { + "name": "binding scoped to labelled namespaces", + "ops": [ + { + "op": "set", + "target": "binding", + "path": "spec.matchResources", + "value": { + "namespaceSelector": { + "matchLabels": { + "scoped": "true" + } + } + } + } + ], + "enforced": false, + "reason": "BindingScoped" + }, + { + "name": "policy fails open", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.failurePolicy", + "value": "Ignore" + } + ], + "enforced": false, + "reason": "PolicyFailsOpen" + }, + { + "name": "policy does not cover UPDATE", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.resourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["v1"], + "operations": ["CREATE", "DELETE"], + "resources": ["secrets", "configmaps"] + } + ] + } + ], + "enforced": false, + "reason": "PolicyScope" + }, + { + "name": "policy does not cover ConfigMaps", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.resourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["v1"], + "operations": ["CREATE", "UPDATE", "DELETE"], + "resources": ["secrets"] + } + ] + } + ], + "enforced": false, + "reason": "PolicyScope" + }, + { + "name": "wildcard rule covers everything", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.resourceRules", + "value": [ + { + "apiGroups": ["*"], + "apiVersions": ["*"], + "operations": ["*"], + "resources": ["*"] + } + ] + } + ], + "enforced": true, + "reason": "Enforced" + }, + { + "name": "matchCondition is not a plain equality", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConditions", + "value": [ + { + "name": "request-is-from-the-operator", + "expression": "request.userInfo.username.startsWith('system:serviceaccount:')" + } + ] + } + ], + "enforced": false, + "reason": "IdentityUnparseable" + }, + { + "name": "matchCondition is negated", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConditions", + "value": [ + { + "name": "request-is-from-the-operator", + "expression": "request.userInfo.username != 'system:serviceaccount:' + 'kn-next-operator-system' + ':' + 'kn-next-operator-controller-manager'" + } + ] + } + ], + "enforced": false, + "reason": "IdentityUnparseable" + }, + { + "name": "policy has no matchCondition", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConditions", + "value": [] + } + ], + "enforced": false, + "reason": "IdentityUnparseable" + }, + { + "name": "identity spelled as one literal", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConditions", + "value": [ + { + "name": "request-is-from-the-operator", + "expression": "request.userInfo.username == 'system:serviceaccount:kn-next-operator-system:kn-next-operator-controller-manager'" + } + ] + } + ], + "enforced": true, + "reason": "Enforced" + }, + { + "name": "excludeResourceRules removes Secrets from the policy", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.excludeResourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["v1"], + "operations": ["*"], + "resources": ["secrets"] + } + ] + } + ], + "enforced": false, + "reason": "PolicyScope" + }, + { + "name": "excludeResourceRules removes ConfigMaps from the policy", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.excludeResourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["*"], + "operations": ["CREATE", "UPDATE"], + "resources": ["configmaps"] + } + ] + } + ], + "enforced": false, + "reason": "PolicyScope" + }, + { + "name": "the covering rule names objects (resourceNames)", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.resourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["v1"], + "operations": ["CREATE", "UPDATE", "DELETE"], + "resources": ["secrets", "configmaps"], + "resourceNames": ["only-this-one"] + } + ] + } + ], + "enforced": false, + "reason": "PolicyScope" + }, + { + "name": "the covering rule is cluster-scoped", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.resourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["v1"], + "operations": ["CREATE", "UPDATE", "DELETE"], + "resources": ["secrets", "configmaps"], + "scope": "Cluster" + } + ] + } + ], + "enforced": false, + "reason": "PolicyScope" + }, + { + "name": "the covering rule is for another apiVersion", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.resourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["v1beta1"], + "operations": ["CREATE", "UPDATE", "DELETE"], + "resources": ["secrets", "configmaps"] + } + ] + } + ], + "enforced": false, + "reason": "PolicyScope" + }, + { + "name": "a rule scoped Namespaced still covers Secrets and ConfigMaps", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.resourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["v1"], + "operations": ["CREATE", "UPDATE", "DELETE"], + "resources": ["secrets", "configmaps"], + "scope": "Namespaced" + } + ] + } + ], + "enforced": true, + "reason": "Enforced" + }, + { + "name": "a wildcard scope and apiVersion still cover them", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.resourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["*"], + "operations": ["CREATE", "UPDATE", "DELETE"], + "resources": ["secrets", "configmaps"], + "scope": "*" + } + ] + } + ], + "enforced": true, + "reason": "Enforced" + }, + { + "name": "one unrestricted rule is enough beside a rule with resourceNames", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.matchConstraints.resourceRules", + "value": [ + { + "apiGroups": [""], + "apiVersions": ["v1"], + "operations": ["CREATE", "UPDATE", "DELETE"], + "resources": ["secrets", "configmaps"], + "resourceNames": ["x"] + }, + { + "apiGroups": [""], + "apiVersions": ["v1"], + "operations": ["CREATE", "UPDATE", "DELETE"], + "resources": ["secrets", "configmaps"] + } + ] + } + ], + "enforced": true, + "reason": "Enforced" + }, + { + "name": "validations replaced by a constant true", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.validations", + "value": [ + { + "expression": "true" + } + ] + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "a validation removed", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.validations", + "value": [ + { + "expression": "variables.allowedSuffixes.exists(s, variables.objectName.endsWith(s) && size(variables.objectName) > size(s))", + "messageExpression": "'the knext operator may not ' + request.operation.lowerAscii() + ' the ' + request.resource.resource + ' object \"' + variables.objectName + '\": it writes only the per-zone signing key and JWKS'", + "reason": "Forbidden" + } + ] + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "a validation added", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.validations", + "value": [ + { + "expression": "variables.allowedSuffixes.exists(s, variables.objectName.endsWith(s) && size(variables.objectName) > size(s))", + "messageExpression": "'the knext operator may not ' + request.operation.lowerAscii() + ' the ' + request.resource.resource + ' object \"' + variables.objectName + '\": it writes only the per-zone signing key and JWKS'", + "reason": "Forbidden" + }, + { + "expression": "request.resource.resource != 'secrets' || request.operation == 'DELETE' || dyn(object).type == variables.allowedSecretType", + "message": "the knext operator may write only Opaque Secrets: a service-account-token or any other Secret type under an allowed name would let it mint a credential", + "reason": "Forbidden" + }, + { + "expression": "true" + } + ] + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "a validation expression weakened", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.validations", + "value": [ + { + "expression": "variables.allowedSuffixes.exists(s, variables.objectName.contains(s))", + "messageExpression": "'the knext operator may not ' + request.operation.lowerAscii() + ' the ' + request.resource.resource + ' object \"' + variables.objectName + '\": it writes only the per-zone signing key and JWKS'", + "reason": "Forbidden" + }, + { + "expression": "request.resource.resource != 'secrets' || request.operation == 'DELETE' || dyn(object).type == variables.allowedSecretType", + "message": "the knext operator may write only Opaque Secrets: a service-account-token or any other Secret type under an allowed name would let it mint a credential", + "reason": "Forbidden" + } + ] + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "the Secret type check dropped", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.validations", + "value": [ + { + "expression": "variables.allowedSuffixes.exists(s, variables.objectName.endsWith(s) && size(variables.objectName) > size(s))", + "messageExpression": "'the knext operator may not ' + request.operation.lowerAscii() + ' the ' + request.resource.resource + ' object \"' + variables.objectName + '\": it writes only the per-zone signing key and JWKS'", + "reason": "Forbidden" + }, + { + "expression": "dyn(object).type != ''", + "message": "the knext operator may write only Opaque Secrets: a service-account-token or any other Secret type under an allowed name would let it mint a credential", + "reason": "Forbidden" + } + ] + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "a validation's messageExpression changed", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.validations", + "value": [ + { + "expression": "variables.allowedSuffixes.exists(s, variables.objectName.endsWith(s) && size(variables.objectName) > size(s))", + "messageExpression": "'refused'", + "reason": "Forbidden" + }, + { + "expression": "request.resource.resource != 'secrets' || request.operation == 'DELETE' || dyn(object).type == variables.allowedSecretType", + "message": "the knext operator may write only Opaque Secrets: a service-account-token or any other Secret type under an allowed name would let it mint a credential", + "reason": "Forbidden" + } + ] + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "an allow-list variable widened", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.variables", + "value": [ + { + "name": "objectName", + "expression": "request.operation == 'DELETE' ? oldObject.metadata.name : object.metadata.name" + }, + { + "name": "allowedSecretSuffixes", + "expression": "['-knext-signing-key', '']" + }, + { + "name": "allowedConfigMapSuffixes", + "expression": "['-knext-jwks']" + }, + { + "name": "allowedSecretType", + "expression": "'Opaque'" + }, + { + "name": "allowedSuffixes", + "expression": "request.resource.resource == 'secrets' ? variables.allowedSecretSuffixes : variables.allowedConfigMapSuffixes" + } + ] + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "a variable removed", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.variables", + "value": [ + { + "name": "allowedSecretSuffixes", + "expression": "['-knext-signing-key']" + }, + { + "name": "allowedConfigMapSuffixes", + "expression": "['-knext-jwks']" + }, + { + "name": "allowedSecretType", + "expression": "'Opaque'" + }, + { + "name": "allowedSuffixes", + "expression": "request.resource.resource == 'secrets' ? variables.allowedSecretSuffixes : variables.allowedConfigMapSuffixes" + } + ] + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "the policy has no validations at all", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.validations", + "value": [] + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "the policy declares paramKind (a binding paramRef that finds nothing then runs no validation)", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.paramKind", + "value": { "apiVersion": "v1", "kind": "ConfigMap" } + } + ], + "enforced": false, + "reason": "PolicyModified" + }, + { + "name": "the binding sets paramRef to a parameter that does not exist, with parameterNotFoundAction Allow", + "ops": [ + { + "op": "set", + "target": "binding", + "path": "spec.paramRef", + "value": { + "name": "does-not-exist", + "namespace": "default", + "parameterNotFoundAction": "Allow" + } + } + ], + "enforced": false, + "reason": "BindingScoped" + }, + { + "name": "the binding sets a paramRef selector that matches nothing", + "ops": [ + { + "op": "set", + "target": "binding", + "path": "spec.paramRef", + "value": { + "selector": { "matchLabels": { "nothing": "matches" } }, + "parameterNotFoundAction": "Allow" + } + } + ], + "enforced": false, + "reason": "BindingScoped" + }, + { + "name": "paramKind on the policy and a paramRef on the binding together", + "ops": [ + { + "op": "set", + "target": "policy", + "path": "spec.paramKind", + "value": { "apiVersion": "v1", "kind": "ConfigMap" } + }, + { + "op": "set", + "target": "binding", + "path": "spec.paramRef", + "value": { + "name": "does-not-exist", + "namespace": "default", + "parameterNotFoundAction": "Allow" + } + } + ], + "enforced": false, + "reason": "PolicyModified" + } + ] +} diff --git a/packages/kn-next-operator/internal/writepolicy/verdict.go b/packages/kn-next-operator/internal/writepolicy/verdict.go new file mode 100644 index 000000000..c681db46a --- /dev/null +++ b/packages/kn-next-operator/internal/writepolicy/verdict.go @@ -0,0 +1,260 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package writepolicy answers one question the operator's bundle cannot answer +// for itself: is the ValidatingAdmissionPolicy that confines this operator's +// Secret and ConfigMap writes actually in force? +// +// The policy (config/admission) is the only thing that bounds the operator's +// cluster-wide create/update on Secrets and ConfigMaps, because RBAC cannot +// restrict `create` by name. It can be off with nothing reporting it: a server +// older than 1.30 rejects the two objects while the rest of the bundle applies; +// an install under another namespace or service-account name leaves a +// matchCondition that matches nobody; a cluster admin or a GitOps prune deletes +// the objects. In each case the operator is simply unrestricted. +// +// Evaluate is the pure verdict; Monitor reads the live objects with a plain +// `get`, publishes the verdict as the knext_operator_write_policy_enforced +// gauge, and warns once per change. It never refuses to start and never writes. +// +// KEEP IN LOCKSTEP with `knext doctor`: the same verdict is computed in +// packages/kn-next/src/cli/doctor/checks/write-policy.ts, and both are held to +// testdata/verdict-cases.json. +package writepolicy + +import ( + "fmt" + "reflect" + "regexp" + "strings" + + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Reason names why the policy is, or is not, in force. The values are stable +// strings: the doctor check and the shared test table use the same spellings. +type Reason string + +const ( + // ReasonEnforced: the policy and its binding exist, the binding denies, and + // the policy names the identity the operator runs as. + ReasonEnforced Reason = "Enforced" + // ReasonPolicyMissing: no ValidatingAdmissionPolicy of the expected name + // (deleted, pruned, or never created because the server is older than 1.30). + ReasonPolicyMissing Reason = "PolicyMissing" + // ReasonBindingMissing: the policy exists but nothing binds it, so it is + // never evaluated. + ReasonBindingMissing Reason = "BindingMissing" + // ReasonIdentityUnknown: the operator could not establish its own username. + ReasonIdentityUnknown Reason = "IdentityUnknown" + // ReasonPolicyFailsOpen: failurePolicy Ignore turns an evaluation error into + // an unrestricted write. + ReasonPolicyFailsOpen Reason = "PolicyFailsOpen" + // ReasonPolicyScope: the policy does not cover create, update and delete on + // both Secrets and ConfigMaps, or narrows by namespace or object selector. + ReasonPolicyScope Reason = "PolicyScope" + // ReasonPolicyModified: the policy's variables or validations are not the CEL + // this operator ships, so what it admits is not what the bundle intended + // (for example a validation replaced by `true`). + ReasonPolicyModified Reason = "PolicyModified" + // ReasonIdentityUnparseable: the matchCondition is not a plain equality on + // request.userInfo.username, so which principal it confines cannot be read. + ReasonIdentityUnparseable Reason = "IdentityUnparseable" + // ReasonIdentityMismatch: the matchCondition names a different principal + // than the operator runs as, so it matches nobody the operator is. + ReasonIdentityMismatch Reason = "IdentityMismatch" + // ReasonBindingPolicyMismatch: the binding names another policy. + ReasonBindingPolicyMismatch Reason = "BindingPolicyMismatch" + // ReasonBindingNotDeny: the binding only warns or audits, which refuses + // nothing. + ReasonBindingNotDeny Reason = "BindingNotDeny" + // ReasonBindingScoped: the binding limits the policy to some namespaces or + // objects, but the operator writes in all of them. + ReasonBindingScoped Reason = "BindingScoped" + // ReasonUnverifiable: the objects or the operator's identity could not be + // read (RBAC, transport). Unknown is reported as not enforced, never as + // enforced. + ReasonUnverifiable Reason = "Unverifiable" +) + +// Verdict is the outcome of one check. +type Verdict struct { + Enforced bool + Reason Reason + // Detail is one sentence for a human; empty only when Enforced. + Detail string +} + +func no(reason Reason, format string, args ...any) Verdict { + return Verdict{Reason: reason, Detail: fmt.Sprintf(format, args...)} +} + +// identityExpr is the one matchCondition shape the shipped policy uses and the +// only one the check will read: an equality on the username against string +// literals joined with +. Anything richer (startsWith, !=, ||) is not an +// exact-identity confinement and is reported rather than guessed at. +var identityExpr = regexp.MustCompile(`^\s*request\.userInfo\.username\s*==\s*('[^']*'(?:\s*\+\s*'[^']*')*)\s*$`) + +var celLiteral = regexp.MustCompile(`'([^']*)'`) + +// confinedUsername returns the username the policy's matchCondition confines. +func confinedUsername(p *admissionregistrationv1.ValidatingAdmissionPolicy) (string, bool) { + if len(p.Spec.MatchConditions) != 1 { + return "", false + } + m := identityExpr.FindStringSubmatch(p.Spec.MatchConditions[0].Expression) + if m == nil { + return "", false + } + var b strings.Builder + for _, l := range celLiteral.FindAllStringSubmatch(m[1], -1) { + b.WriteString(l[1]) + } + return b.String(), true +} + +// selectorNarrows reports whether a label selector selects less than +// everything. The apiserver defaults an absent selector to the empty one, which +// selects everything, so the empty selector is not a narrowing. +func selectorNarrows(s *metav1.LabelSelector) bool { + return s != nil && (len(s.MatchLabels) > 0 || len(s.MatchExpressions) > 0) +} + +// scopeCoversNamespaced: Secrets and ConfigMaps are namespaced, so a rule scoped +// "Cluster" never matches them. An absent scope means all. +func scopeCoversNamespaced(s *admissionregistrationv1.ScopeType) bool { + return s == nil || *s == admissionregistrationv1.NamespacedScope || *s == admissionregistrationv1.AllScopes +} + +func has(list []string, want string) bool { + for _, v := range list { + if v == want || v == "*" { + return true + } + } + return false +} + +// covers reports whether the policy's rules reach every (resource, operation) +// the operator's writes need confining on: Secrets and ConfigMaps in the core +// group, on create, update and delete. +func covers(p *admissionregistrationv1.ValidatingAdmissionPolicy) (missing string) { + mc := p.Spec.MatchConstraints + if mc == nil { + return "it has no matchConstraints" + } + // Any exclusion could be carving Secrets or ConfigMaps (or an operation) back + // out of the rules below, and reading which is a re-implementation of the + // apiserver's matching. The shipped policy has none, so none is accepted. + if len(mc.ExcludeResourceRules) > 0 { + return "it excludes some requests through excludeResourceRules" + } + for _, resource := range []string{"secrets", "configmaps"} { + for _, op := range []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, admissionregistrationv1.Update, admissionregistrationv1.Delete, + } { + found := false + for _, r := range mc.ResourceRules { + ops := make([]string, 0, len(r.Operations)) + for _, o := range r.Operations { + ops = append(ops, string(o)) + } + // A rule that names objects, is cluster-scoped, or is for another + // apiVersion does not cover the operator's writes, however it spells + // the resources. + if has(r.APIGroups, "") && has(r.APIVersions, "v1") && has(r.Resources, resource) && has(ops, string(op)) && + len(r.ResourceNames) == 0 && scopeCoversNamespaced(r.Scope) { + found = true + } + } + if !found { + return fmt.Sprintf("it does not cover %s on %s", op, resource) + } + } + } + return "" +} + +// Evaluate decides whether the write policy is in force for the operator that +// runs as operatorUsername. policy and binding are nil when the object does not +// exist. The checks run in a fixed order, so the first thing wrong is the +// reason reported. +func Evaluate(policy *admissionregistrationv1.ValidatingAdmissionPolicy, binding *admissionregistrationv1.ValidatingAdmissionPolicyBinding, operatorUsername string) Verdict { + if policy == nil { + return no(ReasonPolicyMissing, "the ValidatingAdmissionPolicy that limits the operator's Secret and ConfigMap writes does not exist "+ + "(deleted or pruned, or the server is older than Kubernetes 1.30, which rejects it): the operator is unrestricted") + } + if binding == nil { + return no(ReasonBindingMissing, "the policy %q exists but no ValidatingAdmissionPolicyBinding of that name does, so it is never evaluated: "+ + "the operator is unrestricted", policy.Name) + } + if operatorUsername == "" { + return no(ReasonIdentityUnknown, "the operator could not establish the username it runs as, so the policy's identity cannot be compared with it") + } + if fp := policy.Spec.FailurePolicy; fp != nil && *fp == admissionregistrationv1.Ignore { + return no(ReasonPolicyFailsOpen, "the policy %q has failurePolicy Ignore, so an evaluation error lets the write through", policy.Name) + } + if why := covers(policy); why != "" { + return no(ReasonPolicyScope, "the policy %q does not confine every write the operator makes: %s", policy.Name, why) + } + if mc := policy.Spec.MatchConstraints; selectorNarrows(mc.NamespaceSelector) || selectorNarrows(mc.ObjectSelector) { + return no(ReasonPolicyScope, "the policy %q narrows by namespace or object selector, but the operator writes in every namespace", policy.Name) + } + confined, ok := confinedUsername(policy) + if !ok { + return no(ReasonIdentityUnparseable, "the policy %q does not confine one exact username (its matchCondition is not an equality on "+ + "request.userInfo.username), so which principal it limits cannot be verified", policy.Name) + } + if confined != operatorUsername { + return no(ReasonIdentityMismatch, "the policy %q confines %q but the operator runs as %q: it matches nobody the operator is "+ + "(installed under another namespace or service-account name than the bundle rendered?)", policy.Name, confined, operatorUsername) + } + // The shipped policy takes no parameters. A paramKind lets a binding's paramRef + // resolve to nothing, and the apiserver then runs no validation at all + // (with parameterNotFoundAction Allow), however right everything else is. + if policy.Spec.ParamKind != nil { + return no(ReasonPolicyModified, "the policy %q declares paramKind, which the shipped policy does not: a binding paramRef that "+ + "finds no parameter makes the apiserver skip every validation", policy.Name) + } + if !reflect.DeepEqual(policyCEL(policy), canonicalCEL()) { + return no(ReasonPolicyModified, "the policy %q does not carry the variables and validations this operator ships, so what it admits is not what "+ + "the bundle intended (its validation CEL, an allow-list variable or a messageExpression was changed, added or removed)", policy.Name) + } + if binding.Spec.PolicyName != policy.Name { + return no(ReasonBindingPolicyMismatch, "the binding %q names policy %q, not %q", binding.Name, binding.Spec.PolicyName, policy.Name) + } + deny := false + for _, a := range binding.Spec.ValidationActions { + deny = deny || a == admissionregistrationv1.Deny + } + if !deny { + return no(ReasonBindingNotDeny, "the binding %q does not include the Deny action (actions %v): a violation is warned about or audited, never refused", + binding.Name, binding.Spec.ValidationActions) + } + // Likewise the shipped binding sets no paramRef: one that resolves to no + // parameter (a missing name, a selector matching nothing) runs no validation. + if binding.Spec.ParamRef != nil { + return no(ReasonBindingScoped, "the binding %q sets paramRef, which the shipped binding does not: a parameter that cannot be found "+ + "makes the apiserver skip every validation", binding.Name) + } + if mr := binding.Spec.MatchResources; mr != nil && + (selectorNarrows(mr.NamespaceSelector) || selectorNarrows(mr.ObjectSelector) || + len(mr.ResourceRules) > 0 || len(mr.ExcludeResourceRules) > 0) { + return no(ReasonBindingScoped, "the binding %q limits the policy to some namespaces or objects, but the operator writes in every namespace", binding.Name) + } + return Verdict{Enforced: true, Reason: ReasonEnforced} +} diff --git a/packages/kn-next-operator/internal/writepolicy/verdict_test.go b/packages/kn-next-operator/internal/writepolicy/verdict_test.go new file mode 100644 index 000000000..6c23ba35f --- /dev/null +++ b/packages/kn-next-operator/internal/writepolicy/verdict_test.go @@ -0,0 +1,167 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package writepolicy + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" +) + +// verdictFixture is testdata/verdict-cases.json, which `knext doctor` reads too +// (packages/kn-next/src/__tests__/doctor-write-policy.test.ts): the operator's +// self-check and the CLI must reach the same verdict on the same objects. +type verdictFixture struct { + OperatorUsername string `json:"operatorUsername"` + Policy map[string]any `json:"policy"` + Binding map[string]any `json:"binding"` + Cases []verdictCase `json:"cases"` +} + +type verdictCase struct { + Name string `json:"name"` + Ops []verdictOp `json:"ops"` + Enforced bool `json:"enforced"` + Reason string `json:"reason"` +} + +type verdictOp struct { + Op string `json:"op"` + Target string `json:"target"` + Path string `json:"path"` + Value any `json:"value"` +} + +func loadVerdictFixture(t *testing.T) verdictFixture { + t.Helper() + raw, err := os.ReadFile(filepath.Join("testdata", "verdict-cases.json")) + if err != nil { + t.Fatal(err) + } + var f verdictFixture + if err := json.Unmarshal(raw, &f); err != nil { + t.Fatal(err) + } + if len(f.Cases) < 10 { + t.Fatalf("only %d verdict cases; the table would prove little", len(f.Cases)) + } + return f +} + +// deepCopy round-trips through JSON so a case never mutates the shared base. +func deepCopy(t *testing.T, in map[string]any) map[string]any { + t.Helper() + raw, err := json.Marshal(in) + if err != nil { + t.Fatal(err) + } + var out map[string]any + if err := json.Unmarshal(raw, &out); err != nil { + t.Fatal(err) + } + return out +} + +func setPath(t *testing.T, obj map[string]any, path string, value any) { + t.Helper() + parts := strings.Split(path, ".") + cur := obj + for _, p := range parts[:len(parts)-1] { + next, ok := cur[p].(map[string]any) + if !ok { + next = map[string]any{} + cur[p] = next + } + cur = next + } + cur[parts[len(parts)-1]] = value +} + +// objectsFor applies a case's ops to the fixture's base objects. +func objectsFor(t *testing.T, f verdictFixture, c verdictCase) (*admissionregistrationv1.ValidatingAdmissionPolicy, *admissionregistrationv1.ValidatingAdmissionPolicyBinding, string) { + t.Helper() + policy, binding := deepCopy(t, f.Policy), deepCopy(t, f.Binding) + removePolicy, removeBinding := false, false + username := f.OperatorUsername + for _, op := range c.Ops { + switch op.Op { + case "remove-policy": + removePolicy = true + case "remove-binding": + removeBinding = true + case "identity": + username, _ = op.Value.(string) + case "set": + target := policy + if op.Target == "binding" { + target = binding + } + setPath(t, target, op.Path, op.Value) + default: + t.Fatalf("case %q: unknown op %q", c.Name, op.Op) + } + } + decode := func(in map[string]any, out any) { + raw, err := json.Marshal(in) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(raw, out); err != nil { + t.Fatalf("case %q: %v", c.Name, err) + } + } + var p *admissionregistrationv1.ValidatingAdmissionPolicy + var b *admissionregistrationv1.ValidatingAdmissionPolicyBinding + if !removePolicy { + p = &admissionregistrationv1.ValidatingAdmissionPolicy{} + decode(policy, p) + } + if !removeBinding { + b = &admissionregistrationv1.ValidatingAdmissionPolicyBinding{} + decode(binding, b) + } + return p, b, username +} + +func TestEvaluateMatchesTheSharedVerdictTable(t *testing.T) { + f := loadVerdictFixture(t) + seenEnforced, seenNot := false, false + for _, c := range f.Cases { + t.Run(c.Name, func(t *testing.T) { + p, b, username := objectsFor(t, f, c) + got := Evaluate(p, b, username) + if got.Enforced != c.Enforced || string(got.Reason) != c.Reason { + t.Fatalf("Evaluate = {enforced %v, reason %s (%s)}, want {enforced %v, reason %s}", + got.Enforced, got.Reason, got.Detail, c.Enforced, c.Reason) + } + if !got.Enforced && got.Detail == "" { + t.Errorf("an unenforced verdict must say why in words, got an empty Detail") + } + }) + seenEnforced = seenEnforced || c.Enforced + seenNot = seenNot || !c.Enforced + } + // Both halves: a table of only 0s (or only 1s) cannot tell a real check + // from a constant. + if !seenEnforced || !seenNot { + t.Fatalf("the verdict table must hold both outcomes (enforced %v, not-enforced %v)", seenEnforced, seenNot) + } +} diff --git a/packages/kn-next/src/__tests__/__snapshots__/doctor-golden.test.ts.snap b/packages/kn-next/src/__tests__/__snapshots__/doctor-golden.test.ts.snap index ba8b8d3bd..be390fc17 100644 --- a/packages/kn-next/src/__tests__/__snapshots__/doctor-golden.test.ts.snap +++ b/packages/kn-next/src/__tests__/__snapshots__/doctor-golden.test.ts.snap @@ -10,6 +10,7 @@ PASS NextApp CRD served version: v1alpha1; pair PASS NextApp CRD schema coverage all 95 field(s) this CLI emits are defined by the installed CRD (read from the aggregated OpenAPI v3 discovery document) PASS Operator deployment kn-next-operator-controller-manager Ready in kn-next-operator-system WARN Operator version no release version reported, image sha256:75be42bb6b4c… (a bundle from before the operator version line); compatibility with this CLI cannot be checked (hint: Move to a versioned operator (operator first, then CLI). Apply a specific release's digest-pinned bundle: \`kubectl apply --server-side -f https://github.com/getknext-dev/knext/releases/download/operator-v/install-v.yaml\` (see https://knext-platform.dev/docs/versioning).) +PASS Operator write policy kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps PASS cert-manager webhook cert-manager-webhook Ready PASS Knative ingress-class ingress-class kourier.ingress.networking.knative.dev is served by net-kourier-controller PASS Operator image pullable ghcr.io/getknext-dev/kn-next-operator@sha256:75be42bb6b4c6d03c902b4fc90b36b246cc6cacf2233926fa183a6051521a99d is anonymously pullable @@ -77,6 +78,12 @@ exports[`doctor golden output (#1055 byte-identical pin) reachable — all check "status": "warn", "title": "Operator version", }, + { + "detail": "kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps", + "id": "write-policy", + "status": "pass", + "title": "Operator write policy", + }, { "detail": "cert-manager-webhook Ready", "id": "cert-manager", @@ -134,6 +141,7 @@ SKIP NextApp CRD cluster unreachable — check SKIP NextApp CRD schema coverage cluster unreachable — check skipped SKIP Operator deployment cluster unreachable — check skipped SKIP Operator version cluster unreachable — check skipped +SKIP Operator write policy cluster unreachable — check skipped SKIP cert-manager webhook cluster unreachable — check skipped SKIP Knative ingress-class cluster unreachable — check skipped SKIP Operator image pullable cluster unreachable — check skipped @@ -202,6 +210,12 @@ exports[`doctor golden output (#1055 byte-identical pin) unreachable — reachab "status": "skip", "title": "Operator version", }, + { + "detail": "cluster unreachable — check skipped", + "id": "write-policy", + "status": "skip", + "title": "Operator write policy", + }, { "detail": "cluster unreachable — check skipped", "id": "cert-manager", @@ -259,6 +273,7 @@ SKIP NextApp CRD cluster unreachable — check SKIP NextApp CRD schema coverage cluster unreachable — check skipped SKIP Operator deployment cluster unreachable — check skipped SKIP Operator version cluster unreachable — check skipped +SKIP Operator write policy cluster unreachable — check skipped SKIP cert-manager webhook cluster unreachable — check skipped SKIP Knative ingress-class cluster unreachable — check skipped SKIP Operator image pullable cluster unreachable — check skipped @@ -327,6 +342,12 @@ exports[`doctor golden output (#1055 byte-identical pin) auth-failure — the re "status": "skip", "title": "Operator version", }, + { + "detail": "cluster unreachable — check skipped", + "id": "write-policy", + "status": "skip", + "title": "Operator write policy", + }, { "detail": "cluster unreachable — check skipped", "id": "cert-manager", @@ -384,6 +405,7 @@ SKIP NextApp CRD cluster unreachable — check SKIP NextApp CRD schema coverage cluster unreachable — check skipped SKIP Operator deployment cluster unreachable — check skipped SKIP Operator version cluster unreachable — check skipped +SKIP Operator write policy cluster unreachable — check skipped SKIP cert-manager webhook cluster unreachable — check skipped SKIP Knative ingress-class cluster unreachable — check skipped SKIP Operator image pullable cluster unreachable — check skipped @@ -452,6 +474,12 @@ exports[`doctor golden output (#1055 byte-identical pin) forbidden — the reach "status": "skip", "title": "Operator version", }, + { + "detail": "cluster unreachable — check skipped", + "id": "write-policy", + "status": "skip", + "title": "Operator write policy", + }, { "detail": "cluster unreachable — check skipped", "id": "cert-manager", @@ -509,6 +537,7 @@ PASS NextApp CRD served version: v1alpha1; pair PASS NextApp CRD schema coverage all 95 field(s) this CLI emits are defined by the installed CRD (read from the aggregated OpenAPI v3 discovery document) PASS Operator deployment kn-next-operator-controller-manager Ready in kn-next-operator-system WARN Operator version no release version reported, image sha256:75be42bb6b4c… (a bundle from before the operator version line); compatibility with this CLI cannot be checked (hint: Move to a versioned operator (operator first, then CLI). Apply a specific release's digest-pinned bundle: \`kubectl apply --server-side -f https://github.com/getknext-dev/knext/releases/download/operator-v/install-v.yaml\` (see https://knext-platform.dev/docs/versioning).) +PASS Operator write policy kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps PASS cert-manager webhook cert-manager-webhook Ready PASS Knative ingress-class ingress-class kourier.ingress.networking.knative.dev is served by net-kourier-controller PASS Operator image pullable ghcr.io/getknext-dev/kn-next-operator@sha256:75be42bb6b4c6d03c902b4fc90b36b246cc6cacf2233926fa183a6051521a99d is anonymously pullable @@ -576,6 +605,12 @@ exports[`doctor golden output (#1055 byte-identical pin) budget-exhausted — th "status": "warn", "title": "Operator version", }, + { + "detail": "kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps", + "id": "write-policy", + "status": "pass", + "title": "Operator write policy", + }, { "detail": "cert-manager-webhook Ready", "id": "cert-manager", @@ -633,6 +668,7 @@ PASS NextApp CRD served version: v1alpha1; pair FAIL NextApp CRD schema coverage operator behind CLI: the installed CRD is missing 1 field(s) this CLI emits — they would be dropped. Upgrade the operator first (operator, then CLI). (hint: upgrade the operator/CRD FIRST, then the CLI — see https://knext-platform.dev/docs/upgrading) PASS Operator deployment kn-next-operator-controller-manager Ready in kn-next-operator-system WARN Operator version no release version reported, image sha256:75be42bb6b4c… (a bundle from before the operator version line); compatibility with this CLI cannot be checked (hint: Move to a versioned operator (operator first, then CLI). Apply a specific release's digest-pinned bundle: \`kubectl apply --server-side -f https://github.com/getknext-dev/knext/releases/download/operator-v/install-v.yaml\` (see https://knext-platform.dev/docs/versioning).) +PASS Operator write policy kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps PASS cert-manager webhook cert-manager-webhook Ready PASS Knative ingress-class ingress-class kourier.ingress.networking.knative.dev is served by net-kourier-controller PASS Operator image pullable ghcr.io/getknext-dev/kn-next-operator@sha256:75be42bb6b4c6d03c902b4fc90b36b246cc6cacf2233926fa183a6051521a99d is anonymously pullable @@ -701,6 +737,12 @@ exports[`doctor golden output (#1055 byte-identical pin) crd-schema — an insta "status": "warn", "title": "Operator version", }, + { + "detail": "kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps", + "id": "write-policy", + "status": "pass", + "title": "Operator write policy", + }, { "detail": "cert-manager-webhook Ready", "id": "cert-manager", @@ -758,6 +800,7 @@ FAIL NextApp CRD this CLI emits apps.kn-next.de PASS NextApp CRD schema coverage all 95 field(s) this CLI emits are defined by the installed CRD (read from the aggregated OpenAPI v3 discovery document) PASS Operator deployment kn-next-operator-controller-manager Ready in kn-next-operator-system WARN Operator version no release version reported, image sha256:75be42bb6b4c… (a bundle from before the operator version line); compatibility with this CLI cannot be checked (hint: Move to a versioned operator (operator first, then CLI). Apply a specific release's digest-pinned bundle: \`kubectl apply --server-side -f https://github.com/getknext-dev/knext/releases/download/operator-v/install-v.yaml\` (see https://knext-platform.dev/docs/versioning).) +PASS Operator write policy kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps PASS cert-manager webhook cert-manager-webhook Ready PASS Knative ingress-class ingress-class kourier.ingress.networking.knative.dev is served by net-kourier-controller PASS Operator image pullable ghcr.io/getknext-dev/kn-next-operator@sha256:75be42bb6b4c6d03c902b4fc90b36b246cc6cacf2233926fa183a6051521a99d is anonymously pullable @@ -826,6 +869,12 @@ exports[`doctor golden output (#1055 byte-identical pin) crd-version — a CRD t "status": "warn", "title": "Operator version", }, + { + "detail": "kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps", + "id": "write-policy", + "status": "pass", + "title": "Operator write policy", + }, { "detail": "cert-manager-webhook Ready", "id": "cert-manager", @@ -883,6 +932,7 @@ PASS NextApp CRD served version: v1alpha1; pair PASS NextApp CRD schema coverage all 95 field(s) this CLI emits are defined by the installed CRD (read from the aggregated OpenAPI v3 discovery document) PASS Operator deployment kn-next-operator-controller-manager Ready in kn-next-operator-system PASS Operator version v1.5.0 (from label), image sha256:75be42bb6b4c…; different major from CLI 2.0.0 (the operator keeps its own version line) — supported pairing is decided by the CRD API version, see the NextApp CRD row +PASS Operator write policy kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps PASS cert-manager webhook cert-manager-webhook Ready PASS Knative ingress-class ingress-class kourier.ingress.networking.knative.dev is served by net-kourier-controller PASS Operator image pullable ghcr.io/getknext-dev/kn-next-operator@sha256:75be42bb6b4c6d03c902b4fc90b36b246cc6cacf2233926fa183a6051521a99d is anonymously pullable @@ -949,6 +999,12 @@ exports[`doctor golden output (#1055 byte-identical pin) cli-2.0 — a 2.0 CLI a "status": "pass", "title": "Operator version", }, + { + "detail": "kn-next-operator-secret-configmap-writes is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines system:serviceaccount:kn-next-operator-system:default and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps", + "id": "write-policy", + "status": "pass", + "title": "Operator write policy", + }, { "detail": "cert-manager-webhook Ready", "id": "cert-manager", diff --git a/packages/kn-next/src/__tests__/doctor-golden.test.ts b/packages/kn-next/src/__tests__/doctor-golden.test.ts index de2cbc481..d17236fc4 100644 --- a/packages/kn-next/src/__tests__/doctor-golden.test.ts +++ b/packages/kn-next/src/__tests__/doctor-golden.test.ts @@ -32,6 +32,7 @@ import { runDoctor, } from "../cli/doctor"; import type { KnativeNextConfig } from "../config"; +import { WRITE_POLICY_STUBS } from "./doctor-write-policy-stubs"; const HERE = dirname(fileURLToPath(import.meta.url)); const CRD_YAML = join( @@ -101,6 +102,7 @@ function healthyStubs( ): Record { return { "kubectl get --raw /version": { ok: true, stdout: "{}" }, + ...WRITE_POLICY_STUBS, "kubectl version --client -o json": { ok: true, stdout: JSON.stringify({ diff --git a/packages/kn-next/src/__tests__/doctor-write-policy-canonical.test.ts b/packages/kn-next/src/__tests__/doctor-write-policy-canonical.test.ts new file mode 100644 index 000000000..f47a4c97a --- /dev/null +++ b/packages/kn-next/src/__tests__/doctor-write-policy-canonical.test.ts @@ -0,0 +1,202 @@ +/** + * `knext doctor` write-policy check — the parts that are not the shared verdict + * table: the canonical CEL copy, and what an UNKNOWN is reported as. + * + * The check compares a live policy's variables and validations with the ones the + * bundle ships. That copy lives in write-policy.ts (as does the operator's, in + * internal/writepolicy/canonical.go); these tests fail if config/admission + * changes without it, so neither the CLI nor the operator can quietly start + * reporting a correct policy as modified, or a stale one as fine. + */ + +import { describe, expect, it } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { parse as parseYaml } from "yaml"; +import { + type CheckResult, + type KubectlFn, + type ProbeOutcome, + runDoctor, +} from "../cli/doctor"; +import { CANONICAL_POLICY_CEL } from "../cli/doctor/checks/write-policy"; + +const OPERATOR = join(import.meta.dir, "..", "..", "..", "kn-next-operator"); +const SHIPPED = join( + OPERATOR, + "config", + "admission", + "secret-configmap-writes.yaml", +); +const FIXTURE = join( + OPERATOR, + "internal", + "writepolicy", + "testdata", + "verdict-cases.json", +); + +interface PolicyDoc { + spec: { + variables?: { name: string; expression: string }[]; + validations?: { expression: string; messageExpression?: string }[]; + }; +} + +/** A policy's variables and validations reduced to their CEL. */ +function celOf(p: PolicyDoc) { + return { + variables: (p.spec.variables ?? []).map((v) => ({ + name: v.name, + expression: v.expression, + })), + validations: (p.spec.validations ?? []).map((v) => ({ + expression: v.expression, + messageExpression: v.messageExpression ?? "", + })), + }; +} + +describe("the canonical policy CEL", () => { + const shipped = parseYaml(readFileSync(SHIPPED, "utf-8")) as PolicyDoc; + + it("the shipped policy has CEL to compare (a vacuous comparison could not fail)", () => { + expect(shipped.spec.variables?.length ?? 0).toBeGreaterThan(0); + expect(shipped.spec.validations?.length ?? 0).toBeGreaterThan(0); + }); + + it("equals what config/admission ships", () => { + // JSON round trip: the canonical copy is `as const` (readonly). + expect(JSON.parse(JSON.stringify(CANONICAL_POLICY_CEL))).toEqual( + celOf(shipped), + ); + }); + + it("is what the shared table's base policy carries", () => { + const fixture = JSON.parse(readFileSync(FIXTURE, "utf-8")) as { + policy: PolicyDoc; + }; + expect(celOf(fixture.policy)).toEqual(celOf(shipped)); + }); +}); + +// ---- an unknown is not enforced ------------------------------------------- + +const OP_NS = "kn-next-operator-system"; +const NAME = "kn-next-operator-secret-configmap-writes"; +const DEPLOY_KEY = `kubectl get deployments -n ${OP_NS} -o json`; +const POLICY_KEY = `kubectl get validatingadmissionpolicy ${NAME} -o json`; +const BINDING_KEY = `kubectl get validatingadmissionpolicybinding ${NAME} -o json`; + +type Entry = { ok: boolean; stdout?: string; stderr?: string }; +const okProbe = async (): Promise => "ok"; + +const fixture = JSON.parse(readFileSync(FIXTURE, "utf-8")) as { + policy: unknown; + binding: unknown; +}; + +/** A deployment with NO WRITE_POLICY_NAME env: an operator from before the check. */ +const deploymentWithoutEnv = JSON.stringify({ + items: [ + { + metadata: { + name: "kn-next-operator-controller-manager", + namespace: OP_NS, + }, + spec: { + template: { + spec: { + serviceAccountName: + "kn-next-operator-controller-manager", + containers: [ + { + name: "manager", + image: "ghcr.io/x/op@sha256:aa", + }, + ], + }, + }, + }, + status: { readyReplicas: 1, replicas: 1 }, + }, + ], +}); + +function table(over: Record = {}): Record { + return { + "kubectl get --raw /version": { + ok: true, + stdout: '{"major":"1","minor":"31","gitVersion":"v1.31.2"}', + }, + [DEPLOY_KEY]: { ok: true, stdout: deploymentWithoutEnv }, + [POLICY_KEY]: { ok: true, stdout: JSON.stringify(fixture.policy) }, + [BINDING_KEY]: { ok: true, stdout: JSON.stringify(fixture.binding) }, + ...over, + }; +} + +async function writePolicyRow(t: Record): Promise { + const kubectl: KubectlFn = (args) => { + const hit = t[args.join(" ")]; + return hit + ? { + ok: hit.ok, + stdout: hit.stdout ?? "", + stderr: hit.stderr ?? "", + } + : { + ok: false, + stdout: "", + stderr: `no stub for: ${args.join(" ")}`, + }; + }; + const report = await runDoctor({ kubectl, probeImage: okProbe }); + const found = report.checks.find((c) => c.id === "write-policy"); + expect(found).toBeDefined(); + return found as CheckResult; +} + +describe("doctor write-policy: unknown means not enforced", () => { + it("an operator Deployment with no WRITE_POLICY_NAME is checked under the bundle's rendered name", async () => { + // The Go flag default is the UNPREFIXED base name (it runs inside the + // operator, whose env the bundle always sets); doctor, which may meet an + // operator from before the env existed, falls back to the name the + // bundle renders. They differ on purpose. + const r = await writePolicyRow(table()); + expect(r.status).toBe("pass"); + expect(r.detail).toContain(NAME); + }); + + it("an unclassifiable read failure WARNs (it does not skip)", async () => { + const r = await writePolicyRow( + table({ + [POLICY_KEY]: { + ok: false, + stderr: "something went sideways that matches no known signature", + }, + }), + ); + expect(r.status).toBe("warn"); + expect(r.detail).toMatch(/not verified/i); + expect(r.detail).toMatch(/unrestricted/i); + expect(r.hint).toBeTruthy(); + }); + + it("an unparseable policy WARNs (it does not skip)", async () => { + const r = await writePolicyRow( + table({ [POLICY_KEY]: { ok: true, stdout: "<>" } }), + ); + expect(r.status).toBe("warn"); + expect(r.detail).toMatch(/unreadable/i); + expect(r.detail).toMatch(/unrestricted/i); + }); + + it("an unparseable binding WARNs (it does not skip)", async () => { + const r = await writePolicyRow( + table({ [BINDING_KEY]: { ok: true, stdout: "" } }), + ); + expect(r.status).toBe("warn"); + expect(r.detail).toMatch(/unreadable/i); + }); +}); diff --git a/packages/kn-next/src/__tests__/doctor-write-policy-stubs.ts b/packages/kn-next/src/__tests__/doctor-write-policy-stubs.ts new file mode 100644 index 000000000..4f2c58352 --- /dev/null +++ b/packages/kn-next/src/__tests__/doctor-write-policy-stubs.ts @@ -0,0 +1,54 @@ +/** + * kubectl stub entries that make the `write-policy` doctor row PASS on the + * shared "healthy cluster" fixtures: the bundle's policy and binding, confining + * the identity those fixtures' operator Deployment runs as (no + * serviceAccountName, so the pod runs as `default`). + * + * A fixture that omits them gets a SKIP row ("could not read"), not a pass, so + * a healthy-cluster test that forgot them would fail its all-pass assertion. + */ + +import { CANONICAL_POLICY_CEL } from "../cli/doctor/checks/write-policy"; + +const NAME = "kn-next-operator-secret-configmap-writes"; + +export const WRITE_POLICY_STUBS: Record< + string, + { ok: boolean; stdout?: string; stderr?: string } +> = { + [`kubectl get validatingadmissionpolicy ${NAME} -o json`]: { + ok: true, + stdout: JSON.stringify({ + metadata: { name: NAME }, + spec: { + failurePolicy: "Fail", + variables: CANONICAL_POLICY_CEL.variables, + validations: CANONICAL_POLICY_CEL.validations, + matchConstraints: { + resourceRules: [ + { + apiGroups: [""], + apiVersions: ["v1"], + operations: ["CREATE", "UPDATE", "DELETE"], + resources: ["secrets", "configmaps"], + }, + ], + }, + matchConditions: [ + { + name: "request-is-from-the-operator", + expression: + "request.userInfo.username == 'system:serviceaccount:' + 'kn-next-operator-system' + ':' + 'default'", + }, + ], + }, + }), + }, + [`kubectl get validatingadmissionpolicybinding ${NAME} -o json`]: { + ok: true, + stdout: JSON.stringify({ + metadata: { name: NAME }, + spec: { policyName: NAME, validationActions: ["Deny", "Audit"] }, + }), + }, +}; diff --git a/packages/kn-next/src/__tests__/doctor-write-policy.test.ts b/packages/kn-next/src/__tests__/doctor-write-policy.test.ts new file mode 100644 index 000000000..055450bfc --- /dev/null +++ b/packages/kn-next/src/__tests__/doctor-write-policy.test.ts @@ -0,0 +1,354 @@ +/** + * `knext doctor` — operator write-policy check. + * + * The operator's Secret/ConfigMap writes are bounded by a + * ValidatingAdmissionPolicy that can be off with nothing saying so: a server + * older than Kubernetes 1.30 rejects it, an install under another namespace or + * ServiceAccount name leaves a matchCondition that matches nobody, a prune or an + * admin deletes it. The operator reports the same verdict as a gauge + * (knext_operator_write_policy_enforced); this is the CLI half. + * + * Two kinds of test: the verdict function is held to the SAME table the Go + * operator is held to (so the two cannot drift), and the check is driven + * end-to-end through runDoctor with a stubbed kubectl. + */ + +import { describe, expect, it } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { + type CheckResult, + type KubectlFn, + type ProbeOutcome, + runDoctor, +} from "../cli/doctor"; +import { + evaluateWritePolicy, + type WritePolicyReason, +} from "../cli/doctor/checks/write-policy"; + +const FIXTURE = join( + import.meta.dir, + "..", + "..", + "..", + "kn-next-operator", + "internal", + "writepolicy", + "testdata", + "verdict-cases.json", +); + +type Op = + | { op: "remove-policy" } + | { op: "remove-binding" } + | { op: "identity"; value: string } + | { op: "set"; target: "policy" | "binding"; path: string; value: unknown }; + +interface Fixture { + operatorUsername: string; + policy: Record; + binding: Record; + cases: { + name: string; + ops: Op[]; + enforced: boolean; + reason: WritePolicyReason; + }[]; +} + +const fixture = JSON.parse(readFileSync(FIXTURE, "utf-8")) as Fixture; +const clone = (v: T): T => JSON.parse(JSON.stringify(v)) as T; + +function setPath(obj: Record, path: string, value: unknown) { + const parts = path.split("."); + let cur = obj; + for (const p of parts.slice(0, -1)) { + if (typeof cur[p] !== "object" || cur[p] === null) cur[p] = {}; + cur = cur[p] as Record; + } + cur[parts[parts.length - 1]] = value; +} + +function applyOps(ops: Op[]) { + let policy: Record | undefined = clone(fixture.policy); + let binding: Record | undefined = clone(fixture.binding); + let username: string = fixture.operatorUsername; + const edits: Op[] = []; + for (const o of ops) { + if (o.op === "remove-policy") policy = undefined; + else if (o.op === "remove-binding") binding = undefined; + else if (o.op === "identity") username = o.value; + else edits.push(o); + } + for (const e of edits) { + if (e.op !== "set") continue; + const target = e.target === "policy" ? policy : binding; + if (target) setPath(target, e.path, e.value); + } + return { policy, binding, username }; +} + +describe("evaluateWritePolicy matches the operator's verdict table", () => { + it("the table holds both outcomes (a constant could not pass)", () => { + expect(fixture.cases.some((c) => c.enforced)).toBe(true); + expect(fixture.cases.some((c) => !c.enforced)).toBe(true); + expect(fixture.cases.length).toBeGreaterThanOrEqual(10); + }); + + for (const c of fixture.cases) { + it(c.name, () => { + const { policy, binding, username } = applyOps(c.ops); + const v = evaluateWritePolicy(policy, binding, username); + expect({ enforced: v.enforced, reason: v.reason }).toEqual({ + enforced: c.enforced, + reason: c.reason, + }); + if (!v.enforced) expect(v.detail.length).toBeGreaterThan(0); + }); + } +}); + +// ---- end to end through runDoctor ------------------------------------------- + +const OP_NS = "kn-next-operator-system"; +const OP_SA = "kn-next-operator-controller-manager"; +const POLICY = "kn-next-operator-secret-configmap-writes"; +const DEPLOY_KEY = `kubectl get deployments -n ${OP_NS} -o json`; +const POLICY_KEY = `kubectl get validatingadmissionpolicy ${POLICY} -o json`; +const BINDING_KEY = `kubectl get validatingadmissionpolicybinding ${POLICY} -o json`; + +type Entry = { ok: boolean; stdout?: string; stderr?: string }; + +const okProbe = async (): Promise => "ok"; + +function stubKubectl(table: Record): { + fn: KubectlFn; + calls: string[]; +} { + const calls: string[] = []; + return { + calls, + fn: (args) => { + const key = args.join(" "); + calls.push(key); + const hit = table[key]; + if (!hit) + return { + ok: false, + stdout: "", + stderr: `no stub for: ${key}`, + }; + return { + ok: hit.ok, + stdout: hit.stdout ?? "", + stderr: hit.stderr ?? "", + }; + }, + }; +} + +const deployment = ( + over: { sa?: string; env?: { name: string; value: string }[] } = {}, +) => + JSON.stringify({ + items: [ + { + metadata: { name: "kn-next-operator-controller-manager" }, + spec: { + template: { + spec: { + serviceAccountName: over.sa ?? OP_SA, + containers: [ + { + name: "manager", + image: "ghcr.io/x/op@sha256:aa", + env: over.env ?? [ + { + name: "WRITE_POLICY_NAME", + value: POLICY, + }, + ], + }, + ], + }, + }, + }, + status: { readyReplicas: 1, replicas: 1 }, + }, + ], + }); + +const NOT_FOUND = (kind: string) => + `Error from server (NotFound): ${kind}.admissionregistration.k8s.io "${POLICY}" not found`; + +function healthy( + over: Record = {}, + versionJson = '{"major":"1","minor":"31","gitVersion":"v1.31.2"}', +): Record { + return { + "kubectl get --raw /version": { ok: true, stdout: versionJson }, + [DEPLOY_KEY]: { ok: true, stdout: deployment() }, + [POLICY_KEY]: { ok: true, stdout: JSON.stringify(fixture.policy) }, + [BINDING_KEY]: { ok: true, stdout: JSON.stringify(fixture.binding) }, + ...over, + }; +} + +async function row( + table: Record, +): Promise<{ row: CheckResult; calls: string[]; all: CheckResult[] }> { + const { fn, calls } = stubKubectl(table); + const report = await runDoctor({ kubectl: fn, probeImage: okProbe }); + const found = report.checks.find((c) => c.id === "write-policy"); + expect(found, "doctor must include the write-policy check").toBeDefined(); + return { row: found as CheckResult, calls, all: report.checks }; +} + +describe("doctor write-policy check", () => { + it("PASS when the policy is present, bound with Deny and names the operator", async () => { + const { row: r } = await row(healthy()); + expect(r.status).toBe("pass"); + expect(r.detail).toContain(POLICY); + }); + + it("WARN naming the missing policy when it is deleted", async () => { + const { row: r } = await row( + healthy({ + [POLICY_KEY]: { + ok: false, + stderr: NOT_FOUND("validatingadmissionpolicies"), + }, + }), + ); + expect(r.status).toBe("warn"); + expect(r.detail).toMatch(/does not exist/); + expect(r.detail).toContain("unrestricted"); + expect(r.hint).toBeTruthy(); + }); + + it("WARN when the binding is deleted", async () => { + const { row: r } = await row( + healthy({ + [BINDING_KEY]: { + ok: false, + stderr: NOT_FOUND("validatingadmissionpolicybindings"), + }, + }), + ); + expect(r.status).toBe("warn"); + expect(r.detail).toMatch(/binding/i); + expect(r.detail).toContain("never evaluated"); + }); + + it("WARN when the operator runs as a different ServiceAccount than the policy names", async () => { + const { row: r } = await row( + healthy({ + [DEPLOY_KEY]: { + ok: true, + stdout: deployment({ sa: "renamed-operator" }), + }, + }), + ); + expect(r.status).toBe("warn"); + expect(r.detail).toContain( + `system:serviceaccount:${OP_NS}:renamed-operator`, + ); + expect(r.detail).toMatch(/matches nobody/); + }); + + it("WARN when the binding only audits", async () => { + const audit = clone(fixture.binding); + setPath(audit, "spec.validationActions", ["Audit"]); + const { row: r } = await row( + healthy({ + [BINDING_KEY]: { ok: true, stdout: JSON.stringify(audit) }, + }), + ); + expect(r.status).toBe("warn"); + expect(r.detail).toMatch(/Deny/); + }); + + it("WARN naming the 1.30 floor when the server is older, without asking for the policy", async () => { + const { row: r, calls } = await row( + healthy( + {}, + '{"major":"1","minor":"29+","gitVersion":"v1.29.5-gke.1"}', + ), + ); + expect(r.status).toBe("warn"); + expect(r.detail).toContain("1.30"); + expect(r.detail).toContain("1.29"); + expect(r.detail).toContain("unrestricted"); + // Nothing to look up on a server that cannot hold the objects. + expect(calls).not.toContain(POLICY_KEY); + }); + + it("reads the policy name the operator was configured with", async () => { + const custom = "acme-secret-configmap-writes"; + const customKey = `kubectl get validatingadmissionpolicy ${custom} -o json`; + const customBindingKey = `kubectl get validatingadmissionpolicybinding ${custom} -o json`; + const p = clone(fixture.policy); + setPath(p, "metadata.name", custom); + const b = clone(fixture.binding); + setPath(b, "metadata.name", custom); + setPath(b, "spec.policyName", custom); + const { row: r } = await row( + healthy({ + [DEPLOY_KEY]: { + ok: true, + stdout: deployment({ + env: [{ name: "WRITE_POLICY_NAME", value: custom }], + }), + }, + [customKey]: { ok: true, stdout: JSON.stringify(p) }, + [customBindingKey]: { ok: true, stdout: JSON.stringify(b) }, + }), + ); + expect(r.status).toBe("pass"); + expect(r.detail).toContain(custom); + }); + + it("WARN cannot-determine (not pass) when RBAC denies the read", async () => { + const { row: r } = await row( + healthy({ + [POLICY_KEY]: { + ok: false, + stderr: 'Error from server (Forbidden): validatingadmissionpolicies.admissionregistration.k8s.io "x" is forbidden: User "u" cannot get resource', + }, + }), + ); + expect(r.status).toBe("warn"); + expect(r.detail).toMatch(/cannot determine/i); + }); + + it("SKIPs when the cluster is unreachable", async () => { + const { row: r } = await row({ + "kubectl get --raw /version": { + ok: false, + stderr: "connection refused", + }, + }); + expect(r.status).toBe("skip"); + }); + + it("SKIPs when there is no operator Deployment to take the identity from", async () => { + const { row: r } = await row( + healthy({ + [DEPLOY_KEY]: { ok: true, stdout: '{"items":[]}' }, + }), + ); + expect(r.status).toBe("skip"); + }); + + it("only ever reads (get)", async () => { + const { calls } = await row(healthy()); + for (const c of calls) { + expect( + c.startsWith("kubectl get ") || c.startsWith("kubectl version"), + ).toBe(true); + } + expect(calls).toContain(POLICY_KEY); + expect(calls).toContain(BINDING_KEY); + }); +}); diff --git a/packages/kn-next/src/__tests__/doctor.test.ts b/packages/kn-next/src/__tests__/doctor.test.ts index 552bd2251..b57b2373d 100644 --- a/packages/kn-next/src/__tests__/doctor.test.ts +++ b/packages/kn-next/src/__tests__/doctor.test.ts @@ -40,6 +40,7 @@ import { probeManifest, runDoctor, } from "../cli/doctor"; +import { WRITE_POLICY_STUBS } from "./doctor-write-policy-stubs"; /** * bun's `typeof fetch` carries a `preconnect` property; a bare async arrow does @@ -98,6 +99,7 @@ function healthyStubs(): Record< > { return { "kubectl get --raw /version": { ok: true, stdout: "{}" }, + ...WRITE_POLICY_STUBS, // (g) local client version — modern enough for --validate=strict. "kubectl version --client -o json": { ok: true, @@ -244,6 +246,7 @@ describe("runDoctor — healthy cluster", () => { "crd-schema", "operator", "operator-version", + "write-policy", "cert-manager", "ingress", "image", diff --git a/packages/kn-next/src/__tests__/observability-metric-contract.test.ts b/packages/kn-next/src/__tests__/observability-metric-contract.test.ts index 8be810813..6a5080f26 100644 --- a/packages/kn-next/src/__tests__/observability-metric-contract.test.ts +++ b/packages/kn-next/src/__tests__/observability-metric-contract.test.ts @@ -403,8 +403,8 @@ const RULE_GROUPS: Record< { emitters: readonly EmitterId[]; why: string } > = { "knext.operator": { - emitters: ["operator"], - why: "the Go controller's own registry", + emitters: ["operator", "external"], + why: "the Go controller's own registry, plus Prometheus' own `up` (the write-policy meta-alert keys on a discovered operator target, as the app staleness rules do)", }, "knext.nextapp": { emitters: ["external"], diff --git a/packages/kn-next/src/cli/doctor.ts b/packages/kn-next/src/cli/doctor.ts index a96c1739e..83ac8b1ce 100644 --- a/packages/kn-next/src/cli/doctor.ts +++ b/packages/kn-next/src/cli/doctor.ts @@ -66,6 +66,7 @@ import { storageModeCheck } from "./doctor/checks/storage-mode"; import { turbopackFragmentsCheck } from "./doctor/checks/turbopack-fragments"; import { cacheComponentsCheck } from "./doctor/checks/use-cache"; import { vinextPatchesCheck } from "./doctor/checks/vinext-patches"; +import { writePolicyCheck } from "./doctor/checks/write-policy"; import { kubectlRunner, probeManifest } from "./doctor/kubectl"; import { formatDoctorTable } from "./doctor/report"; import type { @@ -147,6 +148,7 @@ export async function runDoctor( checks.push(...crdSchemaCheck(ctx)); checks.push(...operatorCheck(ctx)); // sets ctx.operatorImage + ctx.operatorManager checks.push(...operatorVersionCheck(ctx)); // reads ctx.operatorManager (#1947) + checks.push(...writePolicyCheck(ctx)); // reads ctx.operatorManager: is the operator's Secret/ConfigMap write policy in force? checks.push(...certManagerCheck(ctx)); checks.push(...ingressCheck(ctx)); checks.push(...(await operatorImageCheck(ctx))); // reads ctx.operatorImage @@ -204,7 +206,9 @@ function readOwnVersion(): string | undefined { const DOCTOR_HELP = `knext doctor — cluster-prereq preflight (read-only) Checks: NextApp CRD, operator readiness, the installed operator's release -version and whether it pairs with this CLI (WARN only), cert-manager webhook, Knative +version and whether it pairs with this CLI (WARN only), whether the operator's +Secret/ConfigMap write policy is in force (WARN only; needs Kubernetes 1.30+), +cert-manager webhook, Knative ingress-class vs its reconciler (#208), operator-image pullability (#198), app-image pullability vs the namespace's pull credentials (#952), NextApps whose Redis key prefixes collide (WARN only), diff --git a/packages/kn-next/src/cli/doctor/checks/write-policy.ts b/packages/kn-next/src/cli/doctor/checks/write-policy.ts new file mode 100644 index 000000000..d76c627db --- /dev/null +++ b/packages/kn-next/src/cli/doctor/checks/write-policy.ts @@ -0,0 +1,487 @@ +/** + * Operator write policy. The operator's cluster-wide create/update on Secrets + * and ConfigMaps is bounded by a ValidatingAdmissionPolicy (RBAC cannot restrict + * `create` by name), and that bound can be off with nothing saying so: + * + * - a server older than Kubernetes 1.30 rejects the two policy objects while the + * rest of the install bundle applies; + * - an install under another namespace or ServiceAccount name than the bundle + * rendered leaves a matchCondition that matches nobody; + * - an admin or a GitOps prune deletes the policy or its binding. + * + * The operator publishes the same verdict as `knext_operator_write_policy_enforced` + * (internal/writepolicy). This is the CLI half: read-only `get`s only, and an + * unknown is reported as not verified, never as enforced. + * + * KEEP IN LOCKSTEP with internal/writepolicy/verdict.go. Both are held to + * `kn-next-operator/internal/writepolicy/testdata/verdict-cases.json`. + */ + +import { excerpt } from "../../shared"; +import { classifyKubectlFailure, infraFailure, safeJson } from "../kubectl"; +import { mk } from "../report"; +import type { CheckContext, CheckResult } from "../types"; +import { OPERATOR_NAMESPACE, SKIP_UNREACHABLE } from "../types"; + +const ID = "write-policy"; +const TITLE = "Operator write policy"; + +/** + * The policy's name AS THE BUNDLE RENDERS IT (the `kn-next-operator-` name + * prefix applied), used when the operator Deployment carries no + * WRITE_POLICY_NAME, i.e. an operator from before the self-check existed. + * + * The operator's own flag default (cmd/main.go, `--write-policy-name`) is the + * UNPREFIXED base name `secret-configmap-writes` on purpose: it runs inside an + * operator whose bundle always sets the env, so the fallback only matters for a + * hand-run `make run`. Doctor runs outside it and is most likely to meet a + * stock bundle, so the rendered name is the useful guess. + */ +const DEFAULT_POLICY_NAME = "kn-next-operator-secret-configmap-writes"; + +/** + * The variables and validations the shipped policy carries, reduced to their + * CEL: what the policy ADMITS, which matchConstraints, identity and binding all + * being right do not guarantee (a validation replaced by `true` admits + * everything). A copy of config/admission/secret-configmap-writes.yaml. + * + * KEEP IN LOCKSTEP: doctor-write-policy-canonical.test.ts fails if + * config/admission changes without this, and the operator holds the same copy + * (internal/writepolicy/canonical.go). Cosmetic fields (message, reason) are not + * compared; messageExpression is, because it is CEL the apiserver evaluates. + * An operator whose bundle is newer than the installed policy reports + * PolicyModified until the bundle is reapplied, which is the honest answer. + */ +export const CANONICAL_POLICY_CEL = { + variables: [ + { + name: "objectName", + expression: + "request.operation == 'DELETE' ? oldObject.metadata.name : object.metadata.name", + }, + { + name: "allowedSecretSuffixes", + expression: "['-knext-signing-key']", + }, + { name: "allowedConfigMapSuffixes", expression: "['-knext-jwks']" }, + { name: "allowedSecretType", expression: "'Opaque'" }, + { + name: "allowedSuffixes", + expression: + "request.resource.resource == 'secrets' ? variables.allowedSecretSuffixes : variables.allowedConfigMapSuffixes", + }, + ], + validations: [ + { + expression: + "variables.allowedSuffixes.exists(s, variables.objectName.endsWith(s) && size(variables.objectName) > size(s))", + messageExpression: + "'the knext operator may not ' + request.operation.lowerAscii() + ' the ' + request.resource.resource + ' object \"' + variables.objectName + '\": it writes only the per-zone signing key and JWKS'", + }, + { + expression: + "request.resource.resource != 'secrets' || request.operation == 'DELETE' || dyn(object).type == variables.allowedSecretType", + messageExpression: "", + }, + ], +} as const; + +/** ValidatingAdmissionPolicy is admissionregistration.k8s.io/v1 from here. */ +const FLOOR = { major: 1, minor: 30 } as const; + +export type WritePolicyReason = + | "Enforced" + | "PolicyMissing" + | "BindingMissing" + | "IdentityUnknown" + | "PolicyFailsOpen" + | "PolicyScope" + | "PolicyModified" + | "IdentityUnparseable" + | "IdentityMismatch" + | "BindingPolicyMismatch" + | "BindingNotDeny" + | "BindingScoped"; + +export interface WritePolicyVerdict { + enforced: boolean; + reason: WritePolicyReason; + /** One sentence for a human; empty only when enforced. */ + detail: string; +} + +interface Selector { + matchLabels?: Record; + matchExpressions?: unknown[]; +} + +interface ResourceRule { + apiGroups?: string[]; + apiVersions?: string[]; + resources?: string[]; + resourceNames?: string[]; + operations?: string[]; + scope?: string; +} + +interface PolicyJson { + metadata?: { name?: string }; + spec?: { + failurePolicy?: string; + matchConstraints?: { + resourceRules?: ResourceRule[]; + excludeResourceRules?: ResourceRule[]; + namespaceSelector?: Selector; + objectSelector?: Selector; + }; + matchConditions?: { name?: string; expression?: string }[]; + paramKind?: { apiVersion?: string; kind?: string } | null; + variables?: { name?: string; expression?: string }[]; + validations?: { expression?: string; messageExpression?: string }[]; + }; +} + +interface BindingJson { + metadata?: { name?: string }; + spec?: { + policyName?: string; + paramRef?: Record | null; + validationActions?: string[]; + matchResources?: { + namespaceSelector?: Selector; + objectSelector?: Selector; + resourceRules?: unknown[]; + excludeResourceRules?: unknown[]; + }; + }; +} + +const no = (reason: WritePolicyReason, detail: string): WritePolicyVerdict => ({ + enforced: false, + reason, + detail, +}); + +/** + * The one matchCondition shape read: an equality on the username against string + * literals joined with +. Anything richer is not an exact-identity confinement. + */ +const IDENTITY_EXPR = + /^\s*request\.userInfo\.username\s*==\s*('[^']*'(?:\s*\+\s*'[^']*')*)\s*$/; + +function confinedUsername(policy: PolicyJson): string | undefined { + const conds = policy.spec?.matchConditions ?? []; + if (conds.length !== 1) return undefined; + const m = IDENTITY_EXPR.exec(conds[0].expression ?? ""); + if (!m) return undefined; + let out = ""; + for (const l of m[1].matchAll(/'([^']*)'/g)) out += l[1]; + return out; +} + +/** An empty selector selects everything; the apiserver defaults an absent one to it. */ +const narrows = (s?: Selector): boolean => + !!s && + (Object.keys(s.matchLabels ?? {}).length > 0 || + (s.matchExpressions ?? []).length > 0); + +const has = (list: string[] | undefined, want: string): boolean => + (list ?? []).some((v) => v === want || v === "*"); + +/** The first (resource, operation) the policy does not reach, or "" if none. */ +function uncovered(policy: PolicyJson): string { + const rules = policy.spec?.matchConstraints?.resourceRules; + if (!policy.spec?.matchConstraints) return "it has no matchConstraints"; + // Any exclusion could be carving Secrets or ConfigMaps (or an operation) + // back out of the rules below, and reading which is a re-implementation of + // the apiserver's matching. The shipped policy has none, so none is accepted. + if ((policy.spec.matchConstraints.excludeResourceRules ?? []).length > 0) { + return "it excludes some requests through excludeResourceRules"; + } + for (const resource of ["secrets", "configmaps"]) { + for (const op of ["CREATE", "UPDATE", "DELETE"]) { + // A rule that names objects, is cluster-scoped, or is for another + // apiVersion does not cover the operator's writes, however it spells + // the resources. Secrets and ConfigMaps are namespaced; an absent + // scope means all. + const found = (rules ?? []).some( + (r) => + has(r.apiGroups, "") && + has(r.apiVersions, "v1") && + has(r.resources, resource) && + has(r.operations, op) && + (r.resourceNames ?? []).length === 0 && + (r.scope === undefined || + r.scope === "Namespaced" || + r.scope === "*"), + ); + if (!found) return `it does not cover ${op} on ${resource}`; + } + } + return ""; +} + +/** Whether the policy's variables and validations are exactly the shipped CEL. */ +function sameCEL(policy: PolicyJson): boolean { + const got = { + variables: (policy.spec?.variables ?? []).map((v) => ({ + name: v.name ?? "", + expression: v.expression ?? "", + })), + validations: (policy.spec?.validations ?? []).map((v) => ({ + expression: v.expression ?? "", + messageExpression: v.messageExpression ?? "", + })), + }; + return JSON.stringify(got) === JSON.stringify(CANONICAL_POLICY_CEL); +} + +/** + * Whether the write policy is in force for the operator that runs as + * `operatorUsername`. `policy`/`binding` are undefined when the object does not + * exist. Checks run in a fixed order, so the first thing wrong is reported. + */ +export function evaluateWritePolicy( + policy: PolicyJson | undefined, + binding: BindingJson | undefined, + operatorUsername: string, +): WritePolicyVerdict { + if (!policy) { + return no( + "PolicyMissing", + "the ValidatingAdmissionPolicy that limits the operator's Secret and ConfigMap writes does not exist (deleted or pruned, or the server is older than Kubernetes 1.30): the operator is unrestricted", + ); + } + const name = policy.metadata?.name ?? ""; + if (!binding) { + return no( + "BindingMissing", + `the policy ${name} exists but no binding of that name does, so it is never evaluated: the operator is unrestricted`, + ); + } + if (!operatorUsername) { + return no( + "IdentityUnknown", + "the username the operator runs as could not be established, so the policy's identity cannot be compared with it", + ); + } + if (policy.spec?.failurePolicy === "Ignore") { + return no( + "PolicyFailsOpen", + `the policy ${name} has failurePolicy Ignore, so an evaluation error lets the write through`, + ); + } + const gap = uncovered(policy); + if (gap) { + return no( + "PolicyScope", + `the policy ${name} does not confine every write the operator makes: ${gap}`, + ); + } + const mc = policy.spec?.matchConstraints; + if (narrows(mc?.namespaceSelector) || narrows(mc?.objectSelector)) { + return no( + "PolicyScope", + `the policy ${name} narrows by namespace or object selector, but the operator writes in every namespace`, + ); + } + const confined = confinedUsername(policy); + if (confined === undefined) { + return no( + "IdentityUnparseable", + `the policy ${name} does not confine one exact username (its matchCondition is not an equality on request.userInfo.username), so which principal it limits cannot be verified`, + ); + } + if (confined !== operatorUsername) { + return no( + "IdentityMismatch", + `the policy ${name} confines ${confined} but the operator runs as ${operatorUsername}: it matches nobody the operator is (installed under another namespace or ServiceAccount name than the bundle rendered?)`, + ); + } + // The shipped policy takes no parameters. A paramKind lets a binding's + // paramRef resolve to nothing, and the apiserver then runs no validation at + // all (with parameterNotFoundAction Allow), however right everything else is. + if (policy.spec?.paramKind != null) { + return no( + "PolicyModified", + `the policy ${name} declares paramKind, which the shipped policy does not: a binding paramRef that finds no parameter makes the apiserver skip every validation`, + ); + } + if (!sameCEL(policy)) { + return no( + "PolicyModified", + `the policy ${name} does not carry the variables and validations this CLI ships, so what it admits is not what the bundle intended (its validation CEL, an allow-list variable or a messageExpression was changed, added or removed)`, + ); + } + if (binding.spec?.policyName !== name) { + return no( + "BindingPolicyMismatch", + `the binding names policy ${binding.spec?.policyName ?? "(none)"}, not ${name}`, + ); + } + if (!(binding.spec?.validationActions ?? []).includes("Deny")) { + return no( + "BindingNotDeny", + `the binding does not include the Deny action (actions ${JSON.stringify(binding.spec?.validationActions ?? [])}): a violation is warned about or audited, never refused`, + ); + } + // Likewise the shipped binding sets no paramRef: one that resolves to no + // parameter (a missing name, a selector matching nothing) runs no validation. + if (binding.spec?.paramRef != null) { + return no( + "BindingScoped", + `the binding sets paramRef, which the shipped binding does not: a parameter that cannot be found makes the apiserver skip every validation`, + ); + } + const mr = binding.spec?.matchResources; + if ( + mr && + (narrows(mr.namespaceSelector) || + narrows(mr.objectSelector) || + (mr.resourceRules ?? []).length > 0 || + (mr.excludeResourceRules ?? []).length > 0) + ) { + return no( + "BindingScoped", + "the binding limits the policy to some namespaces or objects, but the operator writes in every namespace", + ); + } + return { enforced: true, reason: "Enforced", detail: "" }; +} + +/** The server's major.minor from `/version`, or undefined when unreadable. */ +function serverVersion( + stdout: string, +): { major: number; minor: number } | undefined { + const v = safeJson<{ + major?: string; + minor?: string; + gitVersion?: string; + }>(stdout); + if (!v) return undefined; + const git = /^v?(\d+)\.(\d+)/.exec(v.gitVersion ?? ""); + if (git) return { major: Number(git[1]), minor: Number(git[2]) }; + const major = Number.parseInt(v.major ?? "", 10); + const minor = Number.parseInt((v.minor ?? "").replace(/\D+$/, ""), 10); + return Number.isFinite(major) && Number.isFinite(minor) + ? { major, minor } + : undefined; +} + +const REPAIR_HINT = + "reapply the operator bundle (`kubectl apply --server-side -f install.yaml`) on Kubernetes 1.30 or newer; for a repackaged install, make the policy's identity and name match the operator's ServiceAccount and WRITE_POLICY_NAME"; + +export function writePolicyCheck(ctx: CheckContext): CheckResult[] { + if (ctx.skipAll) { + return [mk(ID, TITLE, "skip", SKIP_UNREACHABLE)]; + } + const manager = ctx.operatorManager; + if (!manager) { + return [ + mk( + ID, + TITLE, + "skip", + "no operator Deployment found, so there is no identity to compare the write policy with — skipped", + ), + ]; + } + + const version = serverVersion( + ctx.kubectl(["kubectl", "get", "--raw", "/version"]).stdout, + ); + if ( + version && + (version.major < FLOOR.major || + (version.major === FLOOR.major && version.minor < FLOOR.minor)) + ) { + return [ + mk( + ID, + TITLE, + "warn", + `the server is Kubernetes ${version.major}.${version.minor}, older than the ${FLOOR.major}.${FLOOR.minor} floor: ValidatingAdmissionPolicy is not available, so nothing limits the operator's Secret and ConfigMap writes — the operator is unrestricted`, + `upgrade the cluster to Kubernetes ${FLOOR.major}.${FLOOR.minor} or newer, then reapply the operator bundle`, + ), + ]; + } + + const podSpec = manager.spec?.template?.spec; + const container = + podSpec?.containers?.find((c) => c.name === "manager") ?? + podSpec?.containers?.[0]; + const name = + container?.env?.find((e) => e.name === "WRITE_POLICY_NAME")?.value || + DEFAULT_POLICY_NAME; + const namespace = manager.metadata?.namespace ?? OPERATOR_NAMESPACE; + const sa = podSpec?.serviceAccountName ?? "default"; + const username = `system:serviceaccount:${namespace}:${sa}`; + + const read = (kind: string) => + ctx.kubectl(["kubectl", "get", kind, name, "-o", "json"]); + const policyRes = read("validatingadmissionpolicy"); + const bindingRes = read("validatingadmissionpolicybinding"); + + for (const r of [policyRes, bindingRes]) { + if (r.ok) continue; + const cls = classifyKubectlFailure(r.stderr); + if (cls === "not-found") continue; + if (cls === "forbidden") { + return [ + mk( + ID, + TITLE, + "warn", + `cannot determine whether the operator's write policy ${name} is in force (reading it was denied by RBAC) — treat the operator as UNRESTRICTED until verified`, + "grant `get` on validatingadmissionpolicies and validatingadmissionpolicybindings for this diagnosis, or check the operator's knext_operator_write_policy_enforced gauge", + ), + ]; + } + const infra = infraFailure(r); + if (infra) return [mk(ID, TITLE, "error", infra.detail, infra.hint)]; + return [ + // Unknown is not enforced: a read that failed for a reason doctor does + // not recognise says nothing about the policy, so it is a WARN, not a + // skip that reads as "nothing to see". + mk( + ID, + TITLE, + "warn", + `could not read the operator's write policy ${name} (${excerpt(r.stderr.replace(/[^\x20-\x7e\s]/g, "")) || "no error text"}) — not verified; treat the operator as UNRESTRICTED until it is`, + REPAIR_HINT, + ), + ]; + } + + const policy = policyRes.ok + ? (safeJson(policyRes.stdout) ?? undefined) + : undefined; + const binding = bindingRes.ok + ? (safeJson(bindingRes.stdout) ?? undefined) + : undefined; + // A policy object that exists but could not be parsed is not "missing". + if ((policyRes.ok && !policy) || (bindingRes.ok && !binding)) { + return [ + mk( + ID, + TITLE, + "warn", + `the operator's write policy ${name} returned unreadable output — not verified; treat the operator as UNRESTRICTED until it is`, + REPAIR_HINT, + ), + ]; + } + + const v = evaluateWritePolicy(policy, binding, username); + if (v.enforced) { + return [ + mk( + ID, + TITLE, + "pass", + `${name} is bound with Deny, covers create/update/delete on Secrets and ConfigMaps, confines ${username} and carries the shipped validations, so the operator may write only its per-zone signing-key Secrets and JWKS ConfigMaps`, + ), + ]; + } + return [mk(ID, TITLE, "warn", v.detail, REPAIR_HINT)]; +} diff --git a/packages/kn-next/src/cli/doctor/types.ts b/packages/kn-next/src/cli/doctor/types.ts index ad9740cfd..f4bedcdcd 100644 --- a/packages/kn-next/src/cli/doctor/types.ts +++ b/packages/kn-next/src/cli/doctor/types.ts @@ -264,11 +264,22 @@ export interface DaemonSetRef { } export interface DeploymentJson { - metadata?: { name?: string; labels?: Record }; + metadata?: { + name?: string; + namespace?: string; + labels?: Record; + }; spec?: { template?: { metadata?: { labels?: Record }; - spec?: { containers?: { image?: string }[] }; + spec?: { + serviceAccountName?: string; + containers?: { + name?: string; + image?: string; + env?: { name: string; value?: string }[]; + }[]; + }; }; }; status?: { readyReplicas?: number; replicas?: number }; diff --git a/scripts/lib/prover-lane.mjs b/scripts/lib/prover-lane.mjs index adea09eee..bcb6b1f42 100644 --- a/scripts/lib/prover-lane.mjs +++ b/scripts/lib/prover-lane.mjs @@ -88,6 +88,7 @@ export const NON_JS_TEST_SUITE_PROVERS = Object.freeze([ 'scripts/mutation-prove-2476-propagation-bound.mjs', 'scripts/mutation-prove-2462-operator-write-policy.mjs', 'scripts/mutation-prove-2498-transient-secret.mjs', + 'scripts/mutation-prove-2500-write-policy-selfcheck.mjs', ]); /** The toolchains a `NON_JS_TEST_SUITE_PROVERS` entry may run its guards with. */ diff --git a/scripts/lib/prover-shard.mjs b/scripts/lib/prover-shard.mjs index af8fa1cc7..a4e6a9bf6 100644 --- a/scripts/lib/prover-shard.mjs +++ b/scripts/lib/prover-shard.mjs @@ -74,6 +74,8 @@ export const PROVER_WEIGHT_MINUTES = Object.freeze({ 'scripts/mutation-prove-compat-cell-fingerprint.mjs': 20, 'scripts/mutation-prove-2331-use-cache-tag-replay.mjs': 14, 'scripts/mutation-prove-2462-operator-write-policy.mjs': 14, + // 35 mutations, each a `go test` (the envtest rows start a real apiserver) or a bundle re-render. + 'scripts/mutation-prove-2500-write-policy-selfcheck.mjs': 15, // 12 minutes for 23 mutations with the per-leg cargo dir and generator dir adopted; // the earlier 60-minute estimate was the cold worst case. 'scripts/mutation-prove-2489-rust-scaffold.mjs': 12, @@ -108,6 +110,7 @@ export const WEIGHTED_ON_ONE_REF_ONLY = Object.freeze([ 'scripts/mutation-prove-2052-backendservice.mjs', 'scripts/mutation-prove-2331-use-cache-tag-replay.mjs', 'scripts/mutation-prove-2462-operator-write-policy.mjs', + 'scripts/mutation-prove-2500-write-policy-selfcheck.mjs', 'scripts/mutation-prove-2489-rust-scaffold.mjs', 'scripts/mutation-prove-rust-template-auth.mjs', 'scripts/mutation-prove-go-template-auth.mjs', diff --git a/scripts/mutation-prove-2500-write-policy-selfcheck.mjs b/scripts/mutation-prove-2500-write-policy-selfcheck.mjs new file mode 100644 index 000000000..9b39fa19b --- /dev/null +++ b/scripts/mutation-prove-2500-write-policy-selfcheck.mjs @@ -0,0 +1,594 @@ +#!/usr/bin/env node +/** + * Mutation proof for the operator write-policy self-check (issue #2500): the + * operator's read-only check that the ValidatingAdmissionPolicy limiting its + * Secret/ConfigMap writes is actually in force, the 0/1 gauge it publishes, the + * alert on a 0, the read-only RBAC it needs, the kustomize wiring that gives it + * the rendered policy name, and `knext doctor`'s mirror of the same verdict. + * + * Every row breaks ONE rule and must turn its guard red: + * - the verdict: each way the policy can be off (deleted, binding deleted, + * binding that does not Deny, a username that is not the operator's, a policy + * that fails open or skips a verb or a resource, a binding scoped to some + * namespaces), and the headline row, "the check is forced to true", which + * must red the deleted-policy case on a real apiserver; + * - the gauge: published as 0 when off, one warning per change, run on every + * replica, re-checked on an interval, a failed read reported as not enforced; + * - the RBAC: `get` only, by name, on the two kinds, in a dedicated role; + * - the wiring: the operator's WRITE_POLICY_NAME and the role's resourceNames + * follow the policy's prefixed name, and main registers the monitor; + * + * The alert rule and `knext doctor`'s mirror of the verdict are the TS half: + * mutation-prove-2500-write-policy-ts.mjs. + * + * Graded by `go test` in packages/kn-next-operator (the envtest proof needs + * envtest binaries: KUBEBUILDER_ASSETS or bin/k8s; the bundle rows need + * kustomize: KUSTOMIZE or bin/kustomize or PATH). Each row is BUILD-CHECKED + * first (the mutant must compile its tests), so a row can only count as killed + * by a failing assertion, never by a compile error. Exit 0 = every row went red; 1 = a row survived; 2 = the + * prover could not run (anchor drift, red baseline, an unbuildable mutant, + * missing tool). Never parses output. + * + * Usage: + * node scripts/mutation-prove-2500-write-policy-selfcheck.mjs + * MUTATION_ONLY=V01,K01 node scripts/mutation-prove-2500-write-policy-selfcheck.mjs + */ + +import { spawnSync } from 'node:child_process'; +import { existsSync, readdirSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { assertTreeClean } from './lib/guard-prover.mjs'; +import { mutate, restore, snapshot } from './lib/mutation-harness.mjs'; +import { declareMutations, recordMutation } from './lib/prover-report.mjs'; + +const NAME = 'mutation-prove-2500-write-policy-selfcheck'; +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const OPERATOR_MODULE = `${REPO_ROOT}/packages/kn-next-operator`; + +/** A subject's repo-relative path, made absolute. The map below holds literals so the prover-lane anchor audit can read it. */ +const subjectPath = (key) => { + const rel = PROVER.subjects[key]; + return rel === undefined ? undefined : resolve(REPO_ROOT, rel); +}; + +const PROVER = { + subjects: { + verdict: 'packages/kn-next-operator/internal/writepolicy/verdict.go', + canonical: 'packages/kn-next-operator/internal/writepolicy/canonical.go', + shippedPolicy: 'packages/kn-next-operator/config/admission/secret-configmap-writes.yaml', + monitor: 'packages/kn-next-operator/internal/writepolicy/monitor.go', + source: 'packages/kn-next-operator/internal/writepolicy/source.go', + main: 'packages/kn-next-operator/cmd/main.go', + role: 'packages/kn-next-operator/config/rbac/write_policy_reader_role.yaml', + rbacKust: 'packages/kn-next-operator/config/rbac/kustomization.yaml', + defaultKust: 'packages/kn-next-operator/config/default/kustomization.yaml', + }, +}; + +const WP = './internal/writepolicy/'; +const CTRL = './internal/controller/'; +const INSTALL = './internal/install/'; +const CMD = './cmd/'; + +/** A `go test -run ` guard. */ +const unit = (run, pkg) => ({ kind: 'go', args: ['-run', run, pkg] }); +/** The verdict table and the monitor, no cluster. */ +const VERDICT = unit('TestEvaluateMatchesTheSharedVerdictTable', WP); +/** The canonical CEL copy equals the shipped policy, and the shared table's base policy carries it. */ +const CANON = unit( + 'TestCanonicalPolicyCELEqualsTheShippedPolicy|TestSharedTableBasePolicyCarriesTheShippedCEL', + WP, +); +const MONITOR = unit( + 'TestGaugeIsOneWhenThePolicyIsEnforced|TestGaugeIsZeroAndOneWarningIsLoggedWhenNotEnforced|TestWarningFollowsTheStateAcrossRecoveryAndRegression|TestMonitorRunsOnEveryReplicaAndRechecksOnAnInterval', + WP, +); +/** The self-check against a real kube-apiserver (deleted policy, deleted binding, wrong username, audit-only, the shipped role). */ +const APISERVER = unit('TestSelfCheckAgainstARealApiserver', WP); +/** The RBAC scans: the grant is exactly get-by-name, and no shipped rule writes the policy kinds. */ +const RBAC = unit( + 'TestWritePolicyReaderRoleIsExactlyGetOnTheTwoNamedObjects|TestOperatorCannotEditItsOwnWritePolicy', + CTRL, +); +/** main registers the monitor, with the configured name, on every replica. */ +const WIRING = unit( + 'TestAddWritePolicyMonitorRegistersAnEveryReplicaMonitor|TestWritePolicyNameComesFromTheRenderedEnvironment|TestMainRegistersTheWritePolicyMonitor', + CMD, +); +/** The rendered bundle: re-render dist/install.yaml, then the install tests that read it. */ +const BUNDLE = { kind: 'bundle' }; +const MUTATIONS = [ + // --- the verdict ------------------------------------------------------------ + { + id: 'V01', + claim: 'the check is forced to true: a deleted policy is reported as enforced', + subject: 'monitor', + // The deleted-policy case has to be read off a real apiserver to count. + anchor: ' return Evaluate(policy, binding, username)\n}', + replacement: + ' _ = Evaluate(policy, binding, username)\n return Verdict{Enforced: true, Reason: ReasonEnforced}\n}', + test: APISERVER, + }, + { + id: 'V02', + claim: 'a binding that only audits is accepted (the Deny test is skipped)', + subject: 'verdict', + anchor: ' if !deny {', + replacement: ' if false && !deny {', + test: VERDICT, + }, + { + id: 'V03', + claim: "the username the policy confines is not compared with the operator's", + subject: 'verdict', + anchor: ' if confined != operatorUsername {', + replacement: ' if false && confined != operatorUsername {', + test: VERDICT, + }, + { + id: 'V04', + claim: 'a binding for another policy is accepted', + subject: 'verdict', + anchor: ' if binding.Spec.PolicyName != policy.Name {', + replacement: ' if false && binding.Spec.PolicyName != policy.Name {', + test: VERDICT, + }, + { + id: 'V05', + claim: 'a missing binding is not noticed', + subject: 'verdict', + anchor: ' if binding == nil {', + replacement: ' if binding == nil && false {', + test: VERDICT, + }, + { + id: 'V06', + claim: 'a policy that fails open (failurePolicy Ignore) is accepted', + subject: 'verdict', + anchor: 'fp != nil && *fp == admissionregistrationv1.Ignore {', + replacement: 'fp != nil && *fp == "never" {', + test: VERDICT, + }, + { + id: 'V07', + claim: 'a policy that skips an operation or a resource is accepted', + subject: 'verdict', + anchor: ' if !found {', + replacement: ' if false && !found {', + test: VERDICT, + }, + { + id: 'V08', + claim: 'a binding scoped to some namespaces is accepted (a selector never narrows)', + subject: 'verdict', + anchor: ' return s != nil && (len(s.MatchLabels) > 0 || len(s.MatchExpressions) > 0)', + replacement: ' return false', + test: VERDICT, + }, + { + id: 'V09', + claim: 'a matchCondition that is not a plain equality (startsWith, !=) is read as an identity', + subject: 'verdict', + anchor: 'var identityExpr = regexp.MustCompile(`^\\s*request\\.userInfo\\.username\\s*==\\s*(', + replacement: 'var identityExpr = regexp.MustCompile(`request\\.userInfo\\.username.*?(', + test: VERDICT, + }, + { + id: 'V10', + claim: 'a read that fails (RBAC, transport) is reported as enforced', + subject: 'monitor', + anchor: + ' return no(ReasonUnverifiable, "reading ValidatingAdmissionPolicy %q failed: %v", name, err)', + replacement: ' return Verdict{Enforced: true, Reason: ReasonEnforced}', + test: MONITOR, + }, + // --- narrowed scope and a modified validation body (round-1 review) ---------- + { + id: 'V11', + claim: 'an excludeResourceRules entry that carves Secrets out of the policy is accepted', + subject: 'verdict', + anchor: ' if len(mc.ExcludeResourceRules) > 0 {', + replacement: ' if false && len(mc.ExcludeResourceRules) > 0 {', + test: VERDICT, + }, + { + id: 'V12', + claim: 'a covering rule that names objects (resourceNames) is accepted', + subject: 'verdict', + // Whole tail of the line: the harness appends its residue comment after the + // replacement, which would otherwise comment out the rest of the condition. + anchor: 'len(r.ResourceNames) == 0 && scopeCoversNamespaced(r.Scope) {', + replacement: 'scopeCoversNamespaced(r.Scope) {', + test: VERDICT, + }, + { + id: 'V13', + claim: 'a cluster-scoped rule is accepted as covering namespaced Secrets and ConfigMaps', + subject: 'verdict', + anchor: + ' return s == nil || *s == admissionregistrationv1.NamespacedScope || *s == admissionregistrationv1.AllScopes', + replacement: ' return true', + test: VERDICT, + }, + { + id: 'V14', + claim: 'a rule for another apiVersion is accepted as covering core v1', + subject: 'verdict', + anchor: 'has(r.APIVersions, "v1") && has(r.Resources, resource) && has(ops, string(op)) &&', + replacement: 'has(r.Resources, resource) && has(ops, string(op)) &&', + test: VERDICT, + }, + { + id: 'V15', + claim: + 'the policy body is never compared with the shipped CEL (validations: [true] is Enforced)', + subject: 'verdict', + anchor: ' if !reflect.DeepEqual(policyCEL(policy), canonicalCEL()) {', + replacement: ' if false && !reflect.DeepEqual(policyCEL(policy), canonicalCEL()) {', + test: VERDICT, + }, + { + id: 'V16', + claim: 'the canonical CEL copy drifts from the shipped policy (a variable differs)', + subject: 'canonical', + anchor: '{Name: "allowedSecretType", Expression: "\'Opaque\'"},', + replacement: '{Name: "allowedSecretType", Expression: "\'Opaque2\'"},', + test: CANON, + }, + { + id: 'V17', + claim: 'config/admission changes the shipped CEL without the canonical copy being updated', + subject: 'shippedPolicy', + anchor: ` expression: "['-knext-signing-key']"`, + replacement: ` expression: "['-knext-signing-key', '-extra']"`, + test: CANON, + }, + { + id: 'V18', + claim: + 'a policy that declares paramKind is accepted (a paramRef that finds nothing then runs no validation)', + subject: 'verdict', + anchor: ' if policy.Spec.ParamKind != nil {', + replacement: ' if false && policy.Spec.ParamKind != nil {', + test: VERDICT, + }, + { + id: 'V19', + claim: + 'a binding that sets paramRef is accepted (a missing parameter makes the apiserver skip every validation)', + subject: 'verdict', + anchor: ' if binding.Spec.ParamRef != nil {', + replacement: ' if false && binding.Spec.ParamRef != nil {', + test: VERDICT, + }, + // --- the gauge and the monitor ---------------------------------------------- + { + id: 'G01', + claim: 'the gauge is published as 1 when the policy is off', + subject: 'monitor', + anchor: ' } else {\n enforcedGauge.Set(0)', + replacement: ' } else {\n enforcedGauge.Set(1)', + test: MONITOR, + }, + { + id: 'G02', + claim: 'a warning is logged on every check, not once per change', + subject: 'monitor', + anchor: + ' changed := m.last == nil || m.last.Enforced != v.Enforced || m.last.Reason != v.Reason', + replacement: ' changed := true', + test: MONITOR, + }, + { + id: 'G03', + claim: 'the monitor needs leader election, so a standby replica reports nothing', + subject: 'monitor', + anchor: 'func (m *Monitor) NeedLeaderElection() bool { return false }', + replacement: 'func (m *Monitor) NeedLeaderElection() bool { return true }', + test: MONITOR, + }, + { + id: 'G04', + claim: 'the check runs once at start-up and never again, so a later deletion goes unseen', + subject: 'monitor', + anchor: ' case <-t.C:\n m.CheckOnce(ctx)', + replacement: ' case <-t.C:', + test: MONITOR, + }, + { + id: 'G05', + claim: "the operator's identity is not the one the apiserver reports", + subject: 'source', + anchor: ' return r.Status.UserInfo.Username, nil', + replacement: ' _ = r\n return "", nil', + test: APISERVER, + }, + { + id: 'G06', + claim: 'a deleted policy surfaces as a read error instead of "missing"', + subject: 'source', + anchor: ' if apierrors.IsNotFound(err) {\n return nil, nil\n }\n return p, err', + replacement: ' return p, err', + test: APISERVER, + }, + // --- main ------------------------------------------------------------------- + { + id: 'M01', + claim: 'main never registers the monitor', + subject: 'main', + anchor: + ' if err := addWritePolicyMonitor(mgr.Add, mgr.GetConfig(), writePolicyName, writePolicyCheckInterval); err != nil {', + replacement: ' if err := error(nil); err != nil {', + test: WIRING, + }, + { + id: 'M02', + claim: 'the monitor checks a fixed policy name, not the configured one', + subject: 'main', + anchor: ' Name: name,', + replacement: ' Name: defaultWritePolicyName,', + test: WIRING, + }, + { + id: 'M03', + claim: 'the operator reads the policy name from the wrong environment variable', + subject: 'main', + anchor: ' if v := getenv("WRITE_POLICY_NAME"); v != "" {', + replacement: ' if v := getenv("WRITE_POLICY"); v != "" {', + test: WIRING, + }, + // --- the RBAC --------------------------------------------------------------- + { + id: 'R01', + claim: 'the role also lists admission policies (every policy in the cluster becomes visible)', + subject: 'role', + anchor: ' verbs:\n - get\n', + replacement: ' verbs:\n - get\n - list\n', + test: RBAC, + }, + { + id: 'R02', + claim: "the role is no longer limited to the shipped policy's name", + subject: 'role', + anchor: ' resourceNames:\n - secret-configmap-writes\n', + replacement: '', + test: RBAC, + }, + { + id: 'R03', + claim: 'the role lets the operator edit its own write policy', + subject: 'role', + anchor: ' verbs:\n - get\n', + replacement: ' verbs:\n - get\n - update\n', + test: RBAC, + }, + { + id: 'R04', + claim: 'the role no longer lets the check read the objects (forbidden on a real apiserver)', + subject: 'role', + anchor: ' - validatingadmissionpolicybindings\n', + replacement: '', + test: APISERVER, + }, + { + id: 'R05', + claim: 'the role is dropped from the bundle', + subject: 'rbacKust', + anchor: '- write_policy_reader_role.yaml\n', + replacement: '', + test: BUNDLE, + }, + // --- the kustomize wiring --------------------------------------------------- + { + id: 'K01', + claim: + 'WRITE_POLICY_NAME keeps the base name in a prefixed install (the check reads an object that is not there)', + subject: 'defaultKust', + anchor: + ' kind: Deployment\n name: controller-manager\n fieldPaths:\n - .spec.template.spec.containers.[name=manager].env.[name=WRITE_POLICY_NAME].value', + replacement: + ' kind: Deployment\n name: no-such-deployment\n fieldPaths:\n - .spec.template.spec.containers.[name=manager].env.[name=WRITE_POLICY_NAME].value', + test: BUNDLE, + }, + { + id: 'K02', + claim: + "the role's resourceNames keep the base name in a prefixed install (the check is forbidden from reading the policy)", + subject: 'defaultKust', + anchor: + ' kind: ClusterRole\n name: write-policy-reader\n fieldPaths:\n - .rules.0.resourceNames.0', + replacement: + ' kind: ClusterRole\n name: no-such-role\n fieldPaths:\n - .rules.0.resourceNames.0', + test: BUNDLE, + }, +]; + +// MUTATION_ONLY=V01,K01 runs just those rows. Unset, every row runs; the declared count +// is the rows selected, so declared == run holds either way. +const ONLY = (process.env.MUTATION_ONLY ?? '') + .split(',') + .map((s) => s.trim()) + .filter(Boolean); +const ROWS = ONLY.length === 0 ? MUTATIONS : MUTATIONS.filter((m) => ONLY.includes(m.id)); +if (ONLY.length > 0 && ROWS.length !== ONLY.length) { + console.error(`${NAME}: MUTATION_ONLY names a row that does not exist (${ONLY.join(',')})`); + process.exit(2); +} + +function envtestAssetsPresent() { + if (process.env.KUBEBUILDER_ASSETS) return true; + const dir = `${OPERATOR_MODULE}/bin/k8s`; + return existsSync(dir) && readdirSync(dir).length > 0; +} + +/** kustomize: $KUSTOMIZE, the operator's bin/, then PATH. */ +function kustomizePath() { + if (process.env.KUSTOMIZE) return process.env.KUSTOMIZE; + const local = `${OPERATOR_MODULE}/bin/kustomize`; + if (existsSync(local)) return local; + const which = spawnSync('kustomize', ['version'], { stdio: 'ignore' }); + return which.status === 0 ? 'kustomize' : ''; +} + +/** Run one command; return its exit code. Stderr is kept only to explain a failed build check. */ +function run(bin, args, { cwd = OPERATOR_MODULE, env = {}, showErrors = false } = {}) { + const r = spawnSync(bin, args, { + cwd, + stdio: ['ignore', 'ignore', showErrors ? 'pipe' : 'ignore'], + env: { ...process.env, ...env }, + timeout: 1_800_000, + }); + if (r.error || r.status === null) { + console.error(`${NAME}: cannot run ${bin}: ${r.error?.message ?? r.signal}`); + process.exit(2); + } + if (showErrors && r.status !== 0) console.error(String(r.stderr ?? '').slice(-2000)); + return r.status; +} + +/** + * Re-render the install bundle exactly as `make build-installer` does (without the tool + * downloads), then run the install tests that read it. A render that fails would "kill" + * the row for the wrong reason, so it stops the prover instead of scoring. + */ +function renderBundle(kustomize) { + return run('bash', [ + '-c', + `mkdir -p dist && "${kustomize}" build config/default > dist/install.yaml && ./hack/set-ingress-class.sh dist/install.yaml ""`, + ]); +} + +function guardExit(test, kustomize) { + if (test.kind === 'bundle') { + if (renderBundle(kustomize) !== 0) { + // throw, not process.exit: exiting would skip the caller's restore and leave the + // tree mutated. + throw new Error(`${NAME}: kustomize could not render config/default; the row proves nothing`); + } + return run( + 'go', + [ + 'test', + '-count=1', + '-run', + 'TestRenderedBundleGivesTheSelfCheckTheRenderedPolicyName|TestBaseManifestsNameThePolicyConsistently', + INSTALL, + ], + { env: { KNEXT_REQUIRE_BUNDLE: '1' } }, + ); + } + // -count=1: never trust a cached verdict for a mutated tree. + return run('go', ['test', '-count=1', ...test.args]); +} + +/** The mutated tree must still COMPILE its tests, or the row proves nothing. */ +function buildsExit(file) { + if (!file.endsWith('.go')) return 0; + const build = run('go', ['build', './...'], { showErrors: true }); + if (build !== 0) return build; + return run('go', ['test', '-count=1', '-run', '^$', WP, CTRL, CMD], { showErrors: true }); +} + +declareMutations(ROWS.length); +console.log(`${NAME}: ${ROWS.length} mutations declared`); + +// Preflight: every anchor occurs exactly once BEFORE anything expensive runs. +for (const m of ROWS) { + const path = subjectPath(m.subject); + if (path === undefined) { + console.error(`${NAME}: ${m.id} names no subject ${m.subject}`); + process.exit(2); + } + const count = snapshot(path).bytes.toString('utf8').split(m.anchor).length - 1; + if (count !== 1) { + console.error(`${NAME}: ${m.id}'s anchor occurs ${count} times in ${path}; aborting`); + process.exit(2); + } +} +if (!envtestAssetsPresent()) { + console.error( + `${NAME}: no envtest binaries (set KUBEBUILDER_ASSETS or run \`make setup-envtest\` in packages/kn-next-operator)`, + ); + process.exit(2); +} +const KUSTOMIZE = kustomizePath(); +if (KUSTOMIZE === '') { + console.error( + `${NAME}: no kustomize (set KUSTOMIZE or run \`make kustomize\` in packages/kn-next-operator)`, + ); + process.exit(2); +} + +// Green baseline on a clean tree, for every distinct guard, else every verdict below is noise. +assertTreeClean(REPO_ROOT, 'baseline'); +const seen = new Set(); +for (const m of ROWS) { + const key = JSON.stringify(m.test); + if (seen.has(key)) continue; + seen.add(key); + const code = guardExit(m.test, KUSTOMIZE); + if (code !== 0) { + console.error(`${NAME}: baseline RED for ${m.id}'s guard (exit ${code}); fix the tree first`); + process.exit(2); + } +} +console.log(`${NAME}: baseline green (${seen.size} distinct guards)`); + +// CANARY: prove the harness can SEE red before any green below is trusted. A runner that +// always exits 0 would certify every row while nothing ran. The canary is a real mutation +// (the Deny test skipped) graded by the fast, cluster-free guard. +{ + assertTreeClean(REPO_ROOT, 'before the canary'); + const canary = MUTATIONS.find((m) => m.id === 'V02'); + const snap = snapshot(subjectPath(canary.subject)); + let code; + try { + mutate(snap, canary.anchor, canary.replacement); + code = guardExit(canary.test, KUSTOMIZE); + } finally { + restore(snap); + } + assertTreeClean(REPO_ROOT, 'after the canary'); + if (code === 0) { + console.error( + `${NAME}: the canary mutation (${canary.claim}) left its guard GREEN; the harness cannot observe red`, + ); + process.exit(2); + } + console.log(`${NAME}: canary ok: the harness sees red (exit ${code})`); +} + +let failed = 0; +for (const m of ROWS) { + assertTreeClean(REPO_ROOT, `before ${m.id}`); + const path = subjectPath(m.subject); + const snap = snapshot(path); + let built; + let code; + try { + mutate(snap, m.anchor, m.replacement); + built = buildsExit(path); + code = built === 0 ? guardExit(m.test, KUSTOMIZE) : undefined; + } finally { + restore(snap); + } + assertTreeClean(REPO_ROOT, `after ${m.id}`); + if (built !== 0) { + console.error( + `${NAME}: ${m.id}'s mutation does not build (exit ${built}); the row proves nothing`, + ); + process.exit(2); + } + recordMutation(); + const ok = code !== 0; + if (!ok) failed++; + console.log(`${ok ? 'KILLED ' : '*** SURVIVED ***'} ${m.id} exit=${code} — ${m.claim}`); +} + +// The last bundle row rendered a MUTATED overlay into dist/install.yaml (gitignored). Put the +// restored tree's bundle back so a local `go test ./internal/install/` is not left reading a mutant. +if (ROWS.some((m) => m.test.kind === 'bundle')) renderBundle(KUSTOMIZE); + +console.log(`${NAME}: ${ROWS.length - failed}/${ROWS.length} went red as required`); +process.exit(failed === 0 ? 0 : 1); diff --git a/scripts/mutation-prove-2500-write-policy-ts.mjs b/scripts/mutation-prove-2500-write-policy-ts.mjs new file mode 100644 index 000000000..b1ec7f68c --- /dev/null +++ b/scripts/mutation-prove-2500-write-policy-ts.mjs @@ -0,0 +1,313 @@ +#!/usr/bin/env node +/** + * Mutation prover for #2500, TS half: the alert on the operator's write-policy + * gauge, and `knext doctor`'s mirror of the operator's verdict. + * + * The operator half (the verdict, the gauge, the RBAC, the kustomize wiring) is + * `mutation-prove-2500-write-policy-selfcheck.mjs`. This half proves, by exit + * code, that the two TS consumers BITE: + * + * A1-A3 the alert: the comparison (a 1 instead of a 0), the `for` (a restart + * would flap it), and the severity (every pre-1.30 cluster would page); + * D1-D8 doctor: it must reach the operator's verdict on the same objects, so + * each rule of the verdict is broken in turn (Deny, the username, the + * fail-open policy), plus the version floor, the configured policy name, + * the RBAC-denied and not-found branches, and "forced to pass". + * + * Runs on the shared driver (`lib/guard-prover.mjs`): exit codes only, anchors + * exactly once (preflight), green baseline, a canary red first, tree checked + * clean between rows, partial run = failure. + * + * node scripts/mutation-prove-2500-write-policy-ts.mjs + */ + +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createGuardProver } from './lib/guard-prover.mjs'; +import { declareMutations, recordMutation } from './lib/prover-report.mjs'; + +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const RULE = 'packages/kn-next-operator/config/observability/prometheusrule.yaml'; +const DOCTOR = 'packages/kn-next/src/cli/doctor/checks/write-policy.ts'; +const ALERT_SPEC = 'apps/file-manager/src/app/api/_metrics/prometheusrule.test.ts'; +const DOCTOR_SPEC = 'packages/kn-next/src/__tests__/doctor-write-policy.test.ts'; +const CANONICAL_SPEC = 'packages/kn-next/src/__tests__/doctor-write-policy-canonical.test.ts'; + +const ALERT_ROWS = [ + { + id: 'A1', + expect: 'red', + claim: 'the alert fires on a 1 (enforced) instead of a 0', + subject: 'rule', + anchor: ' expr: knext_operator_write_policy_enforced == 0', + replacement: ' expr: knext_operator_write_policy_enforced == 1', + }, + { + id: 'A2', + expect: 'red', + claim: 'the alert has no `for`, so a restart flaps it', + subject: 'rule', + anchor: ' expr: knext_operator_write_policy_enforced == 0\n for: 15m\n', + replacement: ' expr: knext_operator_write_policy_enforced == 0\n', + }, + { + id: 'A3', + expect: 'red', + claim: 'the alert pages at critical severity on every pre-1.30 cluster', + subject: 'rule', + anchor: + ' expr: knext_operator_write_policy_enforced == 0\n for: 15m\n labels:\n severity: warning', + replacement: + ' expr: knext_operator_write_policy_enforced == 0\n for: 15m\n labels:\n severity: critical', + }, + { + id: 'A4', + expect: 'red', + claim: + 'the companion alert no longer pairs the scraped target with the gauge (it would never fire, or always)', + subject: 'rule', + anchor: 'unless on (namespace, pod) knext_operator_write_policy_enforced\n for: 15m', + replacement: + 'unless on (namespace, pod) knext_operator_write_policy_enforced_typo\n for: 15m', + }, + { + id: 'A5', + expect: 'red', + claim: + 'the companion alert selects a job the operator is not scraped under (silent in exactly the case it exists for)', + subject: 'rule', + anchor: 'up{job=~".*controller-manager-metrics-service"}', + replacement: 'up{job=~"knext-nextapp"}', + }, + { + id: 'A6', + expect: 'red', + claim: 'the companion alert has no `for`, so a scrape gap at start-up pages', + subject: 'rule', + anchor: 'unless on (namespace, pod) knext_operator_write_policy_enforced\n for: 15m\n', + replacement: 'unless on (namespace, pod) knext_operator_write_policy_enforced\n', + }, +]; + +const DOCTOR_ROWS = [ + { + id: 'D1', + expect: 'red', + claim: 'doctor accepts a binding that does not Deny', + subject: 'doctor', + anchor: ' if (!(binding.spec?.validationActions ?? []).includes("Deny")) {', + replacement: ' if (false) {', + }, + { + id: 'D2', + expect: 'red', + claim: "doctor does not compare the policy's username with the operator's", + subject: 'doctor', + anchor: ' if (confined !== operatorUsername) {', + replacement: ' if (false) {', + }, + { + id: 'D3', + expect: 'red', + claim: 'doctor does not compare the server version with the 1.30 floor', + subject: 'doctor', + anchor: 'version.minor < FLOOR.minor', + replacement: 'version.minor < 0', + }, + { + id: 'D4', + expect: 'red', + claim: 'doctor reports PASS whatever the verdict is (forced to true)', + subject: 'doctor', + anchor: ' if (v.enforced) {', + replacement: ' if (true) {', + }, + { + id: 'D5', + expect: 'red', + claim: 'doctor ignores the policy name the operator was configured with', + subject: 'doctor', + anchor: 'container?.env?.find((e) => e.name === "WRITE_POLICY_NAME")?.value ||', + replacement: 'undefined ||', + }, + { + id: 'D6', + expect: 'red', + claim: 'doctor reports an RBAC-denied read as an error, not as "cannot determine"', + subject: 'doctor', + anchor: ' if (cls === "forbidden") {', + replacement: ' if (false) {', + }, + { + id: 'D7', + expect: 'red', + claim: 'doctor does not treat a not-found policy as a missing policy', + subject: 'doctor', + anchor: ' if (cls === "not-found") continue;', + replacement: ' if (false) continue;', + }, + { + id: 'D8', + expect: 'red', + claim: 'doctor treats a policy that fails open as fine', + subject: 'doctor', + anchor: ' if (policy.spec?.failurePolicy === "Ignore") {', + replacement: ' if (false) {', + }, + { + id: 'D9', + expect: 'red', + claim: 'doctor accepts an excludeResourceRules entry that carves resources out of the policy', + subject: 'doctor', + anchor: ' if ((policy.spec.matchConstraints.excludeResourceRules ?? []).length > 0) {', + replacement: ' if (false) {', + }, + { + id: 'D10', + expect: 'red', + claim: 'doctor accepts a covering rule that names objects (resourceNames)', + subject: 'doctor', + anchor: '(r.resourceNames ?? []).length === 0 &&', + replacement: 'true &&', + }, + { + id: 'D11', + expect: 'red', + claim: 'doctor accepts a cluster-scoped rule as covering namespaced Secrets and ConfigMaps', + subject: 'doctor', + anchor: ' (r.scope === undefined ||', + replacement: ' (true ||', + }, + { + id: 'D12', + expect: 'red', + claim: 'doctor accepts a rule for another apiVersion as covering core v1', + subject: 'doctor', + anchor: 'has(r.apiVersions, "v1") &&', + replacement: 'true &&', + }, + { + id: 'D13', + expect: 'red', + claim: + 'doctor never compares the policy body with the shipped CEL (validations: [true] passes)', + subject: 'doctor', + anchor: ' if (!sameCEL(policy)) {', + replacement: ' if (false) {', + }, + { + id: 'D17', + expect: 'red', + claim: + 'doctor accepts a policy that declares paramKind (a paramRef that finds nothing then runs no validation)', + subject: 'doctor', + anchor: ' if (policy.spec?.paramKind != null) {', + replacement: ' if (false) {', + }, + { + id: 'D18', + expect: 'red', + claim: + 'doctor accepts a binding that sets paramRef (a missing parameter makes the apiserver skip every validation)', + subject: 'doctor', + anchor: ' if (binding.spec?.paramRef != null) {', + replacement: ' if (false) {', + }, +]; + +const CANONICAL_ROWS = [ + { + id: 'D14', + expect: 'red', + claim: 'doctor SKIPs, instead of WARNing, on a read failure it cannot classify', + subject: 'doctor', + anchor: ' "warn",\n `could not read the operator\'s write policy', + replacement: + ' "skip",\n `could not read the operator\'s write policy', + }, + { + id: 'D15', + expect: 'red', + claim: 'doctor SKIPs, instead of WARNing, on unparseable policy output', + subject: 'doctor', + anchor: + ' "warn",\n `the operator\'s write policy ${name} returned unreadable output', + replacement: + ' "skip",\n `the operator\'s write policy ${name} returned unreadable output', + }, + { + id: 'D16', + expect: 'red', + claim: "doctor's canonical CEL copy drifts from the shipped policy", + subject: 'doctor', + anchor: '{ name: "allowedSecretType", expression: "\'Opaque\'" },', + replacement: '{ name: "allowedSecretType", expression: "\'Opaque2\'" },', + }, +]; + +// Real mutations of the real subjects, so each also proves its runner reads them. +const ALERT_CANARY = { + subject: 'rule', + anchor: ' expr: knext_operator_write_policy_enforced == 0', + replacement: ' expr: knext_operator_write_policy_enforced == 1', +}; +const CANONICAL_CANARY = { + subject: 'doctor', + anchor: '{ name: "allowedSecretType", expression: "\'Opaque\'" },', + replacement: '{ name: "allowedSecretType", expression: "\'Opaque3\'" },', +}; +const DOCTOR_CANARY = { + subject: 'doctor', + anchor: ' if (v.enforced) {', + replacement: ' if (true) {', +}; + +const alert = createGuardProver({ + repoRoot: REPO_ROOT, + spec: ALERT_SPEC, + subjects: { rule: RULE }, +}); +const doctor = createGuardProver({ + repoRoot: REPO_ROOT, + spec: DOCTOR_SPEC, + subjects: { doctor: DOCTOR }, +}); + +const canonical = createGuardProver({ + repoRoot: REPO_ROOT, + spec: CANONICAL_SPEC, + subjects: { doctor: DOCTOR }, +}); + +console.log('=== mutation proof: the write-policy alert and the doctor mirror (#2500) ==='); +alert.preflight([...ALERT_ROWS, { id: 'canary', ...ALERT_CANARY }]); +doctor.preflight([...DOCTOR_ROWS, { id: 'canary', ...DOCTOR_CANARY }]); +canonical.preflight([...CANONICAL_ROWS, { id: 'canary', ...CANONICAL_CANARY }]); +declareMutations(ALERT_ROWS.length + DOCTOR_ROWS.length + CANONICAL_ROWS.length); + +console.log(`\n=== ${ALERT_SPEC} ===`); +alert.baseline(); +alert.proveCanSeeRed(ALERT_CANARY); +for (const m of ALERT_ROWS) { + alert.run(m); + recordMutation(); +} +alert.finish(ALERT_ROWS.length); + +console.log(`\n=== ${DOCTOR_SPEC} ===`); +doctor.baseline(); +doctor.proveCanSeeRed(DOCTOR_CANARY); +for (const m of DOCTOR_ROWS) { + doctor.run(m); + recordMutation(); +} +doctor.finish(DOCTOR_ROWS.length); + +console.log(`\n=== ${CANONICAL_SPEC} ===`); +canonical.baseline(); +canonical.proveCanSeeRed(CANONICAL_CANARY); +for (const m of CANONICAL_ROWS) { + canonical.run(m); + recordMutation(); +} +canonical.finish(CANONICAL_ROWS.length);