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 <dcanter@microsoft.com>
This commit is contained in:
Daniel Canter 2022-05-26 23:34:41 -07:00
parent c4e29027d4
commit 978ff393d2
4 changed files with 84 additions and 10 deletions

View File

@ -194,8 +194,8 @@ func PodSandboxConfigWithCleanup(t *testing.T, name, ns string, opts ...PodSandb
return sb, sbConfig
}
// Set Windows HostProcess.
func WithWindowsHostProcess(p *runtime.PodSandboxConfig) { //nolint:unused
// Set Windows HostProcess on the pod.
func WithWindowsHostProcessPod(p *runtime.PodSandboxConfig) { //nolint:unused
if p.Windows == nil {
p.Windows = &runtime.WindowsPodSandboxConfig{}
}
@ -252,6 +252,18 @@ func WithWindowsUsername(username string) ContainerOpts { //nolint:unused
}
}
func WithWindowsHostProcessContainer() ContainerOpts { //nolint:unused
return func(c *runtime.ContainerConfig) {
if c.Windows == nil {
c.Windows = &runtime.WindowsContainerConfig{}
}
if c.Windows.SecurityContext == nil {
c.Windows.SecurityContext = &runtime.WindowsContainerSecurityContext{}
}
c.Windows.SecurityContext.HostProcess = true
}
}
// Add container command.
func WithCommand(cmd string, args ...string) ContainerOpts {
return func(c *runtime.ContainerConfig) {

View File

@ -32,6 +32,7 @@ var (
defaultCommand = WithCommand("Powershell", "/c", "$env:CONTAINER_SANDBOX_MOUNT_POINT/pause.exe")
localServiceUsername = WithWindowsUsername("NT AUTHORITY\\Local service")
localSystemUsername = WithWindowsUsername("NT AUTHORITY\\System")
hpcContainerOpt = WithWindowsHostProcessContainer()
)
// Tests to verify the Windows HostProcess
@ -39,34 +40,34 @@ func TestWindowsHostProcess(t *testing.T) {
EnsureImageExists(t, pauseImage)
t.Run("run as Local Service", func(t *testing.T) {
runHostProcess(t, false, pauseImage, localServiceUsername, defaultCommand)
runHostProcess(t, false, pauseImage, hpcContainerOpt, localServiceUsername, defaultCommand)
})
t.Run("run as Local System", func(t *testing.T) {
runHostProcess(t, false, pauseImage, localSystemUsername, defaultCommand)
runHostProcess(t, false, pauseImage, hpcContainerOpt, localSystemUsername, defaultCommand)
})
t.Run("run as unacceptable user", func(t *testing.T) {
runHostProcess(t, true, pauseImage, WithWindowsUsername("Guest"), defaultCommand)
runHostProcess(t, true, pauseImage, hpcContainerOpt, WithWindowsUsername("Guest"), defaultCommand)
})
t.Run("run command on host", func(t *testing.T) {
cmd := WithCommand("Powershell", "/c", "Get-Command containerd.exe")
runHostProcess(t, false, pauseImage, localServiceUsername, cmd)
runHostProcess(t, false, pauseImage, hpcContainerOpt, localServiceUsername, cmd)
})
t.Run("run withHostNetwork", func(t *testing.T) {
hostname, err := os.Hostname()
require.NoError(t, err)
cmd := WithCommand("Powershell", "/c", fmt.Sprintf("if ($env:COMPUTERNAME -ne %s) { exit -1 }", hostname))
runHostProcess(t, false, pauseImage, localServiceUsername, cmd)
runHostProcess(t, false, pauseImage, hpcContainerOpt, localServiceUsername, cmd)
})
t.Run("run with a different os.version image", func(t *testing.T) {
image := "docker.io/e2eteam/busybox:1.29-windows-amd64-1909"
EnsureImageExists(t, image)
runHostProcess(t, false, image, localServiceUsername, defaultCommand)
runHostProcess(t, false, image, hpcContainerOpt, localServiceUsername, defaultCommand)
})
}
func runHostProcess(t *testing.T, expectErr bool, image string, opts ...ContainerOpts) {
t.Logf("Create a pod config and run sandbox container")
sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox1", "hostprocess", WithWindowsHostProcess)
sb, sbConfig := PodSandboxConfigWithCleanup(t, "sandbox1", "hostprocess", WithWindowsHostProcessPod)
t.Logf("Create a container config and run container in a pod")
containerConfig := ContainerConfig(

View File

@ -17,6 +17,7 @@
package server
import (
"errors"
"strconv"
"github.com/containerd/containerd/oci"
@ -50,6 +51,16 @@ func (c *criService) containerSpec(
specOpts := []oci.SpecOpts{
customopts.WithProcessArgs(config, imageConfig),
}
// All containers in a pod need to have HostProcess set if it was set on the pod,
// and vice versa no containers in the pod can be HostProcess if the pods spec
// didn't have the field set. The only case that is valid is if these are the same value.
cntrHpc := config.GetWindows().GetSecurityContext().GetHostProcess()
sandboxHpc := sandboxConfig.GetWindows().GetSecurityContext().GetHostProcess()
if cntrHpc != sandboxHpc {
return nil, errors.New("pod spec and all containers inside must have the HostProcess field set to be valid")
}
if config.GetWorkingDir() != "" {
specOpts = append(specOpts, oci.WithProcessCwd(config.GetWorkingDir()))
} else if imageConfig.WorkingDir != "" {
@ -120,7 +131,7 @@ func (c *criService) containerSpec(
customopts.WithAnnotation(annotations.SandboxName, sandboxConfig.GetMetadata().GetName()),
customopts.WithAnnotation(annotations.ContainerName, containerName),
customopts.WithAnnotation(annotations.ImageName, imageName),
customopts.WithAnnotation(annotations.WindowsHostProcess, strconv.FormatBool(sandboxConfig.GetWindows().GetSecurityContext().GetHostProcess())),
customopts.WithAnnotation(annotations.WindowsHostProcess, strconv.FormatBool(sandboxHpc)),
)
return c.runtimeSpec(id, ociRuntime.BaseRuntimeSpec, specOpts...)
}

View File

@ -83,6 +83,7 @@ func getCreateContainerTestData() (*runtime.ContainerConfig, *runtime.PodSandbox
Namespace: "test-sandbox-ns",
Attempt: 2,
},
Windows: &runtime.WindowsPodSandboxConfig{},
Hostname: "test-hostname",
Annotations: map[string]string{"c": "d"},
}
@ -195,3 +196,52 @@ func TestMountNamedPipe(t *testing.T) {
specCheck(t, testID, testSandboxID, testPid, spec)
checkMount(t, spec.Mounts, `\\.\pipe\foo`, `\\.\pipe\foo`, "", []string{"rw"}, nil)
}
func TestHostProcessRequirements(t *testing.T) {
testID := "test-id"
testSandboxID := "sandbox-id"
testContainerName := "container-name"
testPid := uint32(1234)
containerConfig, sandboxConfig, imageConfig, _ := getCreateContainerTestData()
ociRuntime := config.Runtime{}
c := newTestCRIService()
for desc, test := range map[string]struct {
containerHostProcess bool
sandboxHostProcess bool
expectError bool
}{
"hostprocess container in non-hostprocess sandbox should fail": {
containerHostProcess: true,
sandboxHostProcess: false,
expectError: true,
},
"hostprocess container in hostprocess sandbox should be fine": {
containerHostProcess: true,
sandboxHostProcess: true,
expectError: false,
},
"non-hostprocess container in hostprocess sandbox should fail": {
containerHostProcess: false,
sandboxHostProcess: true,
expectError: true,
},
"non-hostprocess container in non-hostprocess sandbox should be fine": {
containerHostProcess: false,
sandboxHostProcess: false,
expectError: false,
},
} {
t.Run(desc, func(t *testing.T) {
containerConfig.Windows.SecurityContext.HostProcess = test.containerHostProcess
sandboxConfig.Windows.SecurityContext = &runtime.WindowsSandboxSecurityContext{
HostProcess: test.sandboxHostProcess,
}
_, err := c.containerSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
if test.expectError {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
})
}
}