Skip to content

Commit

Permalink
Fixing issue in generatePodSandboxWindowsConfig for hostProcess conta…
Browse files Browse the repository at this point in the history 
…iners 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 <marosset@microsoft.com>
  • Loading branch information
@marosset
marosset committed Jun 2, 2022
1 parent 5379417 commit 0c60888
Show file tree
Hide file tree
Showing 3 changed files with 257 additions and 16 deletions.
213 changes: 213 additions & 0 deletions pkg/kubelet/container/helpers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
39 changes: 23 additions & 16 deletions pkg/kubelet/kuberuntime/kuberuntime_sandbox.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
21 changes: 21 additions & 0 deletions pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 0c60888

Please sign in to comment.