kubeadm: refactor JoinConfigFileAndDefaultsToInternalConfig

Currently JoinConfigFileAndDefaultsToInternalConfig is doing a couple of different things depending on its parameters. It: - loads a versioned JoinConfiguration from an YAML file. - returns defaulted JoinConfiguration allowing for some overrides. In order to make code more manageable, the following steps are taken: - Introduce LoadJoinConfigurationFromFile, which loads a versioned JoinConfiguration from an YAML file, defaults it (both dynamically and statically), converts it to internal JoinConfiguration and validates it. - Introduce DefaultedJoinConfiguration, which returns defaulted (both dynamically and statically) and verified internal JoinConfiguration. The possibility of overwriting defaults via versioned JoinConfiguration is retained. - Re-implement JoinConfigFileAndDefaultsToInternalConfig to use LoadJoinConfigurationFromFile and DefaultedJoinConfiguration. - Replace some calls to JoinConfigFileAndDefaultsToInternalConfig with calls to either LoadJoinConfigurationFromFile or DefaultedJoinConfiguration where appropriate. - Rename JoinConfigFileAndDefaultsToInternalConfig to the more appropriate name LoadOrDefaultJoinConfiguration. Signed-off-by: Rostislav M. Georgiev <rostislavg@vmware.com>
kubernetes · Feb 8, 2019 · 09f753a · 09f753a
1 parent 7672292
commit 09f753a
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 43 deletions.
diff --git a/cmd/kubeadm/app/cmd/config.go b/cmd/kubeadm/app/cmd/config.go
@@ -201,7 +201,7 @@ func getDefaultInitConfigBytes() ([]byte, error) {
 }
 
 func getDefaultNodeConfigBytes() ([]byte, error) {
-	internalcfg, err := configutil.JoinConfigFileAndDefaultsToInternalConfig("", &kubeadmapiv1beta1.JoinConfiguration{
+	internalcfg, err := configutil.DefaultedJoinConfiguration(&kubeadmapiv1beta1.JoinConfiguration{
 		Discovery: kubeadmapiv1beta1.Discovery{
 			BootstrapToken: &kubeadmapiv1beta1.BootstrapTokenDiscovery{
 				Token:                    placeholderToken.Token.String(),

diff --git a/cmd/kubeadm/app/cmd/join.go b/cmd/kubeadm/app/cmd/join.go
@@ -353,7 +353,7 @@ func newJoinData(cmd *cobra.Command, args []string, options *joinOptions, out io
 		klog.V(1).Infoln("[join] found advertiseAddress empty; using default interface's IP address as advertiseAddress")
 	}
 
-	cfg, err := configutil.JoinConfigFileAndDefaultsToInternalConfig(options.cfgPath, options.externalcfg)
+	cfg, err := configutil.LoadOrDefaultJoinConfiguration(options.cfgPath, options.externalcfg)
 	if err != nil {
 		return nil, err
 	}

diff --git a/cmd/kubeadm/app/util/config/common.go b/cmd/kubeadm/app/util/config/common.go
@@ -167,7 +167,7 @@ func MigrateOldConfigFromFile(cfgPath string) ([]byte, error) {
 
 	// Migrate JoinConfiguration if there is any
 	if kubeadmutil.GroupVersionKindsHasJoinConfiguration(gvks...) {
-		o, err := JoinConfigFileAndDefaultsToInternalConfig(cfgPath, &kubeadmapiv1beta1.JoinConfiguration{})
+		o, err := LoadJoinConfigurationFromFile(cfgPath)
 		if err != nil {
 			return []byte{}, err
 		}

diff --git a/cmd/kubeadm/app/util/config/joinconfiguration.go b/cmd/kubeadm/app/util/config/joinconfiguration.go
@@ -54,60 +54,82 @@ func SetJoinControlPlaneDefaults(cfg *kubeadmapi.JoinControlPlane) error {
 	return nil
 }
 
-// JoinConfigFileAndDefaultsToInternalConfig takes a path to a config file and a versioned configuration that can serve as the default config
+// LoadOrDefaultJoinConfiguration takes a path to a config file and a versioned configuration that can serve as the default config
 // If cfgPath is specified, defaultversionedcfg will always get overridden. Otherwise, the default config (often populated by flags) will be used.
 // Then the external, versioned configuration is defaulted and converted to the internal type.
 // Right thereafter, the configuration is defaulted again with dynamic values (like IP addresses of a machine, etc)
 // Lastly, the internal config is validated and returned.
-func JoinConfigFileAndDefaultsToInternalConfig(cfgPath string, defaultversionedcfg *kubeadmapiv1beta1.JoinConfiguration) (*kubeadmapi.JoinConfiguration, error) {
-	internalcfg := &kubeadmapi.JoinConfiguration{}
-
+func LoadOrDefaultJoinConfiguration(cfgPath string, defaultversionedcfg *kubeadmapiv1beta1.JoinConfiguration) (*kubeadmapi.JoinConfiguration, error) {
 	if cfgPath != "" {
 		// Loads configuration from config file, if provided
 		// Nb. --config overrides command line flags, TODO: fix this
-		klog.V(1).Infoln("loading configuration from the given file")
+		return LoadJoinConfigurationFromFile(cfgPath)
+	}
+
+	return DefaultedJoinConfiguration(defaultversionedcfg)
+}
 
-		b, err := ioutil.ReadFile(cfgPath)
-		if err != nil {
-			return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath)
+// LoadJoinConfigurationFromFile loads versioned JoinConfiguration from file, converts it to internal, defaults and validates it
+func LoadJoinConfigurationFromFile(cfgPath string) (*kubeadmapi.JoinConfiguration, error) {
+	klog.V(1).Infof("loading configuration from %q", cfgPath)
+
+	b, err := ioutil.ReadFile(cfgPath)
+	if err != nil {
+		return nil, errors.Wrapf(err, "unable to read config from %q ", cfgPath)
+	}
+
+	gvkmap, err := kubeadmutil.SplitYAMLDocuments(b)
+	if err != nil {
+		return nil, err
+	}
+
+	joinBytes := []byte{}
+	for gvk, bytes := range gvkmap {
+		// not interested in anything other than JoinConfiguration
+		if gvk.Kind != constants.JoinConfigurationKind {
+			continue
 		}
 
-		gvkmap, err := kubeadmutil.SplitYAMLDocuments(b)
-		if err != nil {
+		// check if this version is supported one
+		if err := ValidateSupportedVersion(gvk.GroupVersion()); err != nil {
 			return nil, err
 		}
 
-		joinBytes := []byte{}
-		for gvk, bytes := range gvkmap {
-			// not interested in anything other than JoinConfiguration
-			if gvk.Kind != constants.JoinConfigurationKind {
-				continue
-			}
+		// verify the validity of the YAML
+		strict.VerifyUnmarshalStrict(bytes, gvk)
 
-			// check if this version is supported one
-			if err := ValidateSupportedVersion(gvk.GroupVersion()); err != nil {
-				return nil, err
-			}
-
-			// verify the validity of the YAML
-			strict.VerifyUnmarshalStrict(bytes, gvk)
+		joinBytes = bytes
+	}
 
-			joinBytes = bytes
-		}
+	if len(joinBytes) == 0 {
+		return nil, errors.Errorf("no %s found in config file %q", constants.JoinConfigurationKind, cfgPath)
+	}
 
-		if len(joinBytes) == 0 {
-			return nil, errors.Errorf("no %s found in config file %q", constants.JoinConfigurationKind, cfgPath)
-		}
+	internalcfg := &kubeadmapi.JoinConfiguration{}
+	if err := runtime.DecodeInto(kubeadmscheme.Codecs.UniversalDecoder(), joinBytes, internalcfg); err != nil {
+		return nil, err
+	}
 
-		if err := runtime.DecodeInto(kubeadmscheme.Codecs.UniversalDecoder(), joinBytes, internalcfg); err != nil {
-			return nil, err
-		}
-	} else {
-		// Takes passed flags into account; the defaulting is executed once again enforcing assignement of
-		// static default values to cfg only for values not provided with flags
-		kubeadmscheme.Scheme.Default(defaultversionedcfg)
-		kubeadmscheme.Scheme.Convert(defaultversionedcfg, internalcfg, nil)
+	// Applies dynamic defaults to settings not provided with flags
+	if err := SetJoinDynamicDefaults(internalcfg); err != nil {
+		return nil, err
 	}
+	// Validates cfg (flags/configs + defaults)
+	if err := validation.ValidateJoinConfiguration(internalcfg).ToAggregate(); err != nil {
+		return nil, err
+	}
+
+	return internalcfg, nil
+}
+
+// DefaultedJoinConfiguration takes a versioned JoinConfiguration (usually filled in by command line parameters), defaults it, converts it to internal and validates it
+func DefaultedJoinConfiguration(defaultversionedcfg *kubeadmapiv1beta1.JoinConfiguration) (*kubeadmapi.JoinConfiguration, error) {
+	internalcfg := &kubeadmapi.JoinConfiguration{}
+
+	// Takes passed flags into account; the defaulting is executed once again enforcing assignment of
+	// static default values to cfg only for values not provided with flags
+	kubeadmscheme.Scheme.Default(defaultversionedcfg)
+	kubeadmscheme.Scheme.Convert(defaultversionedcfg, internalcfg, nil)
 
 	// Applies dynamic defaults to settings not provided with flags
 	if err := SetJoinDynamicDefaults(internalcfg); err != nil {

diff --git a/cmd/kubeadm/app/util/config/joinconfiguration_test.go b/cmd/kubeadm/app/util/config/joinconfiguration_test.go
@@ -37,13 +37,13 @@ const (
 	node_invalidYAML    = "testdata/validation/invalid_nodecfg.yaml"
 )
 
-func TestJoinConfigFileAndDefaultsToInternalConfig(t *testing.T) {
+func TestLoadJoinConfigurationFromFile(t *testing.T) {
 	var tests = []struct {
 		name, in, out string
 		groupVersion  schema.GroupVersion
 		expectedErr   bool
 	}{
-		// These tests are reading one file, loading it using JoinConfigFileAndDefaultsToInternalConfig that all of kubeadm is using for unmarshal of our API types,
+		// These tests are reading one file, loading it using LoadJoinConfigurationFromFile that all of kubeadm is using for unmarshal of our API types,
 		// and then marshals the internal object to the expected groupVersion
 		{ // v1alpha3 -> internal
 			name:         "v1alpha3ToInternal",
@@ -69,7 +69,7 @@ func TestJoinConfigFileAndDefaultsToInternalConfig(t *testing.T) {
 			out:          node_v1beta1YAML,
 			groupVersion: kubeadmapiv1beta1.SchemeGroupVersion,
 		},
-		// These tests are reading one file that has only a subset of the fields populated, loading it using JoinConfigFileAndDefaultsToInternalConfig,
+		// These tests are reading one file that has only a subset of the fields populated, loading it using LoadJoinConfigurationFromFile,
 		// and then marshals the internal object to the expected groupVersion
 		{ // v1beta1 -> default -> validate -> internal -> v1beta1
 			name:         "incompleteYAMLToDefaultedv1beta1",
@@ -87,7 +87,7 @@ func TestJoinConfigFileAndDefaultsToInternalConfig(t *testing.T) {
 	for _, rt := range tests {
 		t.Run(rt.name, func(t2 *testing.T) {
 
-			internalcfg, err := JoinConfigFileAndDefaultsToInternalConfig(rt.in, &kubeadmapiv1beta1.JoinConfiguration{})
+			internalcfg, err := LoadJoinConfigurationFromFile(rt.in)
 			if err != nil {
 				if rt.expectedErr {
 					return