Kubelet & implementation changes for Windows GMSA support

This patch comprises the kubelet changes outlined in the Windows GMSA KEP
(https://github.com/kubernetes/enhancements/blob/master/keps/sig-windows/20181221-windows-group-managed-service-accounts-for-container-identity.md)
to add GMSA support to Windows workloads.

Updated tests.

Signed-off-by: Jean Rouge <rougej+github@gmail.com>

pkg/kubelet/dockershim/docker_container_windows.go

@@ -30,7 +30,6 @@ import (
 	dockercontainer "github.com/docker/docker/api/types/container"
 
 	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
-	"k8s.io/kubernetes/pkg/kubelet/kuberuntime"
 )
 
 type containerCleanupInfo struct {
@@ -54,7 +53,7 @@ func (ds *dockerService) applyPlatformSpecificDockerConfig(request *runtimeapi.C
 	return cleanupInfo, nil
 }
 
-// applyGMSAConfig looks at the kuberuntime.GMSASpecContainerAnnotationKey container annotation; if present,
+// applyGMSAConfig looks at the container's .Windows.SecurityContext.GMSACredentialSpec field; if present,
 // it copies its contents to a unique registry value, and sets a SecurityOpt on the config pointing to that registry value.
 // We use registry values instead of files since their location cannot change - as opposed to credential spec files,
 // whose location could potentially change down the line, or even be unknown (eg if docker is not installed on the
@@ -63,7 +62,10 @@ func (ds *dockerService) applyPlatformSpecificDockerConfig(request *runtimeapi.C
 // as it will avoid cluttering the registry - there is a moby PR out for this:
 // https://github.com/moby/moby/pull/38777
 func applyGMSAConfig(config *runtimeapi.ContainerConfig, createConfig *dockertypes.ContainerCreateConfig, cleanupInfo *containerCleanupInfo) error {
-	credSpec := config.Annotations[kuberuntime.GMSASpecContainerAnnotationKey]
+	var credSpec string
+	if config.Windows != nil && config.Windows.SecurityContext != nil {
+		credSpec = config.Windows.SecurityContext.CredentialSpec
+	}
 	if credSpec == "" {
 		return nil
 	}

pkg/kubelet/dockershim/docker_container_windows_test.go

@@ -73,7 +73,11 @@ func TestApplyGMSAConfig(t *testing.T) {
 	expectedValueName := "k8s-cred-spec-" + expectedHex
 
 	containerConfigWithGMSAAnnotation := &runtimeapi.ContainerConfig{
-		Annotations: map[string]string{"container.alpha.windows.kubernetes.io/gmsa-credential-spec": dummyCredSpec},
+		Windows: &runtimeapi.WindowsContainerConfig{
+			SecurityContext: &runtimeapi.WindowsContainerSecurityContext{
+				CredentialSpec: dummyCredSpec,
+			},
+		},
 	}
 
 	t.Run("happy path", func(t *testing.T) {

pkg/kubelet/kuberuntime/kuberuntime_container_windows.go

@@ -36,12 +36,8 @@ func (m *kubeGenericRuntimeManager) applyPlatformSpecificContainerConfig(config
 	if err != nil {
 		return err
 	}
-
-	if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.WindowsGMSA) {
-		determineEffectiveSecurityContext(config, container, pod)
-	}
-
 	config.Windows = windowsConfig
+
 	return nil
 }
 
@@ -100,43 +96,11 @@ func (m *kubeGenericRuntimeManager) generateWindowsContainerConfig(container *v1
 	if username != "" {
 		wc.SecurityContext.RunAsUsername = username
 	}
+	if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.WindowsGMSA) &&
+		effectiveSc.WindowsOptions != nil &&
+		effectiveSc.WindowsOptions.GMSACredentialSpec != nil {
+		wc.SecurityContext.CredentialSpec = *effectiveSc.WindowsOptions.GMSACredentialSpec
+	}
 
 	return wc, nil
 }
-
-const (
-	// GMSASpecContainerAnnotationKey is the container annotation where we store the contents of the GMSA credential spec to use.
-	GMSASpecContainerAnnotationKey = "container.alpha.windows.kubernetes.io/gmsa-credential-spec"
-	// gMSAContainerSpecPodAnnotationKeySuffix is the suffix of the pod annotation where the GMSA webhook admission controller
-	// stores the contents of the GMSA credential spec for a given container (the full annotation being the container's name
-	// with this suffix appended).
-	gMSAContainerSpecPodAnnotationKeySuffix = "." + GMSASpecContainerAnnotationKey
-	// gMSAPodSpecPodAnnotationKey is the pod annotation where the GMSA webhook admission controller stores the contents of the GMSA
-	// credential spec to use for containers that do not have their own specific GMSA cred spec set via a
-	// gMSAContainerSpecPodAnnotationKeySuffix annotation as explained above
-	gMSAPodSpecPodAnnotationKey = "pod.alpha.windows.kubernetes.io/gmsa-credential-spec"
-)
-
-// determineEffectiveSecurityContext determines the effective GMSA credential spec and, if any, copies it to the container's
-// GMSASpecContainerAnnotationKey annotation.
-func determineEffectiveSecurityContext(config *runtimeapi.ContainerConfig, container *v1.Container, pod *v1.Pod) {
-	var containerCredSpec string
-
-	containerGMSAPodAnnotation := container.Name + gMSAContainerSpecPodAnnotationKeySuffix
-	if pod.Annotations[containerGMSAPodAnnotation] != "" {
-		containerCredSpec = pod.Annotations[containerGMSAPodAnnotation]
-	} else if pod.Annotations[gMSAPodSpecPodAnnotationKey] != "" {
-		containerCredSpec = pod.Annotations[gMSAPodSpecPodAnnotationKey]
-	}
-
-	if containerCredSpec != "" {
-		if config.Annotations == nil {
-			config.Annotations = make(map[string]string)
-		}
-		config.Annotations[GMSASpecContainerAnnotationKey] = containerCredSpec
-	} else {
-		// the annotation shouldn't be present, but let's err on the side of caution:
-		// it should only be set here and nowhere else
-		delete(config.Annotations, GMSASpecContainerAnnotationKey)
-	}
-}

pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go

@@ -1,82 +0,0 @@
-/*
-Copyright 2019 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package kuberuntime
-
-import (
-	"github.com/stretchr/testify/assert"
-	"testing"
-
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
-)
-
-func TestDetermineEffectiveSecurityContext(t *testing.T) {
-	containerName := "container_name"
-	container := &corev1.Container{Name: containerName}
-	dummyCredSpec := "test cred spec contents"
-
-	buildPod := func(annotations map[string]string) *corev1.Pod {
-		return &corev1.Pod{
-			ObjectMeta: metav1.ObjectMeta{
-				Annotations: annotations,
-			},
-		}
-	}
-
-	t.Run("when there's a specific GMSA for that container, and no pod-wide GMSA", func(t *testing.T) {
-		containerConfig := &runtimeapi.ContainerConfig{}
-
-		pod := buildPod(map[string]string{
-			"container_name.container.alpha.windows.kubernetes.io/gmsa-credential-spec": dummyCredSpec,
-		})
-
-		determineEffectiveSecurityContext(containerConfig, container, pod)
-
-		assert.Equal(t, dummyCredSpec, containerConfig.Annotations["container.alpha.windows.kubernetes.io/gmsa-credential-spec"])
-	})
-	t.Run("when there's a specific GMSA for that container, and a pod-wide GMSA", func(t *testing.T) {
-		containerConfig := &runtimeapi.ContainerConfig{}
-
-		pod := buildPod(map[string]string{
-			"container_name.container.alpha.windows.kubernetes.io/gmsa-credential-spec": dummyCredSpec,
-			"pod.alpha.windows.kubernetes.io/gmsa-credential-spec":                      "should be ignored",
-		})
-
-		determineEffectiveSecurityContext(containerConfig, container, pod)
-
-		assert.Equal(t, dummyCredSpec, containerConfig.Annotations["container.alpha.windows.kubernetes.io/gmsa-credential-spec"])
-	})
-	t.Run("when there's no specific GMSA for that container, and a pod-wide GMSA", func(t *testing.T) {
-		containerConfig := &runtimeapi.ContainerConfig{}
-
-		pod := buildPod(map[string]string{
-			"pod.alpha.windows.kubernetes.io/gmsa-credential-spec": dummyCredSpec,
-		})
-
-		determineEffectiveSecurityContext(containerConfig, container, pod)
-
-		assert.Equal(t, dummyCredSpec, containerConfig.Annotations["container.alpha.windows.kubernetes.io/gmsa-credential-spec"])
-	})
-	t.Run("when there's no specific GMSA for that container, and no pod-wide GMSA", func(t *testing.T) {
-		containerConfig := &runtimeapi.ContainerConfig{}
-
-		determineEffectiveSecurityContext(containerConfig, container, &corev1.Pod{})
-
-		assert.Nil(t, containerConfig.Annotations)
-	})
-}

pkg/securitycontext/util.go

@@ -66,6 +66,18 @@ func DetermineEffectiveSecurityContext(pod *v1.Pod, container *v1.Container) *v1
 		*effectiveSc.SELinuxOptions = *containerSc.SELinuxOptions
 	}
 
+	if containerSc.WindowsOptions != nil {
+		// only override fields that are set at the container level, not the whole thing
+		if effectiveSc.WindowsOptions == nil {
+			effectiveSc.WindowsOptions = &v1.WindowsSecurityContextOptions{}
+		}
+		if containerSc.WindowsOptions.GMSACredentialSpecName != nil || containerSc.WindowsOptions.GMSACredentialSpec != nil {
+			// both GMSA fields go hand in hand
+			effectiveSc.WindowsOptions.GMSACredentialSpecName = containerSc.WindowsOptions.GMSACredentialSpecName
+			effectiveSc.WindowsOptions.GMSACredentialSpec = containerSc.WindowsOptions.GMSACredentialSpec
+		}
+	}
+
 	if containerSc.Capabilities != nil {
 		effectiveSc.Capabilities = new(v1.Capabilities)
 		*effectiveSc.Capabilities = *containerSc.Capabilities
@@ -120,6 +132,12 @@ func securityContextFromPodSecurityContext(pod *v1.Pod) *v1.SecurityContext {
 		synthesized.SELinuxOptions = &v1.SELinuxOptions{}
 		*synthesized.SELinuxOptions = *pod.Spec.SecurityContext.SELinuxOptions
 	}
+
+	if pod.Spec.SecurityContext.WindowsOptions != nil {
+		synthesized.WindowsOptions = &v1.WindowsSecurityContextOptions{}
+		*synthesized.WindowsOptions = *pod.Spec.SecurityContext.WindowsOptions
+	}
+
 	if pod.Spec.SecurityContext.RunAsUser != nil {
 		synthesized.RunAsUser = new(int64)
 		*synthesized.RunAsUser = *pod.Spec.SecurityContext.RunAsUser

test/e2e/windows/gmsa.go

@@ -46,24 +46,31 @@ var _ = SIGDescribe("[Feature:Windows] [Feature:WindowsGMSA] GMSA [Slow]", func(
 				container2Name := "container2"
 				container2Domain := "contoso.org"
 
-				containers := make([]corev1.Container, 2)
-				for i, name := range []string{container1Name, container2Name} {
-					containers[i] = corev1.Container{
-						Name:  name,
-						Image: imageutils.GetPauseImageName(),
-					}
-				}
-
 				pod := &corev1.Pod{
 					ObjectMeta: metav1.ObjectMeta{
 						Name: podName,
-						Annotations: map[string]string{
-							"pod.alpha.windows.kubernetes.io/gmsa-credential-spec":                         generateDummyCredSpecs(podDomain),
-							container2Name + ".container.alpha.windows.kubernetes.io/gmsa-credential-spec": generateDummyCredSpecs(container2Domain),
-						},
 					},
 					Spec: corev1.PodSpec{
-						Containers: containers,
+						Containers: []corev1.Container{
+							{
+								Name:  container1Name,
+								Image: imageutils.GetPauseImageName(),
+							},
+							{
+								Name:  container2Name,
+								Image: imageutils.GetPauseImageName(),
+								SecurityContext: &corev1.SecurityContext{
+									WindowsOptions: &corev1.WindowsSecurityContextOptions{
+										GMSACredentialSpec: generateDummyCredSpecs(container2Domain),
+									},
+								},
+							},
+						},
+						SecurityContext: &corev1.PodSecurityContext{
+							WindowsOptions: &corev1.WindowsSecurityContextOptions{
+								GMSACredentialSpec: generateDummyCredSpecs(podDomain),
+							},
+						},
 					},
 				}
 
@@ -108,10 +115,10 @@ var _ = SIGDescribe("[Feature:Windows] [Feature:WindowsGMSA] GMSA [Slow]", func(
 	})
 })
 
-func generateDummyCredSpecs(domain string) string {
+func generateDummyCredSpecs(domain string) *string {
 	shortName := strings.ToUpper(strings.Split(domain, ".")[0])
 
-	return fmt.Sprintf(`{
+	credSpecs := fmt.Sprintf(`{
        "ActiveDirectoryConfig":{
           "GroupManagedServiceAccounts":[
              {
@@ -136,4 +143,6 @@ func generateDummyCredSpecs(domain string) string {
           "Sid":"S-1-5-21-2126729477-2524175714-3194792973"
        }
     }`, shortName, domain, domain, domain, shortName)
+
+	return &credSpecs
 }