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 and join Job pods as well as on
+// instance pods, but only instance pods carry injected sidecars. Presence is
+// therefore scoped by cnpg.io/podRole=instance so a lingering bootstrap Job pod
+// is not mistaken for an instance that lost a 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 {
@@ -274,3 +283,66 @@ func AssertInjectedSidecarsPSARestricted(ctx context.Context, c client.Client, n
 		return nil
 	}
 }
+
+// AssertSidecarsInjected returns a checker that succeeds when every sidecar in
+// names is present on every instance pod backing clusterName in ns.
+//
+// This is deliberately separate from AssertInjectedSidecarsPSARestricted. The
+// two fail for different reasons: a missing collector is a plumbing problem,
+// a bad SecurityContext is the #387 problem, and a caller that sees one error
+// should not have to work out which it got. It also gives the monitoring-on
+// path a check that cannot pass vacuously: documentdb-gateway is injected
+// unconditionally, so a PSA check alone still succeeds on a cluster whose
+// otel-collector never got injected at all.
+//
+// names must not be empty, and every name must be a CNPG-I-injected sidecar;
+// either is a bug in the calling spec and fails immediately rather than
+// spinning until the Eventually timeout.
+func AssertSidecarsInjected(ctx context.Context, c client.Client, ns, clusterName string, names ...string) func() error {
+	var argErr error
+	switch {
+	case len(names) == 0:
+		argErr = fmt.Errorf("AssertSidecarsInjected requires at least one sidecar name")
+	default:
+		for _, name := range names {
+			if !injectedSidecarNames[name] {
+				argErr = fmt.Errorf("sidecar %q is not a CNPG-I-injected sidecar", name)
+				break
+			}
+		}
+	}
+	return func() error {
+		if argErr != nil {
+			return argErr
+		}
+		var pods corev1.PodList
+		if err := c.List(ctx, &pods,
+			client.InNamespace(ns),
+			client.MatchingLabels{"cnpg.io/cluster": clusterName}); err != nil {
+			return fmt.Errorf("list pods for cluster %s/%s: %w", ns, clusterName, err)
+		}
+		instancePods := 0
+		for i := range pods.Items {
+			pod := &pods.Items[i]
+			if pod.Labels[cnpgPodRoleLabel] != cnpgPodRoleInstance {
+				continue
+			}
+			instancePods++
+			present := make(map[string]bool, len(pod.Spec.Containers))
+			for j := range pod.Spec.Containers {
+				present[pod.Spec.Containers[j].Name] = true
+			}
+			for _, name := range names {
+				if !present[name] {
+					return fmt.Errorf("instance pod %s is missing injected sidecar %q",
+						pod.Name, name)
+				}
+			}
+		}
+		if instancePods == 0 {
+			return fmt.Errorf("no instance pods (%s=%s) found for cluster %s/%s",
+				cnpgPodRoleLabel, cnpgPodRoleInstance, ns, clusterName)
+		}
+		return nil
+	}
+}

test/e2e/pkg/e2eutils/assertions/assertions_test.go

@@ -159,3 +159,155 @@ 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 assertions
+// select 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 or 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()},
+		corev1.Container{Name: "otel-collector", SecurityContext: psaRestrictedSC()},
+	)
+	// Gateway without a securityContext at all — the #387 regression.
+	bare := clusterPod("bare-0", "bare",
+		corev1.Container{Name: "documentdb-gateway"},
+	)
+	rootSC := psaRestrictedSC()
+	rootSC.RunAsNonRoot = nil
+	asRoot := clusterPod("root-0", "root",
+		corev1.Container{Name: "documentdb-gateway", SecurityContext: rootSC},
+	)
+	noSeccomp := psaRestrictedSC()
+	noSeccomp.SeccompProfile = nil
+	badOtel := clusterPod("badotel-0", "badotel",
+		corev1.Container{Name: "documentdb-gateway", SecurityContext: psaRestrictedSC()},
+		corev1.Container{Name: "otel-collector", SecurityContext: noSeccomp},
+	)
+	noSidecar := clusterPod("none-0", "none",
+		corev1.Container{Name: "postgres"},
+	)
+
+	c := fake.NewClientBuilder().WithScheme(s).
+		WithObjects(compliant, bare, asRoot, badOtel, 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", "badotel")(); err == nil ||
+		!strings.Contains(err.Error(), "seccompProfile") {
+		t.Fatalf("want otel seccomp 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 TestAssertSidecarsInjected(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 a PSA check alone still passes.
+	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()},
+	)
+	// A bootstrap Job pod carries cnpg.io/cluster but no injected sidecars.
+	bootstrap := jobPod("full-1-initdb", "full",
+		corev1.Container{Name: "bootstrap-controller"},
+	)
+	// A cluster whose only pod is a bootstrap Job: nothing to check yet.
+	onlyJob := jobPod("early-1-initdb", "early",
+		corev1.Container{Name: "bootstrap-controller"},
+	)
+
+	c := fake.NewClientBuilder().WithScheme(s).
+		WithObjects(noOtel, withOtel, bootstrap, onlyJob).Build()
+	ctx := context.Background()
+
+	// The case the PSA checker cannot catch on its own.
+	if err := AssertInjectedSidecarsPSARestricted(ctx, c, "ns", "mon")(); err != nil {
+		t.Fatalf("PSA check passes on a gateway-only cluster, as designed: %v", err)
+	}
+	if err := AssertSidecarsInjected(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 := AssertSidecarsInjected(ctx, c, "ns", "full", "otel-collector")(); err != nil {
+		t.Fatalf("otel present: %v", err)
+	}
+	if err := AssertSidecarsInjected(ctx, c, "ns", "full",
+		"documentdb-gateway", "otel-collector")(); err != nil {
+		t.Fatalf("both sidecars present: %v", err)
+	}
+	// The Job pod above shares the cluster label and has no sidecars; it must
+	// not be read as an instance that lost its collector.
+	if err := AssertSidecarsInjected(ctx, c, "ns", "early", "otel-collector")(); err == nil ||
+		!strings.Contains(err.Error(), "no instance pods") {
+		t.Fatalf("want no-instance-pods error, got %v", err)
+	}
+
+	// Argument bugs fail immediately rather than spinning in Eventually.
+	if err := AssertSidecarsInjected(ctx, c, "ns", "full")(); err == nil ||
+		!strings.Contains(err.Error(), "at least one sidecar name") {
+		t.Fatalf("want empty-names error, got %v", err)
+	}
+	if err := AssertSidecarsInjected(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)
+	}
+}

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,27 @@ 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. Two separate assertions because they fail for
+				// different reasons: a missing collector is a plumbing problem,
+				// a bad securityContext is the #387 problem.
+				Eventually(assertions.AssertSidecarsInjected(
+					ctx, c, cr.Namespace, cr.Name, otelContainerName),
+					timeouts.For(timeouts.DocumentDBReady),
+					timeouts.PollInterval(timeouts.DocumentDBReady),
+				).Should(Succeed(), "monitoring-on cluster must have the otel-collector injected")
+
+				// The fixture labels the namespace restricted, so reaching
+				// healthy already proves the sidecars passed admission; the
+				// explicit field checks name the offending field instead of
+				// leaving an opaque pod-creation failure.
+				Eventually(assertions.AssertInjectedSidecarsPSARestricted(
+					ctx, c, cr.Namespace, cr.Name),
+					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",