Merge pull request #9143 from dmcgowan/plugin-config-unknown-fields

Add warning for plugin configs with unknown fields

cmd/containerd/command/config.go

@@ -31,7 +31,7 @@ import (
 	"github.com/urfave/cli"
 )
 
-func outputConfig(config *srvconfig.Config) error {
+func outputConfig(ctx gocontext.Context, config *srvconfig.Config) error {
 	plugins, err := server.LoadPlugins(gocontext.Background(), config)
 	if err != nil {
 		return err
@@ -43,7 +43,7 @@ func outputConfig(config *srvconfig.Config) error {
 				continue
 			}
 
-			pc, err := config.Decode(p)
+			pc, err := config.Decode(ctx, p)
 			if err != nil {
 				return err
 			}
@@ -83,7 +83,7 @@ var configCommand = cli.Command{
 			Name:  "default",
 			Usage: "See the output of the default config",
 			Action: func(context *cli.Context) error {
-				return outputConfig(defaultConfig())
+				return outputConfig(gocontext.Background(), defaultConfig())
 			},
 		},
 		{
@@ -91,11 +91,12 @@ var configCommand = cli.Command{
 			Usage: "See the output of the final main config with imported in subconfig files",
 			Action: func(context *cli.Context) error {
 				config := defaultConfig()
-				if err := srvconfig.LoadConfig(gocontext.Background(), context.GlobalString("config"), config); err != nil && !os.IsNotExist(err) {
+				ctx := gocontext.Background()
+				if err := srvconfig.LoadConfig(ctx, context.GlobalString("config"), config); err != nil && !os.IsNotExist(err) {
 					return err
 				}
 
-				return outputConfig(config)
+				return outputConfig(ctx, config)
 			},
 		},
 	},

integration/build_local_containerd_helper_test.go

@@ -96,7 +96,7 @@ func buildLocalContainerdClient(t *testing.T, tmpDir string) *containerd.Client
 
 		// load the plugin specific configuration if it is provided
 		if p.Config != nil {
-			pc, err := config.Decode(p)
+			pc, err := config.Decode(ctx, p)
 			assert.NoError(t, err)
 
 			initContext.Config = pc

services/server/config/config.go

@@ -17,6 +17,7 @@
 package config
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -177,23 +178,35 @@ type ProxyPlugin struct {
 }
 
 // Decode unmarshals a plugin specific configuration by plugin id
-func (c *Config) Decode(p *plugin.Registration) (interface{}, error) {
+func (c *Config) Decode(ctx context.Context, p *plugin.Registration) (interface{}, error) {
 	id := p.URI()
 	data, ok := c.Plugins[id]
 	if !ok {
 		return p.Config, nil
 	}
-	r, w := io.Pipe()
-	go func() {
-		err := toml.NewEncoder(w).Encode(data)
-		w.CloseWithError(err)
-	}()
 
-	// TODO: Add DisallowUnknownFields, requires better testing and bubbling errors
+	b, err := toml.Marshal(data)
-	if err := toml.NewDecoder(r).Decode(p.Config); err != nil {
+	if err != nil {
 		return nil, err
 	}
 
+	if err := toml.NewDecoder(bytes.NewReader(b)).DisallowUnknownFields().Decode(p.Config); err != nil {
+		var serr *toml.StrictMissingError
+		if errors.As(err, &serr) {
+			for _, derr := range serr.Errors {
+				log.G(ctx).WithFields(log.Fields{
+					"plugin": id,
+					"key":    strings.Join(derr.Key(), " "),
+				}).WithError(err).Warn("Ignoring unknown key in TOML for plugin")
+			}
+			err = toml.Unmarshal(b, p.Config)
+		}
+		if err != nil {
+			return nil, err
+		}
+
+	}
+
 	return p.Config, nil
 }
 

services/server/config/config_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/containerd/containerd/plugin"
+	"github.com/containerd/log/logtest"
 )
 
 func TestMergeConfigs(t *testing.T) {
@@ -191,6 +192,7 @@ imports = ["data1.toml", "data2.toml"]
 }
 
 func TestDecodePlugin(t *testing.T) {
+	ctx := logtest.WithT(context.Background(), t)
 	data := `
 version = 2
 [plugins."io.containerd.runtime.v1.linux"]
@@ -208,7 +210,7 @@ version = 2
 	assert.NoError(t, err)
 
 	pluginConfig := map[string]interface{}{}
-	_, err = out.Decode(&plugin.Registration{Type: "io.containerd.runtime.v1", ID: "linux", Config: &pluginConfig})
+	_, err = out.Decode(ctx, &plugin.Registration{Type: "io.containerd.runtime.v1", ID: "linux", Config: &pluginConfig})
 	assert.NoError(t, err)
 	assert.Equal(t, true, pluginConfig["shim_debug"])
 }

services/server/server.go

@@ -201,6 +201,7 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) {
 	for _, r := range config.RequiredPlugins {
 		required[r] = struct{}{}
 	}
+
 	for _, p := range plugins {
 		id := p.URI()
 		log.G(ctx).WithField("type", p.Type).Infof("loading plugin %q...", id)
@@ -218,7 +219,7 @@ func New(ctx context.Context, config *srvconfig.Config) (*Server, error) {
 
 		// load the plugin specific configuration if it is provided
 		if p.Config != nil {
-			pc, err := config.Decode(p)
+			pc, err := config.Decode(ctx, p)
 			if err != nil {
 				return nil, err
 			}