Move from mergo.Merge to mergo.MergeWithOverwrite

Kubernetes-commit: a1f6d24962f2b9e6002bcc721e1b48d1008d6cbf
This commit is contained in:
Christoph Blecker 2018-07-04 12:52:01 -07:00 committed by Kubernetes Publisher
parent e88f376842
commit 44530d33a7
3 changed files with 24 additions and 26 deletions

View File

@ -175,10 +175,6 @@ func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) {
// only try to read the auth information if we are secure // only try to read the auth information if we are secure
if restclient.IsConfigTransportTLS(*clientConfig) { if restclient.IsConfigTransportTLS(*clientConfig) {
var err error 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 var persister restclient.AuthProviderConfigPersister
if config.configAccess != nil { if config.configAccess != nil {
authInfoName, _ := config.getAuthInfoName() authInfoName, _ := config.getAuthInfoName()
@ -188,13 +184,13 @@ func (config *DirectClientConfig) ClientConfig() (*restclient.Config, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
mergo.Merge(clientConfig, userAuthPartialConfig) mergo.MergeWithOverwrite(clientConfig, userAuthPartialConfig)
serverAuthPartialConfig, err := getServerIdentificationPartialConfig(configAuthInfo, configClusterInfo) serverAuthPartialConfig, err := getServerIdentificationPartialConfig(configAuthInfo, configClusterInfo)
if err != nil { if err != nil {
return nil, err return nil, err
} }
mergo.Merge(clientConfig, serverAuthPartialConfig) mergo.MergeWithOverwrite(clientConfig, serverAuthPartialConfig)
} }
return clientConfig, nil return clientConfig, nil
@ -214,7 +210,7 @@ func getServerIdentificationPartialConfig(configAuthInfo clientcmdapi.AuthInfo,
configClientConfig.CAFile = configClusterInfo.CertificateAuthority configClientConfig.CAFile = configClusterInfo.CertificateAuthority
configClientConfig.CAData = configClusterInfo.CertificateAuthorityData configClientConfig.CAData = configClusterInfo.CertificateAuthorityData
configClientConfig.Insecure = configClusterInfo.InsecureSkipTLSVerify configClientConfig.Insecure = configClusterInfo.InsecureSkipTLSVerify
mergo.Merge(mergedConfig, configClientConfig) mergo.MergeWithOverwrite(mergedConfig, configClientConfig)
return mergedConfig, nil return mergedConfig, nil
} }
@ -279,8 +275,8 @@ func (config *DirectClientConfig) getUserIdentificationPartialConfig(configAuthI
promptedConfig := makeUserIdentificationConfig(*promptedAuthInfo) promptedConfig := makeUserIdentificationConfig(*promptedAuthInfo)
previouslyMergedConfig := mergedConfig previouslyMergedConfig := mergedConfig
mergedConfig = &restclient.Config{} mergedConfig = &restclient.Config{}
mergo.Merge(mergedConfig, promptedConfig) mergo.MergeWithOverwrite(mergedConfig, promptedConfig)
mergo.Merge(mergedConfig, previouslyMergedConfig) mergo.MergeWithOverwrite(mergedConfig, previouslyMergedConfig)
config.promptedCredentials.username = mergedConfig.Username config.promptedCredentials.username = mergedConfig.Username
config.promptedCredentials.password = mergedConfig.Password config.promptedCredentials.password = mergedConfig.Password
} }
@ -423,11 +419,11 @@ func (config *DirectClientConfig) getContext() (clientcmdapi.Context, error) {
mergedContext := clientcmdapi.NewContext() mergedContext := clientcmdapi.NewContext()
if configContext, exists := contexts[contextName]; exists { if configContext, exists := contexts[contextName]; exists {
mergo.Merge(mergedContext, configContext) mergo.MergeWithOverwrite(mergedContext, configContext)
} else if required { } else if required {
return clientcmdapi.Context{}, fmt.Errorf("context %q does not exist", contextName) 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 return *mergedContext, nil
} }
@ -439,11 +435,11 @@ func (config *DirectClientConfig) getAuthInfo() (clientcmdapi.AuthInfo, error) {
mergedAuthInfo := clientcmdapi.NewAuthInfo() mergedAuthInfo := clientcmdapi.NewAuthInfo()
if configAuthInfo, exists := authInfos[authInfoName]; exists { if configAuthInfo, exists := authInfos[authInfoName]; exists {
mergo.Merge(mergedAuthInfo, configAuthInfo) mergo.MergeWithOverwrite(mergedAuthInfo, configAuthInfo)
} else if required { } else if required {
return clientcmdapi.AuthInfo{}, fmt.Errorf("auth info %q does not exist", authInfoName) 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 return *mergedAuthInfo, nil
} }
@ -454,13 +450,13 @@ func (config *DirectClientConfig) getCluster() (clientcmdapi.Cluster, error) {
clusterInfoName, required := config.getClusterName() clusterInfoName, required := config.getClusterName()
mergedClusterInfo := clientcmdapi.NewCluster() mergedClusterInfo := clientcmdapi.NewCluster()
mergo.Merge(mergedClusterInfo, config.overrides.ClusterDefaults) mergo.MergeWithOverwrite(mergedClusterInfo, config.overrides.ClusterDefaults)
if configClusterInfo, exists := clusterInfos[clusterInfoName]; exists { if configClusterInfo, exists := clusterInfos[clusterInfoName]; exists {
mergo.Merge(mergedClusterInfo, configClusterInfo) mergo.MergeWithOverwrite(mergedClusterInfo, configClusterInfo)
} else if required { } else if required {
return clientcmdapi.Cluster{}, fmt.Errorf("cluster %q does not exist", clusterInfoName) 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 // 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" // 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) caLen := len(config.overrides.ClusterInfo.CertificateAuthority)

View File

@ -28,21 +28,23 @@ import (
clientcmdapi "k8s.io/client-go/tools/clientcmd/api" clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
) )
func TestOldMergoLib(t *testing.T) { func TestMergoSemantics(t *testing.T) {
type T struct { type T struct {
X string X string
} }
dst := T{X: "one"} dst := T{X: "one"}
src := T{X: "two"} src := T{X: "two"}
mergo.Merge(&dst, &src) mergo.MergeWithOverwrite(&dst, &src)
if dst.X != "two" { 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 // https://github.com/imdario/mergo/commit/d304790b2ed594794496464fadd89d2bb266600a
// //
// We have to stay with the old version which still does eager // This test verifies that the semantics of the merge are what we expect.
// copying from src to dst in structs. // If they are not, the mergo library may have been updated and broken
t.Errorf("mergo.Merge library found with incompatible, new behavior") // unexpectedly.
t.Errorf("mergo.MergeWithOverwrite did not provide expected output")
} }
} }

View File

@ -211,7 +211,7 @@ func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) {
mapConfig := clientcmdapi.NewConfig() mapConfig := clientcmdapi.NewConfig()
for _, kubeconfig := range kubeconfigs { 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 // 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() nonMapConfig := clientcmdapi.NewConfig()
for i := len(kubeconfigs) - 1; i >= 0; i-- { for i := len(kubeconfigs) - 1; i >= 0; i-- {
kubeconfig := kubeconfigs[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 // 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. // get the values we expect.
config := clientcmdapi.NewConfig() config := clientcmdapi.NewConfig()
mergo.Merge(config, mapConfig) mergo.MergeWithOverwrite(config, mapConfig)
mergo.Merge(config, nonMapConfig) mergo.MergeWithOverwrite(config, nonMapConfig)
if rules.ResolvePaths() { if rules.ResolvePaths() {
if err := ResolveLocalPaths(config); err != nil { if err := ResolveLocalPaths(config); err != nil {