libcontainerd/supervisor: simplify logic for disabling CRI plugin

The existing implementation used a `nil` value for the CRI plugin's configuration
to indicate that the plugin had to be disabled. Effectively, the `Plugins` value
was only used as an intermediate step, only to be removed later on, and to instead
add the given plugin to `DisabledPlugins` in the containerd configuration.

This patch removes the intermediate step; as a result we also don't need to mask
the containerd `Plugins` field, which was added to allow serializing the toml.

A code comment was added as well to explain why we're (currently) disabling the
CRI plugin by default, which may help future visitors of the code to determin
if that default is still needed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

cmd/dockerd/daemon.go

@@ -596,7 +596,19 @@ func (cli *DaemonCli) getContainerdDaemonOpts() ([]supervisor.DaemonOpt, error)
 	}
 
 	if !cli.Config.CriContainerd {
-		opts = append(opts, supervisor.WithPlugin("io.containerd.grpc.v1.cri", nil))
+		// CRI support in the managed daemon is currently opt-in.
+		//
+		// It's disabled by default, originally because it was listening on
+		// a TCP connection at 0.0.0.0:10010, which was considered a security
+		// risk, and could conflict with user's container ports.
+		//
+		// Current versions of containerd started now listen on localhost on
+		// an ephemeral port instead, but could still conflict with container
+		// ports, and running kubernetes using the static binaries is not a
+		// common scenario, so we (for now) continue disabling it by default.
+		//
+		// Also see https://github.com/containerd/containerd/issues/2483#issuecomment-407530608
+		opts = append(opts, supervisor.WithCRIDisabled())
 	}
 
 	return opts, nil

libcontainerd/supervisor/remote_daemon.go

@@ -33,8 +33,6 @@ const (
 type remote struct {
 	sync.RWMutex
 	config.Config
-	// Plugins overrides `Plugins map[string]toml.Tree` in config config.
-	Plugins map[string]interface{} `toml:"plugins"`
 
 	daemonPid int
 	logger    *logrus.Entry
@@ -66,7 +64,6 @@ func Start(ctx context.Context, rootDir, stateDir string, opts ...DaemonOpt) (Da
 			Root:    filepath.Join(rootDir, "daemon"),
 			State:   filepath.Join(stateDir, "daemon"),
 		},
-		Plugins:       make(map[string]interface{}),
 		daemonPid:     -1,
 		logger:        logrus.WithField("module", "libcontainerd"),
 		daemonStartCh: make(chan error, 1),

libcontainerd/supervisor/remote_daemon_linux.go

@@ -28,13 +28,6 @@ func (r *remote) setDefaults() {
 	if r.Debug.Address == "" {
 		r.Debug.Address = filepath.Join(r.stateDir, debugSockFile)
 	}
-
-	for key, conf := range r.Plugins {
-		if conf == nil {
-			r.DisabledPlugins = append(r.DisabledPlugins, key)
-			delete(r.Plugins, key)
-		}
-	}
 }
 
 func (r *remote) stopDaemon() {

libcontainerd/supervisor/remote_daemon_options.go

@@ -9,12 +9,10 @@ func WithLogLevel(lvl string) DaemonOpt {
 	}
 }
 
-// WithPlugin allow configuring a containerd plugin
-// configuration values passed needs to be quoted if quotes are needed in
-// the toml format.
-func WithPlugin(name string, conf interface{}) DaemonOpt {
+// WithCRIDisabled disables the CRI plugin.
+func WithCRIDisabled() DaemonOpt {
 	return func(r *remote) error {
-		r.Plugins[name] = conf
+		r.DisabledPlugins = append(r.DisabledPlugins, "io.containerd.grpc.v1.cri")
 		return nil
 	}
 }