Merge pull request #5203 from tghartland/5008-target-namespace

Support PID NamespaceMode_TARGET

pkg/cri/opts/spec_linux.go

@@ -593,16 +593,16 @@ func WithSupplementalGroups(groups []int64) oci.SpecOpts {
 }
 
 // WithPodNamespaces sets the pod namespaces for the container
-func WithPodNamespaces(config *runtime.LinuxContainerSecurityContext, pid uint32) oci.SpecOpts {
+func WithPodNamespaces(config *runtime.LinuxContainerSecurityContext, sandboxPid uint32, targetPid uint32) oci.SpecOpts {
 	namespaces := config.GetNamespaceOptions()
 
 	opts := []oci.SpecOpts{
-		oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.NetworkNamespace, Path: GetNetworkNamespace(pid)}),
-		oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.IPCNamespace, Path: GetIPCNamespace(pid)}),
-		oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.UTSNamespace, Path: GetUTSNamespace(pid)}),
+		oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.NetworkNamespace, Path: GetNetworkNamespace(sandboxPid)}),
+		oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.IPCNamespace, Path: GetIPCNamespace(sandboxPid)}),
+		oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.UTSNamespace, Path: GetUTSNamespace(sandboxPid)}),
 	}
 	if namespaces.GetPid() != runtime.NamespaceMode_CONTAINER {
-		opts = append(opts, oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.PIDNamespace, Path: GetPIDNamespace(pid)}))
+		opts = append(opts, oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.PIDNamespace, Path: GetPIDNamespace(targetPid)}))
 	}
 	return oci.Compose(opts...)
 }

pkg/cri/server/container_create_linux.go

@@ -270,9 +270,24 @@ func (c *criService) containerSpec(
 		specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue))
 	}
 
+	// 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, errors.Wrapf(err, "invalid target container")
+		}
+
+		status := targetContainer.Status.Get()
+		targetPid = status.Pid
+	}
+
 	specOpts = append(specOpts,
 		customopts.WithOOMScoreAdj(config, c.config.RestrictOOMScoreAdj),
-		customopts.WithPodNamespaces(securityContext, sandboxPid),
+		customopts.WithPodNamespaces(securityContext, sandboxPid, targetPid),
 		customopts.WithSupplementalGroups(supplementalGroups),
 		customopts.WithAnnotation(annotations.ContainerType, annotations.ContainerTypeContainer),
 		customopts.WithAnnotation(annotations.SandboxID, sandboxID),

pkg/cri/server/container_start.go

@@ -84,6 +84,17 @@ func (c *criService) StartContainer(ctx context.Context, r *runtime.StartContain
 		return nil, errors.Errorf("sandbox container %q is not running", sandboxID)
 	}
 
+	// Recheck target container validity in Linux namespace options.
+	if linux := config.GetLinux(); linux != nil {
+		nsOpts := linux.GetSecurityContext().GetNamespaceOptions()
+		if nsOpts.GetPid() == runtime.NamespaceMode_TARGET {
+			_, err := c.validateTargetContainer(sandboxID, nsOpts.TargetId)
+			if err != nil {
+				return nil, errors.Wrap(err, "invalid target container")
+			}
+		}
+	}
+
 	ioCreation := func(id string) (_ containerdio.IO, err error) {
 		stdoutWC, stderrWC, err := c.createContainerLoggers(meta.LogPath, config.GetTty())
 		if err != nil {

pkg/cri/server/helpers.go

@@ -242,6 +242,30 @@ func (c *criService) ensureImageExists(ctx context.Context, ref string, config *
 	return &newImage, nil
 }
 
+// validateTargetContainer checks that a container is a valid
+// target for a container using PID NamespaceMode_TARGET.
+// The target container must be in the same sandbox and must be running.
+// Returns the target container for convenience.
+func (c *criService) validateTargetContainer(sandboxID, targetContainerID string) (containerstore.Container, error) {
+	targetContainer, err := c.containerStore.Get(targetContainerID)
+	if err != nil {
+		return containerstore.Container{}, errors.Wrapf(err, "container %q does not exist", targetContainerID)
+	}
+
+	targetSandboxID := targetContainer.Metadata.SandboxID
+	if targetSandboxID != sandboxID {
+		return containerstore.Container{},
+			errors.Errorf("container %q (sandbox %s) does not belong to sandbox %s", targetContainerID, targetSandboxID, sandboxID)
+	}
+
+	status := targetContainer.Status.Get()
+	if state := status.State(); state != runtime.ContainerState_CONTAINER_RUNNING {
+		return containerstore.Container{}, errors.Errorf("container %q is not running - in state %s", targetContainerID, state)
+	}
+
+	return targetContainer, nil
+}
+
 // isInCRIMounts checks whether a destination is in CRI mount list.
 func isInCRIMounts(dst string, mounts []*runtime.Mount) bool {
 	for _, m := range mounts {

pkg/cri/server/helpers_test.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"io/ioutil"
 	"testing"
+	"time"
 
 	"github.com/containerd/containerd/oci"
 	"github.com/containerd/containerd/plugin"
@@ -34,6 +35,7 @@ import (
 
 	criconfig "github.com/containerd/containerd/pkg/cri/config"
 	"github.com/containerd/containerd/pkg/cri/store"
+	containerstore "github.com/containerd/containerd/pkg/cri/store/container"
 	imagestore "github.com/containerd/containerd/pkg/cri/store/image"
 )
 
@@ -501,3 +503,94 @@ func TestEnsureRemoveAllWithFile(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
+// Helper function for setting up an environment to test PID namespace targeting.
+func addContainer(c *criService, containerID, sandboxID string, PID uint32, createdAt, startedAt, finishedAt int64) error {
+	meta := containerstore.Metadata{
+		ID:        containerID,
+		SandboxID: sandboxID,
+	}
+	status := containerstore.Status{
+		Pid:        PID,
+		CreatedAt:  createdAt,
+		StartedAt:  startedAt,
+		FinishedAt: finishedAt,
+	}
+	container, err := containerstore.NewContainer(meta,
+		containerstore.WithFakeStatus(status),
+	)
+	if err != nil {
+		return err
+	}
+	return c.containerStore.Add(container)
+}
+
+func TestValidateTargetContainer(t *testing.T) {
+	testSandboxID := "test-sandbox-uid"
+
+	// The existing container that will be targeted.
+	testTargetContainerID := "test-target-container"
+	testTargetContainerPID := uint32(4567)
+
+	// A container that has finished running and cannot be targeted.
+	testStoppedContainerID := "stopped-target-container"
+	testStoppedContainerPID := uint32(6789)
+
+	// A container from another pod.
+	testOtherContainerSandboxID := "other-sandbox-uid"
+	testOtherContainerID := "other-target-container"
+	testOtherContainerPID := uint32(7890)
+
+	// Container create/start/stop times.
+	createdAt := time.Now().Add(-15 * time.Second).UnixNano()
+	startedAt := time.Now().Add(-10 * time.Second).UnixNano()
+	finishedAt := time.Now().Add(-5 * time.Second).UnixNano()
+
+	c := newTestCRIService()
+
+	// Create a target container.
+	err := addContainer(c, testTargetContainerID, testSandboxID, testTargetContainerPID, createdAt, startedAt, 0)
+	require.NoError(t, err, "error creating test target container")
+
+	// Create a stopped container.
+	err = addContainer(c, testStoppedContainerID, testSandboxID, testStoppedContainerPID, createdAt, startedAt, finishedAt)
+	require.NoError(t, err, "error creating test stopped container")
+
+	// Create a container in another pod.
+	err = addContainer(c, testOtherContainerID, testOtherContainerSandboxID, testOtherContainerPID, createdAt, startedAt, 0)
+	require.NoError(t, err, "error creating test container in other pod")
+
+	for desc, test := range map[string]struct {
+		targetContainerID string
+		expectError       bool
+	}{
+		"target container in pod": {
+			targetContainerID: testTargetContainerID,
+			expectError:       false,
+		},
+		"target stopped container in pod": {
+			targetContainerID: testStoppedContainerID,
+			expectError:       true,
+		},
+		"target container does not exist": {
+			targetContainerID: "no-container-with-this-id",
+			expectError:       true,
+		},
+		"target container in other pod": {
+			targetContainerID: testOtherContainerID,
+			expectError:       true,
+		},
+	} {
+		t.Run(desc, func(t *testing.T) {
+			targetContainer, err := c.validateTargetContainer(testSandboxID, test.targetContainerID)
+			if test.expectError {
+				require.Error(t, err, "target should have been invalid but no error")
+				return
+			}
+			require.NoErrorf(t, err, "target should have been valid but got error")
+
+			assert.Equal(t, test.targetContainerID, targetContainer.ID, "returned target container does not have expected ID")
+		})
+	}
+
+}