diff --git a/api/v1alpha1/common_types.go b/api/v1alpha1/common_types.go index 7f5d4370..d815d26e 100644 --- a/api/v1alpha1/common_types.go +++ b/api/v1alpha1/common_types.go @@ -199,16 +199,21 @@ type ContainerConfig struct { // +optional Resources corev1.ResourceRequirements `json:"resources,omitempty"` - // RunAsUser sets the UID for the container process. When unset, Kubernetes - // and the container runtime apply the image's default user. - // Images that declare USER by name may need this set to a numeric UID when - // RunAsNonRoot enforcement is required. + // RunAsUser sets the UID for the container process. For Postgres and + // Multipooler, leaving this unset does not fall back to the image's own + // user: the operator applies its own default numeric UID (shared between + // the two so both can reach PGDATA), because pgctld declares USER + // postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + // numeric UID. A custom image that runs as a different user must set + // this explicitly - including when reusing a PersistentVolume whose data + // is already owned by another UID, since a mismatch there fails the pod + // at startup rather than at scheduling. // +optional // +kubebuilder:validation:Minimum=1 RunAsUser *int64 `json:"runAsUser,omitempty"` - // RunAsGroup sets the primary GID for the container process. When unset, - // Kubernetes and the container runtime apply their default group handling. + // RunAsGroup sets the primary GID for the container process. Same + // operator-supplied default and same caveats as RunAsUser. // +optional // +kubebuilder:validation:Minimum=0 RunAsGroup *int64 `json:"runAsGroup,omitempty"` diff --git a/config/crd/bases/multigres.com_multigresclusters.yaml b/config/crd/bases/multigres.com_multigresclusters.yaml index 0eca7968..c0793953 100644 --- a/config/crd/bases/multigres.com_multigresclusters.yaml +++ b/config/crd/bases/multigres.com_multigresclusters.yaml @@ -5418,17 +5418,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer @@ -5499,17 +5504,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer @@ -7913,17 +7923,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer @@ -7994,17 +8009,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer diff --git a/config/crd/bases/multigres.com_shards.yaml b/config/crd/bases/multigres.com_shards.yaml index 7f5a5366..082859d1 100644 --- a/config/crd/bases/multigres.com_shards.yaml +++ b/config/crd/bases/multigres.com_shards.yaml @@ -2552,17 +2552,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer @@ -2632,17 +2637,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer diff --git a/config/crd/bases/multigres.com_shardtemplates.yaml b/config/crd/bases/multigres.com_shardtemplates.yaml index 95b75ea3..4637f5d1 100644 --- a/config/crd/bases/multigres.com_shardtemplates.yaml +++ b/config/crd/bases/multigres.com_shardtemplates.yaml @@ -2137,17 +2137,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer @@ -2217,17 +2222,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer diff --git a/config/crd/bases/multigres.com_tablegroups.yaml b/config/crd/bases/multigres.com_tablegroups.yaml index 3b7d05c7..4f99d3bb 100644 --- a/config/crd/bases/multigres.com_tablegroups.yaml +++ b/config/crd/bases/multigres.com_tablegroups.yaml @@ -2781,17 +2781,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer @@ -2862,17 +2867,22 @@ spec: type: object runAsGroup: description: |- - RunAsGroup sets the primary GID for the container process. When unset, - Kubernetes and the container runtime apply their default group handling. + RunAsGroup sets the primary GID for the container process. Same + operator-supplied default and same caveats as RunAsUser. format: int64 minimum: 0 type: integer runAsUser: description: |- - RunAsUser sets the UID for the container process. When unset, Kubernetes - and the container runtime apply the image's default user. - Images that declare USER by name may need this set to a numeric UID when - RunAsNonRoot enforcement is required. + RunAsUser sets the UID for the container process. For Postgres and + Multipooler, leaving this unset does not fall back to the image's own + user: the operator applies its own default numeric UID (shared between + the two so both can reach PGDATA), because pgctld declares USER + postgres by name and Kubernetes cannot enforce RunAsNonRoot without a + numeric UID. A custom image that runs as a different user must set + this explicitly - including when reusing a PersistentVolume whose data + is already owned by another UID, since a mismatch there fails the pod + at startup rather than at scheduling. format: int64 minimum: 1 type: integer diff --git a/pkg/resource-handler/controller/shard/containers.go b/pkg/resource-handler/controller/shard/containers.go index 2ad75eba..4c427609 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{ @@ -589,7 +589,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 ad3ba21d..10f006cd 100644 --- a/pkg/resource-handler/controller/shard/containers_test.go +++ b/pkg/resource-handler/controller/shard/containers_test.go @@ -184,6 +184,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{ @@ -855,8 +857,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 db937b42..7d290607 100644 --- a/pkg/resource-handler/controller/shard/pool_pod.go +++ b/pkg/resource-handler/controller/shard/pool_pod.go @@ -162,8 +162,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 @@ -196,32 +198,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 @@ -229,22 +226,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) } @@ -268,13 +262,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 d2c81419..976a8970 100644 --- a/pkg/resource-handler/controller/shard/pool_pod_test.go +++ b/pkg/resource-handler/controller/shard/pool_pod_test.go @@ -476,10 +476,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)) @@ -488,9 +488,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],