diff --git a/pkg/client/clientcmd/api/helpers.go b/pkg/client/clientcmd/api/helpers.go index 5ea762a3c57..e2d56570c6e 100644 --- a/pkg/client/clientcmd/api/helpers.go +++ b/pkg/client/clientcmd/api/helpers.go @@ -43,10 +43,10 @@ func MinifyConfig(config *Config) error { return fmt.Errorf("cannot locate context %v", config.CurrentContext) } - newContexts := map[string]Context{} + newContexts := map[string]*Context{} newContexts[config.CurrentContext] = currContext - newClusters := map[string]Cluster{} + newClusters := map[string]*Cluster{} if len(currContext.Cluster) > 0 { if _, exists := config.Clusters[currContext.Cluster]; !exists { return fmt.Errorf("cannot locate cluster %v", currContext.Cluster) @@ -55,7 +55,7 @@ func MinifyConfig(config *Config) error { newClusters[currContext.Cluster] = config.Clusters[currContext.Cluster] } - newAuthInfos := map[string]AuthInfo{} + newAuthInfos := map[string]*AuthInfo{} if len(currContext.AuthInfo) > 0 { if _, exists := config.AuthInfos[currContext.AuthInfo]; !exists { return fmt.Errorf("cannot locate user %v", currContext.AuthInfo) diff --git a/pkg/client/clientcmd/api/helpers_test.go b/pkg/client/clientcmd/api/helpers_test.go index aa62ab1ec40..ca970f3b4ca 100644 --- a/pkg/client/clientcmd/api/helpers_test.go +++ b/pkg/client/clientcmd/api/helpers_test.go @@ -38,13 +38,13 @@ func newMergedConfig(certFile, certContent, keyFile, keyContent, caFile, caConte } return Config{ - AuthInfos: map[string]AuthInfo{ + AuthInfos: map[string]*AuthInfo{ "red-user": {Token: "red-token", ClientCertificateData: []byte(certContent), ClientKeyData: []byte(keyContent)}, "blue-user": {Token: "blue-token", ClientCertificate: certFile, ClientKey: keyFile}}, - Clusters: map[string]Cluster{ + Clusters: map[string]*Cluster{ "cow-cluster": {Server: "http://cow.org:8080", CertificateAuthorityData: []byte(caContent)}, "chicken-cluster": {Server: "http://chicken.org:8080", CertificateAuthority: caFile}}, - Contexts: map[string]Context{ + Contexts: map[string]*Context{ "federal-context": {AuthInfo: "red-user", Cluster: "cow-cluster"}, "shaker-context": {AuthInfo: "blue-user", Cluster: "chicken-cluster"}}, CurrentContext: "federal-context", diff --git a/pkg/client/clientcmd/api/types.go b/pkg/client/clientcmd/api/types.go index fb18e0f1169..434bfa2b23b 100644 --- a/pkg/client/clientcmd/api/types.go +++ b/pkg/client/clientcmd/api/types.go @@ -33,21 +33,21 @@ type Config struct { // Preferences holds general information to be use for cli interactions Preferences Preferences `json:"preferences"` // Clusters is a map of referencable names to cluster configs - Clusters map[string]Cluster `json:"clusters"` + Clusters map[string]*Cluster `json:"clusters"` // AuthInfos is a map of referencable names to user configs - AuthInfos map[string]AuthInfo `json:"users"` + AuthInfos map[string]*AuthInfo `json:"users"` // Contexts is a map of referencable names to context configs - Contexts map[string]Context `json:"contexts"` + Contexts map[string]*Context `json:"contexts"` // CurrentContext is the name of the context that you would like to use by default CurrentContext string `json:"current-context"` // Extensions holds additional information. This is useful for extenders so that reads and writes don't clobber unknown fields - Extensions map[string]runtime.EmbeddedObject `json:"extensions,omitempty"` + Extensions map[string]*runtime.EmbeddedObject `json:"extensions,omitempty"` } type Preferences struct { Colors bool `json:"colors,omitempty"` // Extensions holds additional information. This is useful for extenders so that reads and writes don't clobber unknown fields - Extensions map[string]runtime.EmbeddedObject `json:"extensions,omitempty"` + Extensions map[string]*runtime.EmbeddedObject `json:"extensions,omitempty"` } // Cluster contains information about how to communicate with a kubernetes cluster @@ -65,7 +65,7 @@ type Cluster struct { // CertificateAuthorityData contains PEM-encoded certificate authority certificates. Overrides CertificateAuthority CertificateAuthorityData []byte `json:"certificate-authority-data,omitempty"` // Extensions holds additional information. This is useful for extenders so that reads and writes don't clobber unknown fields - Extensions map[string]runtime.EmbeddedObject `json:"extensions,omitempty"` + Extensions map[string]*runtime.EmbeddedObject `json:"extensions,omitempty"` } // AuthInfo contains information that describes identity information. This is use to tell the kubernetes cluster who you are. @@ -87,7 +87,7 @@ type AuthInfo struct { // Password is the password for basic authentication to the kubernetes cluster. Password string `json:"password,omitempty"` // Extensions holds additional information. This is useful for extenders so that reads and writes don't clobber unknown fields - Extensions map[string]runtime.EmbeddedObject `json:"extensions,omitempty"` + Extensions map[string]*runtime.EmbeddedObject `json:"extensions,omitempty"` } // Context is a tuple of references to a cluster (how do I communicate with a kubernetes cluster), a user (how do I identify myself), and a namespace (what subset of resources do I want to work with) @@ -101,36 +101,36 @@ type Context struct { // Namespace is the default namespace to use on unspecified requests Namespace string `json:"namespace,omitempty"` // Extensions holds additional information. This is useful for extenders so that reads and writes don't clobber unknown fields - Extensions map[string]runtime.EmbeddedObject `json:"extensions,omitempty"` + Extensions map[string]*runtime.EmbeddedObject `json:"extensions,omitempty"` } // NewConfig is a convenience function that returns a new Config object with non-nil maps func NewConfig() *Config { return &Config{ Preferences: *NewPreferences(), - Clusters: make(map[string]Cluster), - AuthInfos: make(map[string]AuthInfo), - Contexts: make(map[string]Context), - Extensions: make(map[string]runtime.EmbeddedObject), + Clusters: make(map[string]*Cluster), + AuthInfos: make(map[string]*AuthInfo), + Contexts: make(map[string]*Context), + Extensions: make(map[string]*runtime.EmbeddedObject), } } // NewConfig is a convenience function that returns a new Config object with non-nil maps func NewContext() *Context { - return &Context{Extensions: make(map[string]runtime.EmbeddedObject)} + return &Context{Extensions: make(map[string]*runtime.EmbeddedObject)} } // NewConfig is a convenience function that returns a new Config object with non-nil maps func NewCluster() *Cluster { - return &Cluster{Extensions: make(map[string]runtime.EmbeddedObject)} + return &Cluster{Extensions: make(map[string]*runtime.EmbeddedObject)} } // NewConfig is a convenience function that returns a new Config object with non-nil maps func NewAuthInfo() *AuthInfo { - return &AuthInfo{Extensions: make(map[string]runtime.EmbeddedObject)} + return &AuthInfo{Extensions: make(map[string]*runtime.EmbeddedObject)} } // NewConfig is a convenience function that returns a new Config object with non-nil maps func NewPreferences() *Preferences { - return &Preferences{Extensions: make(map[string]runtime.EmbeddedObject)} + return &Preferences{Extensions: make(map[string]*runtime.EmbeddedObject)} } diff --git a/pkg/client/clientcmd/api/types_test.go b/pkg/client/clientcmd/api/types_test.go index 3caf18fae62..af351bf2f42 100644 --- a/pkg/client/clientcmd/api/types_test.go +++ b/pkg/client/clientcmd/api/types_test.go @@ -42,35 +42,35 @@ func ExampleEmptyConfig() { func ExampleOfOptionsConfig() { defaultConfig := NewConfig() defaultConfig.Preferences.Colors = true - defaultConfig.Clusters["alfa"] = Cluster{ + defaultConfig.Clusters["alfa"] = &Cluster{ Server: "https://alfa.org:8080", APIVersion: "v1beta2", InsecureSkipTLSVerify: true, CertificateAuthority: "path/to/my/cert-ca-filename", } - defaultConfig.Clusters["bravo"] = Cluster{ + defaultConfig.Clusters["bravo"] = &Cluster{ Server: "https://bravo.org:8080", APIVersion: "v1beta1", InsecureSkipTLSVerify: false, } - defaultConfig.AuthInfos["white-mage-via-cert"] = AuthInfo{ + defaultConfig.AuthInfos["white-mage-via-cert"] = &AuthInfo{ ClientCertificate: "path/to/my/client-cert-filename", ClientKey: "path/to/my/client-key-filename", } - defaultConfig.AuthInfos["red-mage-via-token"] = AuthInfo{ + defaultConfig.AuthInfos["red-mage-via-token"] = &AuthInfo{ Token: "my-secret-token", } - defaultConfig.Contexts["bravo-as-black-mage"] = Context{ + defaultConfig.Contexts["bravo-as-black-mage"] = &Context{ Cluster: "bravo", AuthInfo: "black-mage-via-file", Namespace: "yankee", } - defaultConfig.Contexts["alfa-as-black-mage"] = Context{ + defaultConfig.Contexts["alfa-as-black-mage"] = &Context{ Cluster: "alfa", AuthInfo: "black-mage-via-file", Namespace: "zulu", } - defaultConfig.Contexts["alfa-as-white-mage"] = Context{ + defaultConfig.Contexts["alfa-as-white-mage"] = &Context{ Cluster: "alfa", AuthInfo: "white-mage-via-cert", } diff --git a/pkg/client/clientcmd/api/v1/conversion.go b/pkg/client/clientcmd/api/v1/conversion.go index b88793c676f..5d7746ed5a3 100644 --- a/pkg/client/clientcmd/api/v1/conversion.go +++ b/pkg/client/clientcmd/api/v1/conversion.go @@ -57,19 +57,19 @@ func init() { return err } - out.Clusters = make(map[string]api.Cluster) + out.Clusters = make(map[string]*api.Cluster) if err := s.Convert(&in.Clusters, &out.Clusters, 0); err != nil { return err } - out.AuthInfos = make(map[string]api.AuthInfo) + out.AuthInfos = make(map[string]*api.AuthInfo) if err := s.Convert(&in.AuthInfos, &out.AuthInfos, 0); err != nil { return err } - out.Contexts = make(map[string]api.Context) + out.Contexts = make(map[string]*api.Context) if err := s.Convert(&in.Contexts, &out.Contexts, 0); err != nil { return err } - out.Extensions = make(map[string]runtime.EmbeddedObject) + out.Extensions = make(map[string]*runtime.EmbeddedObject) if err := s.Convert(&in.Extensions, &out.Extensions, 0); err != nil { return err } @@ -99,18 +99,18 @@ func init() { } return nil }, - func(in *[]NamedCluster, out *map[string]api.Cluster, s conversion.Scope) error { + func(in *[]NamedCluster, out *map[string]*api.Cluster, s conversion.Scope) error { for _, curr := range *in { newCluster := api.NewCluster() if err := s.Convert(&curr.Cluster, newCluster, 0); err != nil { return err } - (*out)[curr.Name] = *newCluster + (*out)[curr.Name] = newCluster } return nil }, - func(in *map[string]api.Cluster, out *[]NamedCluster, s conversion.Scope) error { + func(in *map[string]*api.Cluster, out *[]NamedCluster, s conversion.Scope) error { allKeys := make([]string, 0, len(*in)) for key := range *in { allKeys = append(allKeys, key) @@ -120,7 +120,7 @@ func init() { for _, key := range allKeys { newCluster := (*in)[key] oldCluster := &Cluster{} - if err := s.Convert(&newCluster, oldCluster, 0); err != nil { + if err := s.Convert(newCluster, oldCluster, 0); err != nil { return err } @@ -130,18 +130,18 @@ func init() { return nil }, - func(in *[]NamedAuthInfo, out *map[string]api.AuthInfo, s conversion.Scope) error { + func(in *[]NamedAuthInfo, out *map[string]*api.AuthInfo, s conversion.Scope) error { for _, curr := range *in { newAuthInfo := api.NewAuthInfo() if err := s.Convert(&curr.AuthInfo, newAuthInfo, 0); err != nil { return err } - (*out)[curr.Name] = *newAuthInfo + (*out)[curr.Name] = newAuthInfo } return nil }, - func(in *map[string]api.AuthInfo, out *[]NamedAuthInfo, s conversion.Scope) error { + func(in *map[string]*api.AuthInfo, out *[]NamedAuthInfo, s conversion.Scope) error { allKeys := make([]string, 0, len(*in)) for key := range *in { allKeys = append(allKeys, key) @@ -151,7 +151,7 @@ func init() { for _, key := range allKeys { newAuthInfo := (*in)[key] oldAuthInfo := &AuthInfo{} - if err := s.Convert(&newAuthInfo, oldAuthInfo, 0); err != nil { + if err := s.Convert(newAuthInfo, oldAuthInfo, 0); err != nil { return err } @@ -161,18 +161,18 @@ func init() { return nil }, - func(in *[]NamedContext, out *map[string]api.Context, s conversion.Scope) error { + func(in *[]NamedContext, out *map[string]*api.Context, s conversion.Scope) error { for _, curr := range *in { newContext := api.NewContext() if err := s.Convert(&curr.Context, newContext, 0); err != nil { return err } - (*out)[curr.Name] = *newContext + (*out)[curr.Name] = newContext } return nil }, - func(in *map[string]api.Context, out *[]NamedContext, s conversion.Scope) error { + func(in *map[string]*api.Context, out *[]NamedContext, s conversion.Scope) error { allKeys := make([]string, 0, len(*in)) for key := range *in { allKeys = append(allKeys, key) @@ -182,7 +182,7 @@ func init() { for _, key := range allKeys { newContext := (*in)[key] oldContext := &Context{} - if err := s.Convert(&newContext, oldContext, 0); err != nil { + if err := s.Convert(newContext, oldContext, 0); err != nil { return err } @@ -192,18 +192,18 @@ func init() { return nil }, - func(in *[]NamedExtension, out *map[string]runtime.EmbeddedObject, s conversion.Scope) error { + func(in *[]NamedExtension, out *map[string]*runtime.EmbeddedObject, s conversion.Scope) error { for _, curr := range *in { newExtension := &runtime.EmbeddedObject{} if err := s.Convert(&curr.Extension, newExtension, 0); err != nil { return err } - (*out)[curr.Name] = *newExtension + (*out)[curr.Name] = newExtension } return nil }, - func(in *map[string]runtime.EmbeddedObject, out *[]NamedExtension, s conversion.Scope) error { + func(in *map[string]*runtime.EmbeddedObject, out *[]NamedExtension, s conversion.Scope) error { allKeys := make([]string, 0, len(*in)) for key := range *in { allKeys = append(allKeys, key) @@ -213,7 +213,7 @@ func init() { for _, key := range allKeys { newExtension := (*in)[key] oldExtension := &runtime.RawExtension{} - if err := s.Convert(&newExtension, oldExtension, 0); err != nil { + if err := s.Convert(newExtension, oldExtension, 0); err != nil { return err } diff --git a/pkg/client/clientcmd/client_config_test.go b/pkg/client/clientcmd/client_config_test.go index e55bfa3b111..ed6dbdd85f3 100644 --- a/pkg/client/clientcmd/client_config_test.go +++ b/pkg/client/clientcmd/client_config_test.go @@ -32,14 +32,14 @@ func createValidTestConfig() *clientcmdapi.Config { ) config := clientcmdapi.NewConfig() - config.Clusters["clean"] = clientcmdapi.Cluster{ + config.Clusters["clean"] = &clientcmdapi.Cluster{ Server: server, APIVersion: latest.Version, } - config.AuthInfos["clean"] = clientcmdapi.AuthInfo{ + config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ Token: token, } - config.Contexts["clean"] = clientcmdapi.Context{ + config.Contexts["clean"] = &clientcmdapi.Context{ Cluster: "clean", AuthInfo: "clean", } @@ -87,16 +87,16 @@ func TestCertificateData(t *testing.T) { keyData := []byte("key-data") config := clientcmdapi.NewConfig() - config.Clusters["clean"] = clientcmdapi.Cluster{ + config.Clusters["clean"] = &clientcmdapi.Cluster{ Server: "https://localhost:8443", APIVersion: latest.Version, CertificateAuthorityData: caData, } - config.AuthInfos["clean"] = clientcmdapi.AuthInfo{ + config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ ClientCertificateData: certData, ClientKeyData: keyData, } - config.Contexts["clean"] = clientcmdapi.Context{ + config.Contexts["clean"] = &clientcmdapi.Context{ Cluster: "clean", AuthInfo: "clean", } @@ -120,15 +120,15 @@ func TestBasicAuthData(t *testing.T) { password := "mypass" config := clientcmdapi.NewConfig() - config.Clusters["clean"] = clientcmdapi.Cluster{ + config.Clusters["clean"] = &clientcmdapi.Cluster{ Server: "https://localhost:8443", APIVersion: latest.Version, } - config.AuthInfos["clean"] = clientcmdapi.AuthInfo{ + config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ Username: username, Password: password, } - config.Contexts["clean"] = clientcmdapi.Context{ + config.Contexts["clean"] = &clientcmdapi.Context{ Cluster: "clean", AuthInfo: "clean", } diff --git a/pkg/client/clientcmd/loader.go b/pkg/client/clientcmd/loader.go index 7410e5b82cd..596433f6d30 100644 --- a/pkg/client/clientcmd/loader.go +++ b/pkg/client/clientcmd/loader.go @@ -23,6 +23,7 @@ import ( "os" "path" "path/filepath" + "strings" "github.com/ghodss/yaml" "github.com/imdario/mergo" @@ -120,11 +121,6 @@ func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) { if err := mergeConfigWithFile(mapConfig, file); err != nil { errlist = append(errlist, err) } - if rules.ResolvePaths() { - if err := ResolveLocalPaths(file, mapConfig); err != nil { - errlist = append(errlist, err) - } - } } // merge all of the struct values in the reverse order so that priority is given correctly @@ -133,9 +129,6 @@ func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) { for i := len(kubeConfigFiles) - 1; i >= 0; i-- { file := kubeConfigFiles[i] mergeConfigWithFile(nonMapConfig, file) - if rules.ResolvePaths() { - ResolveLocalPaths(file, nonMapConfig) - } } // since values are overwritten, but maps values are not, we can merge the non-map config on top of the map config and @@ -144,6 +137,12 @@ func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) { mergo.Merge(config, mapConfig) mergo.Merge(config, nonMapConfig) + if rules.ResolvePaths() { + if err := ResolveLocalPaths(config); err != nil { + errlist = append(errlist, err) + } + } + return config, errors.NewAggregate(errlist) } @@ -213,49 +212,6 @@ func mergeConfigWithFile(startingConfig *clientcmdapi.Config, filename string) e return nil } -// ResolveLocalPaths resolves all relative paths in the config object with respect to the parent directory of the filename -// this cannot be done directly inside of LoadFromFile because doing so there would make it impossible to load a file without -// modification of its contents. -func ResolveLocalPaths(filename string, config *clientcmdapi.Config) error { - if len(filename) == 0 { - return nil - } - - configDir, err := filepath.Abs(filepath.Dir(filename)) - if err != nil { - return fmt.Errorf("Could not determine the absolute path of config file %s: %v", filename, err) - } - - resolvedClusters := make(map[string]clientcmdapi.Cluster) - for key, cluster := range config.Clusters { - cluster.CertificateAuthority = resolveLocalPath(configDir, cluster.CertificateAuthority) - resolvedClusters[key] = cluster - } - config.Clusters = resolvedClusters - - resolvedAuthInfos := make(map[string]clientcmdapi.AuthInfo) - for key, authInfo := range config.AuthInfos { - authInfo.ClientCertificate = resolveLocalPath(configDir, authInfo.ClientCertificate) - authInfo.ClientKey = resolveLocalPath(configDir, authInfo.ClientKey) - resolvedAuthInfos[key] = authInfo - } - config.AuthInfos = resolvedAuthInfos - - return nil -} - -// resolveLocalPath makes the path absolute with respect to the startingDir -func resolveLocalPath(startingDir, path string) string { - if len(path) == 0 { - return path - } - if filepath.IsAbs(path) { - return path - } - - return filepath.Join(startingDir, path) -} - // LoadFromFile takes a filename and deserializes the contents into Config object func LoadFromFile(filename string) (*clientcmdapi.Config, error) { kubeconfigBytes, err := ioutil.ReadFile(filename) @@ -335,3 +291,159 @@ func Write(config clientcmdapi.Config) ([]byte, error) { func (rules ClientConfigLoadingRules) ResolvePaths() bool { return !rules.DoNotResolvePaths } + +// ResolveLocalPaths resolves all relative paths in the config object with respect to the stanza's LocationOfOrigin +// this cannot be done directly inside of LoadFromFile because doing so there would make it impossible to load a file without +// modification of its contents. +func ResolveLocalPaths(config *clientcmdapi.Config) error { + for _, cluster := range config.Clusters { + if len(cluster.LocationOfOrigin) == 0 { + continue + } + base, err := filepath.Abs(filepath.Dir(cluster.LocationOfOrigin)) + if err != nil { + return fmt.Errorf("Could not determine the absolute path of config file %s: %v", cluster.LocationOfOrigin, err) + } + + if err := ResolvePaths(GetClusterFileReferences(cluster), base); err != nil { + return err + } + } + for _, authInfo := range config.AuthInfos { + if len(authInfo.LocationOfOrigin) == 0 { + continue + } + base, err := filepath.Abs(filepath.Dir(authInfo.LocationOfOrigin)) + if err != nil { + return fmt.Errorf("Could not determine the absolute path of config file %s: %v", authInfo.LocationOfOrigin, err) + } + + if err := ResolvePaths(GetAuthInfoFileReferences(authInfo), base); err != nil { + return err + } + } + + return nil +} + +// RelativizeClusterLocalPaths first absolutizes the paths by calling ResolveLocalPaths. This assumes that any NEW path is already +// absolute, but any existing path will be resolved relative to LocationOfOrigin +func RelativizeClusterLocalPaths(cluster *clientcmdapi.Cluster) error { + if len(cluster.LocationOfOrigin) == 0 { + return fmt.Errorf("no location of origin for %s", cluster.Server) + } + base, err := filepath.Abs(filepath.Dir(cluster.LocationOfOrigin)) + if err != nil { + return fmt.Errorf("could not determine the absolute path of config file %s: %v", cluster.LocationOfOrigin, err) + } + + if err := ResolvePaths(GetClusterFileReferences(cluster), base); err != nil { + return err + } + if err := RelativizePathWithNoBacksteps(GetClusterFileReferences(cluster), base); err != nil { + return err + } + + return nil +} + +// RelativizeAuthInfoLocalPaths first absolutizes the paths by calling ResolveLocalPaths. This assumes that any NEW path is already +// absolute, but any existing path will be resolved relative to LocationOfOrigin +func RelativizeAuthInfoLocalPaths(authInfo *clientcmdapi.AuthInfo) error { + if len(authInfo.LocationOfOrigin) == 0 { + return fmt.Errorf("no location of origin for %v", authInfo) + } + base, err := filepath.Abs(filepath.Dir(authInfo.LocationOfOrigin)) + if err != nil { + return fmt.Errorf("could not determine the absolute path of config file %s: %v", authInfo.LocationOfOrigin, err) + } + + if err := ResolvePaths(GetAuthInfoFileReferences(authInfo), base); err != nil { + return err + } + if err := RelativizePathWithNoBacksteps(GetAuthInfoFileReferences(authInfo), base); err != nil { + return err + } + + return nil +} + +func RelativizeConfigPaths(config *clientcmdapi.Config, base string) error { + return RelativizePathWithNoBacksteps(GetConfigFileReferences(config), base) +} + +func ResolveConfigPaths(config *clientcmdapi.Config, base string) error { + return ResolvePaths(GetConfigFileReferences(config), base) +} + +func GetConfigFileReferences(config *clientcmdapi.Config) []*string { + refs := []*string{} + + for _, cluster := range config.Clusters { + refs = append(refs, GetClusterFileReferences(cluster)...) + } + for _, authInfo := range config.AuthInfos { + refs = append(refs, GetAuthInfoFileReferences(authInfo)...) + } + + return refs +} + +func GetClusterFileReferences(cluster *clientcmdapi.Cluster) []*string { + return []*string{&cluster.CertificateAuthority} +} + +func GetAuthInfoFileReferences(authInfo *clientcmdapi.AuthInfo) []*string { + return []*string{&authInfo.ClientCertificate, &authInfo.ClientKey} +} + +// ResolvePaths updates the given refs to be absolute paths, relative to the given base directory +func ResolvePaths(refs []*string, base string) error { + for _, ref := range refs { + // Don't resolve empty paths + if len(*ref) > 0 { + // Don't resolve absolute paths + if !filepath.IsAbs(*ref) { + *ref = filepath.Join(base, *ref) + } + } + } + return nil +} + +// RelativizePathWithNoBacksteps updates the given refs to be relative paths, relative to the given base directory as long as they do not require backsteps. +// Any path requiring a backstep is left as-is as long it is absolute. Any non-absolute path that can't be relativized produces an error +func RelativizePathWithNoBacksteps(refs []*string, base string) error { + for _, ref := range refs { + // Don't relativize empty paths + if len(*ref) > 0 { + rel, err := MakeRelative(*ref, base) + if err != nil { + return err + } + + // if we have a backstep, don't mess with the path + if strings.HasPrefix(rel, "../") { + if filepath.IsAbs(*ref) { + continue + } + + return fmt.Errorf("%v requires backsteps and is not absolute", *ref) + } + + *ref = rel + } + } + return nil +} + +func MakeRelative(path, base string) (string, error) { + if len(path) > 0 { + rel, err := filepath.Rel(base, path) + if err != nil { + return path, err + } + return rel, nil + } + return path, nil +} diff --git a/pkg/client/clientcmd/loader_test.go b/pkg/client/clientcmd/loader_test.go index 27c897d8a77..b011cc8d4a8 100644 --- a/pkg/client/clientcmd/loader_test.go +++ b/pkg/client/clientcmd/loader_test.go @@ -34,43 +34,43 @@ import ( var ( testConfigAlfa = clientcmdapi.Config{ - AuthInfos: map[string]clientcmdapi.AuthInfo{ + AuthInfos: map[string]*clientcmdapi.AuthInfo{ "red-user": {Token: "red-token"}}, - Clusters: map[string]clientcmdapi.Cluster{ + Clusters: map[string]*clientcmdapi.Cluster{ "cow-cluster": {Server: "http://cow.org:8080"}}, - Contexts: map[string]clientcmdapi.Context{ + Contexts: map[string]*clientcmdapi.Context{ "federal-context": {AuthInfo: "red-user", Cluster: "cow-cluster", Namespace: "hammer-ns"}}, } testConfigBravo = clientcmdapi.Config{ - AuthInfos: map[string]clientcmdapi.AuthInfo{ + AuthInfos: map[string]*clientcmdapi.AuthInfo{ "black-user": {Token: "black-token"}}, - Clusters: map[string]clientcmdapi.Cluster{ + Clusters: map[string]*clientcmdapi.Cluster{ "pig-cluster": {Server: "http://pig.org:8080"}}, - Contexts: map[string]clientcmdapi.Context{ + Contexts: map[string]*clientcmdapi.Context{ "queen-anne-context": {AuthInfo: "black-user", Cluster: "pig-cluster", Namespace: "saw-ns"}}, } testConfigCharlie = clientcmdapi.Config{ - AuthInfos: map[string]clientcmdapi.AuthInfo{ + AuthInfos: map[string]*clientcmdapi.AuthInfo{ "green-user": {Token: "green-token"}}, - Clusters: map[string]clientcmdapi.Cluster{ + Clusters: map[string]*clientcmdapi.Cluster{ "horse-cluster": {Server: "http://horse.org:8080"}}, - Contexts: map[string]clientcmdapi.Context{ + Contexts: map[string]*clientcmdapi.Context{ "shaker-context": {AuthInfo: "green-user", Cluster: "horse-cluster", Namespace: "chisel-ns"}}, } testConfigDelta = clientcmdapi.Config{ - AuthInfos: map[string]clientcmdapi.AuthInfo{ + AuthInfos: map[string]*clientcmdapi.AuthInfo{ "blue-user": {Token: "blue-token"}}, - Clusters: map[string]clientcmdapi.Cluster{ + Clusters: map[string]*clientcmdapi.Cluster{ "chicken-cluster": {Server: "http://chicken.org:8080"}}, - Contexts: map[string]clientcmdapi.Context{ + Contexts: map[string]*clientcmdapi.Context{ "gothic-context": {AuthInfo: "blue-user", Cluster: "chicken-cluster", Namespace: "plane-ns"}}, } testConfigConflictAlfa = clientcmdapi.Config{ - AuthInfos: map[string]clientcmdapi.AuthInfo{ + AuthInfos: map[string]*clientcmdapi.AuthInfo{ "red-user": {Token: "a-different-red-token"}, "yellow-user": {Token: "yellow-token"}}, - Clusters: map[string]clientcmdapi.Cluster{ + Clusters: map[string]*clientcmdapi.Cluster{ "cow-cluster": {Server: "http://a-different-cow.org:8080", InsecureSkipTLSVerify: true}, "donkey-cluster": {Server: "http://donkey.org:8080", InsecureSkipTLSVerify: true}}, CurrentContext: "federal-context", @@ -176,21 +176,21 @@ func TestConflictingCurrentContext(t *testing.T) { func TestResolveRelativePaths(t *testing.T) { pathResolutionConfig1 := clientcmdapi.Config{ - AuthInfos: map[string]clientcmdapi.AuthInfo{ + AuthInfos: map[string]*clientcmdapi.AuthInfo{ "relative-user-1": {ClientCertificate: "relative/client/cert", ClientKey: "../relative/client/key"}, "absolute-user-1": {ClientCertificate: "/absolute/client/cert", ClientKey: "/absolute/client/key"}, }, - Clusters: map[string]clientcmdapi.Cluster{ + Clusters: map[string]*clientcmdapi.Cluster{ "relative-server-1": {CertificateAuthority: "../relative/ca"}, "absolute-server-1": {CertificateAuthority: "/absolute/ca"}, }, } pathResolutionConfig2 := clientcmdapi.Config{ - AuthInfos: map[string]clientcmdapi.AuthInfo{ + AuthInfos: map[string]*clientcmdapi.AuthInfo{ "relative-user-2": {ClientCertificate: "relative/client/cert2", ClientKey: "../relative/client/key2"}, "absolute-user-2": {ClientCertificate: "/absolute/client/cert2", ClientKey: "/absolute/client/key2"}, }, - Clusters: map[string]clientcmdapi.Cluster{ + Clusters: map[string]*clientcmdapi.Cluster{ "relative-server-2": {CertificateAuthority: "../relative/ca2"}, "absolute-server-2": {CertificateAuthority: "/absolute/ca2"}, }, diff --git a/pkg/client/clientcmd/validation.go b/pkg/client/clientcmd/validation.go index 31e927ccbf2..92538759750 100644 --- a/pkg/client/clientcmd/validation.go +++ b/pkg/client/clientcmd/validation.go @@ -95,15 +95,15 @@ func Validate(config clientcmdapi.Config) error { } for contextName, context := range config.Contexts { - validationErrors = append(validationErrors, validateContext(contextName, context, config)...) + validationErrors = append(validationErrors, validateContext(contextName, *context, config)...) } for authInfoName, authInfo := range config.AuthInfos { - validationErrors = append(validationErrors, validateAuthInfo(authInfoName, authInfo)...) + validationErrors = append(validationErrors, validateAuthInfo(authInfoName, *authInfo)...) } for clusterName, clusterInfo := range config.Clusters { - validationErrors = append(validationErrors, validateClusterInfo(clusterName, clusterInfo)...) + validationErrors = append(validationErrors, validateClusterInfo(clusterName, *clusterInfo)...) } return newErrConfigurationInvalid(validationErrors) @@ -131,9 +131,9 @@ func ConfirmUsable(config clientcmdapi.Config, passedContextName string) error { } if exists { - validationErrors = append(validationErrors, validateContext(contextName, context, config)...) - validationErrors = append(validationErrors, validateAuthInfo(context.AuthInfo, config.AuthInfos[context.AuthInfo])...) - validationErrors = append(validationErrors, validateClusterInfo(context.Cluster, config.Clusters[context.Cluster])...) + validationErrors = append(validationErrors, validateContext(contextName, *context, config)...) + validationErrors = append(validationErrors, validateAuthInfo(context.AuthInfo, *config.AuthInfos[context.AuthInfo])...) + validationErrors = append(validationErrors, validateClusterInfo(context.Cluster, *config.Clusters[context.Cluster])...) } return newErrConfigurationInvalid(validationErrors) diff --git a/pkg/client/clientcmd/validation_test.go b/pkg/client/clientcmd/validation_test.go index f93aa03d737..c058dc67aaa 100644 --- a/pkg/client/clientcmd/validation_test.go +++ b/pkg/client/clientcmd/validation_test.go @@ -28,25 +28,25 @@ import ( func TestConfirmUsableBadInfoButOkConfig(t *testing.T) { config := clientcmdapi.NewConfig() - config.Clusters["missing ca"] = clientcmdapi.Cluster{ + config.Clusters["missing ca"] = &clientcmdapi.Cluster{ Server: "anything", CertificateAuthority: "missing", } - config.AuthInfos["error"] = clientcmdapi.AuthInfo{ + config.AuthInfos["error"] = &clientcmdapi.AuthInfo{ Username: "anything", Token: "here", } - config.Contexts["dirty"] = clientcmdapi.Context{ + config.Contexts["dirty"] = &clientcmdapi.Context{ Cluster: "missing ca", AuthInfo: "error", } - config.Clusters["clean"] = clientcmdapi.Cluster{ + config.Clusters["clean"] = &clientcmdapi.Cluster{ Server: "anything", } - config.AuthInfos["clean"] = clientcmdapi.AuthInfo{ + config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ Token: "here", } - config.Contexts["clean"] = clientcmdapi.Context{ + config.Contexts["clean"] = &clientcmdapi.Context{ Cluster: "clean", AuthInfo: "clean", } @@ -64,15 +64,15 @@ func TestConfirmUsableBadInfoButOkConfig(t *testing.T) { } func TestConfirmUsableBadInfoConfig(t *testing.T) { config := clientcmdapi.NewConfig() - config.Clusters["missing ca"] = clientcmdapi.Cluster{ + config.Clusters["missing ca"] = &clientcmdapi.Cluster{ Server: "anything", CertificateAuthority: "missing", } - config.AuthInfos["error"] = clientcmdapi.AuthInfo{ + config.AuthInfos["error"] = &clientcmdapi.AuthInfo{ Username: "anything", Token: "here", } - config.Contexts["first"] = clientcmdapi.Context{ + config.Contexts["first"] = &clientcmdapi.Context{ Cluster: "missing ca", AuthInfo: "error", } @@ -150,7 +150,7 @@ func TestIsConfigurationInvalid(t *testing.T) { func TestValidateMissingReferencesConfig(t *testing.T) { config := clientcmdapi.NewConfig() config.CurrentContext = "anything" - config.Contexts["anything"] = clientcmdapi.Context{Cluster: "missing", AuthInfo: "missing"} + config.Contexts["anything"] = &clientcmdapi.Context{Cluster: "missing", AuthInfo: "missing"} test := configValidationTest{ config: config, expectedErrorSubstring: []string{"user \"missing\" was not found for context \"anything\"", "cluster \"missing\" was not found for context \"anything\""}, @@ -162,7 +162,7 @@ func TestValidateMissingReferencesConfig(t *testing.T) { func TestValidateEmptyContext(t *testing.T) { config := clientcmdapi.NewConfig() config.CurrentContext = "anything" - config.Contexts["anything"] = clientcmdapi.Context{} + config.Contexts["anything"] = &clientcmdapi.Context{} test := configValidationTest{ config: config, expectedErrorSubstring: []string{"user was not specified for context \"anything\"", "cluster was not specified for context \"anything\""}, @@ -174,7 +174,7 @@ func TestValidateEmptyContext(t *testing.T) { func TestValidateEmptyClusterInfo(t *testing.T) { config := clientcmdapi.NewConfig() - config.Clusters["empty"] = clientcmdapi.Cluster{} + config.Clusters["empty"] = &clientcmdapi.Cluster{} test := configValidationTest{ config: config, expectedErrorSubstring: []string{"no server found for"}, @@ -185,7 +185,7 @@ func TestValidateEmptyClusterInfo(t *testing.T) { } func TestValidateMissingCAFileClusterInfo(t *testing.T) { config := clientcmdapi.NewConfig() - config.Clusters["missing ca"] = clientcmdapi.Cluster{ + config.Clusters["missing ca"] = &clientcmdapi.Cluster{ Server: "anything", CertificateAuthority: "missing", } @@ -199,7 +199,7 @@ func TestValidateMissingCAFileClusterInfo(t *testing.T) { } func TestValidateCleanClusterInfo(t *testing.T) { config := clientcmdapi.NewConfig() - config.Clusters["clean"] = clientcmdapi.Cluster{ + config.Clusters["clean"] = &clientcmdapi.Cluster{ Server: "anything", } test := configValidationTest{ @@ -214,7 +214,7 @@ func TestValidateCleanWithCAClusterInfo(t *testing.T) { defer os.Remove(tempFile.Name()) config := clientcmdapi.NewConfig() - config.Clusters["clean"] = clientcmdapi.Cluster{ + config.Clusters["clean"] = &clientcmdapi.Cluster{ Server: "anything", CertificateAuthority: tempFile.Name(), } @@ -228,7 +228,7 @@ func TestValidateCleanWithCAClusterInfo(t *testing.T) { func TestValidateEmptyAuthInfo(t *testing.T) { config := clientcmdapi.NewConfig() - config.AuthInfos["error"] = clientcmdapi.AuthInfo{} + config.AuthInfos["error"] = &clientcmdapi.AuthInfo{} test := configValidationTest{ config: config, } @@ -238,7 +238,7 @@ func TestValidateEmptyAuthInfo(t *testing.T) { } func TestValidateCertFilesNotFoundAuthInfo(t *testing.T) { config := clientcmdapi.NewConfig() - config.AuthInfos["error"] = clientcmdapi.AuthInfo{ + config.AuthInfos["error"] = &clientcmdapi.AuthInfo{ ClientCertificate: "missing", ClientKey: "missing", } @@ -255,7 +255,7 @@ func TestValidateCertDataOverridesFiles(t *testing.T) { defer os.Remove(tempFile.Name()) config := clientcmdapi.NewConfig() - config.AuthInfos["clean"] = clientcmdapi.AuthInfo{ + config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ ClientCertificate: tempFile.Name(), ClientCertificateData: []byte("certdata"), ClientKey: tempFile.Name(), @@ -274,7 +274,7 @@ func TestValidateCleanCertFilesAuthInfo(t *testing.T) { defer os.Remove(tempFile.Name()) config := clientcmdapi.NewConfig() - config.AuthInfos["clean"] = clientcmdapi.AuthInfo{ + config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ ClientCertificate: tempFile.Name(), ClientKey: tempFile.Name(), } @@ -287,7 +287,7 @@ func TestValidateCleanCertFilesAuthInfo(t *testing.T) { } func TestValidateCleanTokenAuthInfo(t *testing.T) { config := clientcmdapi.NewConfig() - config.AuthInfos["clean"] = clientcmdapi.AuthInfo{ + config.AuthInfos["clean"] = &clientcmdapi.AuthInfo{ Token: "any-value", } test := configValidationTest{ @@ -300,7 +300,7 @@ func TestValidateCleanTokenAuthInfo(t *testing.T) { func TestValidateMultipleMethodsAuthInfo(t *testing.T) { config := clientcmdapi.NewConfig() - config.AuthInfos["error"] = clientcmdapi.AuthInfo{ + config.AuthInfos["error"] = &clientcmdapi.AuthInfo{ Token: "token", Username: "username", } @@ -319,7 +319,7 @@ type configValidationTest struct { } func (c configValidationTest) testContext(contextName string, t *testing.T) { - errs := validateContext(contextName, c.config.Contexts[contextName], *c.config) + errs := validateContext(contextName, *c.config.Contexts[contextName], *c.config) if len(c.expectedErrorSubstring) != 0 { if len(errs) == 0 { @@ -379,7 +379,7 @@ func (c configValidationTest) testConfig(t *testing.T) { } } func (c configValidationTest) testCluster(clusterName string, t *testing.T) { - errs := validateClusterInfo(clusterName, c.config.Clusters[clusterName]) + errs := validateClusterInfo(clusterName, *c.config.Clusters[clusterName]) if len(c.expectedErrorSubstring) != 0 { if len(errs) == 0 { @@ -399,7 +399,7 @@ func (c configValidationTest) testCluster(clusterName string, t *testing.T) { } func (c configValidationTest) testAuthInfo(authInfoName string, t *testing.T) { - errs := validateAuthInfo(authInfoName, c.config.AuthInfos[authInfoName]) + errs := validateAuthInfo(authInfoName, *c.config.AuthInfos[authInfoName]) if len(c.expectedErrorSubstring) != 0 { if len(errs) == 0 { diff --git a/pkg/kubectl/cmd/config/config.go b/pkg/kubectl/cmd/config/config.go index fbf9d5d3e30..9876f48dade 100644 --- a/pkg/kubectl/cmd/config/config.go +++ b/pkg/kubectl/cmd/config/config.go @@ -187,8 +187,9 @@ func (o *PathOptions) GetExplicitFile() string { // uses the default destination file to write the results into. This results in multiple file reads, but it's very easy to follow. // Preferences and CurrentContext should always be set in the default destination file. Since we can't distinguish between empty and missing values // (no nil strings), we're forced have separate handling for them. In the kubeconfig cases, newConfig should have at most one difference, -// that means that this code will only write into a single file. -func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config) error { +// that means that this code will only write into a single file. If you want to relativizePaths, you must provide a fully qualified path in any +// modified element. +func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config, relativizePaths bool) error { startingConfig, err := configAccess.GetStartingConfig() if err != nil { return err @@ -223,7 +224,14 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config) erro } configToWrite := getConfigFromFileOrDie(destinationFile) - configToWrite.Clusters[key] = cluster + t := *cluster + configToWrite.Clusters[key] = &t + configToWrite.Clusters[key].LocationOfOrigin = destinationFile + if relativizePaths { + if err := clientcmd.RelativizeClusterLocalPaths(configToWrite.Clusters[key]); err != nil { + return err + } + } if err := clientcmd.WriteToFile(*configToWrite, destinationFile); err != nil { return err @@ -257,7 +265,14 @@ func ModifyConfig(configAccess ConfigAccess, newConfig clientcmdapi.Config) erro } configToWrite := getConfigFromFileOrDie(destinationFile) - configToWrite.AuthInfos[key] = authInfo + t := *authInfo + configToWrite.AuthInfos[key] = &t + configToWrite.AuthInfos[key].LocationOfOrigin = destinationFile + if relativizePaths { + if err := clientcmd.RelativizeAuthInfoLocalPaths(configToWrite.AuthInfos[key]); err != nil { + return err + } + } if err := clientcmd.WriteToFile(*configToWrite, destinationFile); err != nil { return err diff --git a/pkg/kubectl/cmd/config/config_test.go b/pkg/kubectl/cmd/config/config_test.go index 8940ffb025b..5e25b3e53c6 100644 --- a/pkg/kubectl/cmd/config/config_test.go +++ b/pkg/kubectl/cmd/config/config_test.go @@ -21,6 +21,7 @@ import ( "fmt" "io/ioutil" "os" + "path" "reflect" "strings" "testing" @@ -34,11 +35,11 @@ import ( func newRedFederalCowHammerConfig() clientcmdapi.Config { return clientcmdapi.Config{ - AuthInfos: map[string]clientcmdapi.AuthInfo{ + AuthInfos: map[string]*clientcmdapi.AuthInfo{ "red-user": {Token: "red-token"}}, - Clusters: map[string]clientcmdapi.Cluster{ + Clusters: map[string]*clientcmdapi.Cluster{ "cow-cluster": {Server: "http://cow.org:8080"}}, - Contexts: map[string]clientcmdapi.Context{ + Contexts: map[string]*clientcmdapi.Context{ "federal-context": {AuthInfo: "red-user", Cluster: "cow-cluster"}}, CurrentContext: "federal-context", } @@ -79,10 +80,9 @@ func TestSetCurrentContext(t *testing.T) { startingConfig := newRedFederalCowHammerConfig() newContextName := "the-new-context" - newContext := clientcmdapi.NewContext() - startingConfig.Contexts[newContextName] = *newContext - expectedConfig.Contexts[newContextName] = *newContext + startingConfig.Contexts[newContextName] = clientcmdapi.NewContext() + expectedConfig.Contexts[newContextName] = clientcmdapi.NewContext() expectedConfig.CurrentContext = newContextName @@ -108,10 +108,7 @@ func TestSetNonExistantContext(t *testing.T) { func TestSetIntoExistingStruct(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() - a := expectedConfig.AuthInfos["red-user"] - authInfo := &a - authInfo.Password = "new-path-value" - expectedConfig.AuthInfos["red-user"] = *authInfo + expectedConfig.AuthInfos["red-user"].Password = "new-path-value" test := configCommandTest{ args: []string{"set", "users.red-user.password", "new-path-value"}, startingConfig: newRedFederalCowHammerConfig(), @@ -123,10 +120,7 @@ func TestSetIntoExistingStruct(t *testing.T) { func TestSetWithPathPrefixIntoExistingStruct(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() - cc := expectedConfig.Clusters["cow-clusters"] - cinfo := &cc - cinfo.Server = "http://cow.org:8080/foo/baz" - expectedConfig.Clusters["cow-cluster"] = *cinfo + expectedConfig.Clusters["cow-cluster"].Server = "http://cow.org:8080/foo/baz" test := configCommandTest{ args: []string{"set", "clusters.cow-cluster.server", "http://cow.org:8080/foo/baz"}, startingConfig: newRedFederalCowHammerConfig(), @@ -164,7 +158,7 @@ func TestUnsetStruct(t *testing.T) { func TestUnsetField(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() - expectedConfig.AuthInfos["red-user"] = *clientcmdapi.NewAuthInfo() + expectedConfig.AuthInfos["red-user"] = clientcmdapi.NewAuthInfo() test := configCommandTest{ args: []string{"unset", "users.red-user.token"}, startingConfig: newRedFederalCowHammerConfig(), @@ -178,7 +172,7 @@ func TestSetIntoNewStruct(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() cluster := clientcmdapi.NewCluster() cluster.Server = "new-server-value" - expectedConfig.Clusters["big-cluster"] = *cluster + expectedConfig.Clusters["big-cluster"] = cluster test := configCommandTest{ args: []string{"set", "clusters.big-cluster.server", "new-server-value"}, startingConfig: newRedFederalCowHammerConfig(), @@ -192,7 +186,7 @@ func TestSetBoolean(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() cluster := clientcmdapi.NewCluster() cluster.InsecureSkipTLSVerify = true - expectedConfig.Clusters["big-cluster"] = *cluster + expectedConfig.Clusters["big-cluster"] = cluster test := configCommandTest{ args: []string{"set", "clusters.big-cluster.insecure-skip-tls-verify", "true"}, startingConfig: newRedFederalCowHammerConfig(), @@ -206,7 +200,7 @@ func TestSetIntoNewConfig(t *testing.T) { expectedConfig := *clientcmdapi.NewConfig() context := clientcmdapi.NewContext() context.AuthInfo = "fake-user" - expectedConfig.Contexts["new-context"] = *context + expectedConfig.Contexts["new-context"] = context test := configCommandTest{ args: []string{"set", "contexts.new-context.user", "fake-user"}, startingConfig: *clientcmdapi.NewConfig(), @@ -218,7 +212,7 @@ func TestSetIntoNewConfig(t *testing.T) { func TestNewEmptyAuth(t *testing.T) { expectedConfig := *clientcmdapi.NewConfig() - expectedConfig.AuthInfos["the-user-name"] = *clientcmdapi.NewAuthInfo() + expectedConfig.AuthInfos["the-user-name"] = clientcmdapi.NewAuthInfo() test := configCommandTest{ args: []string{"set-credentials", "the-user-name"}, startingConfig: *clientcmdapi.NewConfig(), @@ -232,7 +226,7 @@ func TestAdditionalAuth(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() authInfo := clientcmdapi.NewAuthInfo() authInfo.Token = "token" - expectedConfig.AuthInfos["another-user"] = *authInfo + expectedConfig.AuthInfos["another-user"] = authInfo test := configCommandTest{ args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagBearerToken + "=token"}, startingConfig: newRedFederalCowHammerConfig(), @@ -250,7 +244,7 @@ func TestEmbedClientCert(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() authInfo := clientcmdapi.NewAuthInfo() authInfo.ClientCertificateData = fakeData - expectedConfig.AuthInfos["another-user"] = *authInfo + expectedConfig.AuthInfos["another-user"] = authInfo test := configCommandTest{ args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagCertFile + "=" + fakeCertFile.Name(), "--" + clientcmd.FlagEmbedCerts + "=true"}, @@ -269,7 +263,7 @@ func TestEmbedClientKey(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() authInfo := clientcmdapi.NewAuthInfo() authInfo.ClientKeyData = fakeData - expectedConfig.AuthInfos["another-user"] = *authInfo + expectedConfig.AuthInfos["another-user"] = authInfo test := configCommandTest{ args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagKeyFile + "=" + fakeKeyFile.Name(), "--" + clientcmd.FlagEmbedCerts + "=true"}, @@ -293,13 +287,15 @@ func TestEmbedNoKeyOrCertDisallowed(t *testing.T) { } func TestEmptyTokenAndCertAllowed(t *testing.T) { + fakeCertFile, _ := ioutil.TempFile("", "cert-file") + expectedConfig := newRedFederalCowHammerConfig() authInfo := clientcmdapi.NewAuthInfo() - authInfo.ClientCertificate = "cert-file" - expectedConfig.AuthInfos["another-user"] = *authInfo + authInfo.ClientCertificate = path.Base(fakeCertFile.Name()) + expectedConfig.AuthInfos["another-user"] = authInfo test := configCommandTest{ - args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagCertFile + "=cert-file", "--" + clientcmd.FlagBearerToken + "="}, + args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagCertFile + "=" + fakeCertFile.Name(), "--" + clientcmd.FlagBearerToken + "="}, startingConfig: newRedFederalCowHammerConfig(), expectedConfig: expectedConfig, } @@ -311,10 +307,10 @@ func TestTokenAndCertAllowed(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() authInfo := clientcmdapi.NewAuthInfo() authInfo.Token = "token" - authInfo.ClientCertificate = "cert-file" - expectedConfig.AuthInfos["another-user"] = *authInfo + authInfo.ClientCertificate = "/cert-file" + expectedConfig.AuthInfos["another-user"] = authInfo test := configCommandTest{ - args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagCertFile + "=cert-file", "--" + clientcmd.FlagBearerToken + "=token"}, + args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagCertFile + "=/cert-file", "--" + clientcmd.FlagBearerToken + "=token"}, startingConfig: newRedFederalCowHammerConfig(), expectedConfig: expectedConfig, } @@ -343,10 +339,10 @@ func TestBasicClearsToken(t *testing.T) { authInfoWithBasic.Password = "mypass" startingConfig := newRedFederalCowHammerConfig() - startingConfig.AuthInfos["another-user"] = *authInfoWithToken + startingConfig.AuthInfos["another-user"] = authInfoWithToken expectedConfig := newRedFederalCowHammerConfig() - expectedConfig.AuthInfos["another-user"] = *authInfoWithBasic + expectedConfig.AuthInfos["another-user"] = authInfoWithBasic test := configCommandTest{ args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagUsername + "=myuser", "--" + clientcmd.FlagPassword + "=mypass"}, @@ -366,10 +362,10 @@ func TestTokenClearsBasic(t *testing.T) { authInfoWithToken.Token = "token" startingConfig := newRedFederalCowHammerConfig() - startingConfig.AuthInfos["another-user"] = *authInfoWithBasic + startingConfig.AuthInfos["another-user"] = authInfoWithBasic expectedConfig := newRedFederalCowHammerConfig() - expectedConfig.AuthInfos["another-user"] = *authInfoWithToken + expectedConfig.AuthInfos["another-user"] = authInfoWithToken test := configCommandTest{ args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagBearerToken + "=token"}, @@ -395,10 +391,10 @@ func TestTokenLeavesCert(t *testing.T) { authInfoWithTokenAndCerts.ClientKeyData = []byte("keydata") startingConfig := newRedFederalCowHammerConfig() - startingConfig.AuthInfos["another-user"] = *authInfoWithCerts + startingConfig.AuthInfos["another-user"] = authInfoWithCerts expectedConfig := newRedFederalCowHammerConfig() - expectedConfig.AuthInfos["another-user"] = *authInfoWithTokenAndCerts + expectedConfig.AuthInfos["another-user"] = authInfoWithTokenAndCerts test := configCommandTest{ args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagBearerToken + "=token"}, @@ -415,17 +411,17 @@ func TestCertLeavesToken(t *testing.T) { authInfoWithTokenAndCerts := clientcmdapi.NewAuthInfo() authInfoWithTokenAndCerts.Token = "token" - authInfoWithTokenAndCerts.ClientCertificate = "cert" - authInfoWithTokenAndCerts.ClientKey = "key" + authInfoWithTokenAndCerts.ClientCertificate = "/cert" + authInfoWithTokenAndCerts.ClientKey = "/key" startingConfig := newRedFederalCowHammerConfig() - startingConfig.AuthInfos["another-user"] = *authInfoWithToken + startingConfig.AuthInfos["another-user"] = authInfoWithToken expectedConfig := newRedFederalCowHammerConfig() - expectedConfig.AuthInfos["another-user"] = *authInfoWithTokenAndCerts + expectedConfig.AuthInfos["another-user"] = authInfoWithTokenAndCerts test := configCommandTest{ - args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagCertFile + "=cert", "--" + clientcmd.FlagKeyFile + "=key"}, + args: []string{"set-credentials", "another-user", "--" + clientcmd.FlagCertFile + "=/cert", "--" + clientcmd.FlagKeyFile + "=/key"}, startingConfig: startingConfig, expectedConfig: expectedConfig, } @@ -434,20 +430,22 @@ func TestCertLeavesToken(t *testing.T) { } func TestCAClearsInsecure(t *testing.T) { + fakeCAFile, _ := ioutil.TempFile("", "ca-file") + clusterInfoWithInsecure := clientcmdapi.NewCluster() clusterInfoWithInsecure.InsecureSkipTLSVerify = true clusterInfoWithCA := clientcmdapi.NewCluster() - clusterInfoWithCA.CertificateAuthority = "cafile" + clusterInfoWithCA.CertificateAuthority = path.Base(fakeCAFile.Name()) startingConfig := newRedFederalCowHammerConfig() - startingConfig.Clusters["another-cluster"] = *clusterInfoWithInsecure + startingConfig.Clusters["another-cluster"] = clusterInfoWithInsecure expectedConfig := newRedFederalCowHammerConfig() - expectedConfig.Clusters["another-cluster"] = *clusterInfoWithCA + expectedConfig.Clusters["another-cluster"] = clusterInfoWithCA test := configCommandTest{ - args: []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagCAFile + "=cafile"}, + args: []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagCAFile + "=" + fakeCAFile.Name()}, startingConfig: startingConfig, expectedConfig: expectedConfig, } @@ -460,16 +458,16 @@ func TestCAClearsCAData(t *testing.T) { clusterInfoWithCAData.CertificateAuthorityData = []byte("cadata") clusterInfoWithCA := clientcmdapi.NewCluster() - clusterInfoWithCA.CertificateAuthority = "cafile" + clusterInfoWithCA.CertificateAuthority = "/cafile" startingConfig := newRedFederalCowHammerConfig() - startingConfig.Clusters["another-cluster"] = *clusterInfoWithCAData + startingConfig.Clusters["another-cluster"] = clusterInfoWithCAData expectedConfig := newRedFederalCowHammerConfig() - expectedConfig.Clusters["another-cluster"] = *clusterInfoWithCA + expectedConfig.Clusters["another-cluster"] = clusterInfoWithCA test := configCommandTest{ - args: []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagCAFile + "=cafile", "--" + clientcmd.FlagInsecure + "=false"}, + args: []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagCAFile + "=/cafile", "--" + clientcmd.FlagInsecure + "=false"}, startingConfig: startingConfig, expectedConfig: expectedConfig, } @@ -486,10 +484,10 @@ func TestInsecureClearsCA(t *testing.T) { clusterInfoWithCA.CertificateAuthorityData = []byte("cadata") startingConfig := newRedFederalCowHammerConfig() - startingConfig.Clusters["another-cluster"] = *clusterInfoWithCA + startingConfig.Clusters["another-cluster"] = clusterInfoWithCA expectedConfig := newRedFederalCowHammerConfig() - expectedConfig.Clusters["another-cluster"] = *clusterInfoWithInsecure + expectedConfig.Clusters["another-cluster"] = clusterInfoWithInsecure test := configCommandTest{ args: []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagInsecure + "=true"}, @@ -513,10 +511,10 @@ func TestCADataClearsCA(t *testing.T) { clusterInfoWithCA.CertificateAuthority = "cafile" startingConfig := newRedFederalCowHammerConfig() - startingConfig.Clusters["another-cluster"] = *clusterInfoWithCA + startingConfig.Clusters["another-cluster"] = clusterInfoWithCA expectedConfig := newRedFederalCowHammerConfig() - expectedConfig.Clusters["another-cluster"] = *clusterInfoWithCAData + expectedConfig.Clusters["another-cluster"] = clusterInfoWithCAData test := configCommandTest{ args: []string{"set-cluster", "another-cluster", "--" + clientcmd.FlagCAFile + "=" + fakeCAFile.Name(), "--" + clientcmd.FlagEmbedCerts + "=true"}, @@ -553,10 +551,10 @@ func TestCAAndInsecureDisallowed(t *testing.T) { func TestMergeExistingAuth(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() authInfo := expectedConfig.AuthInfos["red-user"] - authInfo.ClientKey = "key" + authInfo.ClientKey = "/key" expectedConfig.AuthInfos["red-user"] = authInfo test := configCommandTest{ - args: []string{"set-credentials", "red-user", "--" + clientcmd.FlagKeyFile + "=key"}, + args: []string{"set-credentials", "red-user", "--" + clientcmd.FlagKeyFile + "=/key"}, startingConfig: newRedFederalCowHammerConfig(), expectedConfig: expectedConfig, } @@ -566,7 +564,7 @@ func TestMergeExistingAuth(t *testing.T) { func TestNewEmptyCluster(t *testing.T) { expectedConfig := *clientcmdapi.NewConfig() - expectedConfig.Clusters["new-cluster"] = *clientcmdapi.NewCluster() + expectedConfig.Clusters["new-cluster"] = clientcmdapi.NewCluster() test := configCommandTest{ args: []string{"set-cluster", "new-cluster"}, startingConfig: *clientcmdapi.NewConfig(), @@ -578,14 +576,14 @@ func TestNewEmptyCluster(t *testing.T) { func TestAdditionalCluster(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() - cluster := *clientcmdapi.NewCluster() + cluster := clientcmdapi.NewCluster() cluster.APIVersion = testapi.Version() - cluster.CertificateAuthority = "ca-location" + cluster.CertificateAuthority = "/ca-location" cluster.InsecureSkipTLSVerify = false cluster.Server = "serverlocation" expectedConfig.Clusters["different-cluster"] = cluster test := configCommandTest{ - args: []string{"set-cluster", "different-cluster", "--" + clientcmd.FlagAPIServer + "=serverlocation", "--" + clientcmd.FlagInsecure + "=false", "--" + clientcmd.FlagCAFile + "=ca-location", "--" + clientcmd.FlagAPIVersion + "=" + testapi.Version()}, + args: []string{"set-cluster", "different-cluster", "--" + clientcmd.FlagAPIServer + "=serverlocation", "--" + clientcmd.FlagInsecure + "=false", "--" + clientcmd.FlagCAFile + "=/ca-location", "--" + clientcmd.FlagAPIVersion + "=" + testapi.Version()}, startingConfig: newRedFederalCowHammerConfig(), expectedConfig: expectedConfig, } @@ -595,7 +593,7 @@ func TestAdditionalCluster(t *testing.T) { func TestOverwriteExistingCluster(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() - cluster := *clientcmdapi.NewCluster() + cluster := clientcmdapi.NewCluster() cluster.Server = "serverlocation" expectedConfig.Clusters["cow-cluster"] = cluster @@ -610,7 +608,7 @@ func TestOverwriteExistingCluster(t *testing.T) { func TestNewEmptyContext(t *testing.T) { expectedConfig := *clientcmdapi.NewConfig() - expectedConfig.Contexts["new-context"] = *clientcmdapi.NewContext() + expectedConfig.Contexts["new-context"] = clientcmdapi.NewContext() test := configCommandTest{ args: []string{"set-context", "new-context"}, startingConfig: *clientcmdapi.NewConfig(), @@ -622,7 +620,7 @@ func TestNewEmptyContext(t *testing.T) { func TestAdditionalContext(t *testing.T) { expectedConfig := newRedFederalCowHammerConfig() - context := *clientcmdapi.NewContext() + context := clientcmdapi.NewContext() context.Cluster = "some-cluster" context.AuthInfo = "some-user" context.Namespace = "different-namespace" @@ -683,10 +681,13 @@ func TestToBool(t *testing.T) { } -func testConfigCommand(args []string, startingConfig clientcmdapi.Config) (string, clientcmdapi.Config) { +func testConfigCommand(args []string, startingConfig clientcmdapi.Config, t *testing.T) (string, clientcmdapi.Config) { fakeKubeFile, _ := ioutil.TempFile("", "") defer os.Remove(fakeKubeFile.Name()) - clientcmd.WriteToFile(startingConfig, fakeKubeFile.Name()) + err := clientcmd.WriteToFile(startingConfig, fakeKubeFile.Name()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } argsToUse := make([]string, 0, 2+len(args)) argsToUse = append(argsToUse, "--kubeconfig="+fakeKubeFile.Name()) @@ -712,7 +713,7 @@ type configCommandTest struct { } func (test configCommandTest) run(t *testing.T) string { - out, actualConfig := testConfigCommand(test.args, test.startingConfig) + out, actualConfig := testConfigCommand(test.args, test.startingConfig, t) testSetNilMapsToEmpties(reflect.ValueOf(&test.expectedConfig)) testSetNilMapsToEmpties(reflect.ValueOf(&actualConfig)) @@ -755,20 +756,7 @@ func testSetNilMapsToEmpties(curr reflect.Value) { case reflect.Map: for _, mapKey := range actualCurrValue.MapKeys() { currMapValue := actualCurrValue.MapIndex(mapKey) - - // our maps do not hold pointers to structs, they hold the structs themselves. This means that MapIndex returns the struct itself - // That in turn means that they have kinds of type.Struct, which is not a settable type. Because of this, we need to make new struct of that type - // copy all the data from the old value into the new value, then take the .addr of the new value to modify it in the next recursion. - // clear as mud - modifiableMapValue := reflect.New(currMapValue.Type()).Elem() - modifiableMapValue.Set(currMapValue) - - if modifiableMapValue.Kind() == reflect.Struct { - modifiableMapValue = modifiableMapValue.Addr() - } - - testSetNilMapsToEmpties(modifiableMapValue) - actualCurrValue.SetMapIndex(mapKey, reflect.Indirect(modifiableMapValue)) + testSetNilMapsToEmpties(currMapValue) } case reflect.Struct: diff --git a/pkg/kubectl/cmd/config/create_authinfo.go b/pkg/kubectl/cmd/config/create_authinfo.go index 9e157d543bc..8d4811a9ff1 100644 --- a/pkg/kubectl/cmd/config/create_authinfo.go +++ b/pkg/kubectl/cmd/config/create_authinfo.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "io/ioutil" + "path/filepath" "strings" "github.com/spf13/cobra" @@ -108,10 +109,14 @@ func (o createAuthInfoOptions) run() error { return err } - authInfo := o.modifyAuthInfo(config.AuthInfos[o.name]) - config.AuthInfos[o.name] = authInfo + startingStanza, exists := config.AuthInfos[o.name] + if !exists { + startingStanza = clientcmdapi.NewAuthInfo() + } + authInfo := o.modifyAuthInfo(*startingStanza) + config.AuthInfos[o.name] = &authInfo - if err := ModifyConfig(o.configAccess, *config); err != nil { + if err := ModifyConfig(o.configAccess, *config, true); err != nil { return err } @@ -130,6 +135,7 @@ func (o *createAuthInfoOptions) modifyAuthInfo(existingAuthInfo clientcmdapi.Aut modifiedAuthInfo.ClientCertificateData, _ = ioutil.ReadFile(certPath) modifiedAuthInfo.ClientCertificate = "" } else { + certPath, _ = filepath.Abs(certPath) modifiedAuthInfo.ClientCertificate = certPath if len(modifiedAuthInfo.ClientCertificate) > 0 { modifiedAuthInfo.ClientCertificateData = nil @@ -142,6 +148,7 @@ func (o *createAuthInfoOptions) modifyAuthInfo(existingAuthInfo clientcmdapi.Aut modifiedAuthInfo.ClientKeyData, _ = ioutil.ReadFile(keyPath) modifiedAuthInfo.ClientKey = "" } else { + keyPath, _ = filepath.Abs(keyPath) modifiedAuthInfo.ClientKey = keyPath if len(modifiedAuthInfo.ClientKey) > 0 { modifiedAuthInfo.ClientKeyData = nil diff --git a/pkg/kubectl/cmd/config/create_cluster.go b/pkg/kubectl/cmd/config/create_cluster.go index cd19a3e913a..063bb2c98b2 100644 --- a/pkg/kubectl/cmd/config/create_cluster.go +++ b/pkg/kubectl/cmd/config/create_cluster.go @@ -21,6 +21,7 @@ import ( "fmt" "io" "io/ioutil" + "path/filepath" "github.com/spf13/cobra" @@ -94,10 +95,14 @@ func (o createClusterOptions) run() error { return err } - cluster := o.modifyCluster(config.Clusters[o.name]) - config.Clusters[o.name] = cluster + startingStanza, exists := config.Clusters[o.name] + if !exists { + startingStanza = clientcmdapi.NewCluster() + } + cluster := o.modifyCluster(*startingStanza) + config.Clusters[o.name] = &cluster - if err := ModifyConfig(o.configAccess, *config); err != nil { + if err := ModifyConfig(o.configAccess, *config, true); err != nil { return err } @@ -129,6 +134,7 @@ func (o *createClusterOptions) modifyCluster(existingCluster clientcmdapi.Cluste modifiedCluster.InsecureSkipTLSVerify = false modifiedCluster.CertificateAuthority = "" } else { + caPath, _ = filepath.Abs(caPath) modifiedCluster.CertificateAuthority = caPath // Specifying a certificate authority file clears certificate authority data and insecure mode if caPath != "" { diff --git a/pkg/kubectl/cmd/config/create_context.go b/pkg/kubectl/cmd/config/create_context.go index bd8ff3642af..fe7874f1200 100644 --- a/pkg/kubectl/cmd/config/create_context.go +++ b/pkg/kubectl/cmd/config/create_context.go @@ -81,10 +81,14 @@ func (o createContextOptions) run() error { return err } - context := o.modifyContext(config.Contexts[o.name]) - config.Contexts[o.name] = context + startingStanza, exists := config.Contexts[o.name] + if !exists { + startingStanza = clientcmdapi.NewContext() + } + context := o.modifyContext(*startingStanza) + config.Contexts[o.name] = &context - if err := ModifyConfig(o.configAccess, *config); err != nil { + if err := ModifyConfig(o.configAccess, *config, true); err != nil { return err } diff --git a/pkg/kubectl/cmd/config/navigation_step_parser.go b/pkg/kubectl/cmd/config/navigation_step_parser.go index 4b5d1ca96f0..1e0bca9ed5a 100644 --- a/pkg/kubectl/cmd/config/navigation_step_parser.go +++ b/pkg/kubectl/cmd/config/navigation_step_parser.go @@ -50,7 +50,7 @@ func newNavigationSteps(path string) (*navigationSteps, error) { // store them as a single step. In order to do that, we need to determine what set of tokens is a legal step AFTER the name of the map key // This set of reflective code pulls the type of the map values, uses that type to look up the set of legal tags. Those legal tags are used to // walk the list of remaining parts until we find a match to a legal tag or the end of the string. That name is used to burn all the used parts. - mapValueType := currType.Elem() + mapValueType := currType.Elem().Elem() mapValueOptions, err := getPotentialTypeValues(mapValueType) if err != nil { return nil, err @@ -120,6 +120,10 @@ func findNameStep(parts []string, typeOptions util.StringSet) string { // getPotentialTypeValues takes a type and looks up the tags used to represent its fields when serialized. func getPotentialTypeValues(typeValue reflect.Type) (map[string]reflect.Type, error) { + if typeValue.Kind() == reflect.Ptr { + typeValue = typeValue.Elem() + } + if typeValue.Kind() != reflect.Struct { return nil, fmt.Errorf("%v is not of type struct", typeValue) } diff --git a/pkg/kubectl/cmd/config/navigation_step_parser_test.go b/pkg/kubectl/cmd/config/navigation_step_parser_test.go index 6d112386513..4e4a11b0e79 100644 --- a/pkg/kubectl/cmd/config/navigation_step_parser_test.go +++ b/pkg/kubectl/cmd/config/navigation_step_parser_test.go @@ -36,7 +36,7 @@ func TestParseWithDots(t *testing.T) { path: "clusters.my.dot.delimited.name.server", expectedNavigationSteps: navigationSteps{ steps: []navigationStep{ - {"clusters", reflect.TypeOf(make(map[string]clientcmdapi.Cluster))}, + {"clusters", reflect.TypeOf(make(map[string]*clientcmdapi.Cluster))}, {"my.dot.delimited.name", reflect.TypeOf(clientcmdapi.Cluster{})}, {"server", reflect.TypeOf("")}, }, @@ -51,7 +51,7 @@ func TestParseWithDotsEndingWithName(t *testing.T) { path: "contexts.10.12.12.12", expectedNavigationSteps: navigationSteps{ steps: []navigationStep{ - {"contexts", reflect.TypeOf(make(map[string]clientcmdapi.Context))}, + {"contexts", reflect.TypeOf(make(map[string]*clientcmdapi.Context))}, {"10.12.12.12", reflect.TypeOf(clientcmdapi.Context{})}, }, }, @@ -91,5 +91,6 @@ func (test stepParserTest) run(t *testing.T) { if !reflect.DeepEqual(test.expectedNavigationSteps, *actualSteps) { t.Errorf("diff: %v", util.ObjectDiff(test.expectedNavigationSteps, *actualSteps)) + t.Errorf("expected: %#v\n actual: %#v", test.expectedNavigationSteps, *actualSteps) } } diff --git a/pkg/kubectl/cmd/config/set.go b/pkg/kubectl/cmd/config/set.go index 38e397d52bd..585e87c599e 100644 --- a/pkg/kubectl/cmd/config/set.go +++ b/pkg/kubectl/cmd/config/set.go @@ -82,7 +82,7 @@ func (o setOptions) run() error { return err } - if err := ModifyConfig(o.configAccess, *config); err != nil { + if err := ModifyConfig(o.configAccess, *config, false); err != nil { return err } @@ -139,26 +139,15 @@ func modifyConfig(curr reflect.Value, steps *navigationSteps, propertyValue stri needToSetNewMapValue := currMapValue.Kind() == reflect.Invalid if needToSetNewMapValue { - currMapValue = reflect.New(mapValueType).Elem() + currMapValue = reflect.New(mapValueType.Elem()).Elem().Addr() actualCurrValue.SetMapIndex(mapKey, currMapValue) } - // our maps do not hold pointers to structs, they hold the structs themselves. This means that MapIndex returns the struct itself - // That in turn means that they have kinds of type.Struct, which is not a settable type. Because of this, we need to make new struct of that type - // copy all the data from the old value into the new value, then take the .addr of the new value to modify it in the next recursion. - // clear as mud - modifiableMapValue := reflect.New(currMapValue.Type()).Elem() - modifiableMapValue.Set(currMapValue) - - if modifiableMapValue.Kind() == reflect.Struct { - modifiableMapValue = modifiableMapValue.Addr() - } - err := modifyConfig(modifiableMapValue, steps, propertyValue, unset) + err := modifyConfig(currMapValue, steps, propertyValue, unset) if err != nil { return err } - actualCurrValue.SetMapIndex(mapKey, reflect.Indirect(modifiableMapValue)) return nil case reflect.String: @@ -213,5 +202,6 @@ func modifyConfig(curr reflect.Value, steps *navigationSteps, propertyValue stri } - return fmt.Errorf("Unrecognized type: %v", actualCurrValue) + panic(fmt.Errorf("Unrecognized type: %v", actualCurrValue)) + return nil } diff --git a/pkg/kubectl/cmd/config/unset.go b/pkg/kubectl/cmd/config/unset.go index 3e607dc1021..ef418060b62 100644 --- a/pkg/kubectl/cmd/config/unset.go +++ b/pkg/kubectl/cmd/config/unset.go @@ -75,7 +75,7 @@ func (o unsetOptions) run() error { return err } - if err := ModifyConfig(o.configAccess, *config); err != nil { + if err := ModifyConfig(o.configAccess, *config, false); err != nil { return err } diff --git a/pkg/kubectl/cmd/config/use_context.go b/pkg/kubectl/cmd/config/use_context.go index 341c48b6a9e..409f64daf13 100644 --- a/pkg/kubectl/cmd/config/use_context.go +++ b/pkg/kubectl/cmd/config/use_context.go @@ -66,7 +66,7 @@ func (o useContextOptions) run() error { config.CurrentContext = o.contextName - if err := ModifyConfig(o.configAccess, *config); err != nil { + if err := ModifyConfig(o.configAccess, *config, true); err != nil { return err }