From 4ba0db76fbfa4e33043bf51996836fc970984c3e Mon Sep 17 00:00:00 2001 From: Ondra Kupka Date: Fri, 7 Nov 2025 12:05:21 +0100 Subject: [PATCH] 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 --- pkg/cli/login/error_translation.go | 2 + pkg/cli/login/helpers.go | 14 ++--- pkg/cli/login/loginoptions.go | 17 ++++-- pkg/cli/login/loginoptions_test.go | 85 ++++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 15 deletions(-) diff --git a/pkg/cli/login/error_translation.go b/pkg/cli/login/error_translation.go index 4fc61b346d..3b75321bf8 100644 --- a/pkg/cli/login/error_translation.go +++ b/pkg/cli/login/error_translation.go @@ -24,6 +24,8 @@ You can also run this command again providing the path to a config file directly Ensure the specified server supports HTTPS.` invalidServerURLMsg = `Seems you passed an HTML page (console?) instead of server URL. Verify provided address and try again.` + + insecureTransportCertificateAuthorityConflictMsg = "certificate-authority, certificate-authority-data are mutually exclusive with insecure-skip-tls-verify" ) type errInvalidServerURL struct{} diff --git a/pkg/cli/login/helpers.go b/pkg/cli/login/helpers.go index 6b1ca5b9ea..45c2284b00 100644 --- a/pkg/cli/login/helpers.go +++ b/pkg/cli/login/helpers.go @@ -30,20 +30,14 @@ func getMatchingClusters(clientConfig restclient.Config, kubeconfig clientcmdapi return ret } -// findExistingClientCA returns *either* the existing client CA file name as a string, -// *or* data in a []byte for a given host, and true if it exists in the given config -func findExistingClientCA(host string, kubeconfig clientcmdapi.Config) (string, []byte, bool) { +// findClusters returns the first cluster matching the host. +func findCluster(host string, kubeconfig clientcmdapi.Config) *clientcmdapi.Cluster { for _, cluster := range kubeconfig.Clusters { if cluster.Server == host { - if len(cluster.CertificateAuthority) > 0 { - return cluster.CertificateAuthority, nil, true - } - if len(cluster.CertificateAuthorityData) > 0 { - return "", cluster.CertificateAuthorityData, true - } + return cluster } } - return "", nil, false + return nil } // dialToServer takes the Server URL from the given clientConfig and dials to diff --git a/pkg/cli/login/loginoptions.go b/pkg/cli/login/loginoptions.go index 62a998036d..75ffe281c8 100644 --- a/pkg/cli/login/loginoptions.go +++ b/pkg/cli/login/loginoptions.go @@ -163,13 +163,20 @@ func (o *LoginOptions) getClientConfig() (*restclient.Config, error) { clientConfig.Insecure = o.InsecureTLS if !o.InsecureTLS { - // use specified CA or find existing CA + // Try to use the specified CA. Then fall back into searching kubeconfig. if len(o.CAFile) > 0 { clientConfig.CAFile = o.CAFile clientConfig.CAData = nil - } else if caFile, caData, ok := findExistingClientCA(clientConfig.Host, *o.StartingKubeConfig); ok { - clientConfig.CAFile = caFile - clientConfig.CAData = caData + } else if cluster := findCluster(clientConfig.Host, *o.StartingKubeConfig); cluster != nil { + clientConfig.Insecure = cluster.InsecureSkipTLSVerify + clientConfig.CAFile = cluster.CertificateAuthority + clientConfig.CAData = cluster.CertificateAuthorityData + + // It's not allowed to specify both the insecure flag and a CA. + // k8s transport init machinery returns an error in that case. + if clientConfig.Insecure && (clientConfig.CAFile != "" || clientConfig.CAData != nil) { + return nil, errors.New(insecureTransportCertificateAuthorityConflictMsg) + } } } @@ -188,7 +195,7 @@ func (o *LoginOptions) getClientConfig() (*restclient.Config, error) { // connection or if we already have a cluster stanza that tells us to // connect to this particular server insecurely case x509.UnknownAuthorityError, x509.HostnameError, x509.CertificateInvalidError: - if o.InsecureTLS || + if clientConfig.Insecure || hasExistingInsecureCluster(*clientConfig, *o.StartingKubeConfig) || promptForInsecureTLS(o.In, o.Out, err) { clientConfig.Insecure = true diff --git a/pkg/cli/login/loginoptions_test.go b/pkg/cli/login/loginoptions_test.go index 1b95e1f3a8..85ba775bf2 100644 --- a/pkg/cli/login/loginoptions_test.go +++ b/pkg/cli/login/loginoptions_test.go @@ -377,6 +377,91 @@ func TestDialToHTTPSServer(t *testing.T) { } } +func TestGetClientConfig_InsecureSkipTLSVerify(t *testing.T) { + // Test that insecure-skip-tls-verify setting from kubeconfig is respected + // when logging in without the --insecure-skip-tls-verify flag. + + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + testCases := map[string]struct { + insecureFlag bool + kubeconfigInsecure bool + kubeconfigCAData []byte + kubeconfigCAFile string + expectedInsecureClientConfig bool + expectedErrMsg string + }{ + "command flag set": { + insecureFlag: true, + expectedInsecureClientConfig: true, + }, + "kubeconfig flag set": { + kubeconfigInsecure: true, + expectedInsecureClientConfig: true, + }, + "no flag set": { + insecureFlag: false, + kubeconfigInsecure: false, + expectedErrMsg: certificateAuthorityUnknownMsg, + }, + "both command and kubeconfig flag set": { + insecureFlag: true, + kubeconfigInsecure: true, + expectedInsecureClientConfig: true, + }, + "conflict error on certificate authority data set": { + kubeconfigInsecure: true, + kubeconfigCAData: []byte("whatever"), + expectedErrMsg: insecureTransportCertificateAuthorityConflictMsg, + }, + "conflict error on certificate authority file set": { + kubeconfigInsecure: true, + kubeconfigCAFile: "/whatever.crt", + expectedErrMsg: insecureTransportCertificateAuthorityConflictMsg, + }, + } + + for name, test := range testCases { + t.Run(name, func(t *testing.T) { + startingConfig := &kclientcmdapi.Config{ + Clusters: map[string]*kclientcmdapi.Cluster{}, + } + if test.kubeconfigInsecure { + startingConfig.Clusters["test-cluster"] = &kclientcmdapi.Cluster{ + Server: server.URL, + CertificateAuthority: test.kubeconfigCAFile, + CertificateAuthorityData: test.kubeconfigCAData, + InsecureSkipTLSVerify: true, + } + } + + options := &LoginOptions{ + Server: server.URL, + InsecureTLS: test.insecureFlag, + StartingKubeConfig: startingConfig, + } + + clientConfig, err := options.getClientConfig() + if err != nil { + if test.expectedInsecureClientConfig { + t.Fatalf("Expected to succeed with insecure connection, but got error: %v", err) + } + if test.expectedErrMsg != err.Error() { + t.Fatalf("Unexpected error received:\n expected %q\n got %q", test.expectedErrMsg, err.Error()) + } + return + } + + if clientConfig.Insecure != test.expectedInsecureClientConfig { + t.Errorf("expected Insecure=%v, got %v", test.expectedInsecureClientConfig, clientConfig.Insecure) + } + }) + } +} + func TestPreserveExecProviderOnUsernameLogin(t *testing.T) { // Test that when using -u flag with existing OIDC credentials, // the ExecProvider configuration is preserved