Update OCI spec tests for generic platform

Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>

api/runtime/sandbox/v1/sandbox.proto

@@ -37,8 +37,8 @@ service Sandbox {
 	// StartSandbox will start previsouly created sandbox.
 	rpc StartSandbox(StartSandboxRequest) returns (StartSandboxResponse);
 
-	// Platform queries platform sandbox is going to run containers on.
+	// Platform queries the platform the sandbox is going to run containers on.
-	// containerd will use this to generate proper OCI spec.
+	// containerd will use this to generate a proper OCI spec.
 	rpc Platform(PlatformRequest) returns (PlatformResponse);
 
 	// StopSandbox will stop existing sandbox instance

pkg/cri/sbserver/container_create.go

@@ -420,50 +420,12 @@ func (c *criService) buildContainerSpec(
 ) (_ *runtimespec.Spec, retErr error) {
 	var (
 		specOpts []oci.SpecOpts
+
+		// Platform helpers
 		isLinux   = platform.OS == "linux"
 		isWindows = platform.OS == "windows"
 	)
 
-	specOpts = append(specOpts, customopts.WithProcessArgs(config, imageConfig))
-
-	if config.GetWorkingDir() != "" {
-		specOpts = append(specOpts, oci.WithProcessCwd(config.GetWorkingDir()))
-	} else if imageConfig.WorkingDir != "" {
-		specOpts = append(specOpts, oci.WithProcessCwd(imageConfig.WorkingDir))
-	}
-
-	if config.GetTty() {
-		specOpts = append(specOpts, oci.WithTTY)
-	}
-
-	// Apply envs from image config first, so that envs from container config
-	// can override them.
-	env := append([]string{}, imageConfig.Env...)
-	for _, e := range config.GetEnvs() {
-		env = append(env, e.GetKey()+"="+e.GetValue())
-	}
-	specOpts = append(specOpts, oci.WithEnv(env))
-
-	for pKey, pValue := range getPassthroughAnnotations(sandboxConfig.Annotations,
-		ociRuntime.PodAnnotations) {
-		specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue))
-	}
-
-	for pKey, pValue := range getPassthroughAnnotations(config.Annotations,
-		ociRuntime.ContainerAnnotations) {
-		specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue))
-	}
-
-	specOpts = append(specOpts,
-		customopts.WithAnnotation(annotations.ContainerType, annotations.ContainerTypeContainer),
-		customopts.WithAnnotation(annotations.SandboxID, sandboxID),
-		customopts.WithAnnotation(annotations.SandboxNamespace, sandboxConfig.GetMetadata().GetNamespace()),
-		customopts.WithAnnotation(annotations.SandboxUID, sandboxConfig.GetMetadata().GetUid()),
-		customopts.WithAnnotation(annotations.SandboxName, sandboxConfig.GetMetadata().GetName()),
-		customopts.WithAnnotation(annotations.ContainerName, containerName),
-		customopts.WithAnnotation(annotations.ImageName, imageName),
-	)
-
 	if isLinux {
 		specOpts = append(specOpts, oci.WithoutRunMount)
 
@@ -492,80 +454,29 @@ func (c *criService) buildContainerSpec(
 			}
 		}
 		specOpts = append(specOpts, oci.WithEnv([]string{hostnameEnv + "=" + hostname}))
-
-		securityContext := config.GetLinux().GetSecurityContext()
-
-		// Clear all ambient capabilities. The implication of non-root + caps
-		// is not clearly defined in Kubernetes.
-		// See https://github.com/kubernetes/kubernetes/issues/56374
-		// Keep docker's behavior for now.
-		specOpts = append(specOpts, customopts.WithoutAmbientCaps)
-
-		if securityContext.GetPrivileged() {
-			if !sandboxConfig.GetLinux().GetSecurityContext().GetPrivileged() {
-				return nil, errors.New("no privileged container allowed in sandbox")
-			}
-			specOpts = append(specOpts, oci.WithPrivileged)
-			if !ociRuntime.PrivilegedWithoutHostDevices {
-				specOpts = append(specOpts, oci.WithHostDevices, oci.WithAllDevicesAllowed)
-			} else if ociRuntime.PrivilegedWithoutHostDevicesAllDevicesAllowed {
-				// allow rwm on all devices for the container
-				specOpts = append(specOpts, oci.WithAllDevicesAllowed)
-			}
 	}
 
-		// TODO: Figure out whether we should set no new privilege for sandbox container by default
+	specOpts = append(specOpts, customopts.WithProcessArgs(config, imageConfig))
-		if securityContext.GetNoNewPrivs() {
+
-			specOpts = append(specOpts, oci.WithNoNewPrivileges)
+	if config.GetWorkingDir() != "" {
-		}
+		specOpts = append(specOpts, oci.WithProcessCwd(config.GetWorkingDir()))
-		// TODO(random-liu): [P1] Set selinux options (privileged or not).
+	} else if imageConfig.WorkingDir != "" {
-		if securityContext.GetReadonlyRootfs() {
+		specOpts = append(specOpts, oci.WithProcessCwd(imageConfig.WorkingDir))
-			specOpts = append(specOpts, oci.WithRootFSReadonly())
 	}
 
-		if !c.config.DisableProcMount {
+	if config.GetTty() {
-			// Change the default masked/readonly paths to empty slices
+		specOpts = append(specOpts, oci.WithTTY)
-			// See https://github.com/containerd/containerd/issues/5029
-			// TODO: Provide an option to set default paths to the ones in oci.populateDefaultUnixSpec()
-			specOpts = append(specOpts, oci.WithMaskedPaths([]string{}), oci.WithReadonlyPaths([]string{}))
-
-			// Apply masked paths if specified.
-			// If the container is privileged, this will be cleared later on.
-			if maskedPaths := securityContext.GetMaskedPaths(); maskedPaths != nil {
-				specOpts = append(specOpts, oci.WithMaskedPaths(maskedPaths))
 	}
 
-			// Apply readonly paths if specified.
+	// Apply envs from image config first, so that envs from container config
-			// If the container is privileged, this will be cleared later on.
+	// can override them.
-			if readonlyPaths := securityContext.GetReadonlyPaths(); readonlyPaths != nil {
+	env := append([]string{}, imageConfig.Env...)
-				specOpts = append(specOpts, oci.WithReadonlyPaths(readonlyPaths))
+	for _, e := range config.GetEnvs() {
-			}
+		env = append(env, e.GetKey()+"="+e.GetValue())
 	}
+	specOpts = append(specOpts, oci.WithEnv(env))
 
-		supplementalGroups := securityContext.GetSupplementalGroups()
+	if isWindows {
-		specOpts = append(specOpts, customopts.WithSupplementalGroups(supplementalGroups))
-
-		// Default target PID namespace is the sandbox PID.
-		targetPid := sandboxPid
-		// If the container targets another container's PID namespace,
-		// set targetPid to the PID of that container.
-		nsOpts := securityContext.GetNamespaceOptions()
-		if nsOpts.GetPid() == runtime.NamespaceMode_TARGET {
-			targetContainer, err := c.validateTargetContainer(sandboxID, nsOpts.TargetId)
-			if err != nil {
-				return nil, fmt.Errorf("invalid target container: %w", err)
-			}
-
-			status := targetContainer.Status.Get()
-			targetPid = status.Pid
-		}
-
-		specOpts = append(specOpts,
-			// TODO: This is a hack to make this compile. We should move userns support to sbserver.
-			customopts.WithPodNamespaces(securityContext, sandboxPid, targetPid, nil, nil),
-		)
-
-	} else if isWindows {
 		specOpts = append(specOpts,
 			// Clear the root location since hcsshim expects it.
 			// NOTE: readonly rootfs doesn't work on windows.
@@ -606,5 +517,99 @@ func (c *criService) buildContainerSpec(
 
 	specOpts = append(specOpts, platformSpecOpts...)
 
+	if isLinux {
+		securityContext := config.GetLinux().GetSecurityContext()
+
+		if !c.config.DisableProcMount {
+			// Change the default masked/readonly paths to empty slices
+			// See https://github.com/containerd/containerd/issues/5029
+			// TODO: Provide an option to set default paths to the ones in oci.populateDefaultUnixSpec()
+			specOpts = append(specOpts, oci.WithMaskedPaths([]string{}), oci.WithReadonlyPaths([]string{}))
+
+			// Apply masked paths if specified.
+			// If the container is privileged, this will be cleared later on.
+			if maskedPaths := securityContext.GetMaskedPaths(); maskedPaths != nil {
+				specOpts = append(specOpts, oci.WithMaskedPaths(maskedPaths))
+			}
+
+			// Apply readonly paths if specified.
+			// If the container is privileged, this will be cleared later on.
+			if readonlyPaths := securityContext.GetReadonlyPaths(); readonlyPaths != nil {
+				specOpts = append(specOpts, oci.WithReadonlyPaths(readonlyPaths))
+			}
+		}
+
+		if securityContext.GetPrivileged() {
+			if !sandboxConfig.GetLinux().GetSecurityContext().GetPrivileged() {
+				return nil, errors.New("no privileged container allowed in sandbox")
+			}
+			specOpts = append(specOpts, oci.WithPrivileged)
+			if !ociRuntime.PrivilegedWithoutHostDevices {
+				specOpts = append(specOpts, oci.WithHostDevices, oci.WithAllDevicesAllowed)
+			} else if ociRuntime.PrivilegedWithoutHostDevicesAllDevicesAllowed {
+				// allow rwm on all devices for the container
+				specOpts = append(specOpts, oci.WithAllDevicesAllowed)
+			}
+		}
+
+		// Clear all ambient capabilities. The implication of non-root + caps
+		// is not clearly defined in Kubernetes.
+		// See https://github.com/kubernetes/kubernetes/issues/56374
+		// Keep docker's behavior for now.
+		specOpts = append(specOpts, customopts.WithoutAmbientCaps)
+
+		// TODO: Figure out whether we should set no new privilege for sandbox container by default
+		if securityContext.GetNoNewPrivs() {
+			specOpts = append(specOpts, oci.WithNoNewPrivileges)
+		}
+		// TODO(random-liu): [P1] Set selinux options (privileged or not).
+		if securityContext.GetReadonlyRootfs() {
+			specOpts = append(specOpts, oci.WithRootFSReadonly())
+		}
+
+		supplementalGroups := securityContext.GetSupplementalGroups()
+		specOpts = append(specOpts, customopts.WithSupplementalGroups(supplementalGroups))
+
+		// Default target PID namespace is the sandbox PID.
+		targetPid := sandboxPid
+		// If the container targets another container's PID namespace,
+		// set targetPid to the PID of that container.
+		nsOpts := securityContext.GetNamespaceOptions()
+		if nsOpts.GetPid() == runtime.NamespaceMode_TARGET {
+			targetContainer, err := c.validateTargetContainer(sandboxID, nsOpts.TargetId)
+			if err != nil {
+				return nil, fmt.Errorf("invalid target container: %w", err)
+			}
+
+			status := targetContainer.Status.Get()
+			targetPid = status.Pid
+		}
+
+		specOpts = append(specOpts,
+			// TODO: This is a hack to make this compile. We should move userns support to sbserver.
+			customopts.WithPodNamespaces(securityContext, sandboxPid, targetPid, nil, nil),
+		)
+	}
+
+	for pKey, pValue := range getPassthroughAnnotations(sandboxConfig.Annotations,
+		ociRuntime.PodAnnotations) {
+		specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue))
+	}
+
+	for pKey, pValue := range getPassthroughAnnotations(config.Annotations,
+		ociRuntime.ContainerAnnotations) {
+		specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue))
+	}
+
+	specOpts = append(specOpts,
+		customopts.WithAnnotation(annotations.ContainerType, annotations.ContainerTypeContainer),
+		customopts.WithAnnotation(annotations.SandboxID, sandboxID),
+		customopts.WithAnnotation(annotations.SandboxNamespace, sandboxConfig.GetMetadata().GetNamespace()),
+		customopts.WithAnnotation(annotations.SandboxUID, sandboxConfig.GetMetadata().GetUid()),
+		customopts.WithAnnotation(annotations.SandboxName, sandboxConfig.GetMetadata().GetName()),
+		customopts.WithAnnotation(annotations.ContainerName, containerName),
+		customopts.WithAnnotation(annotations.ImageName, imageName),
+	)
+
 	return c.runtimeSpec(id, ociRuntime.BaseRuntimeSpec, specOpts...)
 }

pkg/cri/sbserver/container_create_linux_test.go

@@ -248,7 +248,7 @@ func TestContainerCapabilities(t *testing.T) {
 			c.allCaps = allCaps
 
 			containerConfig.Linux.SecurityContext.Capabilities = test.capability
-			spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+			spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			require.NoError(t, err)
 
 			if selinux.GetEnabled() {
@@ -283,7 +283,7 @@ func TestContainerSpecTty(t *testing.T) {
 	c := newTestCRIService()
 	for _, tty := range []bool{true, false} {
 		containerConfig.Tty = tty
-		spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+		spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 		require.NoError(t, err)
 		specCheck(t, testID, testSandboxID, testPid, spec)
 		assert.Equal(t, tty, spec.Process.Terminal)
@@ -310,7 +310,7 @@ func TestContainerSpecDefaultPath(t *testing.T) {
 			imageConfig.Env = append(imageConfig.Env, pathenv)
 			expected = pathenv
 		}
-		spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+		spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 		require.NoError(t, err)
 		specCheck(t, testID, testSandboxID, testPid, spec)
 		assert.Contains(t, spec.Process.Env, expected)
@@ -327,7 +327,7 @@ func TestContainerSpecReadonlyRootfs(t *testing.T) {
 	c := newTestCRIService()
 	for _, readonly := range []bool{true, false} {
 		containerConfig.Linux.SecurityContext.ReadonlyRootfs = readonly
-		spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+		spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 		require.NoError(t, err)
 		specCheck(t, testID, testSandboxID, testPid, spec)
 		assert.Equal(t, readonly, spec.Root.Readonly)
@@ -366,7 +366,7 @@ func TestContainerSpecWithExtraMounts(t *testing.T) {
 			Readonly:      false,
 		},
 	}
-	spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, extraMounts, ociRuntime)
+	spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, extraMounts, ociRuntime)
 	require.NoError(t, err)
 	specCheck(t, testID, testSandboxID, testPid, spec)
 	var mounts, sysMounts, devMounts []runtimespec.Mount
@@ -434,7 +434,7 @@ func TestContainerAndSandboxPrivileged(t *testing.T) {
 			sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{
 				Privileged: test.sandboxPrivileged,
 			}
-			_, err := c.platformSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+			_, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			if test.expectError {
 				assert.Error(t, err)
 			} else {
@@ -638,7 +638,7 @@ func TestPrivilegedBindMount(t *testing.T) {
 			containerConfig.Linux.SecurityContext.Privileged = test.privileged
 			sandboxConfig.Linux.SecurityContext.Privileged = test.privileged
 
-			spec, err := c.buildContainerSpec(linuxPlatform, t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+			spec, err := c.buildContainerSpec(currentPlatform, t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 
 			assert.NoError(t, err)
 			if test.expectedSysFSRO {
@@ -797,7 +797,7 @@ func TestPidNamespace(t *testing.T) {
 	} {
 		t.Run(desc, func(t *testing.T) {
 			containerConfig.Linux.SecurityContext.NamespaceOptions = &runtime.NamespaceOption{Pid: test.pidNS}
-			spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+			spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			require.NoError(t, err)
 			assert.Contains(t, spec.Linux.Namespaces, test.expected)
 		})
@@ -813,7 +813,7 @@ func TestNoDefaultRunMount(t *testing.T) {
 	ociRuntime := config.Runtime{}
 	c := newTestCRIService()
 
-	spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+	spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 	assert.NoError(t, err)
 	for _, mount := range spec.Mounts {
 		assert.NotEqual(t, "/run", mount.Destination)
@@ -1187,7 +1187,7 @@ func TestMaskedAndReadonlyPaths(t *testing.T) {
 			sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{
 				Privileged: test.privileged,
 			}
-			spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+			spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			require.NoError(t, err)
 			if !test.privileged { // specCheck presumes an unprivileged container
 				specCheck(t, testID, testSandboxID, testPid, spec)
@@ -1235,7 +1235,7 @@ func TestHostname(t *testing.T) {
 			sandboxConfig.Linux.SecurityContext = &runtime.LinuxSandboxSecurityContext{
 				NamespaceOptions: &runtime.NamespaceOption{Network: test.networkNs},
 			}
-			spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+			spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			require.NoError(t, err)
 			specCheck(t, testID, testSandboxID, testPid, spec)
 			assert.Contains(t, spec.Process.Env, test.expectedEnv)
@@ -1248,7 +1248,7 @@ func TestDisableCgroup(t *testing.T) {
 	ociRuntime := config.Runtime{}
 	c := newTestCRIService()
 	c.config.DisableCgroup = true
-	spec, err := c.buildContainerSpec(linuxPlatform, "test-id", "sandbox-id", 1234, "", "container-name", testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+	spec, err := c.buildContainerSpec(currentPlatform, "test-id", "sandbox-id", 1234, "", "container-name", testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 	require.NoError(t, err)
 
 	t.Log("resource limit should not be set")
@@ -1387,7 +1387,7 @@ func TestNonRootUserAndDevices(t *testing.T) {
 				},
 			}
 
-			spec, err := c.buildContainerSpec(linuxPlatform, t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{})
+			spec, err := c.buildContainerSpec(currentPlatform, t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{})
 			assert.NoError(t, err)
 
 			assert.Equal(t, test.expectedDeviceUID, *spec.Linux.Devices[0].UID)
@@ -1454,7 +1454,7 @@ func TestPrivilegedDevices(t *testing.T) {
 				PrivilegedWithoutHostDevices:                  test.privilegedWithoutHostDevices,
 				PrivilegedWithoutHostDevicesAllDevicesAllowed: test.privilegedWithoutHostDevicesAllDevicesAllowed,
 			}
-			spec, err := c.buildContainerSpec(linuxPlatform, t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+			spec, err := c.buildContainerSpec(currentPlatform, t.Name(), testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			assert.NoError(t, err)
 
 			hostDevicesRaw, err := oci.HostDevices()
@@ -1508,7 +1508,7 @@ func TestBaseOCISpec(t *testing.T) {
 	testPid := uint32(1234)
 	containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData()
 
-	spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+	spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 	assert.NoError(t, err)
 
 	specCheck(t, testID, testSandboxID, testPid, spec)
@@ -1642,7 +1642,7 @@ containerEdits:
 		},
 	} {
 		t.Run(test.description, func(t *testing.T) {
-			spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+			spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			require.NoError(t, err)
 
 			specCheck(t, testID, testSandboxID, testPid, spec)

pkg/cri/sbserver/container_create_other_test.go

@@ -21,8 +21,10 @@ package sbserver
 import (
 	"testing"
 
+	"github.com/containerd/containerd/pkg/cri/annotations"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/stretchr/testify/assert"
 	runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
 )
 
@@ -31,10 +33,66 @@ var _ = checkMount
 
 func getCreateContainerTestData() (*runtime.ContainerConfig, *runtime.PodSandboxConfig,
 	*imagespec.ImageConfig, func(*testing.T, string, string, uint32, *runtimespec.Spec)) {
-	config := &runtime.ContainerConfig{}
+	config := &runtime.ContainerConfig{
-	sandboxConfig := &runtime.PodSandboxConfig{}
+		Metadata: &runtime.ContainerMetadata{
-	imageConfig := &imagespec.ImageConfig{}
+			Name:    "test-name",
+			Attempt: 1,
+		},
+		Image: &runtime.ImageSpec{
+			Image: "sha256:c75bebcdd211f41b3a460c7bf82970ed6c75acaab9cd4c9a4e125b03ca113799",
+		},
+		Command:    []string{"test", "command"},
+		Args:       []string{"test", "args"},
+		WorkingDir: "test-cwd",
+		Envs: []*runtime.KeyValue{
+			{Key: "k1", Value: "v1"},
+			{Key: "k2", Value: "v2"},
+			{Key: "k3", Value: "v3=v3bis"},
+			{Key: "k4", Value: "v4=v4bis=foop"},
+		},
+		Labels:      map[string]string{"a": "b"},
+		Annotations: map[string]string{"ca-c": "ca-d"},
+	}
+	sandboxConfig := &runtime.PodSandboxConfig{
+		Metadata: &runtime.PodSandboxMetadata{
+			Name:      "test-sandbox-name",
+			Uid:       "test-sandbox-uid",
+			Namespace: "test-sandbox-ns",
+			Attempt:   2,
+		},
+		Annotations: map[string]string{"c": "d"},
+	}
+	imageConfig := &imagespec.ImageConfig{
+		Env:        []string{"ik1=iv1", "ik2=iv2", "ik3=iv3=iv3bis", "ik4=iv4=iv4bis=boop"},
+		Entrypoint: []string{"/entrypoint"},
+		Cmd:        []string{"cmd"},
+		WorkingDir: "/workspace",
+	}
 	specCheck := func(t *testing.T, id string, sandboxID string, sandboxPid uint32, spec *runtimespec.Spec) {
+		assert.Equal(t, relativeRootfsPath, spec.Root.Path)
+		assert.Equal(t, []string{"test", "command", "test", "args"}, spec.Process.Args)
+		assert.Equal(t, "test-cwd", spec.Process.Cwd)
+		assert.Contains(t, spec.Process.Env, "k1=v1", "k2=v2", "k3=v3=v3bis", "ik4=iv4=iv4bis=boop")
+		assert.Contains(t, spec.Process.Env, "ik1=iv1", "ik2=iv2", "ik3=iv3=iv3bis", "k4=v4=v4bis=foop")
+
+		t.Logf("Check PodSandbox annotations")
+		assert.Contains(t, spec.Annotations, annotations.SandboxID)
+		assert.EqualValues(t, spec.Annotations[annotations.SandboxID], sandboxID)
+
+		assert.Contains(t, spec.Annotations, annotations.ContainerType)
+		assert.EqualValues(t, spec.Annotations[annotations.ContainerType], annotations.ContainerTypeContainer)
+
+		assert.Contains(t, spec.Annotations, annotations.SandboxNamespace)
+		assert.EqualValues(t, spec.Annotations[annotations.SandboxNamespace], "test-sandbox-ns")
+
+		assert.Contains(t, spec.Annotations, annotations.SandboxUID)
+		assert.EqualValues(t, spec.Annotations[annotations.SandboxUID], "test-sandbox-uid")
+
+		assert.Contains(t, spec.Annotations, annotations.SandboxName)
+		assert.EqualValues(t, spec.Annotations[annotations.SandboxName], "test-sandbox-name")
+
+		assert.Contains(t, spec.Annotations, annotations.ImageName)
+		assert.EqualValues(t, spec.Annotations[annotations.ImageName], testImageName)
 	}
 	return config, sandboxConfig, imageConfig, specCheck
 }

pkg/cri/sbserver/container_create_test.go

@@ -35,7 +35,7 @@ import (
 	"github.com/containerd/containerd/pkg/cri/opts"
 )
 
-var linuxPlatform = &types.Platform{OS: "linux"}
+var currentPlatform = &types.Platform{OS: goruntime.GOOS, Architecture: goruntime.GOARCH}
 
 func checkMount(t *testing.T, mounts []runtimespec.Mount, src, dest, typ string,
 	contains, notcontains []string) {
@@ -62,12 +62,11 @@ func TestGeneralContainerSpec(t *testing.T) {
 	testID := "test-id"
 	testPid := uint32(1234)
 	containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData()
-	containerConfig.Command = []string{"/bin/true"}
 	ociRuntime := config.Runtime{}
 	c := newTestCRIService()
 	testSandboxID := "sandbox-id"
 	testContainerName := "container-name"
-	spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+	spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 	require.NoError(t, err)
 	specCheck(t, testID, testSandboxID, testPid, spec)
 }
@@ -138,7 +137,7 @@ func TestPodAnnotationPassthroughContainerSpec(t *testing.T) {
 			ociRuntime := config.Runtime{
 				PodAnnotations: test.podAnnotations,
 			}
-			spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName,
+			spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName,
 				containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			assert.NoError(t, err)
 			assert.NotNil(t, spec)
@@ -395,7 +394,7 @@ func TestContainerAnnotationPassthroughContainerSpec(t *testing.T) {
 				PodAnnotations:       test.podAnnotations,
 				ContainerAnnotations: test.containerAnnotations,
 			}
-			spec, err := c.buildContainerSpec(linuxPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName,
+			spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName,
 				containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			assert.NoError(t, err)
 			assert.NotNil(t, spec)

pkg/cri/sbserver/container_create_windows_test.go

@@ -19,7 +19,6 @@ package sbserver
 import (
 	"testing"
 
-	"github.com/containerd/containerd/api/types"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/assert"
@@ -143,8 +142,6 @@ func getCreateContainerTestData() (*runtime.ContainerConfig, *runtime.PodSandbox
 	return config, sandboxConfig, imageConfig, specCheck
 }
 
-var windowsPlatform = &types.Platform{OS: "windows"}
-
 func TestContainerWindowsNetworkNamespace(t *testing.T) {
 	testID := "test-id"
 	testSandboxID := "sandbox-id"
@@ -154,7 +151,7 @@ func TestContainerWindowsNetworkNamespace(t *testing.T) {
 	c := newTestCRIService()
 
 	containerConfig, sandboxConfig, imageConfig, specCheck := getCreateContainerTestData()
-	spec, err := c.buildContainerSpec(windowsPlatform, testID, testSandboxID, testPid, nsPath, testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{})
+	spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, nsPath, testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{})
 	assert.NoError(t, err)
 	assert.NotNil(t, spec)
 	specCheck(t, testID, testSandboxID, testPid, spec)
@@ -176,7 +173,7 @@ func TestMountCleanPath(t *testing.T) {
 		ContainerPath: "c:/test/container-path",
 		HostPath:      "c:/test/host-path",
 	})
-	spec, err := c.buildContainerSpec(windowsPlatform, testID, testSandboxID, testPid, nsPath, testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{})
+	spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, nsPath, testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{})
 	assert.NoError(t, err)
 	assert.NotNil(t, spec)
 	specCheck(t, testID, testSandboxID, testPid, spec)
@@ -196,7 +193,7 @@ func TestMountNamedPipe(t *testing.T) {
 		ContainerPath: `\\.\pipe\foo`,
 		HostPath:      `\\.\pipe\foo`,
 	})
-	spec, err := c.buildContainerSpec(windowsPlatform, testID, testSandboxID, testPid, nsPath, testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{})
+	spec, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, nsPath, testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, config.Runtime{})
 	assert.NoError(t, err)
 	assert.NotNil(t, spec)
 	specCheck(t, testID, testSandboxID, testPid, spec)
@@ -242,7 +239,7 @@ func TestHostProcessRequirements(t *testing.T) {
 			sandboxConfig.Windows.SecurityContext = &runtime.WindowsSandboxSecurityContext{
 				HostProcess: test.sandboxHostProcess,
 			}
-			_, err := c.platformSpec(testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
+			_, err := c.buildContainerSpec(currentPlatform, testID, testSandboxID, testPid, "", testContainerName, testImageName, containerConfig, sandboxConfig, imageConfig, nil, ociRuntime)
 			if test.expectError {
 				assert.Error(t, err)
 			} else {