From 037fb6103e5f135edb1b4fc66b75f0f93ad4009d Mon Sep 17 00:00:00 2001 From: "Rostislav M. Georgiev" Date: Wed, 21 Nov 2018 12:59:24 +0200 Subject: [PATCH] kubeadm: config migrate handles more valid configs kubeadm config migrate uses AnyConfigFileAndDefaultsToInternal, which can unmarshal config from file only if InitConfiguration or JoinConfiguration are present. Even with that in mind, it can only return a singlie config object, with InitConfiguration taking precendence over JoinConfiguration. Thus, the following cases were not handled properly, while they were perfectly valid for kubeadm init/join: - ClusterConfiguration only file caused kubeadm config migrate to exit with error. - Init + Join configurations in the same file caused Init + Cluster configuration to be produced (ignoring JoinConfiguration). The same is valid when the combo is Init + Cluster + Join configurations. - Cluster + Join configuration ignores ClusterConfiguration and only JoinConfiguration gets migrated. To fix this, the following is done: - Introduce MigrateOldConfigFromFile which migrates old config from a file, while ensuring that all kubeadm originated input config kinds are taken care of. Add comprehensive unit tests for this. - Replace the use of AnyConfigFileAndDefaultsToInternal in kubeadm config migrate with MigrateOldConfigFromFile. - Remove the no longer used and error prone AnyConfigFileAndDefaultsToInternal. Signed-off-by: Rostislav M. Georgiev --- cmd/kubeadm/app/cmd/config.go | 5 +- cmd/kubeadm/app/util/config/BUILD | 1 + cmd/kubeadm/app/util/config/common.go | 66 ++++--- cmd/kubeadm/app/util/config/common_test.go | 214 +++++++++++++++++++++ 4 files changed, 260 insertions(+), 26 deletions(-) diff --git a/cmd/kubeadm/app/cmd/config.go b/cmd/kubeadm/app/cmd/config.go index 5f4ff01b87..48a363cb7a 100644 --- a/cmd/kubeadm/app/cmd/config.go +++ b/cmd/kubeadm/app/cmd/config.go @@ -329,10 +329,7 @@ func NewCmdConfigMigrate(out io.Writer) *cobra.Command { kubeadmutil.CheckErr(errors.New("The --old-config flag is mandatory")) } - internalcfg, err := configutil.AnyConfigFileAndDefaultsToInternal(oldCfgPath) - kubeadmutil.CheckErr(err) - - outputBytes, err := configutil.MarshalKubeadmConfigObject(internalcfg) + outputBytes, err := configutil.MigrateOldConfigFromFile(oldCfgPath) kubeadmutil.CheckErr(err) if newCfgPath == "" { diff --git a/cmd/kubeadm/app/util/config/BUILD b/cmd/kubeadm/app/util/config/BUILD index 012f544e1a..738e33ab58 100644 --- a/cmd/kubeadm/app/util/config/BUILD +++ b/cmd/kubeadm/app/util/config/BUILD @@ -65,6 +65,7 @@ go_test( "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//vendor/github.com/pmezard/go-difflib/difflib:go_default_library", + "//vendor/github.com/renstrom/dedent:go_default_library", ], ) diff --git a/cmd/kubeadm/app/util/config/common.go b/cmd/kubeadm/app/util/config/common.go index 660210706a..eb005b2a38 100644 --- a/cmd/kubeadm/app/util/config/common.go +++ b/cmd/kubeadm/app/util/config/common.go @@ -17,6 +17,7 @@ limitations under the License. package config import ( + "bytes" "io/ioutil" "net" "reflect" @@ -35,28 +36,6 @@ import ( kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" ) -// AnyConfigFileAndDefaultsToInternal reads either a InitConfiguration or JoinConfiguration and unmarshals it -func AnyConfigFileAndDefaultsToInternal(cfgPath string) (runtime.Object, error) { - b, err := ioutil.ReadFile(cfgPath) - if err != nil { - return nil, err - } - - gvks, err := kubeadmutil.GroupVersionKindsFromBytes(b) - if err != nil { - return nil, err - } - - // First, check if the gvk list has InitConfiguration and in that case try to unmarshal it - if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvks...) { - return ConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.InitConfiguration{}) - } - if kubeadmutil.GroupVersionKindsHasJoinConfiguration(gvks...) { - return JoinConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.JoinConfiguration{}) - } - return nil, errors.Errorf("didn't recognize types with GroupVersionKind: %v", gvks) -} - // MarshalKubeadmConfigObject marshals an Object registered in the kubeadm scheme. If the object is a InitConfiguration or ClusterConfiguration, some extra logic is run func MarshalKubeadmConfigObject(obj runtime.Object) ([]byte, error) { switch internalcfg := obj.(type) { @@ -181,3 +160,46 @@ func ChooseAPIServerBindAddress(bindAddress net.IP) (net.IP, error) { } return ip, nil } + +// MigrateOldConfigFromFile migrates an old configuration from a file into a new one (returned as a byte slice). Only kubeadm kinds are migrated. Others are silently ignored. +func MigrateOldConfigFromFile(cfgPath string) ([]byte, error) { + newConfig := [][]byte{} + + cfgBytes, err := ioutil.ReadFile(cfgPath) + if err != nil { + return []byte{}, err + } + + gvks, err := kubeadmutil.GroupVersionKindsFromBytes(cfgBytes) + if err != nil { + return []byte{}, err + } + + // Migrate InitConfiguration and ClusterConfiguration if there are any in the config + if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvks...) || kubeadmutil.GroupVersionKindsHasClusterConfiguration(gvks...) { + o, err := ConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.InitConfiguration{}) + if err != nil { + return []byte{}, err + } + b, err := MarshalKubeadmConfigObject(o) + if err != nil { + return []byte{}, err + } + newConfig = append(newConfig, b) + } + + // Migrate JoinConfiguration if there is any + if kubeadmutil.GroupVersionKindsHasJoinConfiguration(gvks...) { + o, err := JoinConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.JoinConfiguration{}) + if err != nil { + return []byte{}, err + } + b, err := MarshalKubeadmConfigObject(o) + if err != nil { + return []byte{}, err + } + newConfig = append(newConfig, b) + } + + return bytes.Join(newConfig, []byte(constants.YAMLDocumentSeparator)), nil +} diff --git a/cmd/kubeadm/app/util/config/common_test.go b/cmd/kubeadm/app/util/config/common_test.go index 2aa4c44de3..08e0158cc5 100644 --- a/cmd/kubeadm/app/util/config/common_test.go +++ b/cmd/kubeadm/app/util/config/common_test.go @@ -18,10 +18,15 @@ package config import ( "bytes" + "io/ioutil" + "os" "testing" + "github.com/renstrom/dedent" + kubeadmapiv1beta1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta1" "k8s.io/kubernetes/cmd/kubeadm/app/constants" + kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" ) var files = map[string][]byte{ @@ -298,3 +303,212 @@ func TestVerifyAPIServerBindAddress(t *testing.T) { }) } } + +func TestMigrateOldConfigFromFile(t *testing.T) { + tests := []struct { + desc string + oldCfg string + expectedKinds []string + expectErr bool + }{ + { + desc: "empty file produces empty result", + oldCfg: "", + expectErr: false, + }, + { + desc: "bad config produces error", + oldCfg: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1alpha3 + `), + expectErr: true, + }, + { + desc: "InitConfiguration only gets migrated", + oldCfg: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: InitConfiguration + `), + expectedKinds: []string{ + constants.InitConfigurationKind, + constants.ClusterConfigurationKind, + }, + expectErr: false, + }, + { + desc: "ClusterConfiguration only gets migrated", + oldCfg: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: ClusterConfiguration + `), + expectedKinds: []string{ + constants.InitConfigurationKind, + constants.ClusterConfigurationKind, + }, + expectErr: false, + }, + { + desc: "JoinConfiguration only gets migrated", + oldCfg: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: JoinConfiguration + token: abcdef.0123456789abcdef + discoveryTokenAPIServers: + - kube-apiserver:6443 + discoveryTokenUnsafeSkipCAVerification: true + `), + expectedKinds: []string{ + constants.JoinConfigurationKind, + }, + expectErr: false, + }, + { + desc: "Init + Cluster Configurations are migrated", + oldCfg: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: InitConfiguration + --- + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: ClusterConfiguration + `), + expectedKinds: []string{ + constants.InitConfigurationKind, + constants.ClusterConfigurationKind, + }, + expectErr: false, + }, + { + desc: "Init + Join Configurations are migrated", + oldCfg: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: InitConfiguration + --- + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: JoinConfiguration + token: abcdef.0123456789abcdef + discoveryTokenAPIServers: + - kube-apiserver:6443 + discoveryTokenUnsafeSkipCAVerification: true + `), + expectedKinds: []string{ + constants.InitConfigurationKind, + constants.ClusterConfigurationKind, + constants.JoinConfigurationKind, + }, + expectErr: false, + }, + { + desc: "Cluster + Join Configurations are migrated", + oldCfg: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: ClusterConfiguration + --- + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: JoinConfiguration + token: abcdef.0123456789abcdef + discoveryTokenAPIServers: + - kube-apiserver:6443 + discoveryTokenUnsafeSkipCAVerification: true + `), + expectedKinds: []string{ + constants.InitConfigurationKind, + constants.ClusterConfigurationKind, + constants.JoinConfigurationKind, + }, + expectErr: false, + }, + { + desc: "Init + Cluster + Join Configurations are migrated", + oldCfg: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: InitConfiguration + --- + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: ClusterConfiguration + --- + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: JoinConfiguration + token: abcdef.0123456789abcdef + discoveryTokenAPIServers: + - kube-apiserver:6443 + discoveryTokenUnsafeSkipCAVerification: true + `), + expectedKinds: []string{ + constants.InitConfigurationKind, + constants.ClusterConfigurationKind, + constants.JoinConfigurationKind, + }, + expectErr: false, + }, + { + desc: "component configs are not migrated", + oldCfg: dedent.Dedent(` + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: InitConfiguration + --- + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: ClusterConfiguration + --- + apiVersion: kubeadm.k8s.io/v1alpha3 + kind: JoinConfiguration + token: abcdef.0123456789abcdef + discoveryTokenAPIServers: + - kube-apiserver:6443 + discoveryTokenUnsafeSkipCAVerification: true + --- + apiVersion: kubeproxy.config.k8s.io/v1alpha1 + kind: KubeProxyConfiguration + --- + apiVersion: kubelet.config.k8s.io/v1beta1 + kind: KubeletConfiguration + `), + expectedKinds: []string{ + constants.InitConfigurationKind, + constants.ClusterConfigurationKind, + constants.JoinConfigurationKind, + }, + expectErr: false, + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + file, err := ioutil.TempFile("", "") + if err != nil { + t.Fatalf("could not create temporary test file: %v", err) + } + fileName := file.Name() + defer os.Remove(fileName) + + _, err = file.WriteString(test.oldCfg) + file.Close() + if err != nil { + t.Fatalf("could not write contents of old config: %v", err) + } + + b, err := MigrateOldConfigFromFile(fileName) + if test.expectErr { + if err == nil { + t.Fatalf("unexpected success:\n%s", b) + } + } else { + if err != nil { + t.Fatalf("unexpected failure: %v", err) + } + gvks, err := kubeadmutil.GroupVersionKindsFromBytes(b) + if err != nil { + t.Fatalf("unexpected error returned by GroupVersionKindsFromBytes: %v", err) + } + if len(gvks) != len(test.expectedKinds) { + t.Fatalf("length mismatch between resulting gvks and expected kinds:\n\tlen(gvks)=%d\n\tlen(expectedKinds)=%d", + len(gvks), len(test.expectedKinds)) + } + for _, expectedKind := range test.expectedKinds { + if !kubeadmutil.GroupVersionKindsHasKind(gvks, expectedKind) { + t.Fatalf("migration failed to produce config kind: %s", expectedKind) + } + } + } + }) + } +}