Only validate SSL cert file exists if a value is supplied

This fixes a regression in
https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/508. If an
HTTPS internal API URL were used, gitlab-shell would not work at all. We
now handle blank `caFile` properly.

Relates to https://gitlab.com/gitlab-org/gitlab-shell/-/issues/529

2 files changed, 34 insertions, 12 deletions

diff --git a/client/httpclient.go b/client/httpclient.go
index 72238f8..cdf5665 100644
--- a/client/httpclient.go
+++ b/client/httpclient.go
@@ -54,6 +54,22 @@ func WithClientCert(certPath, keyPath string) HTTPClientOpt {
 	}
 }
 
+func validateCaFile(filename string) error {
+	if filename == "" {
+		return nil
+	}
+
+	if _, err := os.Stat(filename); err != nil {
+		if os.IsNotExist(err) {
+			return fmt.Errorf("cannot find cafile '%s': %w", filename, ErrCafileNotFound)
+		}
+
+		return err
+	}
+
+	return nil
+}
+
 // Deprecated: use NewHTTPClientWithOpts - https://gitlab.com/gitlab-org/gitlab-shell/-/issues/484
 func NewHTTPClient(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, selfSignedCert bool, readTimeoutSeconds uint64) *HttpClient {
 	c, err := NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath, selfSignedCert, readTimeoutSeconds, nil)
@@ -73,10 +89,8 @@ func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath stri
 	} else if strings.HasPrefix(gitlabURL, httpProtocol) {
 		transport, host = buildHttpTransport(gitlabURL)
 	} else if strings.HasPrefix(gitlabURL, httpsProtocol) {
-		if _, err := os.Stat(caFile); err != nil {
-			if os.IsNotExist(err) {
-				return nil, fmt.Errorf("cannot find cafile '%s': %w", caFile, ErrCafileNotFound)
-			}
+		err = validateCaFile(caFile)
+		if err != nil {
 			return nil, err
 		}
 
diff --git a/client/httpsclient_test.go b/client/httpsclient_test.go
index d2c2293..debe1bd 100644
--- a/client/httpsclient_test.go
+++ b/client/httpsclient_test.go
@@ -66,10 +66,11 @@ func TestSuccessfulRequests(t *testing.T) {
 
 func TestFailedRequests(t *testing.T) {
 	testCases := []struct {
-		desc          string
-		caFile        string
-		caPath        string
-		expectedError string
+		desc                   string
+		caFile                 string
+		caPath                 string
+		expectedCaFileNotFound bool
+		expectedError          string
 	}{
 		{
 			desc:          "Invalid CaFile",
@@ -77,18 +78,25 @@ func TestFailedRequests(t *testing.T) {
 			expectedError: "Internal API unreachable",
 		},
 		{
-			desc:   "Invalid CaPath",
-			caPath: path.Join(testhelper.TestRoot, "certs/invalid"),
+			desc:                   "Missing CaFile",
+			caFile:                 path.Join(testhelper.TestRoot, "certs/invalid/missing.crt"),
+			expectedCaFileNotFound: true,
 		},
 		{
-			desc: "Empty config",
+			desc:          "Invalid CaPath",
+			caPath:        path.Join(testhelper.TestRoot, "certs/invalid"),
+			expectedError: "Internal API unreachable",
+		},
+		{
+			desc:          "Empty config",
+			expectedError: "Internal API unreachable",
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
 			client, err := setupWithRequests(t, tc.caFile, tc.caPath, "", "", "", false)
-			if tc.caFile == "" {
+			if tc.expectedCaFileNotFound {
 				require.Error(t, err)
 				require.ErrorIs(t, err, ErrCafileNotFound)
 			} else {