diff --git a/test/e2e/pkg/e2eutils/assertions/assertions.go b/test/e2e/pkg/e2eutils/assertions/assertions.go index e56d39e2c..7df7e05dd 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 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 + } +} diff --git a/test/e2e/pkg/e2eutils/assertions/assertions_test.go b/test/e2e/pkg/e2eutils/assertions/assertions_test.go index 19fd2a75a..bff40f2ef 100644 --- a/test/e2e/pkg/e2eutils/assertions/assertions_test.go +++ b/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) + } +} 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..18aae42b0 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,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",