Merge pull request #6996 from dcantah/hpc-validations

Add validations for Windows HostProcess CRI configs

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...)
 }

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)
+			}
+		})
+	}
+}