From cd8686bc57fe2a1e5fcf4b1c0bb82419125ab4d1 Mon Sep 17 00:00:00 2001 From: ravisantoshgudimetla Date: Mon, 27 Jul 2020 11:57:06 -0400 Subject: [PATCH] Strip unnecessary security contexts on Windows As of now, the kubelet is passing the security context to container runtime even if the security context has invalid options for a particular OS. As a result, the pod fails to come up on the node. This error is particularly pronounced on the Windows nodes where kubelet is allowing Linux specific options like SELinux, RunAsUser etc where as in [documentation](https://kubernetes.io/docs/setup/production-environment/windows/intro-windows-in-kubernetes/#v1-container), we clearly state they are not supported. This PR ensures that the kubelet strips the security contexts of the pod, if they don't make sense on the Windows OS. --- .../kuberuntime_container_windows.go | 6 +---- .../kuberuntime/kuberuntime_sandbox.go | 12 ++++++--- test/e2e/windows/security_context.go | 26 ++++++++++++++++++- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go b/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go index eeb08d79bf1..49fec206c4a 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_windows.go @@ -19,7 +19,6 @@ limitations under the License. package kuberuntime import ( - "fmt" "runtime" "k8s.io/api/core/v1" @@ -125,10 +124,7 @@ func (m *kubeGenericRuntimeManager) generateWindowsContainerConfig(container *v1 // setup security context effectiveSc := securitycontext.DetermineEffectiveSecurityContext(pod, container) - // RunAsUser only supports int64 from Kubernetes API, but Windows containers only support username. - if effectiveSc.RunAsUser != nil { - return nil, fmt.Errorf("run as uid (%d) is not supported on Windows", *effectiveSc.RunAsUser) - } + if username != "" { wc.SecurityContext.RunAsUsername = username } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go index 0978044f753..68a38125367 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go @@ -20,6 +20,7 @@ import ( "fmt" "net" "net/url" + "runtime" "sort" v1 "k8s.io/api/core/v1" @@ -143,6 +144,9 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxConfig(pod *v1.Pod, attemp } // generatePodSandboxLinuxConfig generates LinuxPodSandboxConfig from v1.Pod. +// We've to call PodSandboxLinuxConfig always irrespective of the underlying OS as securityContext is not part of +// podSandboxConfig. It is currently part of LinuxPodSandboxConfig. In future, if we have securityContext pulled out +// in podSandboxConfig we should be able to use it. func (m *kubeGenericRuntimeManager) generatePodSandboxLinuxConfig(pod *v1.Pod) (*runtimeapi.LinuxPodSandboxConfig, error) { cgroupParent := m.runtimeHelper.GetPodCgroupParent(pod) lc := &runtimeapi.LinuxPodSandboxConfig{ @@ -169,15 +173,15 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxLinuxConfig(pod *v1.Pod) ( if pod.Spec.SecurityContext != nil { sc := pod.Spec.SecurityContext - if sc.RunAsUser != nil { + if sc.RunAsUser != nil && runtime.GOOS != "windows" { lc.SecurityContext.RunAsUser = &runtimeapi.Int64Value{Value: int64(*sc.RunAsUser)} } - if sc.RunAsGroup != nil { + if sc.RunAsGroup != nil && runtime.GOOS != "windows" { lc.SecurityContext.RunAsGroup = &runtimeapi.Int64Value{Value: int64(*sc.RunAsGroup)} } lc.SecurityContext.NamespaceOptions = namespacesForPod(pod) - if sc.FSGroup != nil { + if sc.FSGroup != nil && runtime.GOOS != "windows" { lc.SecurityContext.SupplementalGroups = append(lc.SecurityContext.SupplementalGroups, int64(*sc.FSGroup)) } if groups := m.runtimeHelper.GetExtraSupplementalGroupsForPod(pod); len(groups) > 0 { @@ -188,7 +192,7 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxLinuxConfig(pod *v1.Pod) ( lc.SecurityContext.SupplementalGroups = append(lc.SecurityContext.SupplementalGroups, int64(sg)) } } - if sc.SELinuxOptions != nil { + if sc.SELinuxOptions != nil && runtime.GOOS != "windows" { lc.SecurityContext.SelinuxOptions = &runtimeapi.SELinuxOption{ User: sc.SELinuxOptions.User, Role: sc.SELinuxOptions.Role, diff --git a/test/e2e/windows/security_context.go b/test/e2e/windows/security_context.go index ff100d7ba20..1f316d3520c 100644 --- a/test/e2e/windows/security_context.go +++ b/test/e2e/windows/security_context.go @@ -31,7 +31,7 @@ import ( const runAsUserNameContainerName = "run-as-username-container" -var _ = SIGDescribe("[Feature:Windows] SecurityContext RunAsUserName", func() { +var _ = SIGDescribe("[Feature:Windows] SecurityContext", func() { f := framework.NewDefaultFramework("windows-run-as-username") ginkgo.It("should be able create pods and run containers with a given username", func() { @@ -71,6 +71,29 @@ var _ = SIGDescribe("[Feature:Windows] SecurityContext RunAsUserName", func() { f.TestContainerOutput("check overridden username", pod, 0, []string{"ContainerUser"}) f.TestContainerOutput("check pod SecurityContext username", pod, 1, []string{"ContainerAdministrator"}) }) + ginkgo.It("should ignore Linux Specific SecurityContext if set", func() { + ginkgo.By("Creating a pod with SELinux options") + // It is sufficient to show that the pod comes up here. Since we're stripping the SELinux and other linux + // security contexts in apiserver and not updating the pod object in the apiserver, we cannot validate the + // the pod object to not have those security contexts. However the pod coming to running state is a sufficient + // enough condition for us to validate since prior to https://github.com/kubernetes/kubernetes/pull/93475 + // the pod would have failed to come up. + windowsPodWithSELinux := createTestPod(f, windowsBusyBoximage, windowsOS) + windowsPodWithSELinux.Spec.Containers[0].Args = []string{"test-webserver-with-selinux"} + windowsPodWithSELinux.Spec.SecurityContext = &v1.PodSecurityContext{} + containerUserName := "ContainerAdministrator" + windowsPodWithSELinux.Spec.SecurityContext.SELinuxOptions = &v1.SELinuxOptions{Level: "s0:c24,c9"} + windowsPodWithSELinux.Spec.Containers[0].SecurityContext = &v1.SecurityContext{ + SELinuxOptions: &v1.SELinuxOptions{Level: "s0:c24,c9"}, + WindowsOptions: &v1.WindowsSecurityContextOptions{RunAsUserName: &containerUserName}} + windowsPodWithSELinux.Spec.Tolerations = []v1.Toleration{{Key: "os", Value: "Windows"}} + windowsPodWithSELinux, err := f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(context.TODO(), + windowsPodWithSELinux, metav1.CreateOptions{}) + framework.ExpectNoError(err) + framework.Logf("Created pod %v", windowsPodWithSELinux) + framework.ExpectNoError(e2epod.WaitForPodNameRunningInNamespace(f.ClientSet, windowsPodWithSELinux.Name, + f.Namespace.Name), "failed to wait for pod %s to be running", windowsPodWithSELinux.Name) + }) }) func runAsUserNamePod(username *string) *v1.Pod { @@ -80,6 +103,7 @@ func runAsUserNamePod(username *string) *v1.Pod { Name: podName, }, Spec: v1.PodSpec{ + NodeSelector: map[string]string{"kubernetes.io/os": "windows"}, Containers: []v1.Container{ { Name: runAsUserNameContainerName,