From 09f753a94cd786c376b16971991179d2e1229330 Mon Sep 17 00:00:00 2001 From: "Rostislav M. Georgiev" Date: Fri, 7 Dec 2018 12:51:38 +0200 Subject: [PATCH] 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 --- cmd/kubeadm/app/cmd/config.go | 2 +- cmd/kubeadm/app/cmd/join.go | 2 +- cmd/kubeadm/app/util/config/common.go | 2 +- .../app/util/config/joinconfiguration.go | 96 ++++++++++++------- .../app/util/config/joinconfiguration_test.go | 8 +- 5 files changed, 66 insertions(+), 44 deletions(-) diff --git a/cmd/kubeadm/app/cmd/config.go b/cmd/kubeadm/app/cmd/config.go index 0f7e43c66a..53422af043 100644 --- 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 index 92bc369c64..3143f663f4 100644 --- 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 index 9a8f4c0cab..f3518c339f 100644 --- 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 index 34ad12ba35..d512424e86 100644 --- a/cmd/kubeadm/app/util/config/joinconfiguration.go +++ b/cmd/kubeadm/app/util/config/joinconfiguration.go @@ -54,59 +54,60 @@ 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) + } - b, err := ioutil.ReadFile(cfgPath) - if err != nil { - return nil, errors.Wrapf(err, "unable to read config from %q ", 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 { + 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 - } + joinBytes = bytes + } - // verify the validity of the YAML - strict.VerifyUnmarshalStrict(bytes, gvk) + if len(joinBytes) == 0 { + return nil, errors.Errorf("no %s found in config file %q", constants.JoinConfigurationKind, cfgPath) + } - joinBytes = bytes - } - - if len(joinBytes) == 0 { - return nil, errors.Errorf("no %s found in config file %q", constants.JoinConfigurationKind, cfgPath) - } - - 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) + internalcfg := &kubeadmapi.JoinConfiguration{} + if err := runtime.DecodeInto(kubeadmscheme.Codecs.UniversalDecoder(), joinBytes, internalcfg); err != nil { + return nil, err } // Applies dynamic defaults to settings not provided with flags @@ -120,3 +121,24 @@ func JoinConfigFileAndDefaultsToInternalConfig(cfgPath string, defaultversionedc 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 { + return nil, err + } + // Validates cfg (flags/configs + defaults) + if err := validation.ValidateJoinConfiguration(internalcfg).ToAggregate(); err != nil { + return nil, err + } + + return internalcfg, nil +} diff --git a/cmd/kubeadm/app/util/config/joinconfiguration_test.go b/cmd/kubeadm/app/util/config/joinconfiguration_test.go index 5ce23dbff3..65244cba85 100644 --- 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