Skip to content

Commit

Permalink
Overriding CA file should override skip TLS and CA data
Browse files Browse the repository at this point in the history
Kubernetes-commit: 857572168e79430af2dbf05e9d4705dfb3f0d99b
  • Loading branch information
liggitt authored and k8s-publishing-bot committed Oct 6, 2019
1 parent b1fd789 commit 24302e4
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 6 deletions.
12 changes: 7 additions & 5 deletions tools/clientcmd/client_config.go
Expand Up @@ -449,13 +449,15 @@ func (config *DirectClientConfig) getCluster() (clientcmdapi.Cluster, error) {
return clientcmdapi.Cluster{}, fmt.Errorf("cluster %q does not exist", clusterInfoName)
}
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"
// * 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".
// * An override of --certificate-authority should also override TLS skip settings and CA data, otherwise existing CA data will take precedence.
caLen := len(config.overrides.ClusterInfo.CertificateAuthority)
caDataLen := len(config.overrides.ClusterInfo.CertificateAuthorityData)
if config.overrides.ClusterInfo.InsecureSkipTLSVerify && caLen == 0 && caDataLen == 0 {
mergedClusterInfo.CertificateAuthority = ""
mergedClusterInfo.CertificateAuthorityData = nil
if config.overrides.ClusterInfo.InsecureSkipTLSVerify || caLen > 0 || caDataLen > 0 {
mergedClusterInfo.InsecureSkipTLSVerify = config.overrides.ClusterInfo.InsecureSkipTLSVerify
mergedClusterInfo.CertificateAuthority = config.overrides.ClusterInfo.CertificateAuthority
mergedClusterInfo.CertificateAuthorityData = config.overrides.ClusterInfo.CertificateAuthorityData
}

return *mergedClusterInfo, nil
Expand Down
26 changes: 25 additions & 1 deletion tools/clientcmd/client_config_test.go
Expand Up @@ -148,14 +148,38 @@ func TestInsecureOverridesCA(t *testing.T) {

actualCfg, err := clientBuilder.ClientConfig()
if err != nil {
t.Errorf("Unexpected error: %v", err)
t.Fatalf("Unexpected error: %v", err)
}

matchBoolArg(true, actualCfg.Insecure, t)
matchStringArg("", actualCfg.TLSClientConfig.CAFile, t)
matchByteArg(nil, actualCfg.TLSClientConfig.CAData, t)
}

func TestCAOverridesCAData(t *testing.T) {
file, err := ioutil.TempFile("", "my.ca")
if err != nil {
t.Fatalf("could not create tempfile: %v", err)
}
defer os.Remove(file.Name())

config := createCAValidTestConfig()
clientBuilder := NewNonInteractiveClientConfig(*config, "clean", &ConfigOverrides{
ClusterInfo: clientcmdapi.Cluster{
CertificateAuthority: file.Name(),
},
}, nil)

actualCfg, err := clientBuilder.ClientConfig()
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}

matchBoolArg(false, actualCfg.Insecure, t)
matchStringArg(file.Name(), actualCfg.TLSClientConfig.CAFile, t)
matchByteArg(nil, actualCfg.TLSClientConfig.CAData, t)
}

func TestMergeContext(t *testing.T) {
const namespace = "overridden-namespace"

Expand Down

0 comments on commit 24302e4

Please sign in to comment.