kubeadm: Replace MigrateOldConfigFromFile

MigrateOldConfigFromFile is a function, whose purpose is to migrate one config
into another. It is working OK for now, but it has some issues:

- It is incredibly inefficient. It can reload and re-parse a single config file
  for up to 3 times.

- Because of the reloads, it has to take a file containing the configuration
  (not a byte slice as most of the rest config functions). However, it returns
  the migrated config in a byte slice (rather asymmetric from the input
  method).

- Due to the above points it's difficult to implement a proper interface for
  deprecated kubeadm config versions.

To fix the issues of MigrateOldConfigFromFile, the following is done:

- Re-implement the function by removing the calls to file loading package
  public APIs and replacing them with newly extracted package private APIs that
  do the job with pre-provided input data in the form of
  map[GroupVersionKind][]byte.

- Take a byte slice of the input configuration as an argument. This makes the
  function input symmetric to its output. Also, it's now renamed to
  MigrateOldConfig to represent the change from config file path as an input
  to byte slice.

- As a bonus (actually forgotten from a previous change) BytesToInternalConfig
  is renamed to the more descriptive BytesToInitConfiguration.

Signed-off-by: Rostislav M. Georgiev <rostislavg@vmware.com>

cmd/kubeadm/app/cmd/config.go

@@ -246,7 +246,10 @@ func NewCmdConfigMigrate(out io.Writer) *cobra.Command {
 				kubeadmutil.CheckErr(errors.New("The --old-config flag is mandatory"))
 			}
 
-			outputBytes, err := configutil.MigrateOldConfigFromFile(oldCfgPath)
+			oldCfgBytes, err := ioutil.ReadFile(oldCfgPath)
+			kubeadmutil.CheckErr(err)
+
+			outputBytes, err := configutil.MigrateOldConfig(oldCfgBytes)
 			kubeadmutil.CheckErr(err)
 
 			if newCfgPath == "" {

cmd/kubeadm/app/phases/upgrade/BUILD

@@ -76,7 +76,6 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
-        "//cmd/kubeadm/app/apis/kubeadm/validation:go_default_library",
         "//cmd/kubeadm/app/constants:go_default_library",
         "//cmd/kubeadm/app/phases/certs:go_default_library",
         "//cmd/kubeadm/app/phases/controlplane:go_default_library",

cmd/kubeadm/app/phases/upgrade/staticpods_test.go

@@ -32,7 +32,6 @@ import (
 	"github.com/pkg/errors"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	"k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	certsphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/certs"
 	controlplanephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/controlplane"
@@ -541,22 +540,7 @@ func getConfig(version, certsDir, etcdDataDir string) (*kubeadmapi.InitConfigura
 	configBytes := []byte(fmt.Sprintf(testConfiguration, certsDir, etcdDataDir, version))
 
 	// Unmarshal the config
-	cfg, err := configutil.BytesToInternalConfig(configBytes)
+	return configutil.BytesToInitConfiguration(configBytes)
-	if err != nil {
-		return nil, err
-	}
-
-	// Applies dynamic defaults to settings not provided with flags
-	if err = configutil.SetInitDynamicDefaults(cfg); err != nil {
-		return nil, err
-	}
-
-	// Validates cfg (flags/configs + defaults + dynamic defaults)
-	if err = validation.ValidateInitConfiguration(cfg).ToAggregate(); err != nil {
-		return nil, err
-	}
-
-	return cfg, nil
 }
 
 func getTempDir(t *testing.T, name string) (string, func()) {

cmd/kubeadm/app/util/config/common.go

@@ -18,7 +18,6 @@ package config
 
 import (
 	"bytes"
-	"io/ioutil"
 	"net"
 	"reflect"
 	"strings"
@@ -138,23 +137,24 @@ 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.
+// MigrateOldConfig migrates an old configuration from a byte slice into a new one (returned again as a byte slice).
-func MigrateOldConfigFromFile(cfgPath string) ([]byte, error) {
+// Only kubeadm kinds are migrated. Others are silently ignored.
+func MigrateOldConfig(oldConfig []byte) ([]byte, error) {
 	newConfig := [][]byte{}
 
-	cfgBytes, err := ioutil.ReadFile(cfgPath)
+	gvkmap, err := kubeadmutil.SplitYAMLDocuments(oldConfig)
 	if err != nil {
 		return []byte{}, err
 	}
 
-	gvks, err := kubeadmutil.GroupVersionKindsFromBytes(cfgBytes)
+	gvks := []schema.GroupVersionKind{}
-	if err != nil {
+	for gvk := range gvkmap {
-		return []byte{}, err
+		gvks = append(gvks, gvk)
 	}
 
 	// Migrate InitConfiguration and ClusterConfiguration if there are any in the config
 	if kubeadmutil.GroupVersionKindsHasInitConfiguration(gvks...) || kubeadmutil.GroupVersionKindsHasClusterConfiguration(gvks...) {
-		o, err := LoadInitConfigurationFromFile(cfgPath)
+		o, err := documentMapToInitConfiguration(gvkmap)
 		if err != nil {
 			return []byte{}, err
 		}
@@ -167,7 +167,7 @@ func MigrateOldConfigFromFile(cfgPath string) ([]byte, error) {
 
 	// Migrate JoinConfiguration if there is any
 	if kubeadmutil.GroupVersionKindsHasJoinConfiguration(gvks...) {
-		o, err := LoadJoinConfigurationFromFile(cfgPath)
+		o, err := documentMapToJoinConfiguration(gvkmap)
 		if err != nil {
 			return []byte{}, err
 		}

cmd/kubeadm/app/util/config/common_test.go

@@ -17,8 +17,6 @@ limitations under the License.
 package config
 
 import (
-	"io/ioutil"
-	"os"
 	"testing"
 
 	"github.com/lithammer/dedent"
@@ -352,20 +350,7 @@ func TestMigrateOldConfigFromFile(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.desc, func(t *testing.T) {
-			file, err := ioutil.TempFile("", "")
+			b, err := MigrateOldConfig([]byte(test.oldCfg))
-			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)

cmd/kubeadm/app/util/config/initconfiguration.go

@@ -190,19 +190,10 @@ func LoadInitConfigurationFromFile(cfgPath string) (*kubeadmapi.InitConfiguratio
 		return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath)
 	}
 
-	internalcfg, err := BytesToInternalConfig(b)
+	internalcfg, err := BytesToInitConfiguration(b)
 	if err != nil {
 		return nil, err
 	}
-
-	// Applies dynamic defaults to settings not provided with flags
-	if err := SetInitDynamicDefaults(internalcfg); err != nil {
-		return nil, err
-	}
-	// Validates cfg (flags/configs + defaults + dynamic defaults)
-	if err := validation.ValidateInitConfiguration(internalcfg).ToAggregate(); err != nil {
-		return nil, err
-	}
 	return internalcfg, nil
 }
 
@@ -221,19 +212,25 @@ func LoadOrDefaultInitConfiguration(cfgPath string, defaultversionedcfg *kubeadm
 	return DefaultedInitConfiguration(defaultversionedcfg)
 }
 
-// BytesToInternalConfig converts a byte slice to an internal, defaulted and validated configuration object.
+// BytesToInitConfiguration converts a byte slice to an internal, defaulted and validated InitConfiguration object.
-// The byte slice may contain one or many different YAML documents. These YAML documents are parsed one-by-one
+// 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 InitConfiguration struct
+// and well-known ComponentConfig GroupVersionKinds are stored inside of the internal InitConfiguration struct.
-func BytesToInternalConfig(b []byte) (*kubeadmapi.InitConfiguration, error) {
+// The resulting InitConfiguration is then dynamically defaulted and validated prior to return.
-	var initcfg *kubeadmapi.InitConfiguration
+func BytesToInitConfiguration(b []byte) (*kubeadmapi.InitConfiguration, error) {
-	var clustercfg *kubeadmapi.ClusterConfiguration
-	decodedComponentConfigObjects := map[componentconfigs.RegistrationKind]runtime.Object{}
-
 	gvkmap, err := kubeadmutil.SplitYAMLDocuments(b)
 	if err != nil {
 		return nil, err
 	}
 
+	return documentMapToInitConfiguration(gvkmap)
+}
+
+// documentMapToInitConfiguration converts a map of GVKs and YAML documents to defaulted and validated configuration object.
+func documentMapToInitConfiguration(gvkmap map[schema.GroupVersionKind][]byte) (*kubeadmapi.InitConfiguration, error) {
+	var initcfg *kubeadmapi.InitConfiguration
+	var clustercfg *kubeadmapi.ClusterConfiguration
+	decodedComponentConfigObjects := map[componentconfigs.RegistrationKind]runtime.Object{}
+
 	for gvk, fileContent := range gvkmap {
 		// first, check if this GVK is supported one
 		if err := ValidateSupportedVersion(gvk.GroupVersion()); err != nil {
@@ -308,6 +305,17 @@ func BytesToInternalConfig(b []byte) (*kubeadmapi.InitConfiguration, error) {
 			fmt.Printf("[config] WARNING: Decoded a kind that couldn't be saved to the internal configuration: %q\n", string(kind))
 		}
 	}
+
+	// Applies dynamic defaults to settings not provided with flags
+	if err := SetInitDynamicDefaults(initcfg); err != nil {
+		return nil, err
+	}
+
+	// Validates cfg (flags/configs + defaults + dynamic defaults)
+	if err := validation.ValidateInitConfiguration(initcfg).ToAggregate(); err != nil {
+		return nil, err
+	}
+
 	return initcfg, nil
 }
 

cmd/kubeadm/app/util/config/joinconfiguration.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/pkg/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/klog"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmscheme "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/scheme"
@@ -83,6 +84,12 @@ func LoadJoinConfigurationFromFile(cfgPath string) (*kubeadmapi.JoinConfiguratio
 		return nil, err
 	}
 
+	return documentMapToJoinConfiguration(gvkmap)
+}
+
+// documentMapToJoinConfiguration takes a map between GVKs and YAML documents (as returned by SplitYAMLDocuments),
+// finds a JoinConfiguration, decodes it, dynamically defaults it and then validates it prior to return.
+func documentMapToJoinConfiguration(gvkmap map[schema.GroupVersionKind][]byte) (*kubeadmapi.JoinConfiguration, error) {
 	joinBytes := []byte{}
 	for gvk, bytes := range gvkmap {
 		// not interested in anything other than JoinConfiguration
@@ -102,7 +109,7 @@ func LoadJoinConfigurationFromFile(cfgPath string) (*kubeadmapi.JoinConfiguratio
 	}
 
 	if len(joinBytes) == 0 {
-		return nil, errors.Errorf("no %s found in config file %q", constants.JoinConfigurationKind, cfgPath)
+		return nil, errors.Errorf("no %s found in the supplied config", constants.JoinConfigurationKind)
 	}
 
 	internalcfg := &kubeadmapi.JoinConfiguration{}