Skip to content

Commit ac38852

Browse files
dcantahdcantah
committed
Add validations for Windows HostProcess CRI configs
HostProcess containers require every container in the pod to be a host process container and have the corresponding field set. The Kubelet usually enforces this so we'd error before even getting here but we recently found a bug in this logic so better to be safe than sorry. Signed-off-by: Daniel Canter <[email protected]> (cherry picked from commit 978ff39) Signed-off-by: Daniel Canter <[email protected]>
1 parent d972571 commit ac38852

4 files changed

Lines changed: 84 additions & 10 deletions

File tree

‎integration/main_test.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,8 @@ func PodSandboxConfigWithCleanup(t *testing.T, name, ns string, opts ...PodSandb
194194
return sb, sbConfig
195195
}
196196

197-
// Set Windows HostProcess.
198-
func WithWindowsHostProcess(p *runtime.PodSandboxConfig) { //nolint:unused
197+
// Set Windows HostProcess on the pod.
198+
func WithWindowsHostProcessPod(p *runtime.PodSandboxConfig) { //nolint:unused
199199
if p.Windows == nil {
200200
p.Windows = &runtime.WindowsPodSandboxConfig{}
201201
}
@@ -252,6 +252,18 @@ func WithWindowsUsername(username string) ContainerOpts { //nolint:unused
252252
}
253253
}
254254

255+
func WithWindowsHostProcessContainer() ContainerOpts { //nolint:unused
256+
return func(c *runtime.ContainerConfig) {
257+
if c.Windows == nil {
258+
c.Windows = &runtime.WindowsContainerConfig{}
259+
}
260+
if c.Windows.SecurityContext == nil {
261+
c.Windows.SecurityContext = &runtime.WindowsContainerSecurityContext{}
262+
}
263+
c.Windows.SecurityContext.HostProcess = true
264+
}
265+
}
266+
255267
// Add container command.
256268
func WithCommand(cmd string, args ...string) ContainerOpts {
257269
return func(c *runtime.ContainerConfig) {

‎integration/windows_hostprocess_test.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,41 +32,42 @@ var (
3232
defaultCommand = WithCommand("Powershell", "/c", "$env:CONTAINER_SANDBOX_MOUNT_POINT/pause.exe")
3333
localServiceUsername = WithWindowsUsername("NT AUTHORITY\\Local service")
3434
localSystemUsername = WithWindowsUsername("NT AUTHORITY\\System")
35+
hpcContainerOpt = WithWindowsHostProcessContainer()
3536
)
3637

3738
// Tests to verify the Windows HostProcess
3839
func TestWindowsHostProcess(t *testing.T) {
3940
EnsureImageExists(t, pauseImage)
4041

4142
t.Run("run as Local Service", func(t *testing.T) {
42-
runHostProcess(t, false, pauseImage, localServiceUsername, defaultCommand)
43+
runHostProcess(t, false, pauseImage, hpcContainerOpt, localServiceUsername, defaultCommand)
4344
})
4445
t.Run("run as Local System", func(t *testing.T) {
45-
runHostProcess(t, false, pauseImage, localSystemUsername, defaultCommand)
46+
runHostProcess(t, false, pauseImage, hpcContainerOpt, localSystemUsername, defaultCommand)
4647
})
4748
t.Run("run as unacceptable user", func(t *testing.T) {
48-
runHostProcess(t, true, pauseImage, WithWindowsUsername("Guest"), defaultCommand)
49+
runHostProcess(t, true, pauseImage, hpcContainerOpt, WithWindowsUsername("Guest"), defaultCommand)
4950
})
5051
t.Run("run command on host", func(t *testing.T) {
5152
cmd := WithCommand("Powershell", "/c", "Get-Command containerd.exe")
52-
runHostProcess(t, false, pauseImage, localServiceUsername, cmd)
53+
runHostProcess(t, false, pauseImage, hpcContainerOpt, localServiceUsername, cmd)
5354
})
5455
t.Run("run withHostNetwork", func(t *testing.T) {
5556
hostname, err := os.Hostname()
5657
require.NoError(t, err)
5758
cmd := WithCommand("Powershell", "/c", fmt.Sprintf("if ($env:COMPUTERNAME -ne %s) { exit -1 }", hostname))
58-
runHostProcess(t, false, pauseImage, localServiceUsername, cmd)
59+
runHostProcess(t, false, pauseImage, hpcContainerOpt, localServiceUsername, cmd)
5960
})
6061
t.Run("run with a different os.version image", func(t *testing.T) {
6162
image := "docker.io/e2eteam/busybox:1.29-windows-amd64-1909"
6263
EnsureImageExists(t, image)
63-
runHostProcess(t, false, image, localServiceUsername, defaultCommand)
64+
runHostProcess(t, false, image, hpcContainerOpt, localServiceUsername, defaultCommand)
6465
})
6566
}
6667

6768
func runHostProcess(t *testing.T, expectErr bool, image string, opts ...ContainerOpts) {
6869
t.Logf("Create a pod config and run sandbox container")
69-
sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox1", "hostprocess", WithWindowsHostProcess)
70+
sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox1", "hostprocess", WithWindowsHostProcessPod)
7071

7172
t.Logf("Create a container config and run container in a pod")
7273
containerConfig := ContainerConfig(

‎pkg/cri/server/container_create_windows.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package server
1818

1919
import (
20+
"errors"
2021
"strconv"
2122

2223
"github.com/containerd/containerd/oci"
@@ -50,6 +51,16 @@ func (c *criService) containerSpec(
5051
specOpts := []oci.SpecOpts{
5152
customopts.WithProcessArgs(config, imageConfig),
5253
}
54+
55+
// All containers in a pod need to have HostProcess set if it was set on the pod,
56+
// and vice versa no containers in the pod can be HostProcess if the pods spec
57+
// didn't have the field set. The only case that is valid is if these are the same value.
58+
cntrHpc := config.GetWindows().GetSecurityContext().GetHostProcess()
59+
sandboxHpc := sandboxConfig.GetWindows().GetSecurityContext().GetHostProcess()
60+
if cntrHpc != sandboxHpc {
61+
return nil, errors.New("pod spec and all containers inside must have the HostProcess field set to be valid")
62+
}
63+
5364
if config.GetWorkingDir() != "" {
5465
specOpts = append(specOpts, oci.WithProcessCwd(config.GetWorkingDir()))
5566
} else if imageConfig.WorkingDir != "" {
@@ -120,7 +131,7 @@ func (c *criService) containerSpec(
120131
customopts.WithAnnotation(annotations.SandboxName, sandboxConfig.GetMetadata().GetName()),
121132
customopts.WithAnnotation(annotations.ContainerName, containerName),
122133
customopts.WithAnnotation(annotations.ImageName, imageName),
123-
customopts.WithAnnotation(annotations.WindowsHostProcess, strconv.FormatBool(sandboxConfig.GetWindows().GetSecurityContext().GetHostProcess())),
134+
customopts.WithAnnotation(annotations.WindowsHostProcess, strconv.FormatBool(sandboxHpc)),
124135
)
125136
return c.runtimeSpec(id, ociRuntime.BaseRuntimeSpec, specOpts...)
126137
}

‎pkg/cri/server/container_create_windows_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ func getCreateContainerTestData() (*runtime.ContainerConfig, *runtime.PodSandbox
8383
Namespace: "test-sandbox-ns",
8484
Attempt: 2,
8585
},
86+
Windows: &runtime.WindowsPodSandboxConfig{},
8687
Hostname: "test-hostname",
8788
Annotations: map[string]string{"c": "d"},
8889
}
@@ -195,3 +196,52 @@ func TestMountNamedPipe(t *testing.T) {
195196
specCheck(t, testID, testSandboxID, testPid, spec)
196197
checkMount(t, spec.Mounts, `\\.\pipe\foo`, `\\.\pipe\foo`, "", []string{"rw"}, nil)
197198
}
199+
200+
func TestHostProcessRequirements(t *testing.T) {
201+
testID := "test-id"
202+
testSandboxID := "sandbox-id"
203+
testContainerName := "container-name"
204+
testPid := uint32(1234)
205+
containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData()
206+
ociRuntime := config.Runtime{}
207+
c := newTestCRIService()
208+
for desc, test := range map[string]struct {
209+
containerHostProcess bool
210+
sandboxHostProcess bool
211+
expectError bool
212+
}{
213+
"hostprocess container in non-hostprocess sandbox should fail": {
214+
containerHostProcess: true,
215+
sandboxHostProcess: false,
216+
expectError: true,
217+
},
218+
"hostprocess container in hostprocess sandbox should be fine": {
219+
containerHostProcess: true,
220+
sandboxHostProcess: true,
221+
expectError: false,
222+
},
223+
"non-hostprocess container in hostprocess sandbox should fail": {
224+
containerHostProcess: false,
225+
sandboxHostProcess: true,
226+
expectError: true,
227+
},
228+
"non-hostprocess container in non-hostprocess sandbox should be fine": {
229+
containerHostProcess: false,
230+
sandboxHostProcess: false,
231+
expectError: false,
232+
},
233+
} {
234+
t.Run(desc, func(t *testing.T) {
235+
containerConfig.Windows.SecurityContext.HostProcess = test.containerHostProcess
236+
sandboxConfig.Windows.SecurityContext = &runtime.WindowsSandboxSecurityContext{
237+
HostProcess: test.sandboxHostProcess,
238+
}
239+
_, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
240+
if test.expectError {
241+
assert.Error(t, err)
242+
} else {
243+
assert.NoError(t, err)
244+
}
245+
})
246+
}
247+
}

0 commit comments

Comments
 (0)