Skip to content

Commit db1f5fa

Browse files
feat(controller): restricted securityContext on backup/restore Jobs
Final piece of #9: the backup CronJob and the restore-assembly Job each have a valkey-image container and an aws-cli container, and had no securityContext, so they were rejected under the restricted Pod Security Standard. Both pods now get the restricted pod/container contexts (user override, else the default). The aws-cli image runs as root by default, so the pod runs every container as the non-root uid 1000 and points HOME at /tmp (aws-cli writes its CLI cache under $HOME, which is otherwise unwritable for a non-root uid). Also fixes a stale comment on configHashFromData: the rendered valkey.conf carries requirepass, so the auth password DOES affect the config hash (which is why an existingSecret change rolls the pods). Closes #9.
1 parent c265c62 commit db1f5fa

4 files changed

Lines changed: 82 additions & 19 deletions

File tree

internal/controller/backup.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ echo "ok"
263263
},
264264
},
265265
{Name: envAWSDefaultRegion, Value: s3.Region},
266+
// aws-cli writes its CLI cache under $HOME; running non-root under the
267+
// restricted PSA, the default HOME may be unwritable, so point it at tmpfs.
268+
{Name: "HOME", Value: "/tmp"},
266269
}
267270
if s3.Endpoint != "" {
268271
awsEnv = append(awsEnv, corev1.EnvVar{Name: envS3EndpointURL, Value: s3.Endpoint})
@@ -296,11 +299,12 @@ echo "ok"
296299
Spec: corev1.PodSpec{
297300
RestartPolicy: corev1.RestartPolicyOnFailure,
298301
InitContainers: []corev1.Container{{
299-
Name: "dump",
300-
Image: dumpImage,
301-
Command: []string{shellCmd, "-c", dumpScript},
302-
Env: env,
303-
VolumeMounts: mounts,
302+
Name: "dump",
303+
Image: dumpImage,
304+
Command: []string{shellCmd, "-c", dumpScript},
305+
Env: env,
306+
VolumeMounts: mounts,
307+
SecurityContext: containerSecurityContext(vc),
304308
}},
305309
Containers: []corev1.Container{{
306310
Name: "upload",
@@ -310,8 +314,10 @@ echo "ok"
310314
VolumeMounts: []corev1.VolumeMount{
311315
{Name: backupVolumeName, MountPath: "/backup"},
312316
},
317+
SecurityContext: containerSecurityContext(vc),
313318
}},
314-
Volumes: volumes,
319+
Volumes: volumes,
320+
SecurityContext: podSecurityContext(vc),
315321
},
316322
},
317323
},

internal/controller/resources.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,9 @@ func tlsSecretName(vc *cachev1beta1.ValkeyCluster) string {
177177

178178
// configHashFromData returns a deterministic short hash of a ConfigMap.Data map,
179179
// used to roll the StatefulSet pods whenever valkey.conf or entrypoint.sh changes.
180-
// The auth password is intentionally NOT included — it lives in a Secret and is
181-
// loaded at process start via env/mount; rotating it is handled separately.
180+
// The rendered valkey.conf carries requirepass, so the auth password DOES affect
181+
// this hash: a rotated existingSecret re-renders the config and rolls the pods
182+
// (the auth.existingSecret watch + the init-script ACL re-seed apply the change).
182183
func configHashFromData(data map[string]string) string {
183184
keys := make([]string, 0, len(data))
184185
for k := range data {

internal/controller/resources_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,3 +1479,53 @@ func TestClusterJobsHaveRestrictedSecurityContext(t *testing.T) {
14791479
})
14801480
}
14811481
}
1482+
1483+
func TestBackupAndRestoreJobsHaveRestrictedSecurityContext(t *testing.T) {
1484+
bvc := minimalCR()
1485+
bvc.Spec.Backup = &cachev1beta1.BackupSpec{
1486+
Enabled: true,
1487+
Schedule: "0 0 * * *",
1488+
S3: &cachev1beta1.S3Spec{Bucket: "b", Region: "r", CredentialsSecret: "creds"},
1489+
}
1490+
backupPod := buildBackupCronJob(bvc, "b").Spec.JobTemplate.Spec.Template.Spec
1491+
1492+
shards := int32(3)
1493+
rvc := minimalCR()
1494+
rvc.Spec.Topology = cachev1beta1.TopologyCluster
1495+
rvc.Spec.Shards = &shards
1496+
rvc.Spec.RestoreFrom = &cachev1beta1.RestoreSpec{
1497+
SourceKey: "base-stamp-shard-{shard}.rdb",
1498+
S3: &cachev1beta1.S3Spec{Bucket: "bkt", Region: "r", CredentialsSecret: "c"},
1499+
}
1500+
restorePod := buildRestoreAssemblyJob(rvc, "pw", "r").Spec.Template.Spec
1501+
1502+
for name, spec := range map[string]corev1.PodSpec{"backup": backupPod, "restore": restorePod} {
1503+
t.Run(name, func(t *testing.T) {
1504+
if spec.SecurityContext == nil || spec.SecurityContext.RunAsNonRoot == nil || !*spec.SecurityContext.RunAsNonRoot {
1505+
t.Errorf("%s pod must have a restricted pod securityContext", name)
1506+
}
1507+
for _, c := range spec.InitContainers {
1508+
assertContainerRestricted(t, name+"/init/"+c.Name, c.SecurityContext)
1509+
}
1510+
for _, c := range spec.Containers {
1511+
assertContainerRestricted(t, name+"/"+c.Name, c.SecurityContext)
1512+
}
1513+
})
1514+
}
1515+
1516+
// The aws-cli containers run non-root, so they need a writable HOME.
1517+
hasHome := func(c corev1.Container) bool {
1518+
for _, e := range c.Env {
1519+
if e.Name == "HOME" && e.Value == "/tmp" {
1520+
return true
1521+
}
1522+
}
1523+
return false
1524+
}
1525+
if !hasHome(backupPod.Containers[0]) {
1526+
t.Error("backup aws-cli (upload) container must set HOME=/tmp for non-root")
1527+
}
1528+
if !hasHome(restorePod.InitContainers[0]) {
1529+
t.Error("restore aws-cli (fetch-manifest) container must set HOME=/tmp for non-root")
1530+
}
1531+
}

internal/controller/restore_assembly.go

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,9 @@ echo "manifest:"; cat /work/manifest.txt
252252
{Name: envAWSAccessKey, ValueFrom: secretRef(rs.S3.CredentialsSecret, envAWSAccessKey)},
253253
{Name: envAWSSecretKey, ValueFrom: secretRef(rs.S3.CredentialsSecret, envAWSSecretKey)},
254254
{Name: envAWSDefaultRegion, Value: rs.S3.Region},
255+
// aws-cli writes its CLI cache under $HOME; running non-root under the
256+
// restricted PSA, the default HOME may be unwritable, so point it at tmpfs.
257+
{Name: "HOME", Value: "/tmp"},
255258
}
256259
if rs.S3.Endpoint != "" {
257260
awsEnv = append(awsEnv, corev1.EnvVar{Name: envS3EndpointURL, Value: rs.S3.Endpoint})
@@ -281,20 +284,23 @@ echo "manifest:"; cat /work/manifest.txt
281284
Spec: corev1.PodSpec{
282285
RestartPolicy: corev1.RestartPolicyOnFailure,
283286
InitContainers: []corev1.Container{{
284-
Name: "fetch-manifest",
285-
Image: awsImage,
286-
Command: []string{shellCmd, "-c", fetchScript},
287-
Env: awsEnv,
288-
VolumeMounts: []corev1.VolumeMount{{Name: workVolumeName, MountPath: "/work"}},
287+
Name: "fetch-manifest",
288+
Image: awsImage,
289+
Command: []string{shellCmd, "-c", fetchScript},
290+
Env: awsEnv,
291+
VolumeMounts: []corev1.VolumeMount{{Name: workVolumeName, MountPath: "/work"}},
292+
SecurityContext: containerSecurityContext(vc),
289293
}},
290294
Containers: []corev1.Container{{
291-
Name: "assemble",
292-
Image: vc.Spec.Image,
293-
Command: []string{shellCmd, "-c", script},
294-
Env: valkeyEnv,
295-
VolumeMounts: mounts,
295+
Name: "assemble",
296+
Image: vc.Spec.Image,
297+
Command: []string{shellCmd, "-c", script},
298+
Env: valkeyEnv,
299+
VolumeMounts: mounts,
300+
SecurityContext: containerSecurityContext(vc),
296301
}},
297-
Volumes: volumes,
302+
Volumes: volumes,
303+
SecurityContext: podSecurityContext(vc),
298304
},
299305
},
300306
},

0 commit comments

Comments
 (0)