Merge pull request kubernetes#93475 from ravisantoshgudimetla/fix-kubelet-scc

Strip unnecessary security contexts on Windows

Author: k8s-ci-robot

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
 	}

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,

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,