OCPBUGS-64619: oc login: Respect insecure flag from kubeconfig

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{}

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

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

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