Move config validation into pkg/config and add unit test.

Signed-off-by: Lantao Liu <lantaol@google.com>

cri.go

@@ -17,10 +17,8 @@ limitations under the License.
 package cri
 
 import (
-	"context"
 	"flag"
 	"path/filepath"
-	"time"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/api/services/containers/v1"
@@ -74,8 +72,8 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	}
 	log.G(ctx).Infof("Start cri plugin with config %+v", c)
 
-	if err := validateConfig(ctx, &c); err != nil {
+	if err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
-		return nil, errors.Wrap(err, "invalid config")
+		return nil, errors.Wrap(err, "invalid plugin config")
 	}
 
 	if err := setGLogLevel(); err != nil {
@@ -111,87 +109,6 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	return s, nil
 }
 
-// validateConfig validates the given configuration.
-func validateConfig(ctx context.Context, c *criconfig.Config) error {
-	if c.ContainerdConfig.Runtimes == nil {
-		c.ContainerdConfig.Runtimes = make(map[string]criconfig.Runtime)
-	}
-
-	// Validation for deprecated untrusted_workload_runtime.
-	if c.ContainerdConfig.UntrustedWorkloadRuntime.Type != "" {
-		log.G(ctx).Warning("`untrusted_workload_runtime` is deprecated, please use `untrusted` runtime in `runtimes` instead")
-		if _, ok := c.ContainerdConfig.Runtimes[criconfig.RuntimeUntrusted]; ok {
-			return errors.Errorf("conflicting definitions: configuration includes both `untrusted_workload_runtime` and `runtimes[%q]`", criconfig.RuntimeUntrusted)
-		}
-		c.ContainerdConfig.Runtimes[criconfig.RuntimeUntrusted] = c.ContainerdConfig.UntrustedWorkloadRuntime
-	}
-
-	// Validation for deprecated default_runtime field.
-	if c.ContainerdConfig.DefaultRuntime.Type != "" {
-		log.G(ctx).Warning("`default_runtime` is deprecated, please use `default_runtime_name` to reference the default configuration you have defined in `runtimes`")
-		c.ContainerdConfig.DefaultRuntimeName = criconfig.RuntimeDefault
-		c.ContainerdConfig.Runtimes[criconfig.RuntimeDefault] = c.ContainerdConfig.DefaultRuntime
-	}
-
-	// Validation for default_runtime_name
-	if c.ContainerdConfig.DefaultRuntimeName == "" {
-		return errors.New("`default_runtime_name` is empty")
-	}
-	if _, ok := c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName]; !ok {
-		return errors.New("no corresponding runtime configured in `runtimes` for `default_runtime_name`")
-	}
-
-	// Validation for deprecated runtime options.
-	if c.SystemdCgroup {
-		if c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName].Type != plugin.RuntimeLinuxV1 {
-			return errors.Errorf("`systemd_cgroup` only works for runtime %s", plugin.RuntimeLinuxV1)
-		}
-		log.G(ctx).Warning("`systemd_cgroup` is deprecated, please use runtime `options` instead")
-	}
-	if c.NoPivot {
-		if c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName].Type != plugin.RuntimeLinuxV1 {
-			return errors.Errorf("`no_pivot` only works for runtime %s", plugin.RuntimeLinuxV1)
-		}
-		// NoPivot can't be deprecated yet, because there is no alternative config option
-		// for `io.containerd.runtime.v1.linux`.
-	}
-	for _, r := range c.ContainerdConfig.Runtimes {
-		if r.Engine != "" {
-			if r.Type != plugin.RuntimeLinuxV1 {
-				return errors.Errorf("`runtime_engine` only works for runtime %s", plugin.RuntimeLinuxV1)
-			}
-			log.G(ctx).Warning("`runtime_engine` is deprecated, please use runtime `options` instead")
-		}
-		if r.Root != "" {
-			if r.Type != plugin.RuntimeLinuxV1 {
-				return errors.Errorf("`runtime_root` only works for runtime %s", plugin.RuntimeLinuxV1)
-			}
-			log.G(ctx).Warning("`runtime_root` is deprecated, please use runtime `options` instead")
-		}
-	}
-
-	// Validation for registry configurations.
-	if len(c.Registry.Auths) != 0 {
-		if c.Registry.Configs == nil {
-			c.Registry.Configs = make(map[string]criconfig.RegistryConfig)
-		}
-		for endpoint, auth := range c.Registry.Auths {
-			config := c.Registry.Configs[endpoint]
-			config.Auth = &auth
-			c.Registry.Configs[endpoint] = config
-		}
-		log.G(ctx).Warning("`auths` is deprecated, please use registry`configs` instead")
-	}
-
-	// Validation for stream_idle_timeout
-	if c.StreamIdleTimeout != "" {
-		if _, err := time.ParseDuration(c.StreamIdleTimeout); err != nil {
-			return errors.Wrap(err, "invalid stream idle timeout")
-		}
-	}
-	return nil
-}
-
 // getServicesOpts get service options from plugin context.
 func getServicesOpts(ic *plugin.InitContext) ([]containerd.ServicesOpt, error) {
 	plugins, err := ic.GetByType(plugin.ServicePlugin)

pkg/config/config.go

@@ -17,8 +17,14 @@ limitations under the License.
 package config
 
 import (
+	"context"
+	"time"
+
 	"github.com/BurntSushi/toml"
 	"github.com/containerd/containerd"
+	"github.com/containerd/containerd/log"
+	"github.com/containerd/containerd/plugin"
+	"github.com/pkg/errors"
 	"k8s.io/kubernetes/pkg/kubelet/server/streaming"
 )
 
@@ -272,3 +278,84 @@ const (
 	// RuntimeDefault is the implicit runtime defined for ContainerdConfig.DefaultRuntime
 	RuntimeDefault = "default"
 )
+
+// ValidatePluginConfig validates the given plugin configuration.
+func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
+	if c.ContainerdConfig.Runtimes == nil {
+		c.ContainerdConfig.Runtimes = make(map[string]Runtime)
+	}
+
+	// Validation for deprecated untrusted_workload_runtime.
+	if c.ContainerdConfig.UntrustedWorkloadRuntime.Type != "" {
+		log.G(ctx).Warning("`untrusted_workload_runtime` is deprecated, please use `untrusted` runtime in `runtimes` instead")
+		if _, ok := c.ContainerdConfig.Runtimes[RuntimeUntrusted]; ok {
+			return errors.Errorf("conflicting definitions: configuration includes both `untrusted_workload_runtime` and `runtimes[%q]`", RuntimeUntrusted)
+		}
+		c.ContainerdConfig.Runtimes[RuntimeUntrusted] = c.ContainerdConfig.UntrustedWorkloadRuntime
+	}
+
+	// Validation for deprecated default_runtime field.
+	if c.ContainerdConfig.DefaultRuntime.Type != "" {
+		log.G(ctx).Warning("`default_runtime` is deprecated, please use `default_runtime_name` to reference the default configuration you have defined in `runtimes`")
+		c.ContainerdConfig.DefaultRuntimeName = RuntimeDefault
+		c.ContainerdConfig.Runtimes[RuntimeDefault] = c.ContainerdConfig.DefaultRuntime
+	}
+
+	// Validation for default_runtime_name
+	if c.ContainerdConfig.DefaultRuntimeName == "" {
+		return errors.New("`default_runtime_name` is empty")
+	}
+	if _, ok := c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName]; !ok {
+		return errors.New("no corresponding runtime configured in `runtimes` for `default_runtime_name`")
+	}
+
+	// Validation for deprecated runtime options.
+	if c.SystemdCgroup {
+		if c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName].Type != plugin.RuntimeLinuxV1 {
+			return errors.Errorf("`systemd_cgroup` only works for runtime %s", plugin.RuntimeLinuxV1)
+		}
+		log.G(ctx).Warning("`systemd_cgroup` is deprecated, please use runtime `options` instead")
+	}
+	if c.NoPivot {
+		if c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName].Type != plugin.RuntimeLinuxV1 {
+			return errors.Errorf("`no_pivot` only works for runtime %s", plugin.RuntimeLinuxV1)
+		}
+		// NoPivot can't be deprecated yet, because there is no alternative config option
+		// for `io.containerd.runtime.v1.linux`.
+	}
+	for _, r := range c.ContainerdConfig.Runtimes {
+		if r.Engine != "" {
+			if r.Type != plugin.RuntimeLinuxV1 {
+				return errors.Errorf("`runtime_engine` only works for runtime %s", plugin.RuntimeLinuxV1)
+			}
+			log.G(ctx).Warning("`runtime_engine` is deprecated, please use runtime `options` instead")
+		}
+		if r.Root != "" {
+			if r.Type != plugin.RuntimeLinuxV1 {
+				return errors.Errorf("`runtime_root` only works for runtime %s", plugin.RuntimeLinuxV1)
+			}
+			log.G(ctx).Warning("`runtime_root` is deprecated, please use runtime `options` instead")
+		}
+	}
+
+	// Validation for deprecated auths options and mapping it to configs.
+	if len(c.Registry.Auths) != 0 {
+		if c.Registry.Configs == nil {
+			c.Registry.Configs = make(map[string]RegistryConfig)
+		}
+		for endpoint, auth := range c.Registry.Auths {
+			config := c.Registry.Configs[endpoint]
+			config.Auth = &auth
+			c.Registry.Configs[endpoint] = config
+		}
+		log.G(ctx).Warning("`auths` is deprecated, please use registry`configs` instead")
+	}
+
+	// Validation for stream_idle_timeout
+	if c.StreamIdleTimeout != "" {
+		if _, err := time.ParseDuration(c.StreamIdleTimeout); err != nil {
+			return errors.Wrap(err, "invalid stream idle timeout")
+		}
+	}
+	return nil
+}

pkg/config/config_test.go

@@ -0,0 +1,334 @@
+/*
+Copyright The containerd Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package config
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/containerd/containerd/plugin"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidateConfig(t *testing.T) {
+	for desc, test := range map[string]struct {
+		config      *PluginConfig
+		expectedErr string
+		expected    *PluginConfig
+	}{
+		"deprecated untrusted_workload_runtime": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					UntrustedWorkloadRuntime: Runtime{
+						Type: "untrusted",
+					},
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: "default",
+						},
+					},
+				},
+			},
+			expected: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					UntrustedWorkloadRuntime: Runtime{
+						Type: "untrusted",
+					},
+					Runtimes: map[string]Runtime{
+						RuntimeUntrusted: {
+							Type: "untrusted",
+						},
+						RuntimeDefault: {
+							Type: "default",
+						},
+					},
+				},
+			},
+		},
+		"both untrusted_workload_runtime and runtime[untrusted]": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					UntrustedWorkloadRuntime: Runtime{
+						Type: "untrusted-1",
+					},
+					Runtimes: map[string]Runtime{
+						RuntimeUntrusted: {
+							Type: "untrusted-2",
+						},
+						RuntimeDefault: {
+							Type: "default",
+						},
+					},
+				},
+			},
+			expectedErr: fmt.Sprintf("conflicting definitions: configuration includes both `untrusted_workload_runtime` and `runtimes[%q]`", RuntimeUntrusted),
+		},
+		"deprecated default_runtime": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntime: Runtime{
+						Type: "default",
+					},
+				},
+			},
+			expected: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntime: Runtime{
+						Type: "default",
+					},
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: "default",
+						},
+					},
+				},
+			},
+		},
+		"no default_runtime_name": {
+			config:      &PluginConfig{},
+			expectedErr: "`default_runtime_name` is empty",
+		},
+		"no runtime[default_runtime_name]": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+				},
+			},
+			expectedErr: "no corresponding runtime configured in `runtimes` for `default_runtime_name`",
+		},
+		"deprecated systemd_cgroup for v1 runtime": {
+			config: &PluginConfig{
+				SystemdCgroup: true,
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: plugin.RuntimeLinuxV1,
+						},
+					},
+				},
+			},
+			expected: &PluginConfig{
+				SystemdCgroup: true,
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: plugin.RuntimeLinuxV1,
+						},
+					},
+				},
+			},
+		},
+		"deprecated systemd_cgroup for v2 runtime": {
+			config: &PluginConfig{
+				SystemdCgroup: true,
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: plugin.RuntimeRuncV1,
+						},
+					},
+				},
+			},
+			expectedErr: fmt.Sprintf("`systemd_cgroup` only works for runtime %s", plugin.RuntimeLinuxV1),
+		},
+		"no_pivot for v1 runtime": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					NoPivot:            true,
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: plugin.RuntimeLinuxV1,
+						},
+					},
+				},
+			},
+			expected: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					NoPivot:            true,
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: plugin.RuntimeLinuxV1,
+						},
+					},
+				},
+			},
+		},
+		"no_pivot for v2 runtime": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					NoPivot:            true,
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: plugin.RuntimeRuncV1,
+						},
+					},
+				},
+			},
+			expectedErr: fmt.Sprintf("`no_pivot` only works for runtime %s", plugin.RuntimeLinuxV1),
+		},
+		"deprecated runtime_engine for v1 runtime": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Engine: "runc",
+							Type:   plugin.RuntimeLinuxV1,
+						},
+					},
+				},
+			},
+			expected: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Engine: "runc",
+							Type:   plugin.RuntimeLinuxV1,
+						},
+					},
+				},
+			},
+		},
+		"deprecated runtime_engine for v2 runtime": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Engine: "runc",
+							Type:   plugin.RuntimeRuncV1,
+						},
+					},
+				},
+			},
+			expectedErr: fmt.Sprintf("`runtime_engine` only works for runtime %s", plugin.RuntimeLinuxV1),
+		},
+		"deprecated runtime_root for v1 runtime": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Root: "/run/containerd/runc",
+							Type: plugin.RuntimeLinuxV1,
+						},
+					},
+				},
+			},
+			expected: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Root: "/run/containerd/runc",
+							Type: plugin.RuntimeLinuxV1,
+						},
+					},
+				},
+			},
+		},
+		"deprecated runtime_root for v2 runtime": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Root: "/run/containerd/runc",
+							Type: plugin.RuntimeRuncV1,
+						},
+					},
+				},
+			},
+			expectedErr: fmt.Sprintf("`runtime_root` only works for runtime %s", plugin.RuntimeLinuxV1),
+		},
+		"deprecated auths": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: plugin.RuntimeRuncV1,
+						},
+					},
+				},
+				Registry: Registry{
+					Auths: map[string]AuthConfig{
+						"https://gcr.io": {Username: "test"},
+					},
+				},
+			},
+			expected: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: plugin.RuntimeRuncV1,
+						},
+					},
+				},
+				Registry: Registry{
+					Configs: map[string]RegistryConfig{
+						"https://gcr.io": {
+							Auth: &AuthConfig{
+								Username: "test",
+							},
+						},
+					},
+					Auths: map[string]AuthConfig{
+						"https://gcr.io": {Username: "test"},
+					},
+				},
+			},
+		},
+		"invalid stream_idle_timeout": {
+			config: &PluginConfig{
+				StreamIdleTimeout: "invalid",
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Type: "default",
+						},
+					},
+				},
+			},
+			expectedErr: "invalid stream idle timeout",
+		},
+	} {
+		t.Run(desc, func(t *testing.T) {
+			err := ValidatePluginConfig(context.Background(), test.config)
+			if test.expectedErr != "" {
+				assert.Contains(t, err.Error(), test.expectedErr)
+			} else {
+				assert.NoError(t, err)
+				assert.Equal(t, test.expected, test.config)
+			}
+		})
+	}
+}