diff --git a/integration/main_test.go b/integration/main_test.go index b82c56cb3..ed1e8f5ef 100644 --- a/integration/main_test.go +++ b/integration/main_test.go @@ -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) { diff --git a/integration/windows_hostprocess_test.go b/integration/windows_hostprocess_test.go index 9e8442160..11495961d 100644 --- a/integration/windows_hostprocess_test.go +++ b/integration/windows_hostprocess_test.go @@ -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( diff --git a/pkg/cri/server/container_create_windows.go b/pkg/cri/server/container_create_windows.go index 2343682f5..6b14818b3 100644 --- a/pkg/cri/server/container_create_windows.go +++ b/pkg/cri/server/container_create_windows.go @@ -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...) } diff --git a/pkg/cri/server/container_create_windows_test.go b/pkg/cri/server/container_create_windows_test.go index 7d7d2e9a1..476d14b7f 100644 --- a/pkg/cri/server/container_create_windows_test.go +++ b/pkg/cri/server/container_create_windows_test.go @@ -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) + } + }) + } +}