Skip to content

Commit 4ba0db7

Browse files
committed
oc login: Respect insecure flag from kubeconfig
When running oc login, the insecure flag from kubeconfig is not consulted properly when calling getClientConfig(). This is now fixed. Assisted-by: Claude Code
1 parent 345800d commit 4ba0db7

File tree

4 files changed

+103
-15
lines changed

4 files changed

+103
-15
lines changed

pkg/cli/login/error_translation.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ You can also run this command again providing the path to a config file directly
2424
Ensure the specified server supports HTTPS.`
2525
invalidServerURLMsg = `Seems you passed an HTML page (console?) instead of server URL.
2626
Verify provided address and try again.`
27+
28+
insecureTransportCertificateAuthorityConflictMsg = "certificate-authority, certificate-authority-data are mutually exclusive with insecure-skip-tls-verify"
2729
)
2830

2931
type errInvalidServerURL struct{}

pkg/cli/login/helpers.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,14 @@ func getMatchingClusters(clientConfig restclient.Config, kubeconfig clientcmdapi
3030
return ret
3131
}
3232

33-
// findExistingClientCA returns *either* the existing client CA file name as a string,
34-
// *or* data in a []byte for a given host, and true if it exists in the given config
35-
func findExistingClientCA(host string, kubeconfig clientcmdapi.Config) (string, []byte, bool) {
33+
// findClusters returns the first cluster matching the host.
34+
func findCluster(host string, kubeconfig clientcmdapi.Config) *clientcmdapi.Cluster {
3635
for _, cluster := range kubeconfig.Clusters {
3736
if cluster.Server == host {
38-
if len(cluster.CertificateAuthority) > 0 {
39-
return cluster.CertificateAuthority, nil, true
40-
}
41-
if len(cluster.CertificateAuthorityData) > 0 {
42-
return "", cluster.CertificateAuthorityData, true
43-
}
37+
return cluster
4438
}
4539
}
46-
return "", nil, false
40+
return nil
4741
}
4842

4943
// dialToServer takes the Server URL from the given clientConfig and dials to

pkg/cli/login/loginoptions.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,20 @@ func (o *LoginOptions) getClientConfig() (*restclient.Config, error) {
163163
clientConfig.Insecure = o.InsecureTLS
164164

165165
if !o.InsecureTLS {
166-
// use specified CA or find existing CA
166+
// Try to use the specified CA. Then fall back into searching kubeconfig.
167167
if len(o.CAFile) > 0 {
168168
clientConfig.CAFile = o.CAFile
169169
clientConfig.CAData = nil
170-
} else if caFile, caData, ok := findExistingClientCA(clientConfig.Host, *o.StartingKubeConfig); ok {
171-
clientConfig.CAFile = caFile
172-
clientConfig.CAData = caData
170+
} else if cluster := findCluster(clientConfig.Host, *o.StartingKubeConfig); cluster != nil {
171+
clientConfig.Insecure = cluster.InsecureSkipTLSVerify
172+
clientConfig.CAFile = cluster.CertificateAuthority
173+
clientConfig.CAData = cluster.CertificateAuthorityData
174+
175+
// It's not allowed to specify both the insecure flag and a CA.
176+
// k8s transport init machinery returns an error in that case.
177+
if clientConfig.Insecure && (clientConfig.CAFile != "" || clientConfig.CAData != nil) {
178+
return nil, errors.New(insecureTransportCertificateAuthorityConflictMsg)
179+
}
173180
}
174181
}
175182

@@ -188,7 +195,7 @@ func (o *LoginOptions) getClientConfig() (*restclient.Config, error) {
188195
// connection or if we already have a cluster stanza that tells us to
189196
// connect to this particular server insecurely
190197
case x509.UnknownAuthorityError, x509.HostnameError, x509.CertificateInvalidError:
191-
if o.InsecureTLS ||
198+
if clientConfig.Insecure ||
192199
hasExistingInsecureCluster(*clientConfig, *o.StartingKubeConfig) ||
193200
promptForInsecureTLS(o.In, o.Out, err) {
194201
clientConfig.Insecure = true

pkg/cli/login/loginoptions_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,91 @@ func TestDialToHTTPSServer(t *testing.T) {
377377
}
378378
}
379379

380+
func TestGetClientConfig_InsecureSkipTLSVerify(t *testing.T) {
381+
// Test that insecure-skip-tls-verify setting from kubeconfig is respected
382+
// when logging in without the --insecure-skip-tls-verify flag.
383+
384+
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
385+
w.WriteHeader(http.StatusOK)
386+
}))
387+
defer server.Close()
388+
389+
testCases := map[string]struct {
390+
insecureFlag bool
391+
kubeconfigInsecure bool
392+
kubeconfigCAData []byte
393+
kubeconfigCAFile string
394+
expectedInsecureClientConfig bool
395+
expectedErrMsg string
396+
}{
397+
"command flag set": {
398+
insecureFlag: true,
399+
expectedInsecureClientConfig: true,
400+
},
401+
"kubeconfig flag set": {
402+
kubeconfigInsecure: true,
403+
expectedInsecureClientConfig: true,
404+
},
405+
"no flag set": {
406+
insecureFlag: false,
407+
kubeconfigInsecure: false,
408+
expectedErrMsg: certificateAuthorityUnknownMsg,
409+
},
410+
"both command and kubeconfig flag set": {
411+
insecureFlag: true,
412+
kubeconfigInsecure: true,
413+
expectedInsecureClientConfig: true,
414+
},
415+
"conflict error on certificate authority data set": {
416+
kubeconfigInsecure: true,
417+
kubeconfigCAData: []byte("whatever"),
418+
expectedErrMsg: insecureTransportCertificateAuthorityConflictMsg,
419+
},
420+
"conflict error on certificate authority file set": {
421+
kubeconfigInsecure: true,
422+
kubeconfigCAFile: "/whatever.crt",
423+
expectedErrMsg: insecureTransportCertificateAuthorityConflictMsg,
424+
},
425+
}
426+
427+
for name, test := range testCases {
428+
t.Run(name, func(t *testing.T) {
429+
startingConfig := &kclientcmdapi.Config{
430+
Clusters: map[string]*kclientcmdapi.Cluster{},
431+
}
432+
if test.kubeconfigInsecure {
433+
startingConfig.Clusters["test-cluster"] = &kclientcmdapi.Cluster{
434+
Server: server.URL,
435+
CertificateAuthority: test.kubeconfigCAFile,
436+
CertificateAuthorityData: test.kubeconfigCAData,
437+
InsecureSkipTLSVerify: true,
438+
}
439+
}
440+
441+
options := &LoginOptions{
442+
Server: server.URL,
443+
InsecureTLS: test.insecureFlag,
444+
StartingKubeConfig: startingConfig,
445+
}
446+
447+
clientConfig, err := options.getClientConfig()
448+
if err != nil {
449+
if test.expectedInsecureClientConfig {
450+
t.Fatalf("Expected to succeed with insecure connection, but got error: %v", err)
451+
}
452+
if test.expectedErrMsg != err.Error() {
453+
t.Fatalf("Unexpected error received:\n expected %q\n got %q", test.expectedErrMsg, err.Error())
454+
}
455+
return
456+
}
457+
458+
if clientConfig.Insecure != test.expectedInsecureClientConfig {
459+
t.Errorf("expected Insecure=%v, got %v", test.expectedInsecureClientConfig, clientConfig.Insecure)
460+
}
461+
})
462+
}
463+
}
464+
380465
func TestPreserveExecProviderOnUsernameLogin(t *testing.T) {
381466
// Test that when using -u flag with existing OIDC credentials,
382467
// the ExecProvider configuration is preserved

0 commit comments

Comments
 (0)