From 051e604adcd8f0f57d710d110d75bdd43010168c Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 3 Aug 2022 10:38:15 +0200 Subject: [PATCH] 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 --- cmd/dockerd/daemon.go | 14 +++++++++++++- libcontainerd/supervisor/remote_daemon.go | 3 --- libcontainerd/supervisor/remote_daemon_linux.go | 7 ------- libcontainerd/supervisor/remote_daemon_options.go | 8 +++----- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 7d3dc6ae1a..5ade4f34ff 100644 --- a/cmd/dockerd/daemon.go +++ b/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 diff --git a/libcontainerd/supervisor/remote_daemon.go b/libcontainerd/supervisor/remote_daemon.go index 60ec588ac6..7de2174809 100644 --- a/libcontainerd/supervisor/remote_daemon.go +++ b/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), diff --git a/libcontainerd/supervisor/remote_daemon_linux.go b/libcontainerd/supervisor/remote_daemon_linux.go index 3f019a162f..14c35ff10c 100644 --- a/libcontainerd/supervisor/remote_daemon_linux.go +++ b/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() { diff --git a/libcontainerd/supervisor/remote_daemon_options.go b/libcontainerd/supervisor/remote_daemon_options.go index 75e8a4c2e7..58350be05b 100644 --- a/libcontainerd/supervisor/remote_daemon_options.go +++ b/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 } }