Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubeadm: refactor JoinConfigFileAndDefaultsToInternalConfig #73745

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/cmd/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/util/config/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
94 changes: 58 additions & 36 deletions cmd/kubeadm/app/util/config/joinconfiguration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not wrong, we should apply defaults before, and then dynamic defaults (otherwise the user will be required to set static default values manually)
Wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The preceding call to DecodeInto applies static defaults upon decoding the object. That's why it actually works in the current code base.

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 {
Expand Down
8 changes: 4 additions & 4 deletions cmd/kubeadm/app/util/config/joinconfiguration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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
Expand Down