diff --git a/test/e2e/pkg/e2eutils/assertions/assertions.go b/test/e2e/pkg/e2eutils/assertions/assertions.go index e56d39e2c..de2d6a2a7 100644 --- a/test/e2e/pkg/e2eutils/assertions/assertions.go +++ b/test/e2e/pkg/e2eutils/assertions/assertions.go @@ -204,6 +204,15 @@ var injectedSidecarNames = map[string]bool{ "otel-collector": true, } +// CNPG stamps cnpg.io/cluster on its bootstrap/join Job pods as well as on +// instance pods, but only instance pods get the injected sidecars. Presence +// checks are therefore scoped by cnpg.io/podRole=instance so a lingering +// bootstrap Job pod is not mistaken for an instance missing its sidecar. +const ( + cnpgPodRoleLabel = "cnpg.io/podRole" + cnpgPodRoleInstance = "instance" +) + // checkPSARestricted returns an error if ctr lacks any SecurityContext field // required by the Kubernetes Pod Security Admission "restricted" profile. func checkPSARestricted(podName string, ctr corev1.Container) error { @@ -242,10 +251,32 @@ func containsCapability(caps []corev1.Capability, want corev1.Capability) bool { // restricted-labeled namespace already implies the pods passed admission; the // explicit field checks turn an otherwise opaque CNPG pod-creation failure into // a precise message. Regression guard for #387 — works for both freshly -// deployed and restored (recovery) clusters. It errors if no injected sidecar -// is found so it cannot pass vacuously. -func AssertInjectedSidecarsPSARestricted(ctx context.Context, c client.Client, ns, clusterName string) func() error { +// deployed and restored (recovery) clusters. +// +// requireSidecars names injected sidecars that must be present on every +// instance pod. documentdb-gateway is injected unconditionally, so without +// this a monitoring-on cluster whose otel-collector failed to inject still +// passes: the gateway alone satisfies the "found at least one sidecar" guard. +// Callers that enable monitoring should pass "otel-collector" so a silently +// missing collector fails instead of being reported as hardened. Passing a +// name that is not an injected sidecar is a spec bug and fails immediately. +func AssertInjectedSidecarsPSARestricted( + ctx context.Context, + c client.Client, + ns, clusterName string, + requireSidecars ...string, +) func() error { + var requireErr error + for _, name := range requireSidecars { + if !injectedSidecarNames[name] { + requireErr = fmt.Errorf("required sidecar %q is not a CNPG-I-injected sidecar", name) + break + } + } return func() error { + if requireErr != nil { + return requireErr + } var pods corev1.PodList if err := c.List(ctx, &pods, client.InNamespace(ns), @@ -256,21 +287,39 @@ func AssertInjectedSidecarsPSARestricted(ctx context.Context, c client.Client, n return fmt.Errorf("no pods found for cluster %s/%s", ns, clusterName) } matched := 0 + instancePods := 0 for i := range pods.Items { - for j := range pods.Items[i].Spec.Containers { - ctr := pods.Items[i].Spec.Containers[j] + pod := &pods.Items[i] + present := make(map[string]bool, len(pod.Spec.Containers)) + for j := range pod.Spec.Containers { + ctr := pod.Spec.Containers[j] if !injectedSidecarNames[ctr.Name] { continue } + present[ctr.Name] = true matched++ - if err := checkPSARestricted(pods.Items[i].Name, ctr); err != nil { + if err := checkPSARestricted(pod.Name, ctr); err != nil { return err } } + if pod.Labels[cnpgPodRoleLabel] != cnpgPodRoleInstance { + continue + } + instancePods++ + for _, name := range requireSidecars { + if !present[name] { + return fmt.Errorf("instance pod %s is missing required injected sidecar %q", + pod.Name, name) + } + } } if matched == 0 { return fmt.Errorf("no injected sidecar containers found on pods for cluster %s/%s", ns, clusterName) } + if len(requireSidecars) > 0 && instancePods == 0 { + return fmt.Errorf("no instance pods (%s=%s) found for cluster %s/%s to check required sidecars %v", + cnpgPodRoleLabel, cnpgPodRoleInstance, ns, clusterName, requireSidecars) + } return nil } } diff --git a/test/e2e/pkg/e2eutils/assertions/assertions_test.go b/test/e2e/pkg/e2eutils/assertions/assertions_test.go index 19fd2a75a..fb4ff1bc8 100644 --- a/test/e2e/pkg/e2eutils/assertions/assertions_test.go +++ b/test/e2e/pkg/e2eutils/assertions/assertions_test.go @@ -159,3 +159,192 @@ func TestAssertConnectionStringMatches(t *testing.T) { t.Fatalf("want regex compile error") } } + +// psaRestrictedSC returns a SecurityContext satisfying every field +// checkPSARestricted requires. +func psaRestrictedSC() *corev1.SecurityContext { + yes, no := true, false + return &corev1.SecurityContext{ + RunAsNonRoot: &yes, + AllowPrivilegeEscalation: &no, + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + SeccompProfile: &corev1.SeccompProfile{Type: corev1.SeccompProfileTypeRuntimeDefault}, + } +} + +// clusterPod builds a CNPG instance pod: the cluster label the assertion +// selects on, plus the podRole marking it an instance rather than a Job pod. +func clusterPod(name, cluster string, ctrs ...corev1.Container) *corev1.Pod { + p := jobPod(name, cluster, ctrs...) + p.Labels[cnpgPodRoleLabel] = cnpgPodRoleInstance + return p +} + +// jobPod builds a CNPG bootstrap/join Job pod. CNPG stamps cnpg.io/cluster on +// these too, but the sidecar injector does not touch them. +func jobPod(name, cluster string, ctrs ...corev1.Container) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "ns", + Labels: map[string]string{"cnpg.io/cluster": cluster}, + }, + Spec: corev1.PodSpec{Containers: ctrs}, + } +} + +func TestAssertInjectedSidecarsPSARestricted(t *testing.T) { + t.Parallel() + s := newScheme(t) + + compliant := clusterPod("ok-0", "ok", + corev1.Container{Name: "postgres"}, + corev1.Container{Name: "documentdb-gateway", SecurityContext: psaRestrictedSC()}, + ) + // Gateway without a securityContext at all — the #387 regression. + bare := clusterPod("bare-0", "bare", + corev1.Container{Name: "documentdb-gateway"}, + ) + // Gateway that runs as root. + rootSC := psaRestrictedSC() + rootSC.RunAsNonRoot = nil + asRoot := clusterPod("root-0", "root", + corev1.Container{Name: "documentdb-gateway", SecurityContext: rootSC}, + ) + // No injected sidecar on the pod at all. + noSidecar := clusterPod("none-0", "none", + corev1.Container{Name: "postgres"}, + ) + + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(compliant, bare, asRoot, noSidecar).Build() + ctx := context.Background() + + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "ok")(); err != nil { + t.Fatalf("compliant: %v", err) + } + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "bare")(); err == nil || + !strings.Contains(err.Error(), "no securityContext") { + t.Fatalf("want missing-securityContext error, got %v", err) + } + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "root")(); err == nil || + !strings.Contains(err.Error(), "runAsNonRoot") { + t.Fatalf("want runAsNonRoot error, got %v", err) + } + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "none")(); err == nil || + !strings.Contains(err.Error(), "no injected sidecar") { + t.Fatalf("want no-injected-sidecar error, got %v", err) + } + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "absent")(); err == nil || + !strings.Contains(err.Error(), "no pods found") { + t.Fatalf("want no-pods error, got %v", err) + } +} + +func TestAssertInjectedSidecarsPSARestrictedRequiredSidecars(t *testing.T) { + t.Parallel() + s := newScheme(t) + + // A monitoring-on cluster whose otel-collector never got injected. The + // always-present gateway is compliant, so the pod looks healthy. + noOtel := clusterPod("mon-0", "mon", + corev1.Container{Name: "postgres"}, + corev1.Container{Name: "documentdb-gateway", SecurityContext: psaRestrictedSC()}, + ) + withOtel := clusterPod("full-0", "full", + corev1.Container{Name: "postgres"}, + corev1.Container{Name: "documentdb-gateway", SecurityContext: psaRestrictedSC()}, + corev1.Container{Name: "otel-collector", SecurityContext: psaRestrictedSC()}, + ) + otelRoot := psaRestrictedSC() + otelRoot.SeccompProfile = nil + badOtel := clusterPod("badotel-0", "badotel", + corev1.Container{Name: "documentdb-gateway", SecurityContext: psaRestrictedSC()}, + corev1.Container{Name: "otel-collector", SecurityContext: otelRoot}, + ) + + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(noOtel, withOtel, badOtel).Build() + ctx := context.Background() + + // Without an explicit requirement the checker cannot tell "monitoring is + // off" from "monitoring is on but otel never got injected": the gateway + // satisfies it either way. That is correct for monitoring-off callers. + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "mon")(); err != nil { + t.Fatalf("gateway-only cluster: %v", err) + } + // Naming otel-collector as required turns the missing sidecar into a failure. + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "mon", "otel-collector")(); err == nil || + !strings.Contains(err.Error(), "otel-collector") { + t.Fatalf("want missing-otel error, got %v", err) + } + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "full", "otel-collector")(); err != nil { + t.Fatalf("otel present and compliant: %v", err) + } + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "badotel", "otel-collector")(); err == nil || + !strings.Contains(err.Error(), "seccompProfile") { + t.Fatalf("want otel seccomp error, got %v", err) + } + // A name that is not an injected sidecar is a spec bug; fail fast rather + // than spin in Eventually until the timeout. + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "full", "postgres")(); err == nil || + !strings.Contains(err.Error(), "not a CNPG-I-injected sidecar") { + t.Fatalf("want unknown-sidecar error, got %v", err) + } +} + +func TestAssertInjectedSidecarsPSARestrictedIgnoresJobPods(t *testing.T) { + t.Parallel() + s := newScheme(t) + + // CNPG labels its bootstrap/join Job pods with cnpg.io/cluster, but the + // sidecar injector never adds containers to them. A lingering Job pod must + // not read as an instance that lost its collector. + instance := clusterPod("jobs-1", "jobs", + corev1.Container{Name: "documentdb-gateway", SecurityContext: psaRestrictedSC()}, + corev1.Container{Name: "otel-collector", SecurityContext: psaRestrictedSC()}, + ) + bootstrap := jobPod("jobs-1-initdb", "jobs", + corev1.Container{Name: "bootstrap-controller"}, + ) + // A cluster whose only pod is a bootstrap Job: nothing to check yet, and + // requiring a sidecar must not pass vacuously. + onlyJob := jobPod("early-1-initdb", "early", + corev1.Container{Name: "bootstrap-controller"}, + ) + + c := fake.NewClientBuilder().WithScheme(s). + WithObjects(instance, bootstrap, onlyJob).Build() + ctx := context.Background() + + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "jobs", "otel-collector")(); err != nil { + t.Fatalf("job pod alongside a healthy instance: %v", err) + } + if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "early", "otel-collector")(); err == nil || + !strings.Contains(err.Error(), "no injected sidecar") { + t.Fatalf("want no-injected-sidecar error, got %v", err) + } +} + +func TestAssertInjectedSidecarsPSARestrictedNeedsAnInstancePod(t *testing.T) { + t.Parallel() + s := newScheme(t) + + // Sidecars present, but on a pod that is not labelled as an instance — + // the shape we would see if CNPG stopped stamping cnpg.io/podRole. The + // per-pod requirement would then match nothing, so without this guard + // requireSidecars would silently stop being enforced. + orphan := jobPod("orphan-0", "orphan", + corev1.Container{Name: "documentdb-gateway", SecurityContext: psaRestrictedSC()}, + ) + c := fake.NewClientBuilder().WithScheme(s).WithObjects(orphan).Build() + + err := AssertInjectedSidecarsPSARestricted(context.Background(), c, "ns", "orphan", "otel-collector")() + if err == nil || !strings.Contains(err.Error(), "no instance pods") { + t.Fatalf("want no-instance-pods error, got %v", err) + } + // With nothing required, the same cluster is still a normal pass. + if err := AssertInjectedSidecarsPSARestricted(context.Background(), c, "ns", "orphan")(); err != nil { + t.Fatalf("no requirement: %v", err) + } +} diff --git a/test/e2e/tests/lifecycle/deploy_test.go b/test/e2e/tests/lifecycle/deploy_test.go index 974b8653e..8538bc66f 100644 --- a/test/e2e/tests/lifecycle/deploy_test.go +++ b/test/e2e/tests/lifecycle/deploy_test.go @@ -105,7 +105,8 @@ var _ = Describe("DocumentDB lifecycle — deploy", // This spec deploys with monitoring off, so only the // always-injected documentdb-gateway sidecar is present; // the otel-collector sidecar (injected only when monitoring - // is enabled) is covered by the sidecar-injector unit test. + // is enabled) is covered end-to-end by the monitoring-on spec + // in tests/resources and by the sidecar-injector unit test. // The shared helper errors if no injected sidecar is found, // so this cannot pass vacuously. Eventually(assertions.AssertInjectedSidecarsPSARestricted(ctx, c, ns, name), diff --git a/test/e2e/tests/resources/sidecar_resources_test.go b/test/e2e/tests/resources/sidecar_resources_test.go index ec65e612f..d80881e6a 100644 --- a/test/e2e/tests/resources/sidecar_resources_test.go +++ b/test/e2e/tests/resources/sidecar_resources_test.go @@ -10,6 +10,8 @@ import ( corev1 "k8s.io/api/core/v1" "github.com/documentdb/documentdb-operator/test/e2e" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/assertions" + "github.com/documentdb/documentdb-operator/test/e2e/pkg/e2eutils/timeouts" ) // These specs validate the pod memory carve-out (sidecar resource isolation). @@ -103,6 +105,20 @@ var _ = Describe("Sidecar memory carve-out", pg := containerByName(pod, postgresContainerName) Expect(pg).ToNot(BeNil(), "postgres container present") assertGuaranteedMemory(pg, wantPostgresWithMon) + + // This is the suite's only monitoring-on cluster, so it is the + // only place the collector's PSA hardening (#387) can be checked + // end-to-end. The fixture labels the namespace restricted, so + // reaching healthy already proves the sidecar passed admission; + // the explicit field checks name the offending field instead of + // leaving an opaque pod-creation failure. otel-collector is + // named as required so a collector that silently fails to inject + // is a failure rather than a vacuous pass on the gateway alone. + Eventually(assertions.AssertInjectedSidecarsPSARestricted( + ctx, c, cr.Namespace, cr.Name, otelContainerName), + timeouts.For(timeouts.DocumentDBReady), + timeouts.PollInterval(timeouts.DocumentDBReady), + ).Should(Succeed(), "monitoring-on cluster pods must carry PSA-restricted securityContext") }) It("derives the envelope from per-container memory when the envelope is omitted",