From b39d8f4777b3a01359046d05b1de17298b818f2f Mon Sep 17 00:00:00 2001 From: Jean Rouge Date: Thu, 16 May 2019 15:34:35 -0700 Subject: [PATCH] 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 --- .../dockershim/docker_container_windows.go | 8 +- .../docker_container_windows_test.go | 6 +- .../kuberuntime_container_windows.go | 48 ++--------- .../kuberuntime_container_windows_test.go | 82 ------------------- pkg/securitycontext/util.go | 18 ++++ test/e2e/windows/gmsa.go | 39 +++++---- 6 files changed, 58 insertions(+), 143 deletions(-) delete mode 100644 pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go diff --git a/pkg/kubelet/dockershim/docker_container_windows.go b/pkg/kubelet/dockershim/docker_container_windows.go index d14168610f5..4276d7c531f 100644 --- a/pkg/kubelet/dockershim/docker_container_windows.go +++ b/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 } diff --git a/pkg/kubelet/dockershim/docker_container_windows_test.go b/pkg/kubelet/dockershim/docker_container_windows_test.go index 2b25ebfbf97..d6c39bcda3f 100644 --- a/pkg/kubelet/dockershim/docker_container_windows_test.go +++ b/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) { diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go b/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go index db03eae34e3..e99660c3c2f 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go +++ b/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) - } -} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go deleted file mode 100644 index f4ea8e4dd5d..00000000000 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_windows_test.go +++ /dev/null @@ -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) - }) -} diff --git a/pkg/securitycontext/util.go b/pkg/securitycontext/util.go index a39ee7571a8..6a012076dc9 100644 --- a/pkg/securitycontext/util.go +++ b/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 diff --git a/test/e2e/windows/gmsa.go b/test/e2e/windows/gmsa.go index 687a600f257..21aef8bd5e4 100644 --- a/test/e2e/windows/gmsa.go +++ b/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 }