Promote RunAsGroup to GA

pkg/api/pod/util.go

@@ -484,8 +484,6 @@ func dropDisabledFields(
 		})
 	}
 
-	dropDisabledRunAsGroupField(podSpec, oldPodSpec)
-
 	dropDisabledFSGroupFields(podSpec, oldPodSpec)
 
 	if !utilfeature.DefaultFeatureGate.Enabled(features.PodOverhead) && !overheadInUse(oldPodSpec) {
@@ -512,22 +510,6 @@ func dropDisabledFields(
 
 }
 
-// dropDisabledRunAsGroupField removes disabled fields from PodSpec related
-// to RunAsGroup
-func dropDisabledRunAsGroupField(podSpec, oldPodSpec *api.PodSpec) {
-	if !utilfeature.DefaultFeatureGate.Enabled(features.RunAsGroup) && !runAsGroupInUse(oldPodSpec) {
-		if podSpec.SecurityContext != nil {
-			podSpec.SecurityContext.RunAsGroup = nil
-		}
-		VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
-			if c.SecurityContext != nil {
-				c.SecurityContext.RunAsGroup = nil
-			}
-			return true
-		})
-	}
-}
-
 // dropDisabledProcMountField removes disabled fields from PodSpec related
 // to ProcMount only if it is not already used by the old spec
 func dropDisabledProcMountField(podSpec, oldPodSpec *api.PodSpec) {
@@ -691,28 +673,6 @@ func emptyDirSizeLimitInUse(podSpec *api.PodSpec) bool {
 	return false
 }
 
-// runAsGroupInUse returns true if the pod spec is non-nil and has a SecurityContext's RunAsGroup field set
-func runAsGroupInUse(podSpec *api.PodSpec) bool {
-	if podSpec == nil {
-		return false
-	}
-
-	if podSpec.SecurityContext != nil && podSpec.SecurityContext.RunAsGroup != nil {
-		return true
-	}
-
-	var inUse bool
-	VisitContainers(podSpec, AllContainers, func(c *api.Container, containerType ContainerType) bool {
-		if c.SecurityContext != nil && c.SecurityContext.RunAsGroup != nil {
-			inUse = true
-			return false
-		}
-		return true
-	})
-
-	return inUse
-}
-
 // subpathExprInUse returns true if the pod spec is non-nil and has a volume mount that makes use of the subPathExpr feature
 func subpathExprInUse(podSpec *api.PodSpec) bool {
 	if podSpec == nil {

pkg/api/pod/util_test.go

@@ -1017,143 +1017,6 @@ func TestDropAppArmor(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 featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RunAsGroup, enabled)()
-
-					var oldPodSpec *api.PodSpec
-					if oldPod != nil {
-						oldPodSpec = &oldPod.Spec
-					}
-					dropDisabledFields(&newPod.Spec, nil, oldPodSpec, nil)
-
-					// 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()))
-						}
-					}
-				})
-			}
-		}
-	}
-}
-
 func TestDropPodSysctls(t *testing.T) {
 	podWithSysctls := func() *api.Pod {
 		return &api.Pod{

pkg/api/podsecuritypolicy/util.go

@@ -28,9 +28,6 @@ func DropDisabledFields(pspSpec, oldPSPSpec *policy.PodSecurityPolicySpec) {
 	if !utilfeature.DefaultFeatureGate.Enabled(features.ProcMountType) && !allowedProcMountTypesInUse(oldPSPSpec) {
 		pspSpec.AllowedProcMountTypes = nil
 	}
-	if !utilfeature.DefaultFeatureGate.Enabled(features.RunAsGroup) && (oldPSPSpec == nil || oldPSPSpec.RunAsGroup == nil) {
-		pspSpec.RunAsGroup = nil
-	}
 	if !utilfeature.DefaultFeatureGate.Enabled(features.Sysctls) && !sysctlsInUse(oldPSPSpec) {
 		pspSpec.AllowedUnsafeSysctls = nil
 		pspSpec.ForbiddenSysctls = nil

pkg/api/podsecuritypolicy/util_test.go

@@ -108,86 +108,6 @@ 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 featuregatetesting.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()))
-						}
-					}
-				})
-			}
-		}
-	}
-}
-
 func TestDropSysctls(t *testing.T) {
 	scWithSysctls := func() *policy.PodSecurityPolicySpec {
 		return &policy.PodSecurityPolicySpec{

pkg/features/kube_features.go

@@ -216,7 +216,8 @@ const (
 	CRIContainerLogRotation featuregate.Feature = "CRIContainerLogRotation"
 
 	// owner: @krmayankk
-	// beta: v1.14
+	// beta:  v1.14
+	// ga:    v1.21
 	//
 	// Enables control over the primary group ID of containers' init processes.
 	RunAsGroup featuregate.Feature = "RunAsGroup"
@@ -730,7 +731,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	CSIMigrationvSphere:                            {Default: false, PreRelease: featuregate.Beta}, // Off by default (requires vSphere CSI driver)
 	CSIMigrationvSphereComplete:                    {Default: false, PreRelease: featuregate.Beta}, // remove in 1.22
 	InTreePluginvSphereUnregister:                  {Default: false, PreRelease: featuregate.Alpha},
-	RunAsGroup:                                     {Default: true, PreRelease: featuregate.Beta},
 	CSIMigrationOpenStack:                          {Default: false, PreRelease: featuregate.Beta}, // Off by default (requires OpenStack Cinder CSI driver)
 	InTreePluginOpenStackUnregister:                {Default: false, PreRelease: featuregate.Alpha},
 	VolumeSubpath:                                  {Default: true, PreRelease: featuregate.GA},
@@ -785,6 +785,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	ServiceLBNodePortControl:                       {Default: false, PreRelease: featuregate.Alpha},
 	MixedProtocolLBService:                         {Default: false, PreRelease: featuregate.Alpha},
 	PreferNominatedNode:                            {Default: false, PreRelease: featuregate.Alpha},
+	RunAsGroup:                                     {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.22
 
 	// inherited features from generic apiserver, relisted here to get a conflict if it is changed
 	// unintentionally on either side:

pkg/security/podsecuritypolicy/factory.go

@@ -19,9 +19,6 @@ package podsecuritypolicy
 import (
 	"fmt"
 
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
-	"k8s.io/kubernetes/pkg/features"
-
 	corev1 "k8s.io/api/core/v1"
 	policy "k8s.io/api/policy/v1beta1"
 	"k8s.io/apimachinery/pkg/util/errors"
@@ -51,11 +48,9 @@ func (f *simpleStrategyFactory) CreateStrategies(psp *policy.PodSecurityPolicy,
 	}
 
 	var groupStrat group.GroupStrategy
-	if utilfeature.DefaultFeatureGate.Enabled(features.RunAsGroup) {
+	groupStrat, err = createRunAsGroupStrategy(psp.Spec.RunAsGroup)
-		groupStrat, err = createRunAsGroupStrategy(psp.Spec.RunAsGroup)
+	if err != nil {
-		if err != nil {
+		errs = append(errs, err)
-			errs = append(errs, err)
-		}
 	}
 
 	seLinuxStrat, err := createSELinuxStrategy(&psp.Spec.SELinux)

pkg/security/podsecuritypolicy/provider.go

@@ -140,15 +140,12 @@ func (s *simpleProvider) mutateContainer(pod *api.Pod, container *api.Container)
 		sc.SetRunAsUser(uid)
 	}
 
-	if utilfeature.DefaultFeatureGate.Enabled(features.RunAsGroup) {
+	if sc.RunAsGroup() == nil {
-		if sc.RunAsGroup() == nil {
+		gid, err := s.strategies.RunAsGroupStrategy.GenerateSingle(pod)
-			gid, err := s.strategies.RunAsGroupStrategy.GenerateSingle(pod)
+		if err != nil {
-			if err != nil {
+			return err
-				return err
-			}
-			sc.SetRunAsGroup(gid)
 		}
-
+		sc.SetRunAsGroup(gid)
 	}
 
 	if sc.SELinuxOptions() == nil {
@@ -337,14 +334,12 @@ func (s *simpleProvider) validateContainer(pod *api.Pod, container *api.Containe
 
 	scPath := containerPath.Child("securityContext")
 	allErrs = append(allErrs, s.strategies.RunAsUserStrategy.Validate(scPath, pod, container, sc.RunAsNonRoot(), sc.RunAsUser())...)
-
+	var runAsGroups []int64
-	if utilfeature.DefaultFeatureGate.Enabled(features.RunAsGroup) {
+	if sc.RunAsGroup() != nil {
-		var runAsGroups []int64
+		runAsGroups = []int64{*sc.RunAsGroup()}
-		if sc.RunAsGroup() != nil {
-			runAsGroups = []int64{*sc.RunAsGroup()}
-		}
-		allErrs = append(allErrs, s.strategies.RunAsGroupStrategy.Validate(scPath, pod, runAsGroups)...)
 	}
+	allErrs = append(allErrs, s.strategies.RunAsGroupStrategy.Validate(scPath, pod, runAsGroups)...)
+
 	allErrs = append(allErrs, s.strategies.SELinuxStrategy.Validate(scPath.Child("seLinuxOptions"), pod, container, sc.SELinuxOptions())...)
 	allErrs = append(allErrs, s.strategies.AppArmorStrategy.Validate(pod, container)...)
 	allErrs = append(allErrs, s.strategies.SeccompStrategy.ValidateContainer(pod, container)...)