diff --git a/tools/clientcmd/client_config.go b/tools/clientcmd/client_config.go index 66331a7a..b8927f71 100644 --- a/tools/clientcmd/client_config.go +++ b/tools/clientcmd/client_config.go @@ -175,10 +175,6 @@ func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) { // only try to read the auth information if we are secure if restclient.IsConfigTransportTLS(*clientConfig) { var err error - - // mergo is a first write wins for map value and a last writing wins for interface values - // NOTE: This behavior changed with https://github.com/imdario/mergo/commit/d304790b2ed594794496464fadd89d2bb266600a. - // Our mergo.Merge version is older than this change. var persister restclient.AuthProviderConfigPersister if config.configAccess != nil { authInfoName, _ := config.getAuthInfoName() @@ -188,13 +184,13 @@ func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) { if err != nil { return nil, err } - mergo.Merge(clientConfig, userAuthPartialConfig) + mergo.MergeWithOverwrite(clientConfig, userAuthPartialConfig) serverAuthPartialConfig, err := getServerIdentificationPartialConfig(configAuthInfo, configClusterInfo) if err != nil { return nil, err } - mergo.Merge(clientConfig, serverAuthPartialConfig) + mergo.MergeWithOverwrite(clientConfig, serverAuthPartialConfig) } return clientConfig, nil @@ -214,7 +210,7 @@ func getServerIdentificationPartialConfig(configAuthInfo clientcmdapi.AuthInfo, configClientConfig.CAFile = configClusterInfo.CertificateAuthority configClientConfig.CAData = configClusterInfo.CertificateAuthorityData configClientConfig.Insecure = configClusterInfo.InsecureSkipTLSVerify - mergo.Merge(mergedConfig, configClientConfig) + mergo.MergeWithOverwrite(mergedConfig, configClientConfig) return mergedConfig, nil } @@ -279,8 +275,8 @@ func (config *DirectClientConfig) getUserIdentificationPartialConfig(configAuthI promptedConfig := makeUserIdentificationConfig(*promptedAuthInfo) previouslyMergedConfig := mergedConfig mergedConfig = &restclient.Config{} - mergo.Merge(mergedConfig, promptedConfig) - mergo.Merge(mergedConfig, previouslyMergedConfig) + mergo.MergeWithOverwrite(mergedConfig, promptedConfig) + mergo.MergeWithOverwrite(mergedConfig, previouslyMergedConfig) config.promptedCredentials.username = mergedConfig.Username config.promptedCredentials.password = mergedConfig.Password } @@ -423,11 +419,11 @@ func (config *DirectClientConfig) getContext() (clientcmdapi.Context, error) { mergedContext := clientcmdapi.NewContext() if configContext, exists := contexts[contextName]; exists { - mergo.Merge(mergedContext, configContext) + mergo.MergeWithOverwrite(mergedContext, configContext) } else if required { return clientcmdapi.Context{}, fmt.Errorf("context %q does not exist", contextName) } - mergo.Merge(mergedContext, config.overrides.Context) + mergo.MergeWithOverwrite(mergedContext, config.overrides.Context) return *mergedContext, nil } @@ -439,11 +435,11 @@ func (config *DirectClientConfig) getAuthInfo() (clientcmdapi.AuthInfo, error) { mergedAuthInfo := clientcmdapi.NewAuthInfo() if configAuthInfo, exists := authInfos[authInfoName]; exists { - mergo.Merge(mergedAuthInfo, configAuthInfo) + mergo.MergeWithOverwrite(mergedAuthInfo, configAuthInfo) } else if required { return clientcmdapi.AuthInfo{}, fmt.Errorf("auth info %q does not exist", authInfoName) } - mergo.Merge(mergedAuthInfo, config.overrides.AuthInfo) + mergo.MergeWithOverwrite(mergedAuthInfo, config.overrides.AuthInfo) return *mergedAuthInfo, nil } @@ -454,13 +450,13 @@ func (config *DirectClientConfig) getCluster() (clientcmdapi.Cluster, error) { clusterInfoName, required := config.getClusterName() mergedClusterInfo := clientcmdapi.NewCluster() - mergo.Merge(mergedClusterInfo, config.overrides.ClusterDefaults) + mergo.MergeWithOverwrite(mergedClusterInfo, config.overrides.ClusterDefaults) if configClusterInfo, exists := clusterInfos[clusterInfoName]; exists { - mergo.Merge(mergedClusterInfo, configClusterInfo) + mergo.MergeWithOverwrite(mergedClusterInfo, configClusterInfo) } else if required { return clientcmdapi.Cluster{}, fmt.Errorf("cluster %q does not exist", clusterInfoName) } - mergo.Merge(mergedClusterInfo, config.overrides.ClusterInfo) + mergo.MergeWithOverwrite(mergedClusterInfo, config.overrides.ClusterInfo) // An override of --insecure-skip-tls-verify=true and no accompanying CA/CA data should clear already-set CA/CA data // otherwise, a kubeconfig containing a CA reference would return an error that "CA and insecure-skip-tls-verify couldn't both be set" caLen := len(config.overrides.ClusterInfo.CertificateAuthority) diff --git a/tools/clientcmd/client_config_test.go b/tools/clientcmd/client_config_test.go index a209da42..b2335f12 100644 --- a/tools/clientcmd/client_config_test.go +++ b/tools/clientcmd/client_config_test.go @@ -28,21 +28,23 @@ import ( clientcmdapi "k8s.io/client-go/tools/clientcmd/api" ) -func TestOldMergoLib(t *testing.T) { +func TestMergoSemantics(t *testing.T) { type T struct { X string } dst := T{X: "one"} src := T{X: "two"} - mergo.Merge(&dst, &src) + mergo.MergeWithOverwrite(&dst, &src) if dst.X != "two" { - // mergo.Merge changed in an incompatible way with + // The mergo library has previously changed in a an incompatible way. + // example: // // https://github.com/imdario/mergo/commit/d304790b2ed594794496464fadd89d2bb266600a // - // We have to stay with the old version which still does eager - // copying from src to dst in structs. - t.Errorf("mergo.Merge library found with incompatible, new behavior") + // This test verifies that the semantics of the merge are what we expect. + // If they are not, the mergo library may have been updated and broken + // unexpectedly. + t.Errorf("mergo.MergeWithOverwrite did not provide expected output") } } diff --git a/tools/clientcmd/loader.go b/tools/clientcmd/loader.go index 3442475e..6038c8d4 100644 --- a/tools/clientcmd/loader.go +++ b/tools/clientcmd/loader.go @@ -211,7 +211,7 @@ func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) { mapConfig := clientcmdapi.NewConfig() for _, kubeconfig := range kubeconfigs { - mergo.Merge(mapConfig, kubeconfig) + mergo.MergeWithOverwrite(mapConfig, kubeconfig) } // merge all of the struct values in the reverse order so that priority is given correctly @@ -219,14 +219,14 @@ func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) { nonMapConfig := clientcmdapi.NewConfig() for i := len(kubeconfigs) - 1; i >= 0; i-- { kubeconfig := kubeconfigs[i] - mergo.Merge(nonMapConfig, kubeconfig) + mergo.MergeWithOverwrite(nonMapConfig, kubeconfig) } // since values are overwritten, but maps values are not, we can merge the non-map config on top of the map config and // get the values we expect. config := clientcmdapi.NewConfig() - mergo.Merge(config, mapConfig) - mergo.Merge(config, nonMapConfig) + mergo.MergeWithOverwrite(config, mapConfig) + mergo.MergeWithOverwrite(config, nonMapConfig) if rules.ResolvePaths() { if err := ResolveLocalPaths(config); err != nil {