From 412c353d05be10d67f9774ca9ef0e7c2799274f9 Mon Sep 17 00:00:00 2001 From: SataQiu Date: Mon, 17 Feb 2025 22:42:14 +0800 Subject: [PATCH] kubeadm: fix panic when no UpgradeConfiguration was found in the config file --- cmd/kubeadm/app/util/config/common.go | 10 --- cmd/kubeadm/app/util/config/common_test.go | 49 ------------ .../app/util/config/upgradeconfiguration.go | 75 +++++++------------ .../util/config/upgradeconfiguration_test.go | 27 +++++-- 4 files changed, 45 insertions(+), 116 deletions(-) diff --git a/cmd/kubeadm/app/util/config/common.go b/cmd/kubeadm/app/util/config/common.go index 1e78452058d..4df881a0bcd 100644 --- a/cmd/kubeadm/app/util/config/common.go +++ b/cmd/kubeadm/app/util/config/common.go @@ -492,13 +492,3 @@ func defaultEmptyMigrateMutators() migrateMutators { return *mutators } - -// isKubeadmConfigPresent checks if a kubeadm config type is found in the provided document map -func isKubeadmConfigPresent(docmap kubeadmapi.DocumentMap) bool { - for gvk := range docmap { - if gvk.Group == kubeadmapi.GroupName { - return true - } - } - return false -} diff --git a/cmd/kubeadm/app/util/config/common_test.go b/cmd/kubeadm/app/util/config/common_test.go index 0e76cc5d0a4..925a4ce90ea 100644 --- a/cmd/kubeadm/app/util/config/common_test.go +++ b/cmd/kubeadm/app/util/config/common_test.go @@ -972,52 +972,3 @@ func TestDefaultMigrateMutators(t *testing.T) { }) } } - -func TestIsKubeadmConfigPresent(t *testing.T) { - var tcases = []struct { - name string - gvkmap kubeadmapi.DocumentMap - expected bool - }{ - { - name: " Wrong Group value", - gvkmap: kubeadmapi.DocumentMap{ - {Group: "foo.k8s.io", Version: "v1", Kind: "Foo"}: []byte(`kind: Foo`), - }, - expected: false, - }, - { - name: "Empty Group value", - gvkmap: kubeadmapi.DocumentMap{ - {Group: "", Version: "v1", Kind: "Empty"}: []byte(`kind: Empty`), - }, - expected: false, - }, - { - name: "Nil value", - gvkmap: nil, - expected: false, - }, - { - name: "Correct Group value 1", - gvkmap: kubeadmapi.DocumentMap{ - {Group: "kubeadm.k8s.io", Version: "v1", Kind: "Empty"}: []byte(`kind: Empty`), - }, - expected: true, - }, - { - name: "Correct Group value 2", - gvkmap: kubeadmapi.DocumentMap{ - {Group: kubeadmapi.GroupName, Version: "v1", Kind: "Empty"}: []byte(`kind: Empty`), - }, - expected: true, - }, - } - for _, tt := range tcases { - t.Run(tt.name, func(t *testing.T) { - if isKubeadmConfigPresent(tt.gvkmap) != tt.expected { - t.Error("unexpected result") - } - }) - } -} diff --git a/cmd/kubeadm/app/util/config/upgradeconfiguration.go b/cmd/kubeadm/app/util/config/upgradeconfiguration.go index d35a8841641..c5688a551f7 100644 --- a/cmd/kubeadm/app/util/config/upgradeconfiguration.go +++ b/cmd/kubeadm/app/util/config/upgradeconfiguration.go @@ -22,28 +22,34 @@ import ( "github.com/pkg/errors" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" - kubeproxyconfig "k8s.io/kube-proxy/config/v1alpha1" - kubeletconfig "k8s.io/kubelet/config/v1beta1" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme" kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta4" "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation" "k8s.io/kubernetes/cmd/kubeadm/app/componentconfigs" + "k8s.io/kubernetes/cmd/kubeadm/app/constants" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" "k8s.io/kubernetes/cmd/kubeadm/app/util/config/strict" ) -var componentCfgGV = sets.New(kubeproxyconfig.GroupName, kubeletconfig.GroupName) - // documentMapToUpgradeConfiguration takes a map between GVKs and YAML documents (as returned by SplitYAMLDocuments), // finds a UpgradeConfiguration, decodes it, dynamically defaults it and then validates it prior to return. func documentMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap, allowDeprecated bool) (*kubeadmapi.UpgradeConfiguration, error) { - var internalcfg *kubeadmapi.UpgradeConfiguration + upgradeBytes := []byte{} for gvk, bytes := range gvkmap { + if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvk) || kubeadmutil.GroupVersionKindsHasClusterConfiguration(gvk) || componentconfigs.Scheme.IsGroupRegistered(gvk.Group) { + klog.Warningf("[config] WARNING: YAML document with GroupVersionKind %v is deprecated for upgrade, please use config file with kind of UpgradeConfiguration instead \n", gvk) + continue + } + + if gvk.Kind != constants.UpgradeConfigurationKind { + klog.Warningf("[config] WARNING: Ignored YAML document with GroupVersionKind %v\n", gvk) + continue + } + // check if this version is supported and possibly not deprecated if err := validateSupportedVersion(gvk, allowDeprecated, true); err != nil { return nil, err @@ -54,37 +60,19 @@ func documentMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap, allowDepre klog.Warning(err.Error()) } - if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvk) || kubeadmutil.GroupVersionKindsHasClusterConfiguration(gvk) { - klog.Warningf("[config] WARNING: YAML document with GroupVersionKind %v is deprecated for upgrade, please use config file with kind of UpgradeConfiguration instead \n", gvk) - continue - } - - if kubeadmutil.GroupVersionKindsHasUpgradeConfiguration(gvk) { - // Set internalcfg to an empty struct value the deserializer will populate - internalcfg = &kubeadmapi.UpgradeConfiguration{} - // Decode the bytes into the internal struct. Under the hood, the bytes will be unmarshalled into the - // right external version, defaulted, and converted into the internal version. - if err := runtime.DecodeInto(kubeadmscheme.Codecs.UniversalDecoder(), bytes, internalcfg); err != nil { - return nil, err - } - continue - } - - // If the group is neither a kubeadm core type or of a supported component config group, we dump a warning about it being ignored - if !componentconfigs.Scheme.IsGroupRegistered(gvk.Group) { - klog.Warningf("[config] WARNING: Ignored YAML document with GroupVersionKind %v\n", gvk) - } + upgradeBytes = bytes } - // If UpgradeConfiguration wasn't given, default it by creating an external struct instance, default it and convert into the internal type - if internalcfg == nil { - extinitcfg := &kubeadmapiv1.UpgradeConfiguration{} - kubeadmscheme.Scheme.Default(extinitcfg) - // Set upgradeCfg to an empty struct value the deserializer will populate - internalcfg = &kubeadmapi.UpgradeConfiguration{} - if err := kubeadmscheme.Scheme.Convert(extinitcfg, internalcfg, nil); err != nil { - return nil, err - } + if len(upgradeBytes) == 0 { + return nil, errors.Errorf("no %s found in the supplied config", constants.UpgradeConfigurationKind) + } + + // Set internalcfg to an empty struct value the deserializer will populate + internalcfg := &kubeadmapi.UpgradeConfiguration{} + // Decode the bytes into the internal struct. Under the hood, the bytes will be unmarshalled into the + // right external version, defaulted, and converted into the internal version. + if err := runtime.DecodeInto(kubeadmscheme.Codecs.UniversalDecoder(), upgradeBytes, internalcfg); err != nil { + return nil, err } // Validates cfg @@ -96,9 +84,6 @@ func documentMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap, allowDepre } // DocMapToUpgradeConfiguration converts documentMap to an internal, defaulted and validated UpgradeConfiguration object. -// The map may contain many different YAML documents. These YAML documents are parsed one-by-one -// and well-known ComponentConfig GroupVersionKinds are stored inside of the internal UpgradeConfiguration struct. -// The resulting UpgradeConfiguration is then dynamically defaulted and validated prior to return. func DocMapToUpgradeConfiguration(gvkmap kubeadmapi.DocumentMap) (*kubeadmapi.UpgradeConfiguration, error) { return documentMapToUpgradeConfiguration(gvkmap, false) } @@ -123,18 +108,8 @@ func LoadUpgradeConfigurationFromFile(cfgPath string, _ LoadOrDefaultConfigurati // Convert documentMap to internal UpgradeConfiguration, InitConfiguration and ClusterConfiguration from config file will be ignored. // Upgrade should respect the cluster configuration from the existing cluster, re-configure the cluster with a InitConfiguration and // ClusterConfiguration from the config file is not allowed for upgrade. - if isKubeadmConfigPresent(docmap) { - if upgradeCfg, err = DocMapToUpgradeConfiguration(docmap); err != nil { - return nil, err - } - } - - // Check is there any component configs defined in the config file. - for gvk := range docmap { - if componentCfgGV.Has(gvk.Group) { - klog.Warningf("[config] WARNING: YAML document with Component Configs %v is deprecated for upgrade and will be ignored \n", gvk.Group) - continue - } + if upgradeCfg, err = DocMapToUpgradeConfiguration(docmap); err != nil { + return nil, err } return upgradeCfg, nil diff --git a/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go b/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go index 6c38218e429..790cb3b2f39 100644 --- a/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go +++ b/cmd/kubeadm/app/util/config/upgradeconfiguration_test.go @@ -38,9 +38,10 @@ import ( func TestDocMapToUpgradeConfiguration(t *testing.T) { tests := []struct { - name string - cfg kubeadmapiv1.UpgradeConfiguration - expectedCfg kubeadmapi.UpgradeConfiguration + name string + cfg interface{} + expectedCfg kubeadmapi.UpgradeConfiguration + expectedError bool }{ { name: "default config is set correctly", @@ -94,6 +95,16 @@ func TestDocMapToUpgradeConfiguration(t *testing.T) { }, }, }, + { + name: "no UpgradeConfiguration found", + cfg: kubeadmapiv1.InitConfiguration{ + TypeMeta: metav1.TypeMeta{ + APIVersion: kubeadmapiv1.SchemeGroupVersion.String(), + Kind: constants.InitConfigurationKind, + }, + }, + expectedError: true, + }, } for _, tc := range tests { @@ -107,11 +118,13 @@ func TestDocMapToUpgradeConfiguration(t *testing.T) { t.Fatalf("Unexpected error of SplitYAMLDocuments: %v", err) } cfg, err := DocMapToUpgradeConfiguration(docmap) - if err != nil { - t.Fatalf("unexpected error of DocMapToUpgradeConfiguration: %v\nconfig: %s", err, string(b)) + if (err != nil) != tc.expectedError { + t.Fatalf("failed DocMapToUpgradeConfiguration:\n\texpected error: %t\n\t actual error: %v", tc.expectedError, err) } - if diff := cmp.Diff(*cfg, tc.expectedCfg, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" { - t.Fatalf("DocMapToUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff) + if err == nil { + if diff := cmp.Diff(*cfg, tc.expectedCfg, cmpopts.IgnoreFields(kubeadmapi.UpgradeConfiguration{}, "Timeouts")); diff != "" { + t.Fatalf("DocMapToUpgradeConfiguration returned unexpected diff (-want,+got):\n%s", diff) + } } }) }