As we reevaluate how to best support and maintain Staging Ref in the future, we encourage development teams using this environment to highlight their use cases in the following issue: https://gitlab.com/gitlab-com/gl-infra/software-delivery/framework/software-delivery-framework-issue-tracker/-/issues/36.

Skip to content
Snippets Groups Projects
Unverified Commit d2f64237 authored by Stan Hu's avatar Stan Hu
Browse files

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
parent a7c424fe
No related branches found
No related tags found
No related merge requests found
Loading
Loading
@@ -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)
Loading
Loading
@@ -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
}
Loading
Loading
Loading
Loading
@@ -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",
Loading
Loading
@@ -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 {
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment