From 0537c1da563603ef670081d6011027bf9291ac3d Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Wed, 28 Jun 2023 19:34:00 +0300 Subject: [PATCH] kubeadm: move migrate / validate tests out of config_tests.go Place the tests in common_tests.go on the backend side in common_tests.go. A test for migrate TestMigrateOldConfig was already present there. Apply slightly better coverage to it and rename desc -> name. Fix typo in argument oldConfig -> config in ValidateConfig(). --- cmd/kubeadm/app/cmd/config_test.go | 186 --------------------- cmd/kubeadm/app/util/config/common.go | 4 +- cmd/kubeadm/app/util/config/common_test.go | 140 ++++++++++++++-- 3 files changed, 129 insertions(+), 201 deletions(-) diff --git a/cmd/kubeadm/app/cmd/config_test.go b/cmd/kubeadm/app/cmd/config_test.go index 920dfb42d0e..49dbc7034b8 100644 --- a/cmd/kubeadm/app/cmd/config_test.go +++ b/cmd/kubeadm/app/cmd/config_test.go @@ -38,7 +38,6 @@ import ( outputapischeme "k8s.io/kubernetes/cmd/kubeadm/app/apis/output/scheme" "k8s.io/kubernetes/cmd/kubeadm/app/constants" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" - configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config" "k8s.io/kubernetes/cmd/kubeadm/app/util/output" utilruntime "k8s.io/kubernetes/cmd/kubeadm/app/util/runtime" ) @@ -52,44 +51,6 @@ var ( // kubeadm lookup dl.k8s.io to resolve what the latest stable release is dummyKubernetesVersion = constants.MinimumControlPlaneVersion dummyKubernetesVersionStr = dummyKubernetesVersion.String() - - // predefined configuration contents for migration and validation - cfgInvalidSubdomain = []byte(dedent.Dedent(fmt.Sprintf(` -apiVersion: %s -kind: InitConfiguration -nodeRegistration: - criSocket: %s - name: foo bar # not a valid subdomain -`, kubeadmapiv1.SchemeGroupVersion.String(), constants.UnknownCRISocket))) - - cfgUnknownAPI = []byte(dedent.Dedent(fmt.Sprintf(` -apiVersion: foo/bar # not a valid GroupVersion -kind: zzz # not a valid Kind -nodeRegistration: - criSocket: %s -`, constants.UnknownCRISocket))) - - cfgLegacyAPI = []byte(dedent.Dedent(fmt.Sprintf(` -apiVersion: kubeadm.k8s.io/v1beta1 # legacy API -kind: InitConfiguration -nodeRegistration: - criSocket: %s -`, constants.UnknownCRISocket))) - - cfgUnknownField = []byte(dedent.Dedent(fmt.Sprintf(` -apiVersion: %s -kind: InitConfiguration -foo: bar -nodeRegistration: - criSocket: %s -`, kubeadmapiv1.SchemeGroupVersion.String(), constants.UnknownCRISocket))) - - cfgValid = []byte(dedent.Dedent(fmt.Sprintf(` -apiVersion: %s -kind: InitConfiguration -nodeRegistration: - criSocket: %s -`, kubeadmapiv1.SchemeGroupVersion.String(), constants.UnknownCRISocket))) ) func TestNewCmdConfigImagesList(t *testing.T) { @@ -428,153 +389,6 @@ func TestImagesPull(t *testing.T) { } } -func TestMigrate(t *testing.T) { - cfgFileInvalidSubdomain, cleanup := tempConfig(t, cfgInvalidSubdomain) - defer cleanup() - cfgFileUnknownAPI, cleanup := tempConfig(t, cfgUnknownAPI) - defer cleanup() - cfgFileLegacyAPI, cleanup := tempConfig(t, cfgLegacyAPI) - defer cleanup() - cfgFileUnknownField, cleanup := tempConfig(t, cfgUnknownField) - defer cleanup() - cfgFileValid, cleanup := tempConfig(t, cfgValid) - defer cleanup() - - testcases := []struct { - name string - cfg string - expectedError bool - }{ - { - name: "invalid subdomain", - cfg: cfgFileInvalidSubdomain, - expectedError: true, - }, - { - name: "unknown API GVK", - cfg: cfgFileUnknownAPI, - expectedError: true, - }, - { - name: "legacy API GVK", - cfg: cfgFileLegacyAPI, - expectedError: true, - }, - { - name: "unknown field", - cfg: cfgFileUnknownField, - expectedError: true, - }, - { - name: "valid", - cfg: cfgFileValid, - expectedError: false, - }, - } - - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - var output bytes.Buffer - command := newCmdConfigMigrate(&output) - if err := command.Flags().Set("old-config", tc.cfg); err != nil { - t.Fatalf("failed to set old-config flag") - } - newConfigPath := filepath.Join(filepath.Dir(tc.cfg), "new-migrated-config") - if err := command.Flags().Set("new-config", newConfigPath); err != nil { - t.Fatalf("failed to set new-config flag") - } - err := command.RunE(nil, nil) - if (err != nil) != tc.expectedError { - t.Fatalf("Expected error from validate command: %v, got: %v, error: %v", - tc.expectedError, err != nil, err) - } - if err != nil { - return - } - if _, err := configutil.LoadInitConfigurationFromFile(newConfigPath); err != nil { - t.Fatalf("Could not read output back into internal type: %v", err) - } - }) - } - -} - -func TestValidate(t *testing.T) { - cfgFileInvalidSubdomain, cleanup := tempConfig(t, cfgInvalidSubdomain) - defer cleanup() - cfgFileUnknownAPI, cleanup := tempConfig(t, cfgUnknownAPI) - defer cleanup() - cfgFileLegacyAPI, cleanup := tempConfig(t, cfgLegacyAPI) - defer cleanup() - cfgFileUnknownField, cleanup := tempConfig(t, cfgUnknownField) - defer cleanup() - cfgFileValid, cleanup := tempConfig(t, cfgValid) - defer cleanup() - - testcases := []struct { - name string - cfg string - expectedError bool - }{ - { - name: "invalid subdomain", - cfg: cfgFileInvalidSubdomain, - expectedError: true, - }, - { - name: "unknown API GVK", - cfg: cfgFileUnknownAPI, - expectedError: true, - }, - { - name: "legacy API GVK", - cfg: cfgFileLegacyAPI, - expectedError: true, - }, - { - name: "unknown field", - cfg: cfgFileUnknownField, - expectedError: true, - }, - { - name: "valid", - cfg: cfgFileValid, - expectedError: false, - }, - } - - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - var output bytes.Buffer - command := newCmdConfigValidate(&output) - if err := command.Flags().Set("config", tc.cfg); err != nil { - t.Fatalf("Failed to set config flag") - } - if err := command.RunE(nil, nil); (err != nil) != tc.expectedError { - t.Fatalf("Expected error from validate command: %v, got: %v, error: %v", - tc.expectedError, err != nil, err) - } - }) - } -} - -// Returns the name of the file created and a cleanup callback -func tempConfig(t *testing.T, config []byte) (string, func()) { - t.Helper() - tmpDir, err := os.MkdirTemp("", "kubeadm-migration-test") - if err != nil { - t.Fatalf("Unable to create temporary directory: %v", err) - } - configFilePath := filepath.Join(tmpDir, "test-config-file") - if err := os.WriteFile(configFilePath, config, 0644); err != nil { - os.RemoveAll(tmpDir) - t.Fatalf("Failed writing a config file: %v", err) - } - return configFilePath, func() { - os.RemoveAll(tmpDir) - } -} - func TestNewCmdConfigPrintActionDefaults(t *testing.T) { tests := []struct { name string diff --git a/cmd/kubeadm/app/util/config/common.go b/cmd/kubeadm/app/util/config/common.go index 3ecc5685ae1..f5d7b1f9c16 100644 --- a/cmd/kubeadm/app/util/config/common.go +++ b/cmd/kubeadm/app/util/config/common.go @@ -289,8 +289,8 @@ func MigrateOldConfig(oldConfig []byte, allowExperimental bool) ([]byte, error) // ValidateConfig takes a byte slice containing a kubeadm configuration and performs conversion // to internal types and validation. -func ValidateConfig(oldConfig []byte, allowExperimental bool) error { - gvkmap, err := kubeadmutil.SplitYAMLDocuments(oldConfig) +func ValidateConfig(config []byte, allowExperimental bool) error { + gvkmap, err := kubeadmutil.SplitYAMLDocuments(config) if err != nil { return err } diff --git a/cmd/kubeadm/app/util/config/common_test.go b/cmd/kubeadm/app/util/config/common_test.go index 879d04aef7f..c1e7db1d222 100644 --- a/cmd/kubeadm/app/util/config/common_test.go +++ b/cmd/kubeadm/app/util/config/common_test.go @@ -226,26 +226,34 @@ func TestMigrateOldConfig(t *testing.T) { gvExperimental = kubeadmapiv1.SchemeGroupVersion.String() ) tests := []struct { - desc string + name string oldCfg string expectedKinds []string expectErr bool allowExperimental bool }{ { - desc: "empty file produces empty result", + name: "empty file produces empty result", oldCfg: "", expectErr: false, }, { - desc: "bad config produces error", + name: "bad config produces error", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s `, gv)), expectErr: true, }, { - desc: "InitConfiguration only gets migrated", + name: "unknown API produces error", + oldCfg: dedent.Dedent(fmt.Sprintf(` + apiVersion: %s + kind: Foo + `, gv)), + expectErr: true, + }, + { + name: "InitConfiguration only gets migrated", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s kind: InitConfiguration @@ -257,7 +265,7 @@ func TestMigrateOldConfig(t *testing.T) { expectErr: false, }, { - desc: "ClusterConfiguration only gets migrated", + name: "ClusterConfiguration only gets migrated", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s kind: ClusterConfiguration @@ -269,7 +277,7 @@ func TestMigrateOldConfig(t *testing.T) { expectErr: false, }, { - desc: "JoinConfiguration only gets migrated", + name: "JoinConfiguration only gets migrated", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s kind: JoinConfiguration @@ -285,7 +293,7 @@ func TestMigrateOldConfig(t *testing.T) { expectErr: false, }, { - desc: "Init + Cluster Configurations are migrated", + name: "Init + Cluster Configurations are migrated", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s kind: InitConfiguration @@ -300,7 +308,7 @@ func TestMigrateOldConfig(t *testing.T) { expectErr: false, }, { - desc: "Init + Join Configurations are migrated", + name: "Init + Join Configurations are migrated", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s kind: InitConfiguration @@ -321,7 +329,7 @@ func TestMigrateOldConfig(t *testing.T) { expectErr: false, }, { - desc: "Cluster + Join Configurations are migrated", + name: "Cluster + Join Configurations are migrated", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s kind: ClusterConfiguration @@ -342,7 +350,7 @@ func TestMigrateOldConfig(t *testing.T) { expectErr: false, }, { - desc: "Init + Cluster + Join Configurations are migrated", + name: "Init + Cluster + Join Configurations are migrated", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s kind: InitConfiguration @@ -366,7 +374,7 @@ func TestMigrateOldConfig(t *testing.T) { expectErr: false, }, { - desc: "component configs are not migrated", + name: "component configs are not migrated", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s kind: InitConfiguration @@ -396,7 +404,7 @@ func TestMigrateOldConfig(t *testing.T) { expectErr: false, }, { - desc: "ClusterConfiguration gets migrated from experimental API", + name: "ClusterConfiguration gets migrated from experimental API", oldCfg: dedent.Dedent(fmt.Sprintf(` apiVersion: %s kind: ClusterConfiguration @@ -408,10 +416,19 @@ func TestMigrateOldConfig(t *testing.T) { allowExperimental: true, expectErr: false, }, + { + name: "ClusterConfiguration from experimental API cannot be migrated", + oldCfg: dedent.Dedent(fmt.Sprintf(` + apiVersion: %s + kind: ClusterConfiguration + `, gvExperimental)), + allowExperimental: false, + expectErr: true, + }, } for _, test := range tests { - t.Run(test.desc, func(t *testing.T) { + t.Run(test.name, func(t *testing.T) { b, err := MigrateOldConfig([]byte(test.oldCfg), test.allowExperimental) if test.expectErr { if err == nil { @@ -439,6 +456,103 @@ func TestMigrateOldConfig(t *testing.T) { } } +// NOTE: do not delete this test once an older API is removed and there is only one API left. +// Update the inline "gv" and "gvExperimental" variables, to have the GroupVersion String of +// the API to be tested. If there are no experimental APIs make "gvExperimental" point to +// an non-experimental API. +func TestValidateConfig(t *testing.T) { + var ( + gv = kubeadmapiv1old.SchemeGroupVersion.String() + gvExperimental = kubeadmapiv1.SchemeGroupVersion.String() + ) + tests := []struct { + name string + cfg string + expectedError bool + allowExperimental bool + }{ + { + name: "invalid subdomain", + cfg: dedent.Dedent(fmt.Sprintf(` + apiVersion: %s + kind: InitConfiguration + nodeRegistration: + criSocket: %s + name: foo bar # not a valid subdomain + `, gv, constants.UnknownCRISocket)), + expectedError: true, + }, + { + name: "unknown API GVK", + cfg: dedent.Dedent(fmt.Sprintf(` + apiVersion: foo/bar # not a valid GroupVersion + kind: zzz # not a valid Kind + nodeRegistration: + criSocket: %s + `, constants.UnknownCRISocket)), + expectedError: true, + }, + { + name: "legacy API GVK", + cfg: dedent.Dedent(fmt.Sprintf(` + apiVersion: kubeadm.k8s.io/v1beta1 # legacy API + kind: InitConfiguration + nodeRegistration: + criSocket: %s + `, constants.UnknownCRISocket)), + expectedError: true, + }, + { + name: "unknown field", + cfg: dedent.Dedent(fmt.Sprintf(` + apiVersion: %s + kind: InitConfiguration + foo: bar + nodeRegistration: + criSocket: %s + `, gv, constants.UnknownCRISocket)), + expectedError: true, + }, + { + name: "valid", + cfg: dedent.Dedent(fmt.Sprintf(` + apiVersion: %s + kind: InitConfiguration + nodeRegistration: + criSocket: %s + `, gv, constants.UnknownCRISocket)), + expectedError: false, + }, + { + name: "valid: experimental API", + cfg: dedent.Dedent(fmt.Sprintf(` + apiVersion: %s + kind: InitConfiguration + `, gvExperimental)), + expectedError: false, + allowExperimental: true, + }, + { + name: "invalid: experimental API", + cfg: dedent.Dedent(fmt.Sprintf(` + apiVersion: %s + kind: InitConfiguration + `, gvExperimental)), + expectedError: true, + allowExperimental: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := ValidateConfig([]byte(test.cfg), test.allowExperimental) + if (err != nil) != test.expectedError { + t.Fatalf("expected error: %v, got: %v, error: %v", test.expectedError, (err != nil), err) + } + }) + } +} + func TestIsKubeadmPrereleaseVersion(t *testing.T) { validVersionInfo := &apimachineryversion.Info{Major: "1", GitVersion: "v1.23.0-alpha.1"} tests := []struct {