Skip to content

Commit 3dad67e

Browse files
mikebrowmikebrow
authored
Merge pull request #5203 from tghartland/5008-target-namespace
Support PID NamespaceMode_TARGET
2 parents cb64dc8 + efcb187 commit 3dad67e

5 files changed

Lines changed: 149 additions & 6 deletions

File tree

‎pkg/cri/opts/spec_linux.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -593,16 +593,16 @@ func WithSupplementalGroups(groups []int64) oci.SpecOpts {
593593
}
594594

595595
// WithPodNamespaces sets the pod namespaces for the container
596-
func WithPodNamespaces(config *runtime.LinuxContainerSecurityContext, pid uint32) oci.SpecOpts {
596+
func WithPodNamespaces(config *runtime.LinuxContainerSecurityContext, sandboxPid uint32, targetPid uint32) oci.SpecOpts {
597597
namespaces := config.GetNamespaceOptions()
598598

599599
opts := []oci.SpecOpts{
600-
oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.NetworkNamespace, Path: GetNetworkNamespace(pid)}),
601-
oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.IPCNamespace, Path: GetIPCNamespace(pid)}),
602-
oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.UTSNamespace, Path: GetUTSNamespace(pid)}),
600+
oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.NetworkNamespace, Path: GetNetworkNamespace(sandboxPid)}),
601+
oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.IPCNamespace, Path: GetIPCNamespace(sandboxPid)}),
602+
oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.UTSNamespace, Path: GetUTSNamespace(sandboxPid)}),
603603
}
604604
if namespaces.GetPid() != runtime.NamespaceMode_CONTAINER {
605-
opts = append(opts, oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.PIDNamespace, Path: GetPIDNamespace(pid)}))
605+
opts = append(opts, oci.WithLinuxNamespace(runtimespec.LinuxNamespace{Type: runtimespec.PIDNamespace, Path: GetPIDNamespace(targetPid)}))
606606
}
607607
return oci.Compose(opts...)
608608
}

‎pkg/cri/server/container_create_linux.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -270,9 +270,24 @@ func (c *criService) containerSpec(
270270
specOpts = append(specOpts, customopts.WithAnnotation(pKey, pValue))
271271
}
272272

273+
// Default target PID namespace is the sandbox PID.
274+
targetPid := sandboxPid
275+
// If the container targets another container's PID namespace,
276+
// set targetPid to the PID of that container.
277+
nsOpts := securityContext.GetNamespaceOptions()
278+
if nsOpts.GetPid() == runtime.NamespaceMode_TARGET {
279+
targetContainer, err := c.validateTargetContainer(sandboxID, nsOpts.TargetId)
280+
if err != nil {
281+
return nil, errors.Wrapf(err, "invalid target container")
282+
}
283+
284+
status := targetContainer.Status.Get()
285+
targetPid = status.Pid
286+
}
287+
273288
specOpts = append(specOpts,
274289
customopts.WithOOMScoreAdj(config, c.config.RestrictOOMScoreAdj),
275-
customopts.WithPodNamespaces(securityContext, sandboxPid),
290+
customopts.WithPodNamespaces(securityContext, sandboxPid, targetPid),
276291
customopts.WithSupplementalGroups(supplementalGroups),
277292
customopts.WithAnnotation(annotations.ContainerType, annotations.ContainerTypeContainer),
278293
customopts.WithAnnotation(annotations.SandboxID, sandboxID),

‎pkg/cri/server/container_start.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ func (c *criService) StartContainer(ctx context.Context, r *runtime.StartContain
8484
return nil, errors.Errorf("sandbox container %q is not running", sandboxID)
8585
}
8686

87+
// Recheck target container validity in Linux namespace options.
88+
if linux := config.GetLinux(); linux != nil {
89+
nsOpts := linux.GetSecurityContext().GetNamespaceOptions()
90+
if nsOpts.GetPid() == runtime.NamespaceMode_TARGET {
91+
_, err := c.validateTargetContainer(sandboxID, nsOpts.TargetId)
92+
if err != nil {
93+
return nil, errors.Wrap(err, "invalid target container")
94+
}
95+
}
96+
}
97+
8798
ioCreation := func(id string) (_ containerdio.IO, err error) {
8899
stdoutWC, stderrWC, err := c.createContainerLoggers(meta.LogPath, config.GetTty())
89100
if err != nil {

‎pkg/cri/server/helpers.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,30 @@ func (c *criService) ensureImageExists(ctx context.Context, ref string, config *
242242
return &newImage, nil
243243
}
244244

245+
// validateTargetContainer checks that a container is a valid
246+
// target for a container using PID NamespaceMode_TARGET.
247+
// The target container must be in the same sandbox and must be running.
248+
// Returns the target container for convenience.
249+
func (c *criService) validateTargetContainer(sandboxID, targetContainerID string) (containerstore.Container, error) {
250+
targetContainer, err := c.containerStore.Get(targetContainerID)
251+
if err != nil {
252+
return containerstore.Container{}, errors.Wrapf(err, "container %q does not exist", targetContainerID)
253+
}
254+
255+
targetSandboxID := targetContainer.Metadata.SandboxID
256+
if targetSandboxID != sandboxID {
257+
return containerstore.Container{},
258+
errors.Errorf("container %q (sandbox %s) does not belong to sandbox %s", targetContainerID, targetSandboxID, sandboxID)
259+
}
260+
261+
status := targetContainer.Status.Get()
262+
if state := status.State(); state != runtime.ContainerState_CONTAINER_RUNNING {
263+
return containerstore.Container{}, errors.Errorf("container %q is not running - in state %s", targetContainerID, state)
264+
}
265+
266+
return targetContainer, nil
267+
}
268+
245269
// isInCRIMounts checks whether a destination is in CRI mount list.
246270
func isInCRIMounts(dst string, mounts []*runtime.Mount) bool {
247271
for _, m := range mounts {

‎pkg/cri/server/helpers_test.go‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"io/ioutil"
2222
"testing"
23+
"time"
2324

2425
"github.com/containerd/containerd/oci"
2526
"github.com/containerd/containerd/plugin"
@@ -34,6 +35,7 @@ import (
3435

3536
criconfig "github.com/containerd/containerd/pkg/cri/config"
3637
"github.com/containerd/containerd/pkg/cri/store"
38+
containerstore "github.com/containerd/containerd/pkg/cri/store/container"
3739
imagestore "github.com/containerd/containerd/pkg/cri/store/image"
3840
)
3941

@@ -501,3 +503,94 @@ func TestEnsureRemoveAllWithFile(t *testing.T) {
501503
t.Fatal(err)
502504
}
503505
}
506+
507+
// Helper function for setting up an environment to test PID namespace targeting.
508+
func addContainer(c *criService, containerID, sandboxID string, PID uint32, createdAt, startedAt, finishedAt int64) error {
509+
meta := containerstore.Metadata{
510+
ID: containerID,
511+
SandboxID: sandboxID,
512+
}
513+
status := containerstore.Status{
514+
Pid: PID,
515+
CreatedAt: createdAt,
516+
StartedAt: startedAt,
517+
FinishedAt: finishedAt,
518+
}
519+
container, err := containerstore.NewContainer(meta,
520+
containerstore.WithFakeStatus(status),
521+
)
522+
if err != nil {
523+
return err
524+
}
525+
return c.containerStore.Add(container)
526+
}
527+
528+
func TestValidateTargetContainer(t *testing.T) {
529+
testSandboxID := "test-sandbox-uid"
530+
531+
// The existing container that will be targeted.
532+
testTargetContainerID := "test-target-container"
533+
testTargetContainerPID := uint32(4567)
534+
535+
// A container that has finished running and cannot be targeted.
536+
testStoppedContainerID := "stopped-target-container"
537+
testStoppedContainerPID := uint32(6789)
538+
539+
// A container from another pod.
540+
testOtherContainerSandboxID := "other-sandbox-uid"
541+
testOtherContainerID := "other-target-container"
542+
testOtherContainerPID := uint32(7890)
543+
544+
// Container create/start/stop times.
545+
createdAt := time.Now().Add(-15 * time.Second).UnixNano()
546+
startedAt := time.Now().Add(-10 * time.Second).UnixNano()
547+
finishedAt := time.Now().Add(-5 * time.Second).UnixNano()
548+
549+
c := newTestCRIService()
550+
551+
// Create a target container.
552+
err := addContainer(c, testTargetContainerID, testSandboxID, testTargetContainerPID, createdAt, startedAt, 0)
553+
require.NoError(t, err, "error creating test target container")
554+
555+
// Create a stopped container.
556+
err = addContainer(c, testStoppedContainerID, testSandboxID, testStoppedContainerPID, createdAt, startedAt, finishedAt)
557+
require.NoError(t, err, "error creating test stopped container")
558+
559+
// Create a container in another pod.
560+
err = addContainer(c, testOtherContainerID, testOtherContainerSandboxID, testOtherContainerPID, createdAt, startedAt, 0)
561+
require.NoError(t, err, "error creating test container in other pod")
562+
563+
for desc, test := range map[string]struct {
564+
targetContainerID string
565+
expectError bool
566+
}{
567+
"target container in pod": {
568+
targetContainerID: testTargetContainerID,
569+
expectError: false,
570+
},
571+
"target stopped container in pod": {
572+
targetContainerID: testStoppedContainerID,
573+
expectError: true,
574+
},
575+
"target container does not exist": {
576+
targetContainerID: "no-container-with-this-id",
577+
expectError: true,
578+
},
579+
"target container in other pod": {
580+
targetContainerID: testOtherContainerID,
581+
expectError: true,
582+
},
583+
} {
584+
t.Run(desc, func(t *testing.T) {
585+
targetContainer, err := c.validateTargetContainer(testSandboxID, test.targetContainerID)
586+
if test.expectError {
587+
require.Error(t, err, "target should have been invalid but no error")
588+
return
589+
}
590+
require.NoErrorf(t, err, "target should have been valid but got error")
591+
592+
assert.Equal(t, test.targetContainerID, targetContainer.ID, "returned target container does not have expected ID")
593+
})
594+
}
595+
596+
}

0 commit comments

Comments
 (0)