Skip to content

Commit 09723a6

Browse files
fuweidfuweid
authored
Merge pull request containerd#9275 from abel-von/sandbox-plugin-1019
sandbox: podsandbox controller init its own client
2 parents 1a54a21 + 32bf805 commit 09723a6

23 files changed

Lines changed: 289 additions & 349 deletions

‎client/client.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,6 @@ func (c *Client) SandboxStore() sandbox.Store {
720720

721721
// SandboxController returns the underlying sandbox controller client
722722
func (c *Client) SandboxController(name string) sandbox.Controller {
723-
// default sandboxer is shim
724723
if c.sandboxers != nil {
725724
return c.sandboxers[name]
726725
}

‎client/services.go‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,6 @@ func WithSnapshotters(snapshotters map[string]snapshots.Snapshotter) ServicesOpt
8787
}
8888
}
8989

90-
// WithSandboxers sets the sandbox controllers.
91-
func WithSandboxers(sandboxers map[string]sandbox.Controller) ServicesOpt {
92-
return func(s *services) {
93-
s.sandboxers = make(map[string]sandbox.Controller)
94-
for n, sn := range sandboxers {
95-
s.sandboxers[n] = sn
96-
}
97-
}
98-
}
99-
10090
// WithContainerClient sets the container service to use using a containers client.
10191
func WithContainerClient(containerService containersapi.ContainersClient) ServicesOpt {
10292
return func(s *services) {
@@ -218,9 +208,6 @@ func WithInMemoryServices(ic *plugin.InitContext) Opt {
218208
srv.SnapshotsService: func(s interface{}) ServicesOpt {
219209
return WithSnapshotters(s.(map[string]snapshots.Snapshotter))
220210
},
221-
srv.SandboxControllersService: func(s interface{}) ServicesOpt {
222-
return WithSandboxers(s.(map[string]sandbox.Controller))
223-
},
224211
srv.ContainersService: func(s interface{}) ServicesOpt {
225212
return WithContainerClient(s.(containersapi.ContainersClient))
226213
},
@@ -251,3 +238,18 @@ func WithInMemoryServices(ic *plugin.InitContext) Opt {
251238
return nil
252239
}
253240
}
241+
242+
func WithInMemorySandboxControllers(ic *plugin.InitContext) Opt {
243+
return func(c *clientOpts) error {
244+
sandboxers, err := ic.GetByType(plugins.SandboxControllerPlugin)
245+
if err != nil {
246+
return err
247+
}
248+
sc := make(map[string]sandbox.Controller)
249+
for name, p := range sandboxers {
250+
sc[name] = p.(sandbox.Controller)
251+
}
252+
c.services.sandboxers = sc
253+
return nil
254+
}
255+
}

‎integration/build_local_containerd_helper_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ func buildLocalContainerdClient(t *testing.T, tmpDir string) *containerd.Client
118118
containerd.WithDefaultNamespace(constants.K8sContainerdNamespace),
119119
containerd.WithDefaultPlatform(platforms.Default()),
120120
containerd.WithInMemoryServices(lastInitContext),
121+
containerd.WithInMemorySandboxControllers(lastInitContext),
121122
)
122123
assert.NoError(t, err)
123124

‎pkg/cri/config/config.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ import (
2424
"time"
2525

2626
"github.com/containerd/log"
27+
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
2728

29+
"github.com/containerd/containerd/v2/pkg/cri/annotations"
2830
"github.com/containerd/containerd/v2/pkg/deprecation"
2931
)
3032

@@ -454,3 +456,55 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) ([]deprecation.W
454456
}
455457
return warnings, nil
456458
}
459+
460+
func (config *Config) GetSandboxRuntime(podSandboxConfig *runtime.PodSandboxConfig, runtimeHandler string) (Runtime, error) {
461+
if untrustedWorkload(podSandboxConfig) {
462+
// If the untrusted annotation is provided, runtimeHandler MUST be empty.
463+
if runtimeHandler != "" && runtimeHandler != RuntimeUntrusted {
464+
return Runtime{}, errors.New("untrusted workload with explicit runtime handler is not allowed")
465+
}
466+
467+
// If the untrusted workload is requesting access to the host/node, this request will fail.
468+
//
469+
// Note: If the workload is marked untrusted but requests privileged, this can be granted, as the
470+
// runtime may support this. For example, in a virtual-machine isolated runtime, privileged
471+
// is a supported option, granting the workload to access the entire guest VM instead of host.
472+
// TODO(windows): Deprecate this so that we don't need to handle it for windows.
473+
if hostAccessingSandbox(podSandboxConfig) {
474+
return Runtime{}, errors.New("untrusted workload with host access is not allowed")
475+
}
476+
477+
runtimeHandler = RuntimeUntrusted
478+
}
479+
480+
if runtimeHandler == "" {
481+
runtimeHandler = config.DefaultRuntimeName
482+
}
483+
484+
r, ok := config.Runtimes[runtimeHandler]
485+
if !ok {
486+
return Runtime{}, fmt.Errorf("no runtime for %q is configured", runtimeHandler)
487+
}
488+
return r, nil
489+
490+
}
491+
492+
// untrustedWorkload returns true if the sandbox contains untrusted workload.
493+
func untrustedWorkload(config *runtime.PodSandboxConfig) bool {
494+
return config.GetAnnotations()[annotations.UntrustedWorkload] == "true"
495+
}
496+
497+
// hostAccessingSandbox returns true if the sandbox configuration
498+
// requires additional host access for the sandbox.
499+
func hostAccessingSandbox(config *runtime.PodSandboxConfig) bool {
500+
securityContext := config.GetLinux().GetSecurityContext()
501+
502+
namespaceOptions := securityContext.GetNamespaceOptions()
503+
if namespaceOptions.GetNetwork() == runtime.NamespaceMode_NODE ||
504+
namespaceOptions.GetPid() == runtime.NamespaceMode_NODE ||
505+
namespaceOptions.GetIpc() == runtime.NamespaceMode_NODE {
506+
return true
507+
}
508+
509+
return false
510+
}

‎pkg/cri/config/config_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"testing"
2222

2323
"github.com/stretchr/testify/assert"
24+
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
2425

2526
"github.com/containerd/containerd/v2/pkg/deprecation"
2627
)
@@ -232,3 +233,50 @@ func TestValidateConfig(t *testing.T) {
232233
})
233234
}
234235
}
236+
237+
func TestHostAccessingSandbox(t *testing.T) {
238+
privilegedContext := &runtime.PodSandboxConfig{
239+
Linux: &runtime.LinuxPodSandboxConfig{
240+
SecurityContext: &runtime.LinuxSandboxSecurityContext{
241+
Privileged: true,
242+
},
243+
},
244+
}
245+
nonPrivilegedContext := &runtime.PodSandboxConfig{
246+
Linux: &runtime.LinuxPodSandboxConfig{
247+
SecurityContext: &runtime.LinuxSandboxSecurityContext{
248+
Privileged: false,
249+
},
250+
},
251+
}
252+
hostNamespace := &runtime.PodSandboxConfig{
253+
Linux: &runtime.LinuxPodSandboxConfig{
254+
SecurityContext: &runtime.LinuxSandboxSecurityContext{
255+
Privileged: false,
256+
NamespaceOptions: &runtime.NamespaceOption{
257+
Network: runtime.NamespaceMode_NODE,
258+
Pid: runtime.NamespaceMode_NODE,
259+
Ipc: runtime.NamespaceMode_NODE,
260+
},
261+
},
262+
},
263+
}
264+
tests := []struct {
265+
name string
266+
config *runtime.PodSandboxConfig
267+
want bool
268+
}{
269+
{"Security Context is nil", nil, false},
270+
{"Security Context is privileged", privilegedContext, false},
271+
{"Security Context is not privileged", nonPrivilegedContext, false},
272+
{"Security Context namespace host access", hostNamespace, true},
273+
}
274+
for _, tt := range tests {
275+
tt := tt
276+
t.Run(tt.name, func(t *testing.T) {
277+
if got := hostAccessingSandbox(tt.config); got != tt.want {
278+
t.Errorf("hostAccessingSandbox() = %v, want %v", got, tt.want)
279+
}
280+
})
281+
}
282+
}

‎pkg/cri/cri.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ func init() {
5050
plugins.ServicePlugin,
5151
plugins.NRIApiPlugin,
5252
plugins.WarningPlugin,
53+
plugins.SandboxControllerPlugin,
5354
},
5455
InitFn: initCRIService,
5556
})
@@ -92,6 +93,7 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
9293
containerd.WithDefaultNamespace(constants.K8sContainerdNamespace),
9394
containerd.WithDefaultPlatform(platforms.Default()),
9495
containerd.WithInMemoryServices(ic),
96+
containerd.WithInMemorySandboxControllers(ic),
9597
)
9698
if err != nil {
9799
return nil, fmt.Errorf("failed to create containerd client: %w", err)

‎pkg/cri/server/container_create.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"strings"
2626
"time"
2727

28+
"github.com/containerd/log"
2829
"github.com/containerd/typeurl/v2"
2930
"github.com/davecgh/go-spew/spew"
3031
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -45,7 +46,6 @@ import (
4546
containerstore "github.com/containerd/containerd/v2/pkg/cri/store/container"
4647
"github.com/containerd/containerd/v2/pkg/cri/util"
4748
"github.com/containerd/containerd/v2/platforms"
48-
"github.com/containerd/log"
4949
)
5050

5151
func init() {
@@ -63,7 +63,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
6363
return nil, fmt.Errorf("failed to find sandbox id %q: %w", r.GetPodSandboxId(), err)
6464
}
6565

66-
controller, err := c.getSandboxController(sandbox.Config, sandbox.RuntimeHandler)
66+
controller, err := c.sandboxService.SandboxController(sandbox.Config, sandbox.RuntimeHandler)
6767
if err != nil {
6868
return nil, fmt.Errorf("failed to get sandbox controller: %w", err)
6969
}
@@ -163,7 +163,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
163163
log.G(ctx).Debugf("Ignoring volumes defined in image %v because IgnoreImageDefinedVolumes is set", image.ID)
164164
}
165165

166-
ociRuntime, err := c.getSandboxRuntime(sandboxConfig, sandbox.Metadata.RuntimeHandler)
166+
ociRuntime, err := c.config.GetSandboxRuntime(sandboxConfig, sandbox.Metadata.RuntimeHandler)
167167
if err != nil {
168168
return nil, fmt.Errorf("failed to get sandbox runtime: %w", err)
169169
}

‎pkg/cri/server/container_start.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func (c *criService) StartContainer(ctx context.Context, r *runtime.StartContain
109109
return cntr.IO, nil
110110
}
111111

112-
ociRuntime, err := c.getSandboxRuntime(sandbox.Config, sandbox.Metadata.RuntimeHandler)
112+
ociRuntime, err := c.config.GetSandboxRuntime(sandbox.Config, sandbox.Metadata.RuntimeHandler)
113113
if err != nil {
114114
return nil, fmt.Errorf("failed to get sandbox runtime: %w", err)
115115
}

‎pkg/cri/server/container_stats_list.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,16 @@ import (
2626
wstats "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/stats"
2727
cg1 "github.com/containerd/cgroups/v3/cgroup1/stats"
2828
cg2 "github.com/containerd/cgroups/v3/cgroup2/stats"
29-
"github.com/containerd/containerd/v2/api/services/tasks/v1"
30-
"github.com/containerd/containerd/v2/api/types"
31-
"github.com/containerd/containerd/v2/errdefs"
32-
"github.com/containerd/containerd/v2/pkg/cri/store/stats"
33-
"github.com/containerd/containerd/v2/protobuf"
3429
"github.com/containerd/log"
3530
"github.com/containerd/typeurl/v2"
3631
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
3732

33+
"github.com/containerd/containerd/v2/api/services/tasks/v1"
34+
"github.com/containerd/containerd/v2/api/types"
35+
"github.com/containerd/containerd/v2/errdefs"
3836
containerstore "github.com/containerd/containerd/v2/pkg/cri/store/container"
37+
"github.com/containerd/containerd/v2/pkg/cri/store/stats"
38+
"github.com/containerd/containerd/v2/protobuf"
3939
)
4040

4141
// ListContainerStats returns stats of all running containers.
@@ -68,7 +68,7 @@ func (c *criService) getMetricsHandler(ctx context.Context, sandboxID string) (m
6868
if err != nil {
6969
return nil, fmt.Errorf("failed to find sandbox id %q: %w", sandboxID, err)
7070
}
71-
controller, err := c.getSandboxController(sandbox.Config, sandbox.RuntimeHandler)
71+
controller, err := c.sandboxService.SandboxController(sandbox.Config, sandbox.RuntimeHandler)
7272
if err != nil {
7373
return nil, fmt.Errorf("failed to get sandbox controller: %w", err)
7474
}
@@ -81,7 +81,7 @@ func (c *criService) getMetricsHandler(ctx context.Context, sandboxID string) (m
8181
return nil, err
8282
}
8383

84-
ociRuntime, err := c.getSandboxRuntime(sandbox.Config, sandbox.RuntimeHandler)
84+
ociRuntime, err := c.config.GetSandboxRuntime(sandbox.Config, sandbox.RuntimeHandler)
8585
if err != nil {
8686
return nil, fmt.Errorf("failed to get runtimeHandler %q: %w", sandbox.RuntimeHandler, err)
8787
}

‎pkg/cri/server/images/image_pull.go‎

Lines changed: 3 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"crypto/tls"
2222
"encoding/base64"
23-
"errors"
2423
"fmt"
2524
"io"
2625
"net"
@@ -33,6 +32,8 @@ import (
3332
"sync/atomic"
3433
"time"
3534

35+
"github.com/containerd/log"
36+
distribution "github.com/distribution/reference"
3637
imagedigest "github.com/opencontainers/go-digest"
3738
imagespec "github.com/opencontainers/image-spec/specs-go/v1"
3839
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
@@ -47,8 +48,6 @@ import (
4748
"github.com/containerd/containerd/v2/remotes/docker"
4849
"github.com/containerd/containerd/v2/remotes/docker/config"
4950
"github.com/containerd/containerd/v2/tracing"
50-
"github.com/containerd/log"
51-
distribution "github.com/distribution/reference"
5251
)
5352

5453
// For image management:
@@ -755,7 +754,7 @@ func (c *CRIImageService) snapshotterFromPodSandboxConfig(ctx context.Context, i
755754
}
756755

757756
// TODO: Find other way to retrieve sandbox runtime, this must belong to the Runtime part of the CRI.
758-
ociRuntime, err := c.getSandboxRuntime(s, runtimeHandler)
757+
ociRuntime, err := c.config.GetSandboxRuntime(s, runtimeHandler)
759758
if err != nil {
760759
return "", fmt.Errorf("experimental: failed to get sandbox runtime for %s: %w", runtimeHandler, err)
761760
}
@@ -764,55 +763,3 @@ func (c *CRIImageService) snapshotterFromPodSandboxConfig(ctx context.Context, i
764763
log.G(ctx).Infof("experimental: PullImage %q for runtime %s, using snapshotter %s", imageRef, runtimeHandler, snapshotter)
765764
return snapshotter, nil
766765
}
767-
768-
// TODO: copy-pasted from the runtime service implementation. This should not be in image service.
769-
func (c *CRIImageService) getSandboxRuntime(config *runtime.PodSandboxConfig, runtimeHandler string) (criconfig.Runtime, error) {
770-
if untrustedWorkload(config) {
771-
// If the untrusted annotation is provided, runtimeHandler MUST be empty.
772-
if runtimeHandler != "" && runtimeHandler != criconfig.RuntimeUntrusted {
773-
return criconfig.Runtime{}, errors.New("untrusted workload with explicit runtime handler is not allowed")
774-
}
775-
776-
// If the untrusted workload is requesting access to the host/node, this request will fail.
777-
//
778-
// Note: If the workload is marked untrusted but requests privileged, this can be granted, as the
779-
// runtime may support this. For example, in a virtual-machine isolated runtime, privileged
780-
// is a supported option, granting the workload to access the entire guest VM instead of host.
781-
// TODO(windows): Deprecate this so that we don't need to handle it for windows.
782-
if hostAccessingSandbox(config) {
783-
return criconfig.Runtime{}, errors.New("untrusted workload with host access is not allowed")
784-
}
785-
786-
runtimeHandler = criconfig.RuntimeUntrusted
787-
}
788-
789-
if runtimeHandler == "" {
790-
runtimeHandler = c.config.ContainerdConfig.DefaultRuntimeName
791-
}
792-
793-
handler, ok := c.config.ContainerdConfig.Runtimes[runtimeHandler]
794-
if !ok {
795-
return criconfig.Runtime{}, fmt.Errorf("no runtime for %q is configured", runtimeHandler)
796-
}
797-
return handler, nil
798-
}
799-
800-
// untrustedWorkload returns true if the sandbox contains untrusted workload.
801-
func untrustedWorkload(config *runtime.PodSandboxConfig) bool {
802-
return config.GetAnnotations()[annotations.UntrustedWorkload] == "true"
803-
}
804-
805-
// hostAccessingSandbox returns true if the sandbox configuration
806-
// requires additional host access for the sandbox.
807-
func hostAccessingSandbox(config *runtime.PodSandboxConfig) bool {
808-
securityContext := config.GetLinux().GetSecurityContext()
809-
810-
namespaceOptions := securityContext.GetNamespaceOptions()
811-
if namespaceOptions.GetNetwork() == runtime.NamespaceMode_NODE ||
812-
namespaceOptions.GetPid() == runtime.NamespaceMode_NODE ||
813-
namespaceOptions.GetIpc() == runtime.NamespaceMode_NODE {
814-
return true
815-
}
816-
817-
return false
818-
}

0 commit comments

Comments
 (0)