Merge pull request #9319 from samuelkarp/config-deprecation-warnings

cri: add deprecation warnings for mirrors, auths, and configs

pkg/cri/config/config.go

@@ -24,6 +24,8 @@ import (
 	"time"
 
 	"github.com/containerd/log"
+
+	"github.com/containerd/containerd/v2/pkg/deprecation"
 )
 
 type SandboxControllerMode string
@@ -365,22 +367,23 @@ const (
 )
 
 // ValidatePluginConfig validates the given plugin configuration.
-func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
+func ValidatePluginConfig(ctx context.Context, c *PluginConfig) ([]deprecation.Warning, error) {
+	var warnings []deprecation.Warning
 	if c.ContainerdConfig.Runtimes == nil {
 		c.ContainerdConfig.Runtimes = make(map[string]Runtime)
 	}
 
 	// Validation for default_runtime_name
 	if c.ContainerdConfig.DefaultRuntimeName == "" {
-		return errors.New("`default_runtime_name` is empty")
+		return warnings, errors.New("`default_runtime_name` is empty")
 	}
 	if _, ok := c.ContainerdConfig.Runtimes[c.ContainerdConfig.DefaultRuntimeName]; !ok {
-		return fmt.Errorf("no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"%s\"", c.ContainerdConfig.DefaultRuntimeName)
+		return warnings, fmt.Errorf("no corresponding runtime configured in `containerd.runtimes` for `containerd` `default_runtime_name = \"%s\"", c.ContainerdConfig.DefaultRuntimeName)
 	}
 
 	for k, r := range c.ContainerdConfig.Runtimes {
 		if !r.PrivilegedWithoutHostDevices && r.PrivilegedWithoutHostDevicesAllDevicesAllowed {
-			return errors.New("`privileged_without_host_devices_all_devices_allowed` requires `privileged_without_host_devices` to be enabled")
+			return warnings, errors.New("`privileged_without_host_devices_all_devices_allowed` requires `privileged_without_host_devices` to be enabled")
 		}
 		// If empty, use default podSandbox mode
 		if len(r.Sandboxer) == 0 {
@@ -392,11 +395,17 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
 	useConfigPath := c.Registry.ConfigPath != ""
 	if len(c.Registry.Mirrors) > 0 {
 		if useConfigPath {
-			return errors.New("`mirrors` cannot be set when `config_path` is provided")
+			return warnings, errors.New("`mirrors` cannot be set when `config_path` is provided")
 		}
+		warnings = append(warnings, deprecation.CRIRegistryMirrors)
 		log.G(ctx).Warning("`mirrors` is deprecated, please use `config_path` instead")
 	}
 
+	if len(c.Registry.Configs) != 0 {
+		warnings = append(warnings, deprecation.CRIRegistryConfigs)
+		log.G(ctx).Warning("`configs` is deprecated, please use `config_path` instead")
+	}
+
 	// Validation for deprecated auths options and mapping it to configs.
 	if len(c.Registry.Auths) != 0 {
 		if c.Registry.Configs == nil {
@@ -406,7 +415,7 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
 			auth := auth
 			u, err := url.Parse(endpoint)
 			if err != nil {
-				return fmt.Errorf("failed to parse registry url %q from `registry.auths`: %w", endpoint, err)
+				return warnings, fmt.Errorf("failed to parse registry url %q from `registry.auths`: %w", endpoint, err)
 			}
 			if u.Scheme != "" {
 				// Do not include the scheme in the new registry config.
@@ -416,28 +425,29 @@ func ValidatePluginConfig(ctx context.Context, c *PluginConfig) error {
 			config.Auth = &auth
 			c.Registry.Configs[endpoint] = config
 		}
-		log.G(ctx).Warning("`auths` is deprecated, please use `configs` instead")
+		warnings = append(warnings, deprecation.CRIRegistryAuths)
+		log.G(ctx).Warning("`auths` is deprecated, please use `ImagePullSecrets` instead")
 	}
 
 	// Validation for stream_idle_timeout
 	if c.StreamIdleTimeout != "" {
 		if _, err := time.ParseDuration(c.StreamIdleTimeout); err != nil {
-			return fmt.Errorf("invalid stream idle timeout: %w", err)
+			return warnings, fmt.Errorf("invalid stream idle timeout: %w", err)
 		}
 	}
 
 	// Validation for image_pull_progress_timeout
 	if c.ImagePullProgressTimeout != "" {
 		if _, err := time.ParseDuration(c.ImagePullProgressTimeout); err != nil {
-			return fmt.Errorf("invalid image pull progress timeout: %w", err)
+			return warnings, fmt.Errorf("invalid image pull progress timeout: %w", err)
 		}
 	}
 
 	// Validation for drain_exec_sync_io_timeout
 	if c.DrainExecSyncIOTimeout != "" {
 		if _, err := time.ParseDuration(c.DrainExecSyncIOTimeout); err != nil {
-			return fmt.Errorf("invalid `drain_exec_sync_io_timeout`: %w", err)
+			return warnings, fmt.Errorf("invalid `drain_exec_sync_io_timeout`: %w", err)
 		}
 	}
-	return nil
+	return warnings, nil
 }

pkg/cri/config/config_test.go

@@ -21,6 +21,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"github.com/containerd/containerd/v2/pkg/deprecation"
 )
 
 func TestValidateConfig(t *testing.T) {
@@ -28,6 +30,7 @@ func TestValidateConfig(t *testing.T) {
 		config      *PluginConfig
 		expectedErr string
 		expected    *PluginConfig
+		warnings    []deprecation.Warning
 	}{
 		"no default_runtime_name": {
 			config:      &PluginConfig{},
@@ -78,6 +81,7 @@ func TestValidateConfig(t *testing.T) {
 					},
 				},
 			},
+			warnings: []deprecation.Warning{deprecation.CRIRegistryAuths},
 		},
 		"invalid stream_idle_timeout": {
 			config: &PluginConfig{
@@ -112,6 +116,76 @@ func TestValidateConfig(t *testing.T) {
 			},
 			expectedErr: "`mirrors` cannot be set when `config_path` is provided",
 		},
+		"deprecated mirrors": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {},
+					},
+				},
+				Registry: Registry{
+					Mirrors: map[string]Mirror{
+						"example.com": {},
+					},
+				},
+			},
+			expected: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Sandboxer: string(ModePodSandbox),
+						},
+					},
+				},
+				Registry: Registry{
+					Mirrors: map[string]Mirror{
+						"example.com": {},
+					},
+				},
+			},
+			warnings: []deprecation.Warning{deprecation.CRIRegistryMirrors},
+		},
+		"deprecated configs": {
+			config: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {},
+					},
+				},
+				Registry: Registry{
+					Configs: map[string]RegistryConfig{
+						"gcr.io": {
+							Auth: &AuthConfig{
+								Username: "test",
+							},
+						},
+					},
+				},
+			},
+			expected: &PluginConfig{
+				ContainerdConfig: ContainerdConfig{
+					DefaultRuntimeName: RuntimeDefault,
+					Runtimes: map[string]Runtime{
+						RuntimeDefault: {
+							Sandboxer: string(ModePodSandbox),
+						},
+					},
+				},
+				Registry: Registry{
+					Configs: map[string]RegistryConfig{
+						"gcr.io": {
+							Auth: &AuthConfig{
+								Username: "test",
+							},
+						},
+					},
+				},
+			},
+			warnings: []deprecation.Warning{deprecation.CRIRegistryConfigs},
+		},
 		"privileged_without_host_devices_all_devices_allowed without privileged_without_host_devices": {
 			config: &PluginConfig{
 				ContainerdConfig: ContainerdConfig{
@@ -143,13 +217,18 @@ func TestValidateConfig(t *testing.T) {
 		},
 	} {
 		t.Run(desc, func(t *testing.T) {
-			err := ValidatePluginConfig(context.Background(), test.config)
+			w, 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)
 			}
+			if len(test.warnings) > 0 {
+				assert.ElementsMatch(t, test.warnings, w)
+			} else {
+				assert.Len(t, w, 0)
+			}
 		})
 	}
 }

pkg/cri/cri.go

@@ -21,20 +21,21 @@ import (
 	"fmt"
 	"path/filepath"
 
-	containerd "github.com/containerd/containerd/v2/client"
-	"github.com/containerd/containerd/v2/pkg/cri/nri"
-	"github.com/containerd/containerd/v2/pkg/cri/server"
-	nriservice "github.com/containerd/containerd/v2/pkg/nri"
-	"github.com/containerd/containerd/v2/platforms"
-	"github.com/containerd/containerd/v2/plugins"
 	"github.com/containerd/log"
 	"github.com/containerd/plugin"
 	"github.com/containerd/plugin/registry"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	"k8s.io/klog/v2"
 
+	containerd "github.com/containerd/containerd/v2/client"
 	criconfig "github.com/containerd/containerd/v2/pkg/cri/config"
 	"github.com/containerd/containerd/v2/pkg/cri/constants"
+	"github.com/containerd/containerd/v2/pkg/cri/nri"
+	"github.com/containerd/containerd/v2/pkg/cri/server"
+	nriservice "github.com/containerd/containerd/v2/pkg/nri"
+	"github.com/containerd/containerd/v2/platforms"
+	"github.com/containerd/containerd/v2/plugins"
+	"github.com/containerd/containerd/v2/services/warning"
 )
 
 // Register CRI service plugin
@@ -48,6 +49,7 @@ func init() {
 			plugins.EventPlugin,
 			plugins.ServicePlugin,
 			plugins.NRIApiPlugin,
+			plugins.WarningPlugin,
 		},
 		InitFn: initCRIService,
 	})
@@ -58,8 +60,17 @@ func initCRIService(ic *plugin.InitContext) (interface{}, error) {
 	ic.Meta.Exports = map[string]string{"CRIVersion": constants.CRIVersion}
 	ctx := ic.Context
 	pluginConfig := ic.Config.(*criconfig.PluginConfig)
-	if err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
+	if warnings, err := criconfig.ValidatePluginConfig(ctx, pluginConfig); err != nil {
 		return nil, fmt.Errorf("invalid plugin config: %w", err)
+	} else if len(warnings) > 0 {
+		ws, err := ic.GetSingle(plugins.WarningPlugin)
+		if err != nil {
+			return nil, err
+		}
+		warn := ws.(warning.Service)
+		for _, w := range warnings {
+			warn.Emit(ctx, w)
+		}
 	}
 
 	c := criconfig.Config{

pkg/deprecation/deprecation.go

@@ -25,12 +25,24 @@ const (
 	PullSchema1Image Warning = Prefix + "pull-schema-1-image"
 	// GoPluginLibrary is a warning for the use of dynamic library Go plugins
 	GoPluginLibrary Warning = Prefix + "go-plugin-library"
+	// CRIRegistryMirrors is a warning for the use of the `mirrors` property
+	CRIRegistryMirrors Warning = Prefix + "cri-registry-mirrors"
+	// CRIRegistryAuths is a warning for the use of the `auths` property
+	CRIRegistryAuths Warning = Prefix + "cri-registry-auths"
+	// CRIRegistryConfigs is a warning for the use of the `configs` property
+	CRIRegistryConfigs Warning = Prefix + "cri-registry-configs"
 )
 
 var messages = map[Warning]string{
 	PullSchema1Image: "Schema 1 images are deprecated since containerd v1.7 and removed in containerd v2.0. " +
 		`Since containerd v1.7.8, schema 1 images are identified by the "io.containerd.image/converted-docker-schema1" label.`,
 	GoPluginLibrary: "Dynamically-linked Go plugins as containerd runtimes are deprecated since containerd v2.0 and removed in containerd v2.1.",
+	CRIRegistryMirrors: "The `mirrors` property of `[plugins.\"io.containerd.grpc.v1.cri\".registry]` is deprecated since containerd v1.5 and will be removed in containerd v2.0." +
+		"Use `config_path` instead.",
+	CRIRegistryAuths: "The `auths` property of `[plugins.\"io.containerd.grpc.v1.cri\".registry]` is deprecated since containerd v1.3 and will be removed in containerd v2.0." +
+		"Use `ImagePullSecrets` instead.",
+	CRIRegistryConfigs: "The `configs` property of `[plugins.\"io.containerd.grpc.v1.cri\".registry]` is deprecated since containerd v1.5 and will be removed in containerd v2.0." +
+		"Use `config_path` instead.",
 }
 
 // Valid checks whether a given Warning is valid