Merge pull request #73745 from rosti/refactor-JoinConfigFileAndDefaultsToInternalConfig

kubeadm: refactor JoinConfigFileAndDefaultsToInternalConfig

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(),

cmd/kubeadm/app/cmd/join.go

@@ -334,7 +334,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
 	}

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
 		}

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

@@ -54,18 +54,24 @@ 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) {
+func LoadOrDefaultJoinConfiguration(cfgPath string, defaultversionedcfg *kubeadmapiv1beta1.JoinConfiguration) (*kubeadmapi.JoinConfiguration, error) {
-	internalcfg := &kubeadmapi.JoinConfiguration{}
-
 	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)
+}
+
+// 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 {
@@ -99,15 +105,31 @@ func JoinConfigFileAndDefaultsToInternalConfig(cfgPath string, defaultversionedc
 		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
 	}
-	} else {
+
-		// Takes passed flags into account; the defaulting is executed once again enforcing assignement of
+	// Applies dynamic defaults to settings not provided with flags
-		// static default values to cfg only for values not provided with flags
+	if err := SetJoinDynamicDefaults(internalcfg); err != nil {
-		kubeadmscheme.Scheme.Default(defaultversionedcfg)
+		return nil, err
-		kubeadmscheme.Scheme.Convert(defaultversionedcfg, internalcfg, nil)
+	}
-	}
+	// 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 {

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