diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 22e2e11ad2b..eca0309b86c 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -911,3 +911,140 @@ func TestDropEmptyDirSizeLimit(t *testing.T) { } } } + +func TestDropRunAsGroup(t *testing.T) { + group := func() *int64 { + testGroup := int64(1000) + return &testGroup + } + defaultProcMount := api.DefaultProcMount + defaultSecurityContext := func() *api.SecurityContext { + return &api.SecurityContext{ProcMount: &defaultProcMount} + } + securityContextWithRunAsGroup := func() *api.SecurityContext { + return &api.SecurityContext{ProcMount: &defaultProcMount, RunAsGroup: group()} + } + podWithoutRunAsGroup := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + SecurityContext: &api.PodSecurityContext{}, + Containers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: defaultSecurityContext()}}, + InitContainers: []api.Container{{Name: "initContainer1", Image: "testimage", SecurityContext: defaultSecurityContext()}}, + }, + } + } + podWithRunAsGroupInPod := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + SecurityContext: &api.PodSecurityContext{RunAsGroup: group()}, + Containers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: defaultSecurityContext()}}, + InitContainers: []api.Container{{Name: "initContainer1", Image: "testimage", SecurityContext: defaultSecurityContext()}}, + }, + } + } + podWithRunAsGroupInContainers := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + SecurityContext: &api.PodSecurityContext{}, + Containers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: securityContextWithRunAsGroup()}}, + InitContainers: []api.Container{{Name: "initContainer1", Image: "testimage", SecurityContext: defaultSecurityContext()}}, + }, + } + } + podWithRunAsGroupInInitContainers := func() *api.Pod { + return &api.Pod{ + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyNever, + SecurityContext: &api.PodSecurityContext{}, + Containers: []api.Container{{Name: "container1", Image: "testimage", SecurityContext: defaultSecurityContext()}}, + InitContainers: []api.Container{{Name: "initContainer1", Image: "testimage", SecurityContext: securityContextWithRunAsGroup()}}, + }, + } + } + + podInfo := []struct { + description string + hasRunAsGroup bool + pod func() *api.Pod + }{ + { + description: "have RunAsGroup in Pod", + hasRunAsGroup: true, + pod: podWithRunAsGroupInPod, + }, + { + description: "have RunAsGroup in Container", + hasRunAsGroup: true, + pod: podWithRunAsGroupInContainers, + }, + { + description: "have RunAsGroup in InitContainer", + hasRunAsGroup: true, + pod: podWithRunAsGroupInInitContainers, + }, + { + description: "does not have RunAsGroup", + hasRunAsGroup: false, + pod: podWithoutRunAsGroup, + }, + { + description: "is nil", + hasRunAsGroup: false, + pod: func() *api.Pod { return nil }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldPodInfo := range podInfo { + for _, newPodInfo := range podInfo { + oldPodHasRunAsGroup, oldPod := oldPodInfo.hasRunAsGroup, oldPodInfo.pod() + newPodHasRunAsGroup, newPod := newPodInfo.hasRunAsGroup, newPodInfo.pod() + if newPod == nil { + continue + } + + t.Run(fmt.Sprintf("feature enabled=%v, old pod %v, new pod %v", enabled, oldPodInfo.description, newPodInfo.description), func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RunAsGroup, enabled)() + + var oldPodSpec *api.PodSpec + if oldPod != nil { + oldPodSpec = &oldPod.Spec + } + DropDisabledFields(&newPod.Spec, oldPodSpec) + + // old pod should never be changed + if !reflect.DeepEqual(oldPod, oldPodInfo.pod()) { + t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodInfo.pod())) + } + + switch { + case enabled || oldPodHasRunAsGroup: + // new pod should not be changed if the feature is enabled, or if the old pod had RunAsGroup + if !reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) + } + case newPodHasRunAsGroup: + // new pod should be changed + if reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("%v", oldPod) + t.Errorf("%v", newPod) + t.Errorf("new pod was not changed") + } + // new pod should not have RunAsGroup + if !reflect.DeepEqual(newPod, podWithoutRunAsGroup()) { + t.Errorf("new pod had RunAsGroup: %v", diff.ObjectReflectDiff(newPod, podWithoutRunAsGroup())) + } + default: + // new pod should not need to be changed + if !reflect.DeepEqual(newPod, newPodInfo.pod()) { + t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodInfo.pod())) + } + } + }) + } + } + } +} diff --git a/pkg/api/podsecuritypolicy/util_test.go b/pkg/api/podsecuritypolicy/util_test.go index 433d4241968..7ec614c1719 100644 --- a/pkg/api/podsecuritypolicy/util_test.go +++ b/pkg/api/podsecuritypolicy/util_test.go @@ -107,3 +107,83 @@ func TestDropAllowedProcMountTypes(t *testing.T) { } } } + +func TestDropRunAsGroup(t *testing.T) { + group := func() *policy.RunAsGroupStrategyOptions { + return &policy.RunAsGroupStrategyOptions{} + } + scWithoutRunAsGroup := func() *policy.PodSecurityPolicySpec { + return &policy.PodSecurityPolicySpec{} + } + scWithRunAsGroup := func() *policy.PodSecurityPolicySpec { + return &policy.PodSecurityPolicySpec{ + RunAsGroup: group(), + } + } + scInfo := []struct { + description string + hasRunAsGroup bool + sc func() *policy.PodSecurityPolicySpec + }{ + { + description: "PodSecurityPolicySpec Without RunAsGroup", + hasRunAsGroup: false, + sc: scWithoutRunAsGroup, + }, + { + description: "PodSecurityPolicySpec With RunAsGroup", + hasRunAsGroup: true, + sc: scWithRunAsGroup, + }, + { + description: "is nil", + hasRunAsGroup: false, + sc: func() *policy.PodSecurityPolicySpec { return nil }, + }, + } + + for _, enabled := range []bool{true, false} { + for _, oldPSPSpecInfo := range scInfo { + for _, newPSPSpecInfo := range scInfo { + oldPSPSpecHasRunAsGroup, oldPSPSpec := oldPSPSpecInfo.hasRunAsGroup, oldPSPSpecInfo.sc() + newPSPSpecHasRunAsGroup, newPSPSpec := newPSPSpecInfo.hasRunAsGroup, newPSPSpecInfo.sc() + if newPSPSpec == nil { + continue + } + + t.Run(fmt.Sprintf("feature enabled=%v, old PodSecurityPolicySpec %v, new PodSecurityPolicySpec %v", enabled, oldPSPSpecInfo.description, newPSPSpecInfo.description), func(t *testing.T) { + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RunAsGroup, enabled)() + + DropDisabledFields(newPSPSpec, oldPSPSpec) + + // old PodSecurityPolicySpec should never be changed + if !reflect.DeepEqual(oldPSPSpec, oldPSPSpecInfo.sc()) { + t.Errorf("old PodSecurityPolicySpec changed: %v", diff.ObjectReflectDiff(oldPSPSpec, oldPSPSpecInfo.sc())) + } + + switch { + case enabled || oldPSPSpecHasRunAsGroup: + // new PodSecurityPolicySpec should not be changed if the feature is enabled, or if the old PodSecurityPolicySpec had RunAsGroup + if !reflect.DeepEqual(newPSPSpec, newPSPSpecInfo.sc()) { + t.Errorf("new PodSecurityPolicySpec changed: %v", diff.ObjectReflectDiff(newPSPSpec, newPSPSpecInfo.sc())) + } + case newPSPSpecHasRunAsGroup: + // new PodSecurityPolicySpec should be changed + if reflect.DeepEqual(newPSPSpec, newPSPSpecInfo.sc()) { + t.Errorf("new PodSecurityPolicySpec was not changed") + } + // new PodSecurityPolicySpec should not have RunAsGroup + if !reflect.DeepEqual(newPSPSpec, scWithoutRunAsGroup()) { + t.Errorf("new PodSecurityPolicySpec had RunAsGroup: %v", diff.ObjectReflectDiff(newPSPSpec, scWithoutRunAsGroup())) + } + default: + // new PodSecurityPolicySpec should not need to be changed + if !reflect.DeepEqual(newPSPSpec, newPSPSpecInfo.sc()) { + t.Errorf("new PodSecurityPolicySpec changed: %v", diff.ObjectReflectDiff(newPSPSpec, newPSPSpecInfo.sc())) + } + } + }) + } + } + } +}