fix(e2e): stop the PSA sidecar guard passing when otel-collector is absent

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
 	}
 }

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)
+	}
+}

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),

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",