Fix Workhorse acceleration for encoded project IDs in API

All API endpoints support integer project IDs, but a number of others
also support the URL-encoded form (e.g. `group%2Fproject`). For the sake
of consistency, Workhorse should match on both integer and encoded
formats. Endpoints that do not support the encoded form will return 400
whether they are accelerated by Workhorse or not.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/324814

changelogs/unreleased/sh-fix-encoded-api-project-urls.yml

+---
+title: Fix Workhorse acceleration for encoded project IDs in API
+merge_request: 56731
+author:
+type: performance

workhorse/internal/api/api.go

@@ -170,6 +170,13 @@ func rebaseUrl(url *url.URL, onto *url.URL, suffix string) *url.URL {
 	newUrl.Scheme = onto.Scheme
 	newUrl.Host = onto.Host
 	if suffix != "" {
+		if newUrl.RawPath != "" {
+			// Since the GitLab API cares about encoded slashes (%2F),
+			// we need to adjust both RawPath and Path for Go to use the
+			// original encoded form. See https://go-review.googlesource.com/c/go/+/11302/.
+			newUrl.RawPath = singleJoiningSlash(url.RawPath, suffix)
+		}
+
 		newUrl.Path = singleJoiningSlash(url.Path, suffix)
 	}
 	if onto.RawQuery == "" || newUrl.RawQuery == "" {

workhorse/internal/upstream/routes.go

@@ -57,6 +57,7 @@ const (
 	ciAPIPattern         = `^/ci/api/`
 	gitProjectPattern    = `^/.+\.git/`
 	projectPattern       = `^/([^/]+/){1,}[^/]+/`
+	apiProjectPattern    = apiPattern + `v4/projects/([^/]+){1,}/` // API: Projects can be encoded via group%2Fsubgroup%2Fproject
 	snippetUploadPattern = `^/uploads/personal_snippet`
 	userUploadPattern    = `^/uploads/user`
 	importPattern        = `^/import/`
@@ -253,32 +254,39 @@ func configureRoutes(u *upstream) {
 		u.route("", apiPattern+`v4/jobs/request\z`, ciAPILongPolling),
 		u.route("", ciAPIPattern+`v1/builds/register.json\z`, ciAPILongPolling),
 
+		// Not all API endpoints support encoded project IDs
+		// (e.g. `group%2Fproject`), but for the sake of consistency we
+		// use the apiProjectPattern regex throughout. API endpoints
+		// that do not support this will return 400 regardless of
+		// whether they are accelerated by Workhorse or not.  See
+		// https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56731.
+
 		// Maven Artifact Repository
-		u.route("PUT", apiPattern+`v4/projects/[0-9]+/packages/maven/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
+		u.route("PUT", apiProjectPattern+`packages/maven/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
 
 		// Conan Artifact Repository
 		u.route("PUT", apiPattern+`v4/packages/conan/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
-		u.route("PUT", apiPattern+`v4/projects/[0-9]+/packages/conan/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
+		u.route("PUT", apiProjectPattern+`packages/conan/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
 
 		// Generic Packages Repository
-		u.route("PUT", apiPattern+`v4/projects/[0-9]+/packages/generic/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
+		u.route("PUT", apiProjectPattern+`packages/generic/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
 
 		// NuGet Artifact Repository
-		u.route("PUT", apiPattern+`v4/projects/[0-9]+/packages/nuget/`, upload.Accelerate(api, signingProxy, preparers.packages)),
+		u.route("PUT", apiProjectPattern+`packages/nuget/`, upload.Accelerate(api, signingProxy, preparers.packages)),
 
 		// PyPI Artifact Repository
-		u.route("POST", apiPattern+`v4/projects/[0-9]+/packages/pypi`, upload.Accelerate(api, signingProxy, preparers.packages)),
+		u.route("POST", apiProjectPattern+`packages/pypi`, upload.Accelerate(api, signingProxy, preparers.packages)),
 
 		// Debian Artifact Repository
-		u.route("PUT", apiPattern+`v4/projects/[0-9]+/packages/debian/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
+		u.route("PUT", apiProjectPattern+`packages/debian/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
 
 		// Gem Artifact Repository
-		u.route("POST", apiPattern+`v4/projects/[0-9]+/packages/rubygems/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
+		u.route("POST", apiProjectPattern+`packages/rubygems/`, upload.BodyUploader(api, signingProxy, preparers.packages)),
 
 		// We are porting API to disk acceleration
 		// we need to declare each routes until we have fixed all the routes on the rails codebase.
 		// Overall status can be seen at https://gitlab.com/groups/gitlab-org/-/epics/1802#current-status
-		u.route("POST", apiPattern+`v4/projects/[0-9]+/wikis/attachments\z`, uploadAccelerateProxy),
+		u.route("POST", apiProjectPattern+`wikis/attachments\z`, uploadAccelerateProxy),
 		u.route("POST", apiPattern+`graphql\z`, uploadAccelerateProxy),
 		u.route("POST", apiPattern+`v4/groups/import`, upload.Accelerate(api, signingProxy, preparers.uploads)),
 		u.route("POST", apiPattern+`v4/projects/import`, upload.Accelerate(api, signingProxy, preparers.uploads)),
@@ -289,7 +297,7 @@ func configureRoutes(u *upstream) {
 		u.route("POST", importPattern+`gitlab_group`, upload.Accelerate(api, signingProxy, preparers.uploads)),
 
 		// Metric image upload
-		u.route("POST", apiPattern+`v4/projects/[0-9]+/issues/[0-9]+/metric_images\z`, upload.Accelerate(api, signingProxy, preparers.uploads)),
+		u.route("POST", apiProjectPattern+`issues/[0-9]+/metric_images\z`, upload.Accelerate(api, signingProxy, preparers.uploads)),
 
 		// Requirements Import via UI upload acceleration
 		u.route("POST", projectPattern+`requirements_management/requirements/import_csv`, upload.Accelerate(api, signingProxy, preparers.uploads)),

workhorse/upload_test.go

@@ -41,7 +41,7 @@ func testArtifactsUpload(t *testing.T, uploadArtifacts uploadArtifactsFunction)
 	reqBody, contentType, err := multipartBodyWithFile()
 	require.NoError(t, err)
 
-	ts := signedUploadTestServer(t, nil)
+	ts := signedUploadTestServer(t, nil, nil)
 	defer ts.Close()
 
 	ws := startWorkhorseServer(ts.URL)
@@ -66,7 +66,7 @@ func expectSignedRequest(t *testing.T, r *http.Request) {
 	require.NoError(t, err)
 }
 
-func uploadTestServer(t *testing.T, extraTests func(r *http.Request)) *httptest.Server {
+func uploadTestServer(t *testing.T, authorizeTests func(r *http.Request), extraTests func(r *http.Request)) *httptest.Server {
 	return testhelper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) {
 		if strings.HasSuffix(r.URL.Path, "/authorize") {
 			expectSignedRequest(t, r)
@@ -74,6 +74,10 @@ func uploadTestServer(t *testing.T, extraTests func(r *http.Request)) *httptest.
 			w.Header().Set("Content-Type", api.ResponseContentType)
 			_, err := fmt.Fprintf(w, `{"TempPath":"%s"}`, scratchDir)
 			require.NoError(t, err)
+
+			if authorizeTests != nil {
+				authorizeTests(r)
+			}
 			return
 		}
 
@@ -91,10 +95,10 @@ func uploadTestServer(t *testing.T, extraTests func(r *http.Request)) *httptest.
 	})
 }
 
-func signedUploadTestServer(t *testing.T, extraTests func(r *http.Request)) *httptest.Server {
+func signedUploadTestServer(t *testing.T, authorizeTests func(r *http.Request), extraTests func(r *http.Request)) *httptest.Server {
 	t.Helper()
 
-	return uploadTestServer(t, func(r *http.Request) {
+	return uploadTestServer(t, authorizeTests, func(r *http.Request) {
 		expectSignedRequest(t, r)
 
 		if extraTests != nil {
@@ -113,20 +117,28 @@ func TestAcceleratedUpload(t *testing.T) {
 		{"POST", `/uploads/personal_snippet`, true},
 		{"POST", `/uploads/user`, true},
 		{"POST", `/api/v4/projects/1/wikis/attachments`, false},
+		{"POST", `/api/v4/projects/group%2Fproject/wikis/attachments`, false},
 		{"POST", `/api/graphql`, false},
 		{"PUT", "/api/v4/projects/9001/packages/nuget/v1/files", true},
+		{"PUT", "/api/v4/projects/group%2Fproject/packages/nuget/v1/files", true},
 		{"POST", `/api/v4/groups/import`, true},
 		{"POST", `/api/v4/projects/import`, true},
 		{"POST", `/import/gitlab_project`, true},
 		{"POST", `/import/gitlab_group`, true},
 		{"POST", `/api/v4/projects/9001/packages/pypi`, true},
+		{"POST", `/api/v4/projects/group%2Fproject/packages/pypi`, true},
 		{"POST", `/api/v4/projects/9001/issues/30/metric_images`, true},
+		{"POST", `/api/v4/projects/project%2Fgroup/issues/30/metric_images`, true},
 		{"POST", `/my/project/-/requirements_management/requirements/import_csv`, true},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.resource, func(t *testing.T) {
 			ts := uploadTestServer(t,
+				func(r *http.Request) {
+					// Validate %2F characters haven't been unescaped
+					require.Equal(t, tt.resource+"/authorize", r.URL.String())
+				},
 				func(r *http.Request) {
 					if tt.signedFinalization {
 						expectSignedRequest(t, r)
@@ -433,6 +445,11 @@ func TestPackageFilesUpload(t *testing.T) {
 		{"PUT", "/api/v4/projects/2412/packages/generic/mypackage/0.0.1/myfile.tar.gz"},
 		{"PUT", "/api/v4/projects/2412/packages/debian/libsample0_1.2.3~alpha2-1_amd64.deb"},
 		{"POST", "/api/v4/projects/2412/packages/rubygems/api/v1/gems/sample.gem"},
+		{"PUT", "/api/v4/projects/group%2Fproject/packages/conan/v1/files"},
+		{"PUT", "/api/v4/projects/group%2Fproject/packages/maven/v1/files"},
+		{"PUT", "/api/v4/projects/group%2Fproject/packages/generic/mypackage/0.0.1/myfile.tar.gz"},
+		{"PUT", "/api/v4/projects/group%2Fproject/packages/debian/libsample0_1.2.3~alpha2-1_amd64.deb"},
+		{"POST", "/api/v4/projects/group%2Fproject/packages/rubygems/api/v1/gems/sample.gem"},
 	}
 
 	for _, r := range routes {