From 9dcbc0bf909a794cf77a801bfd29e306791b1e24 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 3 Mar 2020 13:16:50 -0500 Subject: [PATCH] update override behavior for kubectl --tls-server-name --- .../api/v1/zz_generated.conversion.go | 2 ++ .../tools/clientcmd/client_config.go | 6 +++- .../tools/clientcmd/client_config_test.go | 17 +++++++++++ .../kubectl/pkg/cmd/config/create_cluster.go | 3 ++ .../pkg/cmd/config/create_cluster_test.go | 28 +++++++++++++++++-- 5 files changed, 53 insertions(+), 3 deletions(-) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go index 31e00ea6e4f..8f3631e151b 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.go @@ -233,6 +233,7 @@ func Convert_api_AuthProviderConfig_To_v1_AuthProviderConfig(in *api.AuthProvide func autoConvert_v1_Cluster_To_api_Cluster(in *Cluster, out *api.Cluster, s conversion.Scope) error { out.Server = in.Server + out.TLSServerName = in.TLSServerName out.InsecureSkipTLSVerify = in.InsecureSkipTLSVerify out.CertificateAuthority = in.CertificateAuthority out.CertificateAuthorityData = *(*[]byte)(unsafe.Pointer(&in.CertificateAuthorityData)) @@ -250,6 +251,7 @@ func Convert_v1_Cluster_To_api_Cluster(in *Cluster, out *api.Cluster, s conversi func autoConvert_api_Cluster_To_v1_Cluster(in *api.Cluster, out *Cluster, s conversion.Scope) error { // INFO: in.LocationOfOrigin opted out of conversion generation out.Server = in.Server + out.TLSServerName = in.TLSServerName out.InsecureSkipTLSVerify = in.InsecureSkipTLSVerify out.CertificateAuthority = in.CertificateAuthority out.CertificateAuthorityData = *(*[]byte)(unsafe.Pointer(&in.CertificateAuthorityData)) diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go b/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go index 6b5f3f7375d..a9806384aab 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/client_config.go @@ -461,7 +461,11 @@ func (config *DirectClientConfig) getCluster() (clientcmdapi.Cluster, error) { mergedClusterInfo.CertificateAuthorityData = config.overrides.ClusterInfo.CertificateAuthorityData } - if config.overrides.ClusterInfo.TLSServerName != "" { + // if the --tls-server-name has been set in overrides, use that value. + // if the --server has been set in overrides, then use the value of --tls-server-name specified on the CLI too. This gives the property + // that setting a --server will effectively clear the KUBECONFIG value of tls-server-name if it is specified on the command line which is + // usually correct. + if config.overrides.ClusterInfo.TLSServerName != "" || config.overrides.ClusterInfo.Server != "" { mergedClusterInfo.TLSServerName = config.overrides.ClusterInfo.TLSServerName } diff --git a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go index e89ce147bd0..3232d8b04ec 100644 --- a/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go +++ b/staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.go @@ -199,6 +199,23 @@ func TestTLSServerName(t *testing.T) { matchByteArg(nil, actualCfg.TLSClientConfig.CAData, t) } +func TestTLSServerNameClearsWhenServerNameSet(t *testing.T) { + config := createValidTestConfig() + + clientBuilder := NewNonInteractiveClientConfig(*config, "clean", &ConfigOverrides{ + ClusterInfo: clientcmdapi.Cluster{ + Server: "http://something", + }, + }, nil) + + actualCfg, err := clientBuilder.ClientConfig() + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + + matchStringArg("", actualCfg.ServerName, t) +} + func TestMergeContext(t *testing.T) { const namespace = "overridden-namespace" diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/create_cluster.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/create_cluster.go index f4b4c0ac1b9..2b7f2c9185d 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/create_cluster.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/create_cluster.go @@ -124,6 +124,9 @@ func (o *createClusterOptions) modifyCluster(existingCluster clientcmdapi.Cluste if o.server.Provided() { modifiedCluster.Server = o.server.Value() + // specifying a --server on the command line, overrides the TLSServerName that was specified in the kubeconfig file. + // if both are specified, then the next if block will write the new TLSServerName. + modifiedCluster.TLSServerName = "" } if o.tlsServerName.Provided() { modifiedCluster.TLSServerName = o.tlsServerName.Value() diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/config/create_cluster_test.go b/staging/src/k8s.io/kubectl/pkg/cmd/config/create_cluster_test.go index 1a94436c2a2..00f00c566fc 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/config/create_cluster_test.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/config/create_cluster_test.go @@ -58,7 +58,7 @@ func TestCreateCluster(t *testing.T) { func TestModifyCluster(t *testing.T) { conf := clientcmdapi.Config{ Clusters: map[string]*clientcmdapi.Cluster{ - "my-cluster": {Server: "https://192.168.0.1"}, + "my-cluster": {Server: "https://192.168.0.1", TLSServerName: "to-be-cleared"}, }, } test := createClusterTest{ @@ -78,6 +78,30 @@ func TestModifyCluster(t *testing.T) { test.run(t) } +func TestModifyClusterServerAndTLS(t *testing.T) { + conf := clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{ + "my-cluster": {Server: "https://192.168.0.1"}, + }, + } + test := createClusterTest{ + description: "Testing 'kubectl config set-cluster' with an existing cluster", + config: conf, + args: []string{"my-cluster"}, + flags: []string{ + "--server=https://192.168.0.99", + "--tls-server-name=my-cluster-name", + }, + expected: `Cluster "my-cluster" set.` + "\n", + expectedConfig: clientcmdapi.Config{ + Clusters: map[string]*clientcmdapi.Cluster{ + "my-cluster": {Server: "https://192.168.0.99", TLSServerName: "my-cluster-name"}, + }, + }, + } + test.run(t) +} + func (test createClusterTest) run(t *testing.T) { fakeKubeFile, err := ioutil.TempFile(os.TempDir(), "") if err != nil { @@ -117,7 +141,7 @@ func (test createClusterTest) run(t *testing.T) { t.Errorf("Fail in %q\n expected cluster server %v\n but got %v\n ", test.description, test.expectedConfig.Clusters[test.args[0]].Server, cluster.Server) } if cluster.TLSServerName != test.expectedConfig.Clusters[test.args[0]].TLSServerName { - t.Errorf("Fail in %q\n expected cluster TLS server name %v\n but got %v\n ", test.description, test.expectedConfig.Clusters[test.args[0]].TLSServerName, cluster.TLSServerName) + t.Errorf("Fail in %q\n expected cluster TLS server name %q\n but got %q\n ", test.description, test.expectedConfig.Clusters[test.args[0]].TLSServerName, cluster.TLSServerName) } } }