Skip to content

Commit 7ed8677

Browse files
benjaminjbbenjaminjb
authored
Add fsGroupChangePolicy to pod (CrunchyData#3296)
* Add fsGroupChangePolicy to pod Issue [sc-14235] * Bump test behavior around fsGroupChangePolicy * bump test k8s 1.19=>1.20 for github k3d test action * specify 1.19 for kubernetes-api test action, alter tests to check for k8s version and pass with 1.19 (no fsGroupChangePolicy in check) and >=1.20 (fsGroupChangePolicy in check)
1 parent 436a2c2 commit 7ed8677

File tree

7 files changed

+260
-12
lines changed

7 files changed

+260
-12
lines changed

‎.github/workflows/test.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ jobs:
2525
strategy:
2626
fail-fast: false
2727
matrix:
28-
kubernetes: [default]
28+
kubernetes: ["1.19.2"] # TODO(benjaminjb)(issue sc-11672): bump to 1.20.2 or higher after we update controller-runtime
2929
steps:
3030
- uses: actions/checkout@v3
3131
- uses: actions/setup-go@v3
@@ -51,7 +51,7 @@ jobs:
5151
strategy:
5252
fail-fast: false
5353
matrix:
54-
kubernetes: [latest, v1.19]
54+
kubernetes: [latest, v1.20]
5555
steps:
5656
- uses: actions/checkout@v3
5757
- uses: actions/setup-go@v3

‎internal/controller/postgrescluster/pgadmin_test.go‎

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package postgrescluster
2121
import (
2222
"context"
2323
"io"
24+
"os"
2425
"testing"
2526

2627
"github.com/pkg/errors"
@@ -503,7 +504,11 @@ labels:
503504
postgres-operator.crunchydata.com/data: pgadmin
504505
postgres-operator.crunchydata.com/role: pgadmin
505506
`))
506-
assert.Assert(t, cmp.MarshalMatches(template.Spec, `
507+
508+
// TODO(benjaminjb)(issue sc-11672): after we update controller-runtime and
509+
// are no longer testing in Github actions with K8s 1.19.2, reduce the following comparison
510+
// to simply testing against a given, fixed string.
511+
compare := `
507512
automountServiceAccountToken: false
508513
containers: null
509514
dnsPolicy: ClusterFirst
@@ -512,9 +517,25 @@ restartPolicy: Always
512517
schedulerName: default-scheduler
513518
securityContext:
514519
fsGroup: 26
520+
fsGroupChangePolicy: OnRootMismatch
515521
runAsNonRoot: true
516522
terminationGracePeriodSeconds: 30
517-
`))
523+
`
524+
if os.Getenv("ENVTEST_K8S_VERSION") == "1.19.2" {
525+
compare = `
526+
automountServiceAccountToken: false
527+
containers: null
528+
dnsPolicy: ClusterFirst
529+
enableServiceLinks: false
530+
restartPolicy: Always
531+
schedulerName: default-scheduler
532+
securityContext:
533+
fsGroup: 26
534+
runAsNonRoot: true
535+
terminationGracePeriodSeconds: 30
536+
`
537+
}
538+
assert.Assert(t, cmp.MarshalMatches(template.Spec, compare))
518539
})
519540

520541
t.Run("verify customized deployment", func(t *testing.T) {
@@ -614,7 +635,11 @@ labels:
614635
postgres-operator.crunchydata.com/data: pgadmin
615636
postgres-operator.crunchydata.com/role: pgadmin
616637
`))
617-
assert.Assert(t, cmp.MarshalMatches(template.Spec, `
638+
639+
// TODO(benjaminjb)(issue sc-11672): after we update controller-runtime and
640+
// are no longer testing in Github actions with K8s 1.19.2, reduce the following comparison
641+
// to simply testing against a given, fixed string.
642+
compare := `
618643
affinity:
619644
nodeAffinity:
620645
requiredDuringSchedulingIgnoredDuringExecution:
@@ -632,6 +657,7 @@ restartPolicy: Always
632657
schedulerName: default-scheduler
633658
securityContext:
634659
fsGroup: 26
660+
fsGroupChangePolicy: OnRootMismatch
635661
runAsNonRoot: true
636662
terminationGracePeriodSeconds: 30
637663
tolerations:
@@ -648,7 +674,45 @@ topologySpreadConstraints:
648674
maxSkew: 1
649675
topologyKey: fakekey
650676
whenUnsatisfiable: ScheduleAnyway
651-
`))
677+
`
678+
if os.Getenv("ENVTEST_K8S_VERSION") == "1.19.2" {
679+
compare = `
680+
affinity:
681+
nodeAffinity:
682+
requiredDuringSchedulingIgnoredDuringExecution:
683+
nodeSelectorTerms:
684+
- matchExpressions:
685+
- key: key
686+
operator: Exists
687+
automountServiceAccountToken: false
688+
containers: null
689+
dnsPolicy: ClusterFirst
690+
enableServiceLinks: false
691+
imagePullSecrets:
692+
- name: myImagePullSecret
693+
restartPolicy: Always
694+
schedulerName: default-scheduler
695+
securityContext:
696+
fsGroup: 26
697+
runAsNonRoot: true
698+
terminationGracePeriodSeconds: 30
699+
tolerations:
700+
- key: sometoleration
701+
topologySpreadConstraints:
702+
- labelSelector:
703+
matchExpressions:
704+
- key: postgres-operator.crunchydata.com/cluster
705+
operator: In
706+
values:
707+
- somename
708+
- key: postgres-operator.crunchydata.com/data
709+
operator: Exists
710+
maxSkew: 1
711+
topologyKey: fakekey
712+
whenUnsatisfiable: ScheduleAnyway
713+
`
714+
}
715+
assert.Assert(t, cmp.MarshalMatches(template.Spec, compare))
652716
})
653717
}
654718

‎internal/controller/postgrescluster/pgbackrest_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2518,6 +2518,7 @@ containers:
25182518
enableServiceLinks: false
25192519
restartPolicy: Never
25202520
securityContext:
2521+
fsGroupChangePolicy: OnRootMismatch
25212522
runAsNonRoot: true
25222523
volumes:
25232524
- name: pgbackrest-config

‎internal/controller/postgrescluster/pgbouncer_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ containers: null
469469
enableServiceLinks: false
470470
restartPolicy: Always
471471
securityContext:
472+
fsGroupChangePolicy: OnRootMismatch
472473
runAsNonRoot: true
473474
shareProcessNamespace: true
474475
topologySpreadConstraints:

‎internal/controller/postgrescluster/volumes_test.go‎

Lines changed: 178 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ package postgrescluster
2121
import (
2222
"context"
2323
"errors"
24+
"os"
2425
"testing"
2526
"time"
2627

@@ -755,7 +756,61 @@ func TestReconcileMoveDirectories(t *testing.T) {
755756

756757
for i := range moveJobs.Items {
757758
if moveJobs.Items[i].Name == "testcluster-move-pgdata-dir" {
758-
assert.Assert(t, marshalMatches(moveJobs.Items[i].Spec.Template.Spec, `
759+
// TODO(benjaminjb)(issue sc-11672): after we update controller-runtime and
760+
// are no longer testing in Github actions with K8s 1.19.2, reduce the following comparison
761+
// to simply testing against a given, fixed string.
762+
763+
compare := `
764+
automountServiceAccountToken: false
765+
containers:
766+
- command:
767+
- bash
768+
- -ceu
769+
- "echo \"Preparing cluster testcluster volumes for PGO v5.x\"\n echo \"pgdata_pvc=testpgdata\"\n
770+
\ echo \"Current PG data directory volume contents:\" \n ls -lh \"/pgdata\"\n
771+
\ echo \"Now updating PG data directory...\"\n [ -d \"/pgdata/testpgdatadir\"
772+
] && mv \"/pgdata/testpgdatadir\" \"/pgdata/pg13_bootstrap\"\n rm -f \"/pgdata/pg13/patroni.dynamic.json\"\n
773+
\ echo \"Updated PG data directory contents:\" \n ls -lh \"/pgdata\"\n echo
774+
\"PG Data directory preparation complete\"\n "
775+
image: example.com/crunchy-postgres-ha:test
776+
imagePullPolicy: Always
777+
name: pgdata-move-job
778+
resources:
779+
requests:
780+
cpu: 1m
781+
securityContext:
782+
allowPrivilegeEscalation: false
783+
capabilities:
784+
drop:
785+
- ALL
786+
privileged: false
787+
readOnlyRootFilesystem: true
788+
runAsNonRoot: true
789+
terminationMessagePath: /dev/termination-log
790+
terminationMessagePolicy: File
791+
volumeMounts:
792+
- mountPath: /pgdata
793+
name: postgres-data
794+
dnsPolicy: ClusterFirst
795+
enableServiceLinks: false
796+
imagePullSecrets:
797+
- name: test-secret
798+
priorityClassName: some-priority-class
799+
restartPolicy: Never
800+
schedulerName: default-scheduler
801+
securityContext:
802+
fsGroup: 26
803+
fsGroupChangePolicy: OnRootMismatch
804+
runAsNonRoot: true
805+
terminationGracePeriodSeconds: 30
806+
volumes:
807+
- name: postgres-data
808+
persistentVolumeClaim:
809+
claimName: testpgdata
810+
`
811+
812+
if os.Getenv("ENVTEST_K8S_VERSION") == "1.19.2" {
813+
compare = `
759814
automountServiceAccountToken: false
760815
containers:
761816
- command:
@@ -801,7 +856,10 @@ volumes:
801856
- name: postgres-data
802857
persistentVolumeClaim:
803858
claimName: testpgdata
804-
`+"\n"))
859+
`
860+
}
861+
862+
assert.Assert(t, marshalMatches(moveJobs.Items[i].Spec.Template.Spec, compare+"\n"))
805863
}
806864
}
807865

@@ -811,7 +869,60 @@ volumes:
811869

812870
for i := range moveJobs.Items {
813871
if moveJobs.Items[i].Name == "testcluster-move-pgwal-dir" {
814-
assert.Assert(t, marshalMatches(moveJobs.Items[i].Spec.Template.Spec, `
872+
// TODO(benjaminjb)(issue sc-11672): after we update controller-runtime and
873+
// are no longer testing in Github actions with K8s 1.19.2, reduce the following comparison
874+
// to simply testing against a given, fixed string.
875+
876+
compare := `
877+
automountServiceAccountToken: false
878+
containers:
879+
- command:
880+
- bash
881+
- -ceu
882+
- "echo \"Preparing cluster testcluster volumes for PGO v5.x\"\n echo \"pg_wal_pvc=testwal\"\n
883+
\ echo \"Current PG WAL directory volume contents:\"\n ls -lh \"/pgwal\"\n
884+
\ echo \"Now updating PG WAL directory...\"\n [ -d \"/pgwal/testwaldir\"
885+
] && mv \"/pgwal/testwaldir\" \"/pgwal/testcluster-wal\"\n echo \"Updated PG
886+
WAL directory contents:\"\n ls -lh \"/pgwal\"\n echo \"PG WAL directory
887+
preparation complete\"\n "
888+
image: example.com/crunchy-postgres-ha:test
889+
imagePullPolicy: Always
890+
name: pgwal-move-job
891+
resources:
892+
requests:
893+
cpu: 1m
894+
securityContext:
895+
allowPrivilegeEscalation: false
896+
capabilities:
897+
drop:
898+
- ALL
899+
privileged: false
900+
readOnlyRootFilesystem: true
901+
runAsNonRoot: true
902+
terminationMessagePath: /dev/termination-log
903+
terminationMessagePolicy: File
904+
volumeMounts:
905+
- mountPath: /pgwal
906+
name: postgres-wal
907+
dnsPolicy: ClusterFirst
908+
enableServiceLinks: false
909+
imagePullSecrets:
910+
- name: test-secret
911+
priorityClassName: some-priority-class
912+
restartPolicy: Never
913+
schedulerName: default-scheduler
914+
securityContext:
915+
fsGroup: 26
916+
fsGroupChangePolicy: OnRootMismatch
917+
runAsNonRoot: true
918+
terminationGracePeriodSeconds: 30
919+
volumes:
920+
- name: postgres-wal
921+
persistentVolumeClaim:
922+
claimName: testwal
923+
`
924+
if os.Getenv("ENVTEST_K8S_VERSION") == "1.19.2" {
925+
compare = `
815926
automountServiceAccountToken: false
816927
containers:
817928
- command:
@@ -857,7 +968,9 @@ volumes:
857968
- name: postgres-wal
858969
persistentVolumeClaim:
859970
claimName: testwal
860-
`+"\n"))
971+
`
972+
}
973+
assert.Assert(t, marshalMatches(moveJobs.Items[i].Spec.Template.Spec, compare+"\n"))
861974
}
862975
}
863976

@@ -867,7 +980,64 @@ volumes:
867980

868981
for i := range moveJobs.Items {
869982
if moveJobs.Items[i].Name == "testcluster-move-pgbackrest-repo-dir" {
870-
assert.Assert(t, marshalMatches(moveJobs.Items[i].Spec.Template.Spec, `
983+
984+
// TODO(benjaminjb)(issue sc-11672): after we update controller-runtime and
985+
// are no longer testing in Github actions with K8s 1.19.2, reduce the following comparison
986+
// to simply testing against a given, fixed string.
987+
988+
compare := `
989+
automountServiceAccountToken: false
990+
containers:
991+
- command:
992+
- bash
993+
- -ceu
994+
- "echo \"Preparing cluster testcluster pgBackRest repo volume for PGO v5.x\"\n
995+
\ echo \"repo_pvc=testrepo\"\n echo \"pgbackrest directory:\"\n ls -lh
996+
/pgbackrest\n echo \"Current pgBackRest repo directory volume contents:\" \n
997+
\ ls -lh \"/pgbackrest/testrepodir\"\n echo \"Now updating repo directory...\"\n
998+
\ [ -d \"/pgbackrest/testrepodir\" ] && mv -t \"/pgbackrest/\" \"/pgbackrest/testrepodir/archive\"\n
999+
\ [ -d \"/pgbackrest/testrepodir\" ] && mv -t \"/pgbackrest/\" \"/pgbackrest/testrepodir/backup\"\n
1000+
\ echo \"Updated /pgbackrest directory contents:\"\n ls -lh \"/pgbackrest\"\n
1001+
\ echo \"Repo directory preparation complete\"\n "
1002+
image: example.com/crunchy-pgbackrest:test
1003+
imagePullPolicy: Always
1004+
name: repo-move-job
1005+
resources:
1006+
requests:
1007+
cpu: 1m
1008+
securityContext:
1009+
allowPrivilegeEscalation: false
1010+
capabilities:
1011+
drop:
1012+
- ALL
1013+
privileged: false
1014+
readOnlyRootFilesystem: true
1015+
runAsNonRoot: true
1016+
terminationMessagePath: /dev/termination-log
1017+
terminationMessagePolicy: File
1018+
volumeMounts:
1019+
- mountPath: /pgbackrest
1020+
name: pgbackrest-repo
1021+
dnsPolicy: ClusterFirst
1022+
enableServiceLinks: false
1023+
imagePullSecrets:
1024+
- name: test-secret
1025+
priorityClassName: some-priority-class
1026+
restartPolicy: Never
1027+
schedulerName: default-scheduler
1028+
securityContext:
1029+
fsGroup: 26
1030+
fsGroupChangePolicy: OnRootMismatch
1031+
runAsNonRoot: true
1032+
terminationGracePeriodSeconds: 30
1033+
volumes:
1034+
- name: pgbackrest-repo
1035+
persistentVolumeClaim:
1036+
claimName: testrepo
1037+
`
1038+
1039+
if os.Getenv("ENVTEST_K8S_VERSION") == "1.19.2" {
1040+
compare = `
8711041
automountServiceAccountToken: false
8721042
containers:
8731043
- command:
@@ -915,7 +1085,9 @@ volumes:
9151085
- name: pgbackrest-repo
9161086
persistentVolumeClaim:
9171087
claimName: testrepo
918-
`+"\n"))
1088+
`
1089+
}
1090+
assert.Assert(t, marshalMatches(moveJobs.Items[i].Spec.Template.Spec, compare+"\n"))
9191091
}
9201092
}
9211093

‎internal/initialize/security.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,14 @@ import (
2222
// RestrictedPodSecurityContext returns a v1.PodSecurityContext with safe defaults.
2323
// See https://docs.k8s.io/concepts/security/pod-security-standards/
2424
func RestrictedPodSecurityContext() *corev1.PodSecurityContext {
25+
onRootMismatch := corev1.FSGroupChangeOnRootMismatch
2526
return &corev1.PodSecurityContext{
2627
// Fail to start a container if its image runs as UID 0 (root).
2728
RunAsNonRoot: Bool(true),
29+
30+
// If set to "OnRootMismatch", if the root of the volume already has
31+
// the correct permissions, the recursive permission change can be skipped
32+
FSGroupChangePolicy: &onRootMismatch,
2833
}
2934
}
3035

0 commit comments

Comments
 (0)