From 0c6088861b9d3b59d0cb0ddb4e7c508107a05663 Mon Sep 17 00:00:00 2001 From: Mark Rossetti Date: Wed, 18 May 2022 16:54:51 -0700 Subject: [PATCH] Fixing issue in generatePodSandboxWindowsConfig for hostProcess containers by where pod sandbox won't have HostProcess bit set if pod does not have a security context but containers specify HostProcess. Signed-off-by: Mark Rossetti --- pkg/kubelet/container/helpers_test.go | 213 ++++++++++++++++++ .../kuberuntime/kuberuntime_sandbox.go | 39 ++-- .../kuberuntime/kuberuntime_sandbox_test.go | 21 ++ 3 files changed, 257 insertions(+), 16 deletions(-) diff --git a/pkg/kubelet/container/helpers_test.go b/pkg/kubelet/container/helpers_test.go index 42baf048008a..9e894b2072f8 100644 --- a/pkg/kubelet/container/helpers_test.go +++ b/pkg/kubelet/container/helpers_test.go @@ -699,3 +699,216 @@ func TestShouldRecordEvent(t *testing.T) { _, actual = innerEventRecorder.shouldRecordEvent(nilObj) assert.Equal(t, false, actual, "should not panic if the typed nil was used, see https://github.com/kubernetes/kubernetes/issues/95552") } + +func TestHasWindowsHostProcessContainer(t *testing.T) { + trueVar := true + falseVar := false + const containerName = "container" + + testCases := []struct { + name string + podSpec *v1.PodSpec + expectedResult bool + }{ + { + name: "hostprocess not set anywhere", + podSpec: &v1.PodSpec{ + Containers: []v1.Container{{ + Name: containerName, + }}, + }, + expectedResult: false, + }, + { + name: "pod with hostprocess=false", + podSpec: &v1.PodSpec{ + HostNetwork: true, + SecurityContext: &v1.PodSecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &falseVar, + }, + }, + Containers: []v1.Container{{ + Name: containerName, + }}, + }, + expectedResult: false, + }, + { + name: "pod with hostprocess=true", + podSpec: &v1.PodSpec{ + HostNetwork: true, + SecurityContext: &v1.PodSecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &trueVar, + }, + }, + Containers: []v1.Container{{ + Name: containerName, + }}, + }, + expectedResult: true, + }, + { + name: "container with hostprocess=false", + podSpec: &v1.PodSpec{ + HostNetwork: true, + Containers: []v1.Container{{ + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &falseVar, + }, + }, + }}, + }, + expectedResult: false, + }, + { + name: "container with hostprocess=true", + podSpec: &v1.PodSpec{ + HostNetwork: true, + Containers: []v1.Container{{ + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &trueVar, + }, + }, + }}, + }, + expectedResult: true, + }, + { + name: "pod with hostprocess=false, container with hostprocess=true", + podSpec: &v1.PodSpec{ + HostNetwork: true, + SecurityContext: &v1.PodSecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &falseVar, + }, + }, + Containers: []v1.Container{{ + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &trueVar, + }, + }, + }}, + }, + expectedResult: true, + }, + { + name: "pod with hostprocess=true, container with hostprocess=flase", + podSpec: &v1.PodSpec{ + HostNetwork: true, + SecurityContext: &v1.PodSecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &trueVar, + }, + }, + Containers: []v1.Container{{ + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &falseVar, + }, + }, + }}, + }, + expectedResult: false, + }, + { + name: "containers with hostproces=mixed", + podSpec: &v1.PodSpec{ + Containers: []v1.Container{ + { + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &falseVar, + }, + }, + }, + { + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &trueVar, + }, + }, + }, + }, + }, + expectedResult: true, + }, + { + name: "pod with hostProcess=false, containers with hostproces=mixed", + podSpec: &v1.PodSpec{ + SecurityContext: &v1.PodSecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &falseVar, + }, + }, + Containers: []v1.Container{ + { + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &falseVar, + }, + }, + }, + { + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &trueVar, + }, + }, + }, + }, + }, + expectedResult: true, + }, + { + name: "pod with hostProcess=true, containers with hostproces=mixed", + podSpec: &v1.PodSpec{ + SecurityContext: &v1.PodSecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &trueVar, + }, + }, + Containers: []v1.Container{ + { + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &falseVar, + }, + }, + }, + { + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &trueVar, + }, + }, + }, + }, + }, + expectedResult: true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + pod := &v1.Pod{} + pod.Spec = *testCase.podSpec + result := HasWindowsHostProcessContainer(pod) + assert.Equal(t, result, testCase.expectedResult) + }) + } +} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go index b03ac0f1c392..e6c8fc350124 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go @@ -228,6 +228,29 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxWindowsConfig(pod *v1.Pod) SecurityContext: &runtimeapi.WindowsSandboxSecurityContext{}, } + // If all of the containers in a pod are HostProcess containers, set the pod's HostProcess field + // explicitly because the container runtime requires this information at sandbox creation time. + if kubecontainer.HasWindowsHostProcessContainer(pod) { + // Pods containing HostProcess containers should fail to schedule if feature is not + // enabled instead of trying to schedule containers as regular containers as stated in + // PRR review. + if !utilfeature.DefaultFeatureGate.Enabled(features.WindowsHostProcessContainers) { + return nil, fmt.Errorf("pod contains HostProcess containers but feature 'WindowsHostProcessContainers' is not enabled") + } + + // At present Windows all containers in a Windows pod must be HostProcess containers + // and HostNetwork is required to be set. + if !kubecontainer.AllContainersAreWindowsHostProcess(pod) { + return nil, fmt.Errorf("pod must not contain both HostProcess and non-HostProcess containers") + } + + if !kubecontainer.IsHostNetworkPod(pod) { + return nil, fmt.Errorf("hostNetwork is required if Pod contains HostProcess containers") + } + + wc.SecurityContext.HostProcess = true + } + sc := pod.Spec.SecurityContext if sc == nil || sc.WindowsOptions == nil { return wc, nil @@ -243,26 +266,10 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxWindowsConfig(pod *v1.Pod) } if kubecontainer.HasWindowsHostProcessContainer(pod) { - // Pods containing HostProcess containers should fail to schedule if feature is not - // enabled instead of trying to schedule containers as regular containers as stated in - // PRR review. - if !utilfeature.DefaultFeatureGate.Enabled(features.WindowsHostProcessContainers) { - return nil, fmt.Errorf("pod contains HostProcess containers but feature 'WindowsHostProcessContainers' is not enabled") - } if wo.HostProcess != nil && !*wo.HostProcess { return nil, fmt.Errorf("pod must not contain any HostProcess containers if Pod's WindowsOptions.HostProcess is set to false") } - // At present Windows all containers in a Windows pod must be HostProcess containers - // and HostNetwork is required to be set. - if !kubecontainer.AllContainersAreWindowsHostProcess(pod) { - return nil, fmt.Errorf("pod must not contain both HostProcess and non-HostProcess containers") - } - if !kubecontainer.IsHostNetworkPod(pod) { - return nil, fmt.Errorf("hostNetwork is required if Pod contains HostProcess containers") - } - - wc.SecurityContext.HostProcess = true } return wc, nil diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go index 1d36a3e6db14..00d3b3281f38 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go @@ -345,6 +345,27 @@ func TestGeneratePodSandboxWindowsConfig(t *testing.T) { expectedWindowsConfig: nil, expectedError: fmt.Errorf("pod must not contain any HostProcess containers if Pod's WindowsOptions.HostProcess is set to false"), }, + { + name: "Pod's security context doesn't specify HostProcess containers but Container's security context does", + hostProcessFeatureEnabled: true, + podSpec: &v1.PodSpec{ + HostNetwork: true, + Containers: []v1.Container{{ + Name: containerName, + SecurityContext: &v1.SecurityContext{ + WindowsOptions: &v1.WindowsSecurityContextOptions{ + HostProcess: &trueVar, + }, + }, + }}, + }, + expectedWindowsConfig: &runtimeapi.WindowsPodSandboxConfig{ + SecurityContext: &runtimeapi.WindowsSandboxSecurityContext{ + HostProcess: true, + }, + }, + expectedError: nil, + }, } for _, testCase := range testCases {