From 9547d269a188c28e563470a597c8da7deb5b0734 Mon Sep 17 00:00:00 2001
From: Michael Crosby <crosbymichael@gmail.com>
Date: Tue, 4 Jun 2019 17:55:58 +0000
Subject: [PATCH] Add v2 server config support with plugin URIs

Closes #3210

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
---
 .travis.yml                                  |  6 ++
 client_test.go                               |  4 +
 cmd/containerd/command/config.go             |  2 +-
 cmd/containerd/command/config_linux.go       |  5 +-
 cmd/containerd/command/config_unsupported.go |  5 +-
 cmd/containerd/command/config_windows.go     |  5 +-
 daemon_config_linux_test.go                  |  2 +
 plugin/plugin.go                             | 49 ++++++++----
 services/server/config/config.go             | 79 ++++++++++++++++++--
 services/server/server.go                    | 23 ++++--
 10 files changed, 147 insertions(+), 33 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index c2698acbe..f0711aac5 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -74,6 +74,11 @@ script:
   - go build -i .
   - make check
   - if [ "$GOOS" = "linux" ]; then make check-protos check-api-descriptors; fi
+  - |
+    sudo mkdir -p /etc/containerd
+    sudo bash -c "cat > /etc/containerd/config.toml <<EOF
+    version = 1
+    EOF"
   - make build
   - make binaries
   - if [ "$TRAVIS_GOOS" = "linux" ]; then sudo make install ; fi
@@ -86,6 +91,7 @@ script:
     if [ "$TRAVIS_GOOS" = "linux" ]; then
       sudo mkdir -p /etc/containerd
       sudo bash -c "cat > /etc/containerd/config.toml <<EOF
+      version = 1
       [plugins.cri.containerd.default_runtime]
         runtime_type = \"${TEST_RUNTIME}\"
     EOF"
diff --git a/client_test.go b/client_test.go
index 75426da77..30ed5b107 100644
--- a/client_test.go
+++ b/client_test.go
@@ -390,6 +390,10 @@ func createShimDebugConfig() string {
 		os.Exit(1)
 	}
 	defer f.Close()
+	if _, err := f.WriteString("version = 1\n"); err != nil {
+		fmt.Fprintf(os.Stderr, "Failed to write to config file %s: %s\n", f.Name(), err)
+		os.Exit(1)
+	}
 
 	if _, err := f.WriteString("[plugins.linux]\n\tshim_debug = true\n"); err != nil {
 		fmt.Fprintf(os.Stderr, "Failed to write to config file %s: %s\n", f.Name(), err)
diff --git a/cmd/containerd/command/config.go b/cmd/containerd/command/config.go
index 9fd870c35..de9790ecf 100644
--- a/cmd/containerd/command/config.go
+++ b/cmd/containerd/command/config.go
@@ -60,7 +60,7 @@ var configCommand = cli.Command{
 						if p.Config == nil {
 							continue
 						}
-						config.Plugins[p.ID] = p.Config
+						config.Plugins[p.URI()] = p.Config
 					}
 				}
 				_, err = config.WriteTo(os.Stdout)
diff --git a/cmd/containerd/command/config_linux.go b/cmd/containerd/command/config_linux.go
index 111410ad6..3bbc0ab0d 100644
--- a/cmd/containerd/command/config_linux.go
+++ b/cmd/containerd/command/config_linux.go
@@ -23,8 +23,9 @@ import (
 
 func defaultConfig() *srvconfig.Config {
 	return &srvconfig.Config{
-		Root:  defaults.DefaultRootDir,
-		State: defaults.DefaultStateDir,
+		Version: 2,
+		Root:    defaults.DefaultRootDir,
+		State:   defaults.DefaultStateDir,
 		GRPC: srvconfig.GRPCConfig{
 			Address:        defaults.DefaultAddress,
 			MaxRecvMsgSize: defaults.DefaultMaxRecvMsgSize,
diff --git a/cmd/containerd/command/config_unsupported.go b/cmd/containerd/command/config_unsupported.go
index c19ab1ee7..0408ac230 100644
--- a/cmd/containerd/command/config_unsupported.go
+++ b/cmd/containerd/command/config_unsupported.go
@@ -25,8 +25,9 @@ import (
 
 func defaultConfig() *srvconfig.Config {
 	return &srvconfig.Config{
-		Root:  defaults.DefaultRootDir,
-		State: defaults.DefaultStateDir,
+		Version: 2,
+		Root:    defaults.DefaultRootDir,
+		State:   defaults.DefaultStateDir,
 		GRPC: srvconfig.GRPCConfig{
 			Address: defaults.DefaultAddress,
 		},
diff --git a/cmd/containerd/command/config_windows.go b/cmd/containerd/command/config_windows.go
index 111410ad6..3bbc0ab0d 100644
--- a/cmd/containerd/command/config_windows.go
+++ b/cmd/containerd/command/config_windows.go
@@ -23,8 +23,9 @@ import (
 
 func defaultConfig() *srvconfig.Config {
 	return &srvconfig.Config{
-		Root:  defaults.DefaultRootDir,
-		State: defaults.DefaultStateDir,
+		Version: 2,
+		Root:    defaults.DefaultRootDir,
+		State:   defaults.DefaultStateDir,
 		GRPC: srvconfig.GRPCConfig{
 			Address:        defaults.DefaultAddress,
 			MaxRecvMsgSize: defaults.DefaultMaxRecvMsgSize,
diff --git a/daemon_config_linux_test.go b/daemon_config_linux_test.go
index e82c32778..d11d1d9f8 100644
--- a/daemon_config_linux_test.go
+++ b/daemon_config_linux_test.go
@@ -127,6 +127,7 @@ func TestDaemonRuntimeRoot(t *testing.T) {
 		}
 	}()
 	configTOML := `
+version = 1
 [plugins]
  [plugins.cri]
    stream_server_port = "0"
@@ -221,6 +222,7 @@ func TestDaemonCustomCgroup(t *testing.T) {
 
 	customCgroup := fmt.Sprintf("%d", time.Now().Nanosecond())
 	configTOML := `
+version = 1
 [cgroup]
   path = "` + customCgroup + `"`
 
diff --git a/plugin/plugin.go b/plugin/plugin.go
index 9ae8bbeb5..c7d272414 100644
--- a/plugin/plugin.go
+++ b/plugin/plugin.go
@@ -30,7 +30,8 @@ var (
 	ErrNoType = errors.New("plugin: no type")
 	// ErrNoPluginID is returned when no id is specified
 	ErrNoPluginID = errors.New("plugin: no id")
-
+	// ErrIDRegistered is returned when a duplicate id is already registered
+	ErrIDRegistered = errors.New("plugin: id already registered")
 	// ErrSkipPlugin is used when a plugin is not initialized and should not be loaded,
 	// this allows the plugin loader differentiate between a plugin which is configured
 	// not to load and one that fails to load.
@@ -100,6 +101,8 @@ type Registration struct {
 	// context are passed in. The init function may modify the registration to
 	// add exports, capabilities and platform support declarations.
 	InitFn func(*InitContext) (interface{}, error)
+	// Disable the plugin from loading
+	Disable bool
 }
 
 // Init the registered plugin
@@ -157,12 +160,16 @@ func Load(path string) (err error) {
 func Register(r *Registration) {
 	register.Lock()
 	defer register.Unlock()
+
 	if r.Type == "" {
 		panic(ErrNoType)
 	}
 	if r.ID == "" {
 		panic(ErrNoPluginID)
 	}
+	if err := checkUnique(r); err != nil {
+		panic(err)
+	}
 
 	var last bool
 	for _, requires := range r.Requires {
@@ -177,24 +184,36 @@ func Register(r *Registration) {
 	register.r = append(register.r, r)
 }
 
+func checkUnique(r *Registration) error {
+	for _, registered := range register.r {
+		if r.URI() == registered.URI() {
+			return errors.Wrap(ErrIDRegistered, r.URI())
+		}
+	}
+	return nil
+}
+
+// DisableFilter filters out disabled plugins
+type DisableFilter func(r *Registration) bool
+
 // Graph returns an ordered list of registered plugins for initialization.
 // Plugins in disableList specified by id will be disabled.
-func Graph(disableList []string) (ordered []*Registration) {
+func Graph(filter DisableFilter) (ordered []*Registration) {
 	register.RLock()
 	defer register.RUnlock()
-	for _, d := range disableList {
-		for i, r := range register.r {
-			if r.ID == d {
-				register.r = append(register.r[:i], register.r[i+1:]...)
-				break
-			}
+
+	for _, r := range register.r {
+		if filter(r) {
+			r.Disable = true
 		}
 	}
 
 	added := map[*Registration]bool{}
 	for _, r := range register.r {
-
-		children(r.ID, r.Requires, added, &ordered)
+		if r.Disable {
+			continue
+		}
+		children(r, added, &ordered)
 		if !added[r] {
 			ordered = append(ordered, r)
 			added[r] = true
@@ -203,11 +222,13 @@ func Graph(disableList []string) (ordered []*Registration) {
 	return ordered
 }
 
-func children(id string, types []Type, added map[*Registration]bool, ordered *[]*Registration) {
-	for _, t := range types {
+func children(reg *Registration, added map[*Registration]bool, ordered *[]*Registration) {
+	for _, t := range reg.Requires {
 		for _, r := range register.r {
-			if r.ID != id && (t == "*" || r.Type == t) {
-				children(r.ID, r.Requires, added, ordered)
+			if !r.Disable &&
+				r.URI() != reg.URI() &&
+				(t == "*" || r.Type == t) {
+				children(r, added, ordered)
 				if !added[r] {
 					*ordered = append(*ordered, r)
 					added[r] = true
diff --git a/services/server/config/config.go b/services/server/config/config.go
index 26fb92599..13ccd5b6f 100644
--- a/services/server/config/config.go
+++ b/services/server/config/config.go
@@ -17,13 +17,18 @@
 package config
 
 import (
+	"strings"
+
 	"github.com/BurntSushi/toml"
 	"github.com/containerd/containerd/errdefs"
+	"github.com/containerd/containerd/plugin"
 	"github.com/pkg/errors"
 )
 
 // Config provides containerd configuration data for the server
 type Config struct {
+	// Version of the config file
+	Version int `toml:"version"`
 	// Root is the path to a directory where containerd will store persistent data
 	Root string `toml:"root"`
 	// State is the path to a directory where containerd will store transient data
@@ -54,6 +59,42 @@ type Config struct {
 	md toml.MetaData
 }
 
+// GetVersion returns the config file's version
+func (c *Config) GetVersion() int {
+	if c.Version == 0 {
+		return 1
+	}
+	return c.Version
+}
+
+// ValidateV2 validates the config for a v2 file
+func (c *Config) ValidateV2() error {
+	if c.GetVersion() != 2 {
+		return nil
+	}
+	for _, p := range c.DisabledPlugins {
+		if len(strings.Split(p, ".")) < 4 {
+			return errors.Errorf("invalid disabled plugin URI %q expect io.containerd.x.vx", p)
+		}
+	}
+	for _, p := range c.RequiredPlugins {
+		if len(strings.Split(p, ".")) < 4 {
+			return errors.Errorf("invalid required plugin URI %q expect io.containerd.x.vx", p)
+		}
+	}
+	for p := range c.Plugins {
+		if len(strings.Split(p, ".")) < 4 {
+			return errors.Errorf("invalid plugin key URI %q expect io.containerd.x.vx", p)
+		}
+	}
+	for p := range c.ProxyPlugins {
+		if len(strings.Split(p, ".")) < 4 {
+			return errors.Errorf("invalid proxy plugin key URI %q expect io.containerd.x.vx", p)
+		}
+	}
+	return nil
+}
+
 // GRPCConfig provides GRPC configuration for the socket
 type GRPCConfig struct {
 	Address        string `toml:"address"`
@@ -130,15 +171,19 @@ func (bc *BoltConfig) Validate() error {
 }
 
 // Decode unmarshals a plugin specific configuration by plugin id
-func (c *Config) Decode(id string, v interface{}) (interface{}, error) {
+func (c *Config) Decode(p *plugin.Registration) (interface{}, error) {
+	id := p.URI()
+	if c.GetVersion() == 1 {
+		id = p.ID
+	}
 	data, ok := c.Plugins[id]
 	if !ok {
-		return v, nil
+		return p.Config, nil
 	}
-	if err := c.md.PrimitiveDecode(data, v); err != nil {
+	if err := c.md.PrimitiveDecode(data, p.Config); err != nil {
 		return nil, err
 	}
-	return v, nil
+	return p.Config, nil
 }
 
 // LoadConfig loads the containerd server config from the provided path
@@ -151,5 +196,29 @@ func LoadConfig(path string, v *Config) error {
 		return err
 	}
 	v.md = md
-	return nil
+	return v.ValidateV2()
+}
+
+// V1DisabledFilter matches based on ID
+func V1DisabledFilter(list []string) plugin.DisableFilter {
+	set := make(map[string]struct{}, len(list))
+	for _, l := range list {
+		set[l] = struct{}{}
+	}
+	return func(r *plugin.Registration) bool {
+		_, ok := set[r.ID]
+		return ok
+	}
+}
+
+// V2DisabledFilter matches based on URI
+func V2DisabledFilter(list []string) plugin.DisableFilter {
+	set := make(map[string]struct{}, len(list))
+	for _, l := range list {
+		set[l] = struct{}{}
+	}
+	return func(r *plugin.Registration) bool {
+		_, ok := set[r.URI()]
+		return ok
+	}
 }
diff --git a/services/server/server.go b/services/server/server.go
index 14d9a5ef1..3c85b4b7c 100644
--- a/services/server/server.go
+++ b/services/server/server.go
@@ -126,6 +126,10 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) {
 	}
 	for _, p := range plugins {
 		id := p.URI()
+		reqID := id
+		if config.GetVersion() == 1 {
+			reqID = p.ID
+		}
 		log.G(ctx).WithField("type", p.Type).Infof("loading plugin %q...", id)
 
 		initContext := plugin.NewContext(
@@ -140,11 +144,11 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) {
 
 		// load the plugin specific configuration if it is provided
 		if p.Config != nil {
-			pluginConfig, err := config.Decode(p.ID, p.Config)
+			pc, err := config.Decode(p)
 			if err != nil {
 				return nil, err
 			}
-			initContext.Config = pluginConfig
+			initContext.Config = pc
 		}
 		result := p.Init(initContext)
 		if err := initialized.Add(result); err != nil {
@@ -158,12 +162,13 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) {
 			} else {
 				log.G(ctx).WithError(err).Warnf("failed to load plugin %s", id)
 			}
-			if _, ok := required[p.ID]; ok {
+			if _, ok := required[reqID]; ok {
 				return nil, errors.Wrapf(err, "load required plugin %s", id)
 			}
 			continue
 		}
-		delete(required, p.ID)
+
+		delete(required, reqID)
 		// check for grpc services that should be registered with the server
 		if src, ok := instance.(plugin.Service); ok {
 			grpcServices = append(grpcServices, src)
@@ -266,7 +271,7 @@ func (s *Server) Stop() {
 		p := s.plugins[i]
 		instance, err := p.Instance()
 		if err != nil {
-			log.L.WithError(err).WithField("id", p.Registration.ID).
+			log.L.WithError(err).WithField("id", p.Registration.URI()).
 				Errorf("could not get plugin instance")
 			continue
 		}
@@ -275,7 +280,7 @@ func (s *Server) Stop() {
 			continue
 		}
 		if err := closer.Close(); err != nil {
-			log.L.WithError(err).WithField("id", p.Registration.ID).
+			log.L.WithError(err).WithField("id", p.Registration.URI()).
 				Errorf("failed to close plugin")
 		}
 	}
@@ -415,8 +420,12 @@ func LoadPlugins(ctx context.Context, config *srvconfig.Config) ([]*plugin.Regis
 
 	}
 
+	filter := srvconfig.V2DisabledFilter
+	if config.GetVersion() == 1 {
+		filter = srvconfig.V1DisabledFilter
+	}
 	// return the ordered graph for plugins
-	return plugin.Graph(config.DisabledPlugins), nil
+	return plugin.Graph(filter(config.DisabledPlugins)), nil
 }
 
 type proxyClients struct {