Merge pull request kubernetes#88654 from ddebroy/gmsa-disable1

Promote GMSA support for Windows to GA

Author: k8s-ci-robot

api/openapi-spec/swagger.json

@@ -387,8 +387,6 @@ func dropDisabledFields(
 
 	dropDisabledRunAsGroupField(podSpec, oldPodSpec)
 
-	dropDisabledGMSAFields(podSpec, oldPodSpec)
-
 	if !utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) && !runtimeClassInUse(oldPodSpec) {
 		// Set RuntimeClassName to nil only if feature is disabled and it is not used
 		podSpec.RuntimeClassName = nil
@@ -432,39 +430,6 @@ func dropDisabledRunAsGroupField(podSpec, oldPodSpec *api.PodSpec) {
 	}
 }
 
-// dropDisabledGMSAFields removes disabled fields related to Windows GMSA
-// from the given PodSpec.
-func dropDisabledGMSAFields(podSpec, oldPodSpec *api.PodSpec) {
-	if utilfeature.DefaultFeatureGate.Enabled(features.WindowsGMSA) ||
-		gMSAFieldsInUse(oldPodSpec) {
-		return
-	}
-
-	if podSpec.SecurityContext != nil {
-		dropDisabledGMSAFieldsFromWindowsSecurityOptions(podSpec.SecurityContext.WindowsOptions)
-	}
-	dropDisabledGMSAFieldsFromContainers(podSpec.Containers)
-	dropDisabledGMSAFieldsFromContainers(podSpec.InitContainers)
-}
-
-// dropDisabledGMSAFieldsFromWindowsSecurityOptions removes disabled fields
-// related to Windows GMSA from the given WindowsSecurityContextOptions.
-func dropDisabledGMSAFieldsFromWindowsSecurityOptions(windowsOptions *api.WindowsSecurityContextOptions) {
-	if windowsOptions != nil {
-		windowsOptions.GMSACredentialSpecName = nil
-		windowsOptions.GMSACredentialSpec = nil
-	}
-}
-
-// dropDisabledGMSAFieldsFromContainers removes disabled fields
-func dropDisabledGMSAFieldsFromContainers(containers []api.Container) {
-	for i := range containers {
-		if containers[i].SecurityContext != nil {
-			dropDisabledGMSAFieldsFromWindowsSecurityOptions(containers[i].SecurityContext.WindowsOptions)
-		}
-	}
-}
-
 // 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) {
@@ -655,44 +620,6 @@ func runAsGroupInUse(podSpec *api.PodSpec) bool {
 	return inUse
 }
 
-// gMSAFieldsInUse returns true if the pod spec is non-nil and has one of any
-// SecurityContext's GMSACredentialSpecName or GMSACredentialSpec fields set.
-func gMSAFieldsInUse(podSpec *api.PodSpec) bool {
-	if podSpec == nil {
-		return false
-	}
-
-	if podSpec.SecurityContext != nil && gMSAFieldsInUseInWindowsSecurityOptions(podSpec.SecurityContext.WindowsOptions) {
-		return true
-	}
-
-	return gMSAFieldsInUseInAnyContainer(podSpec.Containers) ||
-		gMSAFieldsInUseInAnyContainer(podSpec.InitContainers)
-}
-
-// gMSAFieldsInUseInWindowsSecurityOptions returns true if the given WindowsSecurityContextOptions is
-// non-nil and one of its GMSACredentialSpecName or GMSACredentialSpec fields is set.
-func gMSAFieldsInUseInWindowsSecurityOptions(windowsOptions *api.WindowsSecurityContextOptions) bool {
-	if windowsOptions == nil {
-		return false
-	}
-
-	return windowsOptions.GMSACredentialSpecName != nil ||
-		windowsOptions.GMSACredentialSpec != nil
-}
-
-// gMSAFieldsInUseInAnyContainer returns true if any of the given Containers has its
-// SecurityContext's GMSACredentialSpecName or GMSACredentialSpec fields set.
-func gMSAFieldsInUseInAnyContainer(containers []api.Container) bool {
-	for _, container := range containers {
-		if container.SecurityContext != nil && gMSAFieldsInUseInWindowsSecurityOptions(container.SecurityContext.WindowsOptions) {
-			return true
-		}
-	}
-
-	return false
-}
-
 // 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.go

@@ -1171,208 +1171,6 @@ func TestDropRunAsGroup(t *testing.T) {
 	}
 }
 
-func TestDropGMSAFields(t *testing.T) {
-	defaultContainerSecurityContextFactory := func() *api.SecurityContext {
-		defaultProcMount := api.DefaultProcMount
-		return &api.SecurityContext{ProcMount: &defaultProcMount}
-	}
-	podWithoutWindowsOptionsFactory := func() *api.Pod {
-		return &api.Pod{
-			Spec: api.PodSpec{
-				RestartPolicy:   api.RestartPolicyNever,
-				SecurityContext: &api.PodSecurityContext{},
-				Containers:      []api.Container{{Name: "container1", Image: "testimage", SecurityContext: defaultContainerSecurityContextFactory()}},
-				InitContainers:  []api.Container{{Name: "initContainer1", Image: "testimage", SecurityContext: defaultContainerSecurityContextFactory()}},
-			},
-		}
-	}
-
-	type podFactoryInfo struct {
-		description  string
-		hasGMSAField bool
-		// this factory should generate the input pod whose spec will be fed to dropDisabledFields
-		podFactory func() *api.Pod
-		// this factory should generate the expected pod after the GMSA fields have been dropped
-		// we can't just use podWithoutWindowsOptionsFactory as is for this, since in some cases
-		// we'll be left with a WindowsSecurityContextOptions struct with no GMSA field set, as opposed
-		// to a nil pointer in the pod generated by podWithoutWindowsOptionsFactory
-		// if this field is not set, it will default to the podFactory
-		strippedPodFactory func() *api.Pod
-	}
-	podFactoryInfos := []podFactoryInfo{
-		{
-			description:  "does not have any GMSA field set",
-			hasGMSAField: false,
-			podFactory:   podWithoutWindowsOptionsFactory,
-		},
-		{
-			description:  "has a pod-level WindowsSecurityContextOptions struct with no GMSA field set",
-			hasGMSAField: false,
-			podFactory: func() *api.Pod {
-				pod := podWithoutWindowsOptionsFactory()
-				pod.Spec.SecurityContext.WindowsOptions = &api.WindowsSecurityContextOptions{}
-				return pod
-			},
-		},
-		{
-			description:  "has a WindowsSecurityContextOptions struct with no GMSA field set on a container",
-			hasGMSAField: false,
-			podFactory: func() *api.Pod {
-				pod := podWithoutWindowsOptionsFactory()
-				pod.Spec.Containers[0].SecurityContext.WindowsOptions = &api.WindowsSecurityContextOptions{}
-				return pod
-			},
-		},
-		{
-			description:  "has a WindowsSecurityContextOptions struct with no GMSA field set on an init container",
-			hasGMSAField: false,
-			podFactory: func() *api.Pod {
-				pod := podWithoutWindowsOptionsFactory()
-				pod.Spec.InitContainers[0].SecurityContext.WindowsOptions = &api.WindowsSecurityContextOptions{}
-				return pod
-			},
-		},
-		{
-			description:  "is nil",
-			hasGMSAField: false,
-			podFactory:   func() *api.Pod { return nil },
-		},
-	}
-
-	toPtr := func(s string) *string {
-		return &s
-	}
-	addGMSACredentialSpecName := func(windowsOptions *api.WindowsSecurityContextOptions) {
-		windowsOptions.GMSACredentialSpecName = toPtr("dummy-gmsa-cred-spec-name")
-	}
-	addGMSACredentialSpec := func(windowsOptions *api.WindowsSecurityContextOptions) {
-		windowsOptions.GMSACredentialSpec = toPtr("dummy-gmsa-cred-spec-contents")
-	}
-	addBothGMSAFields := func(windowsOptions *api.WindowsSecurityContextOptions) {
-		addGMSACredentialSpecName(windowsOptions)
-		addGMSACredentialSpec(windowsOptions)
-	}
-
-	for fieldName, windowsOptionsTransformingFunc := range map[string]func(*api.WindowsSecurityContextOptions){
-		"GMSACredentialSpecName field": addGMSACredentialSpecName,
-		"GMSACredentialSpec field":     addGMSACredentialSpec,
-		"both GMSA fields":             addBothGMSAFields,
-	} {
-		// yes, these variables are indeed needed for the closure to work
-		// properly, please do NOT remove them
-		name := fieldName
-		transformingFunc := windowsOptionsTransformingFunc
-
-		windowsOptionsWithGMSAFieldFactory := func() *api.WindowsSecurityContextOptions {
-			windowsOptions := &api.WindowsSecurityContextOptions{}
-			transformingFunc(windowsOptions)
-			return windowsOptions
-		}
-
-		podFactoryInfos = append(podFactoryInfos,
-			podFactoryInfo{
-				description:  fmt.Sprintf("has %s in Pod", name),
-				hasGMSAField: true,
-				podFactory: func() *api.Pod {
-					pod := podWithoutWindowsOptionsFactory()
-					pod.Spec.SecurityContext.WindowsOptions = windowsOptionsWithGMSAFieldFactory()
-					return pod
-				},
-				strippedPodFactory: func() *api.Pod {
-					pod := podWithoutWindowsOptionsFactory()
-					pod.Spec.SecurityContext.WindowsOptions = &api.WindowsSecurityContextOptions{}
-					return pod
-				},
-			},
-			podFactoryInfo{
-				description:  fmt.Sprintf("has %s in Container", name),
-				hasGMSAField: true,
-				podFactory: func() *api.Pod {
-					pod := podWithoutWindowsOptionsFactory()
-					pod.Spec.Containers[0].SecurityContext.WindowsOptions = windowsOptionsWithGMSAFieldFactory()
-					return pod
-				},
-				strippedPodFactory: func() *api.Pod {
-					pod := podWithoutWindowsOptionsFactory()
-					pod.Spec.Containers[0].SecurityContext.WindowsOptions = &api.WindowsSecurityContextOptions{}
-					return pod
-				},
-			},
-			podFactoryInfo{
-				description:  fmt.Sprintf("has %s in InitContainer", name),
-				hasGMSAField: true,
-				podFactory: func() *api.Pod {
-					pod := podWithoutWindowsOptionsFactory()
-					pod.Spec.InitContainers[0].SecurityContext.WindowsOptions = windowsOptionsWithGMSAFieldFactory()
-					return pod
-				},
-				strippedPodFactory: func() *api.Pod {
-					pod := podWithoutWindowsOptionsFactory()
-					pod.Spec.InitContainers[0].SecurityContext.WindowsOptions = &api.WindowsSecurityContextOptions{}
-					return pod
-				},
-			})
-	}
-
-	for _, enabled := range []bool{true, false} {
-		for _, oldPodFactoryInfo := range podFactoryInfos {
-			for _, newPodFactoryInfo := range podFactoryInfos {
-				newPodHasGMSAField, newPod := newPodFactoryInfo.hasGMSAField, newPodFactoryInfo.podFactory()
-				if newPod == nil {
-					continue
-				}
-				oldPodHasGMSAField, oldPod := oldPodFactoryInfo.hasGMSAField, oldPodFactoryInfo.podFactory()
-
-				t.Run(fmt.Sprintf("feature enabled=%v, old pod %s, new pod %s", enabled, oldPodFactoryInfo.description, newPodFactoryInfo.description), func(t *testing.T) {
-					defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.WindowsGMSA, 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, oldPodFactoryInfo.podFactory()) {
-						t.Errorf("old pod changed: %v", diff.ObjectReflectDiff(oldPod, oldPodFactoryInfo.podFactory()))
-					}
-
-					switch {
-					case enabled || oldPodHasGMSAField:
-						// new pod should not be changed if the feature is enabled, or if the old pod had any GMSA field set
-						if !reflect.DeepEqual(newPod, newPodFactoryInfo.podFactory()) {
-							t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodFactoryInfo.podFactory()))
-						}
-					case newPodHasGMSAField:
-						// new pod should be changed
-						if reflect.DeepEqual(newPod, newPodFactoryInfo.podFactory()) {
-							t.Errorf("%v", oldPod)
-							t.Errorf("%v", newPod)
-							t.Errorf("new pod was not changed")
-						}
-						// new pod should not have any GMSA field set
-						var expectedStrippedPod *api.Pod
-						if newPodFactoryInfo.strippedPodFactory == nil {
-							expectedStrippedPod = newPodFactoryInfo.podFactory()
-						} else {
-							expectedStrippedPod = newPodFactoryInfo.strippedPodFactory()
-						}
-
-						if !reflect.DeepEqual(newPod, expectedStrippedPod) {
-							t.Errorf("new pod had some GMSA field set: %v", diff.ObjectReflectDiff(newPod, expectedStrippedPod))
-						}
-					default:
-						// new pod should not need to be changed
-						if !reflect.DeepEqual(newPod, newPodFactoryInfo.podFactory()) {
-							t.Errorf("new pod changed: %v", diff.ObjectReflectDiff(newPod, newPodFactoryInfo.podFactory()))
-						}
-					}
-				})
-			}
-		}
-	}
-}
-
 func TestDropPodSysctls(t *testing.T) {
 	podWithSysctls := func() *api.Pod {
 		return &api.Pod{

pkg/api/pod/util_test.go

@@ -5092,14 +5092,12 @@ type SELinuxOptions struct {
 // WindowsSecurityContextOptions contain Windows-specific options and credentials.
 type WindowsSecurityContextOptions struct {
 	// GMSACredentialSpecName is the name of the GMSA credential spec to use.
-	// This field is alpha-level and is only honored by servers that enable the WindowsGMSA feature flag.
 	// +optional
 	GMSACredentialSpecName *string
 
 	// GMSACredentialSpec is where the GMSA admission webhook
 	// (https://github.com/kubernetes-sigs/windows-gmsa) inlines the contents of the
 	// GMSA credential spec named by the GMSACredentialSpecName field.
-	// This field is alpha-level and is only honored by servers that enable the WindowsGMSA feature flag.
 	// +optional
 	GMSACredentialSpec *string
 

pkg/apis/core/types.go

@@ -632,7 +632,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	ProcMountType:                  {Default: false, PreRelease: featuregate.Alpha},
 	TTLAfterFinished:               {Default: false, PreRelease: featuregate.Alpha},
 	KubeletPodResources:            {Default: true, PreRelease: featuregate.Beta},
-	WindowsGMSA:                    {Default: true, PreRelease: featuregate.Beta},
+	WindowsGMSA:                    {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.20
 	WindowsRunAsUserName:           {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.20
 	ServiceLoadBalancerFinalizer:   {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 	LocalStorageCapacityIsolationFSQuotaMonitoring: {Default: false, PreRelease: featuregate.Alpha},

pkg/features/kube_features.go

@@ -5839,14 +5839,12 @@ type SELinuxOptions struct {
 // WindowsSecurityContextOptions contain Windows-specific options and credentials.
 type WindowsSecurityContextOptions struct {
 	// GMSACredentialSpecName is the name of the GMSA credential spec to use.
-	// This field is alpha-level and is only honored by servers that enable the WindowsGMSA feature flag.
 	// +optional
 	GMSACredentialSpecName *string `json:"gmsaCredentialSpecName,omitempty" protobuf:"bytes,1,opt,name=gmsaCredentialSpecName"`
 
 	// GMSACredentialSpec is where the GMSA admission webhook
 	// (https://github.com/kubernetes-sigs/windows-gmsa) inlines the contents of the
 	// GMSA credential spec named by the GMSACredentialSpecName field.
-	// This field is alpha-level and is only honored by servers that enable the WindowsGMSA feature flag.
 	// +optional
 	GMSACredentialSpec *string `json:"gmsaCredentialSpec,omitempty" protobuf:"bytes,2,opt,name=gmsaCredentialSpec"`
 

staging/src/k8s.io/api/core/v1/generated.proto

@@ -79,7 +79,7 @@ const (
 	gmsaWebhookDeployScriptURL = "https://raw.githubusercontent.com/kubernetes-sigs/windows-gmsa/master/admission-webhook/deploy/deploy-gmsa-webhook.sh"
 )
 
-var _ = SIGDescribe("[Feature:Windows] [Feature:WindowsGMSA] GMSA Full [Slow]", func() {
+var _ = SIGDescribe("[Feature:Windows] GMSA Full [Slow]", func() {
 	f := framework.NewDefaultFramework("gmsa-full-test-windows")
 
 	ginkgo.Describe("GMSA support", func() {

staging/src/k8s.io/api/core/v1/types.go

@@ -35,7 +35,7 @@ import (
 	"github.com/onsi/gomega"
 )
 
-var _ = SIGDescribe("[Feature:Windows] [Feature:WindowsGMSA] GMSA Kubelet [Slow]", func() {
+var _ = SIGDescribe("[Feature:Windows] GMSA Kubelet [Slow]", func() {
 	f := framework.NewDefaultFramework("gmsa-kubelet-test-windows")
 
 	ginkgo.Describe("kubelet GMSA support", func() {