diff --git a/cmd/containerd-shim-runc-v2/process/init.go b/cmd/containerd-shim-runc-v2/process/init.go index 47097faa7f562..ca808bb6e8b19 100644 --- a/cmd/containerd-shim-runc-v2/process/init.go +++ b/cmd/containerd-shim-runc-v2/process/init.go @@ -21,7 +21,6 @@ package process import ( "context" "encoding/json" - "errors" "fmt" "io" "os" @@ -142,12 +141,6 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error { opts.ConsoleSocket = socket } - // runc ignores silently features it doesn't know about, so for things that this is - // problematic let's check if this runc version supports them. - if err := p.validateRuncFeatures(ctx, r.Bundle); err != nil { - return fmt.Errorf("failed to detect OCI runtime features: %w", err) - } - if err := p.runtime.Create(ctx, r.ID, r.Bundle, opts); err != nil { return p.runtimeError(err, "OCI runtime create failed") } @@ -181,60 +174,6 @@ func (p *Init) Create(ctx context.Context, r *CreateConfig) error { return nil } -func (p *Init) validateRuncFeatures(ctx context.Context, bundle string) error { - // TODO: We should remove the logic from here and rebase on #8509. - // This way we can avoid the call to readConfig() here and the call to p.runtime.Features() - // in validateIDMapMounts(). - // But that PR is not yet merged nor it is clear if it will be refactored. - // Do this contained hack for now. - spec, err := readConfig(bundle) - if err != nil { - return fmt.Errorf("failed to read config: %w", err) - } - - if err := p.validateIDMapMounts(ctx, spec); err != nil { - return fmt.Errorf("OCI runtime doesn't support idmap mounts: %w", err) - } - - return nil -} - -func (p *Init) validateIDMapMounts(ctx context.Context, spec *specs.Spec) error { - var used bool - for _, m := range spec.Mounts { - if m.UIDMappings != nil || m.GIDMappings != nil { - used = true - break - } - if sliceContainsStr(m.Options, "idmap") || sliceContainsStr(m.Options, "ridmap") { - used = true - break - } - } - - if !used { - return nil - } - - // From here onwards, we require idmap mounts. So if we fail to check, we return an error. - features, err := p.runtime.Features(ctx) - if err != nil { - // If the features command is not implemented, then runc is too old. - return fmt.Errorf("features command failed: %w", err) - - } - - if features.Linux.MountExtensions == nil || features.Linux.MountExtensions.IDMap == nil { - return errors.New("missing `mountExtensions.idmap` entry in `features` command") - } - - if enabled := features.Linux.MountExtensions.IDMap.Enabled; enabled == nil || !*enabled { - return errors.New("idmap mounts not supported") - } - - return nil -} - func (p *Init) openStdin(path string) error { sc, err := fifo.OpenFifo(context.Background(), path, unix.O_WRONLY|unix.O_NONBLOCK, 0) if err != nil { @@ -556,12 +495,3 @@ func withConditionalIO(c stdio.Stdio) runc.IOOpt { o.OpenStderr = c.Stderr != "" } } - -func sliceContainsStr(s []string, str string) bool { - for _, s := range s { - if s == str { - return true - } - } - return false -} diff --git a/cmd/containerd-shim-runc-v2/process/utils.go b/cmd/containerd-shim-runc-v2/process/utils.go index 5a1c2ffafbe10..547279093375d 100644 --- a/cmd/containerd-shim-runc-v2/process/utils.go +++ b/cmd/containerd-shim-runc-v2/process/utils.go @@ -21,7 +21,6 @@ package process import ( "context" "encoding/json" - "errors" "fmt" "io" "os" @@ -32,7 +31,6 @@ import ( "github.com/containerd/errdefs" runc "github.com/containerd/go-runc" - specs "github.com/opencontainers/runtime-spec/specs-go" "golang.org/x/sys/unix" ) @@ -188,23 +186,3 @@ func stateName(v interface{}) string { } panic(fmt.Errorf("invalid state %v", v)) } - -func readConfig(path string) (spec *specs.Spec, err error) { - cfg := filepath.Join(path, configFile) - f, err := os.Open(cfg) - if err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("JSON specification file %s not found", cfg) - } - return nil, err - } - defer f.Close() - - if err = json.NewDecoder(f).Decode(&spec); err != nil { - return nil, fmt.Errorf("failed to parse config: %w", err) - } - if spec == nil { - return nil, errors.New("config cannot be null") - } - return spec, nil -} diff --git a/core/runtime/v2/manager.go b/core/runtime/v2/manager.go index 62c3b535b871e..0096bb80e3df7 100644 --- a/core/runtime/v2/manager.go +++ b/core/runtime/v2/manager.go @@ -45,6 +45,9 @@ import ( "github.com/containerd/platforms" "github.com/containerd/plugin" "github.com/containerd/plugin/registry" + "github.com/containerd/typeurl/v2" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/opencontainers/runtime-spec/specs-go/features" ) // Config for the v2 runtime @@ -455,6 +458,10 @@ func (m *TaskManager) Create(ctx context.Context, taskID string, opts runtime.Cr return nil, err } + if err := m.validateRuntimeFeatures(ctx, opts); err != nil { + return nil, fmt.Errorf("failed to validate OCI runtime features: %w", err) + } + t, err := shimTask.Create(ctx, opts) if err != nil { // NOTE: ctx contains required namespace information. @@ -568,3 +575,83 @@ func (m *TaskManager) PluginInfo(ctx context.Context, request interface{}) (inte } return &info, nil } + +func (m *TaskManager) validateRuntimeFeatures(ctx context.Context, opts runtime.CreateOpts) error { + var spec specs.Spec + if err := typeurl.UnmarshalTo(opts.Spec, &spec); err != nil { + return fmt.Errorf("unmarshal spec: %w", err) + } + + // Only ask for the PluginInfo if idmap mounts are used. + if !usesIDMapMounts(spec) { + return nil + } + + pInfo, err := m.PluginInfo(ctx, &apitypes.RuntimeRequest{RuntimePath: opts.Runtime}) + if err != nil { + return fmt.Errorf("runtime info: %w", err) + } + + pluginInfo, ok := pInfo.(*apitypes.RuntimeInfo) + if !ok { + return fmt.Errorf("invalid runtime info type: %T", pInfo) + } + + feat, err := typeurl.UnmarshalAny(pluginInfo.Features) + if err != nil { + return fmt.Errorf("unmarshal runtime features: %w", err) + } + + // runc-compatible runtimes silently ignores features it doesn't know about. But ignoring + // our request to use idmap mounts can break permissions in the volume, so let's make sure + // it supports it. For more info, see: + // https://github.com/opencontainers/runtime-spec/pull/1219 + // + features, ok := feat.(*features.Features) + if !ok { + // Leave alone non runc-compatible runtimes that don't provide the features info, + // they might not be affected by this. + return nil + } + + if err := supportsIDMapMounts(features); err != nil { + return fmt.Errorf("idmap mounts: %w", err) + } + + return nil +} + +func usesIDMapMounts(spec specs.Spec) bool { + for _, m := range spec.Mounts { + if m.UIDMappings != nil || m.GIDMappings != nil { + return true + } + // TODO (rata): When we switch to go 1.21, we can use the slices package from stdlib + // instead of sliceContainsStr(). + if sliceContainsStr(m.Options, "idmap") || sliceContainsStr(m.Options, "ridmap") { + return true + } + + } + return false +} + +func supportsIDMapMounts(features *features.Features) error { + if features.Linux.MountExtensions == nil || features.Linux.MountExtensions.IDMap == nil { + return errors.New("missing `mountExtensions.idmap` entry in `features` command") + + } + if enabled := features.Linux.MountExtensions.IDMap.Enabled; enabled == nil || !*enabled { + return errors.New("not supported or disabled") + } + return nil +} + +func sliceContainsStr(s []string, str string) bool { + for _, s := range s { + if s == str { + return true + } + } + return false +}