diff --git a/cmd/containerd/command/config.go b/cmd/containerd/command/config.go index 2ac567934..fc23c91be 100644 --- a/cmd/containerd/command/config.go +++ b/cmd/containerd/command/config.go @@ -91,7 +91,7 @@ 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(context.GlobalString("config"), config); err != nil && !os.IsNotExist(err) { + if err := srvconfig.LoadConfig(gocontext.Background(), context.GlobalString("config"), config); err != nil && !os.IsNotExist(err) { return err } diff --git a/cmd/containerd/command/main.go b/cmd/containerd/command/main.go index 93f3a9a9d..88aa233e8 100644 --- a/cmd/containerd/command/main.go +++ b/cmd/containerd/command/main.go @@ -121,7 +121,7 @@ can be used and modified as necessary as a custom configuration.` configPath := context.GlobalString("config") _, err := os.Stat(configPath) if !os.IsNotExist(err) || context.GlobalIsSet("config") { - if err := srvconfig.LoadConfig(configPath, config); err != nil { + if err := srvconfig.LoadConfig(ctx, configPath, config); err != nil { return err } } diff --git a/integration/client/restart_monitor_test.go b/integration/client/restart_monitor_test.go index 3765c895d..c8b7e37ac 100644 --- a/integration/client/restart_monitor_test.go +++ b/integration/client/restart_monitor_test.go @@ -60,7 +60,7 @@ func newDaemonWithConfig(t *testing.T, configTOML string) (*Client, *daemon, fun t.Fatal(err) } - if err := srvconfig.LoadConfig(configTOMLFile, &configTOMLDecoded); err != nil { + if err := srvconfig.LoadConfig(context.TODO(), configTOMLFile, &configTOMLDecoded); err != nil { t.Fatal(err) } diff --git a/services/server/config/config.go b/services/server/config/config.go index 890290659..44c18cd68 100644 --- a/services/server/config/config.go +++ b/services/server/config/config.go @@ -17,6 +17,7 @@ package config import ( + "context" "errors" "fmt" "io" @@ -29,6 +30,7 @@ import ( "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/plugin" + "github.com/containerd/log" ) // NOTE: Any new map fields added also need to be handled in mergeConfig. @@ -196,7 +198,7 @@ func (c *Config) Decode(p *plugin.Registration) (interface{}, error) { } // LoadConfig loads the containerd server config from the provided path -func LoadConfig(path string, out *Config) error { +func LoadConfig(ctx context.Context, path string, out *Config) error { if out == nil { return fmt.Errorf("argument out must not be nil: %w", errdefs.ErrInvalidArgument) } @@ -214,7 +216,7 @@ func LoadConfig(path string, out *Config) error { continue } - config, err := loadConfigFile(path) + config, err := loadConfigFile(ctx, path) if err != nil { return err } @@ -246,7 +248,7 @@ func LoadConfig(path string, out *Config) error { } // loadConfigFile decodes a TOML file at the given path -func loadConfigFile(path string) (*Config, error) { +func loadConfigFile(ctx context.Context, path string) (*Config, error) { config := &Config{} f, err := os.Open(path) @@ -255,9 +257,40 @@ func loadConfigFile(path string) (*Config, error) { } defer f.Close() - // TODO: Add DisallowUnknownFields, requires better testing and bubbling errors - if err := toml.NewDecoder(f).Decode(config); err != nil { - return nil, fmt.Errorf("failed to unmarshal TOML: %w", err) + if err := toml.NewDecoder(f).DisallowUnknownFields().Decode(config); err != nil { + var serr *toml.StrictMissingError + if errors.As(err, &serr) { + for _, derr := range serr.Errors { + row, col := derr.Position() + log.G(ctx).WithFields(log.Fields{ + "file": path, + "row": row, + "column": col, + "key": strings.Join(derr.Key(), " "), + }).WithError(err).Warn("Ignoring unknown key in TOML") + } + + // Try decoding again with unknown fields + config = &Config{} + if _, seekerr := f.Seek(0, io.SeekStart); seekerr != nil { + return nil, fmt.Errorf("unable to seek file to start %w: failed to unmarshal TOML with unknown fields: %w", seekerr, err) + } + err = toml.NewDecoder(f).Decode(config) + } + if err != nil { + var derr *toml.DecodeError + if errors.As(err, &derr) { + row, column := derr.Position() + log.G(ctx).WithFields(log.Fields{ + "file": path, + "row": row, + "column": column, + }).WithError(err).Error("Failure unmarshaling TOML") + return nil, fmt.Errorf("failed to unmarshal TOML at row %d column %d: %w", row, column, err) + } + return nil, fmt.Errorf("failed to unmarshal TOML: %w", err) + } + } return config, nil diff --git a/services/server/config/config_test.go b/services/server/config/config_test.go index ccdaff2e8..c94f075b5 100644 --- a/services/server/config/config_test.go +++ b/services/server/config/config_test.go @@ -17,6 +17,7 @@ package config import ( + "context" "os" "path/filepath" "sort" @@ -115,7 +116,7 @@ root = "/var/lib/containerd" assert.NoError(t, err) var out Config - err = LoadConfig(path, &out) + err = LoadConfig(context.Background(), path, &out) assert.NoError(t, err) assert.Equal(t, 2, out.Version) assert.Equal(t, "/var/lib/containerd", out.Root) @@ -147,7 +148,7 @@ disabled_plugins = ["io.containerd.v1.xyz"] assert.NoError(t, err) var out Config - err = LoadConfig(filepath.Join(tempDir, "data1.toml"), &out) + err = LoadConfig(context.Background(), filepath.Join(tempDir, "data1.toml"), &out) assert.NoError(t, err) assert.Equal(t, 2, out.Version) @@ -175,7 +176,7 @@ imports = ["data1.toml", "data2.toml"] assert.NoError(t, err) var out Config - err = LoadConfig(filepath.Join(tempDir, "data1.toml"), &out) + err = LoadConfig(context.Background(), filepath.Join(tempDir, "data1.toml"), &out) assert.NoError(t, err) assert.Equal(t, 2, out.Version) @@ -203,7 +204,7 @@ version = 2 assert.NoError(t, err) var out Config - err = LoadConfig(path, &out) + err = LoadConfig(context.Background(), path, &out) assert.NoError(t, err) pluginConfig := map[string]interface{}{} @@ -225,6 +226,6 @@ func TestDecodePluginInV1Config(t *testing.T) { assert.NoError(t, err) var out Config - err = LoadConfig(path, &out) + err = LoadConfig(context.Background(), path, &out) assert.ErrorContains(t, err, "config version `1` is no longer supported") }