update override behavior for kubectl --tls-server-name

Author: deads2k

staging/src/k8s.io/client-go/tools/clientcmd/api/v1/zz_generated.conversion.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
 	}
 

staging/src/k8s.io/client-go/tools/clientcmd/client_config.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"
 

staging/src/k8s.io/client-go/tools/clientcmd/client_config_test.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()

staging/src/k8s.io/kubectl/pkg/cmd/config/create_cluster.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)
 		}
 	}
 }