gitlab: Handle query parameters in REST trait

This allows us to reliably pass query parameters in the gitlab REST
trait. Else, we'd get everything URL-encoded and it would not work.

Signed-off-by: Juan Antonio Osorio <[email protected]>

internal/providers/gitlab/gitlab_rest.go

@@ -40,13 +40,11 @@ func (c *gitlabClient) GetBaseURL() string {
 
 // NewRequest implements the REST provider interface
 func (c *gitlabClient) NewRequest(method, requestPath string, body any) (*http.Request, error) {
-	base, err := url.Parse(c.glcfg.Endpoint)
+	u, err := getParsedURL(c.glcfg.Endpoint, requestPath)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse URL: %w", err)
+		return nil, err
 	}
 
-	u := base.JoinPath(requestPath)
-
 	var buf io.ReadWriter
 	if body != nil {
 		buf = &bytes.Buffer{}
@@ -118,3 +116,26 @@ func glRESTGet[T any](ctx context.Context, cli genericRESTClient, path string, o
 
 	return nil
 }
+
+func getParsedURL(endpoint, path string) (*url.URL, error) {
+	base, err := url.Parse(endpoint)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse URL: %w", err)
+	}
+
+	// Explicitly parse path and query parameters. This is to ensure that
+	// the path is properly escaped and that the query parameters are
+	// properly encoded.
+	parsedPathAndQuery, err := url.Parse(path)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse path: %w", err)
+	}
+
+	u := base.JoinPath(parsedPathAndQuery.Path)
+
+	// These have already been escaped by the URL parser
+	u.RawQuery = parsedPathAndQuery.RawQuery
+	u.Fragment = parsedPathAndQuery.Fragment
+
+	return u, nil
+}

internal/providers/gitlab/gitlab_rest_test.go

@@ -24,12 +24,11 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-
 	"github.com/stacklok/minder/internal/providers/credentials"
 	"github.com/stacklok/minder/internal/util/ptr"
 	minderv1 "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 type mockGitlabClient struct {
@@ -273,3 +272,97 @@ func Test_glRESTGet(t *testing.T) {
 		})
 	}
 }
+
+func Test_getParsedURL(t *testing.T) {
+	t.Parallel()
+
+	type args struct {
+		endpoint string
+		// This may include query parameters
+		path string
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    *url.URL
+		wantErr bool
+	}{
+		{
+			name: "valid URL",
+			args: args{
+				endpoint: "http://example.com/v1/api",
+				path:     "/test",
+			},
+			want: &url.URL{
+				Scheme: "http",
+				Host:   "example.com",
+				Path:   "/v1/api/test",
+			},
+		},
+		{
+			name: "valid URL with query parameters",
+			args: args{
+				endpoint: "http://example.com/v1/api",
+				path:     "/test?param1=value1&param2=value2",
+			},
+			want: &url.URL{
+				Scheme:   "http",
+				Host:     "example.com",
+				Path:     "/v1/api/test",
+				RawQuery: "param1=value1&param2=value2",
+			},
+		},
+		{
+			name: "valid URL with fragment identifier",
+			args: args{
+				endpoint: "http://example.com/v1/api",
+				path:     "/test#section",
+			},
+			want: &url.URL{
+				Scheme:   "http",
+				Host:     "example.com",
+				Path:     "/v1/api/test",
+				Fragment: "section",
+			},
+		},
+		{
+			name: "invalid URL",
+			args: args{
+				endpoint: "://example.com:80:80",
+				path:     "/test",
+			},
+			wantErr: true,
+		},
+		{
+			name: "query parameter that needs escaping",
+			args: args{
+				endpoint: "http://example.com",
+				path:     "/test?param1=va lue1",
+			},
+			want: &url.URL{
+				Scheme:   "http",
+				Host:     "example.com",
+				Path:     "test",
+				RawQuery: "param1=va%20lue1",
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			got, err := getParsedURL(tt.args.endpoint, tt.args.path)
+			if tt.wantErr {
+				assert.Error(t, err)
+				return
+			}
+
+			assert.NoError(t, err)
+			assert.Equal(t, tt.want.Scheme, got.Scheme, "Expected scheme to be equal")
+			assert.Equal(t, tt.want.Host, got.Host, "Expected host to be equal")
+			assert.Equal(t, tt.want.Path, got.Path, "Expected path to be equal")
+			assert.Equal(t, tt.want.Query(), got.Query(), "Expected query to be equal")
+			assert.Equal(t, tt.want.Fragment, got.Fragment, "Expected fragment to be equal")
+		})
+	}
+}