From 78d9e25af607840c9252af0f10395c96e96ff7c7 Mon Sep 17 00:00:00 2001 From: Brent Graveland Date: Fri, 4 Sep 2026 09:28:03 -0600 Subject: [PATCH] fix(shard): apply default runtime identity to any pool image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit effectivePgctldIdentity supplied the numeric UID/GID only when the image string was exactly DefaultPostgresImage, so overriding spec.images.postgres left containers carrying RunAsNonRoot with no RunAsUser. pgctld declares USER postgres by name, so the kubelet cannot prove the user is not root and every pool pod fails: CreateContainerConfigError: container has runAsNonRoot and image has non-numeric user (postgres) Reproduced with ghcr.io/multigres/pgctld:main, which differs from the pinned sha- tag by reference alone: the pool never converges and the e2e suite times out. The constant's own comment already gave the reason — pgctld declares USER by name — and that holds for every pgctld build rather than one tag. buildPostgresExporterContainer already supplies its identity unconditionally. effectiveMultipoolerIdentity carried the same gate, latent only because the multigres image uses numeric USER 65532. Leaving it would break the invariant validatePoolRuntimeIdentity enforces, that postgres and multipooler share a UID to reach PGDATA. An image that runs as a different numeric user must now set pool.Postgres.RunAsUser rather than inheriting its identity implicitly. Closes #466 Signed-off-by: Brent Graveland --- .../controller/shard/containers.go | 4 +- .../controller/shard/containers_test.go | 9 +++- .../controller/shard/pool_pod.go | 45 +++++++------------ .../controller/shard/pool_pod_test.go | 25 ++++++++--- 4 files changed, 45 insertions(+), 38 deletions(-) diff --git a/pkg/resource-handler/controller/shard/containers.go b/pkg/resource-handler/controller/shard/containers.go index 9401c134..9d884c60 100644 --- a/pkg/resource-handler/controller/shard/containers.go +++ b/pkg/resource-handler/controller/shard/containers.go @@ -432,7 +432,7 @@ func buildPgctldSidecar( Resources: pool.Postgres.Resources, RestartPolicy: &sidecarRestartPolicy, Env: env, - SecurityContext: buildPgctldSecurityContext(image, pool.Postgres), + SecurityContext: buildPgctldSecurityContext(pool.Postgres), VolumeMounts: volumeMounts, StartupProbe: &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ @@ -591,7 +591,7 @@ func buildMultipoolerContainer( Args: args, Ports: buildMultipoolerContainerPorts(), Resources: pool.Multipooler.Resources, - SecurityContext: buildMultipoolerSecurityContext(shard, pool), + SecurityContext: buildMultipoolerSecurityContext(pool), StartupProbe: &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ diff --git a/pkg/resource-handler/controller/shard/containers_test.go b/pkg/resource-handler/controller/shard/containers_test.go index 57bf0984..56b9c6c4 100644 --- a/pkg/resource-handler/controller/shard/containers_test.go +++ b/pkg/resource-handler/controller/shard/containers_test.go @@ -188,6 +188,8 @@ func TestBuildMultipoolerContainer(t *testing.T) { Resources: corev1.ResourceRequirements{}, SecurityContext: &corev1.SecurityContext{ RunAsNonRoot: ptr.To(true), + RunAsUser: ptr.To(DefaultMultipoolerUID), + RunAsGroup: ptr.To(DefaultMultipoolerGID), }, StartupProbe: &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ @@ -852,8 +854,11 @@ func TestBuildPgctldSidecar(t *testing.T) { if c.Image != "custom/pgctld:v1" { t.Errorf("Image = %q, want %q", c.Image, "custom/pgctld:v1") } - assert.Nil(t, c.SecurityContext.RunAsUser) - assert.Nil(t, c.SecurityContext.RunAsGroup) + // The numeric identity must not depend on the image reference: pgctld + // declares USER postgres by name, so without it RunAsNonRoot makes the + // kubelet reject the container regardless of which tag is in use. + assert.Equal(t, ptr.To(DefaultPostgresUID), c.SecurityContext.RunAsUser) + assert.Equal(t, ptr.To(DefaultPostgresGID), c.SecurityContext.RunAsGroup) }) t.Run("with observability", func(t *testing.T) { diff --git a/pkg/resource-handler/controller/shard/pool_pod.go b/pkg/resource-handler/controller/shard/pool_pod.go index ff9c4b9b..91935e71 100644 --- a/pkg/resource-handler/controller/shard/pool_pod.go +++ b/pkg/resource-handler/controller/shard/pool_pod.go @@ -153,8 +153,10 @@ func buildPoolPodSecurityContext(poolSpec multigresv1alpha1.PoolSpec) *corev1.Po const ( // DefaultPostgresUID and DefaultPostgresGID identify the postgres user in - // the default pgctld image. That image declares USER postgres by name, so - // Kubernetes needs the numeric identity to enforce RunAsNonRoot. + // the pgctld image. That image declares USER postgres by name, so + // Kubernetes needs the numeric identity to enforce RunAsNonRoot. This is a + // property of every pgctld build, not of any one tag, so it is applied + // whenever the spec leaves the identity unset — see effectivePgctldIdentity. DefaultPostgresUID int64 = 999 DefaultPostgresGID int64 = 999 @@ -187,32 +189,27 @@ func buildContainerSecurityContext(runAsUser, runAsGroup *int64) *corev1.Securit } func buildPgctldSecurityContext( - image string, config multigresv1alpha1.ContainerConfig, ) *corev1.SecurityContext { - runAsUser, runAsGroup := effectivePgctldIdentity(image, config) + runAsUser, runAsGroup := effectivePgctldIdentity(config) return buildContainerSecurityContext(runAsUser, runAsGroup) } func effectivePgctldIdentity( - image string, config multigresv1alpha1.ContainerConfig, ) (runAsUser, runAsGroup *int64) { runAsUser = config.RunAsUser runAsGroup = config.RunAsGroup - if image == multigresv1alpha1.DefaultPostgresImage { - if runAsUser == nil { - runAsUser = ptr.To(DefaultPostgresUID) - } - if runAsGroup == nil { - runAsGroup = ptr.To(DefaultPostgresGID) - } + if runAsUser == nil { + runAsUser = ptr.To(DefaultPostgresUID) + } + if runAsGroup == nil { + runAsGroup = ptr.To(DefaultPostgresGID) } return runAsUser, runAsGroup } func effectiveMultipoolerIdentity( - shard *multigresv1alpha1.Shard, pool multigresv1alpha1.PoolSpec, ) (runAsUser, runAsGroup *int64) { runAsUser = pool.Multipooler.RunAsUser @@ -220,22 +217,19 @@ func effectiveMultipoolerIdentity( if runAsUser == nil && pool.Postgres.RunAsUser != nil { runAsUser = ptr.To(*pool.Postgres.RunAsUser) } - if resolvedMultipoolerImage(shard) == multigresv1alpha1.DefaultMultipoolerImage { - if runAsUser == nil { - runAsUser = ptr.To(DefaultMultipoolerUID) - } - if runAsGroup == nil { - runAsGroup = ptr.To(DefaultMultipoolerGID) - } + if runAsUser == nil { + runAsUser = ptr.To(DefaultMultipoolerUID) + } + if runAsGroup == nil { + runAsGroup = ptr.To(DefaultMultipoolerGID) } return runAsUser, runAsGroup } func buildMultipoolerSecurityContext( - shard *multigresv1alpha1.Shard, pool multigresv1alpha1.PoolSpec, ) *corev1.SecurityContext { - runAsUser, runAsGroup := effectiveMultipoolerIdentity(shard, pool) + runAsUser, runAsGroup := effectiveMultipoolerIdentity(pool) return buildContainerSecurityContext(runAsUser, runAsGroup) } @@ -259,13 +253,6 @@ func validatePoolRuntimeIdentity(pool multigresv1alpha1.PoolSpec) error { return nil } -func resolvedMultipoolerImage(shard *multigresv1alpha1.Shard) string { - if shard.Spec.Images.Multipooler != "" { - return string(shard.Spec.Images.Multipooler) - } - return multigresv1alpha1.DefaultMultipoolerImage -} - // buildHeadlessServiceName constructs the headless service name for DNS // resolution. Matches the naming used by pool_service.go. func buildHeadlessServiceName(shard *multigresv1alpha1.Shard, poolName, cellName string) string { diff --git a/pkg/resource-handler/controller/shard/pool_pod_test.go b/pkg/resource-handler/controller/shard/pool_pod_test.go index 70a1e355..2d8ce9f4 100644 --- a/pkg/resource-handler/controller/shard/pool_pod_test.go +++ b/pkg/resource-handler/controller/shard/pool_pod_test.go @@ -468,10 +468,10 @@ func TestBuildPoolPod_SecurityContext(t *testing.T) { } func TestBuildPoolPod_FSGroupDoesNotOverrideContainerRuntimeIdentity(t *testing.T) { - t.Run("custom images retain their identity", func(t *testing.T) { + t.Run("custom images get the default identity", func(t *testing.T) { shard := newTestShard() - shard.Spec.Images.Postgres = "example/pgctld:user-1000-group-1001" - shard.Spec.Images.Multipooler = "example/multipooler:user-1000-group-1001" + shard.Spec.Images.Postgres = "example/pgctld:custom" + shard.Spec.Images.Multipooler = "example/multipooler:custom" pool := newTestPoolSpec() pool.FSGroup = ptr.To(int64(2000)) @@ -480,9 +480,24 @@ func TestBuildPoolPod_FSGroupDoesNotOverrideContainerRuntimeIdentity(t *testing. t.Fatalf("unexpected error: %v", err) } + // Overriding the image must not drop the numeric identity. pgctld + // declares USER postgres by name, so leaving RunAsUser unset pairs + // RunAsNonRoot with a username the kubelet cannot resolve, and every + // pod fails CreateContainerConfigError. An image that genuinely runs + // as something else sets RunAsUser explicitly — see the case below. assertPoolPodFSGroup(t, pod, 2000) - assertContainerIdentity(t, pod.Spec.InitContainers[0], nil, nil) - assertContainerIdentity(t, pod.Spec.Containers[0], nil, nil) + assertContainerIdentity( + t, + pod.Spec.InitContainers[0], + ptr.To(DefaultPostgresUID), + ptr.To(DefaultPostgresGID), + ) + assertContainerIdentity( + t, + pod.Spec.Containers[0], + ptr.To(DefaultMultipoolerUID), + ptr.To(DefaultMultipoolerGID), + ) assertContainerIdentity( t, pod.Spec.Containers[1],