Merge branch 'cat-geo-except-internalapi-from-proxying' into 'master'

Exclude internal API and Geo git_ssh routes from secondary proxying

See merge request gitlab-org/gitlab!73202

workhorse/internal/upstream/routes.go

@@ -374,6 +374,10 @@ func configureRoutes(u *upstream) {
 		// Geo API routes
 		u.route("", "^/api/v4/geo_nodes", defaultUpstream),
 		u.route("", "^/api/v4/geo_replication", defaultUpstream),
+		u.route("", "^/api/v4/geo/proxy_git_ssh", defaultUpstream),
+
+		// Internal API routes
+		u.route("", "^/api/v4/internal", defaultUpstream),
 
 		// Don't define a catch-all route. If a route does not match, then we know
 		// the request should be proxied.

workhorse/internal/upstream/routes_test.go

+package upstream
+
+import (
+	"testing"
+)
+
+func TestProjectNotExistingGitHttpPullWithGeoProxy(t *testing.T) {
+	testCases := []testCase{
+		{"secondary info/refs", "/group/project.git/info/refs", "Local Rails server received request to path /group/project.git/info/refs"},
+		{"primary info/refs", "/-/push_from_secondary/2/group/project.git/info/refs", "Geo primary received request to path /-/push_from_secondary/2/group/project.git/info/refs"},
+		{"primary upload-pack", "/-/push_from_secondary/2/group/project.git/git-upload-pack", "Geo primary received request to path /-/push_from_secondary/2/group/project.git/git-upload-pack"},
+	}
+
+	runTestCasesWithGeoProxyEnabled(t, testCases)
+}
+
+func TestProjectNotExistingGitHttpPushWithGeoProxy(t *testing.T) {
+	testCases := []testCase{
+		{"secondary info/refs", "/group/project.git/info/refs", "Local Rails server received request to path /group/project.git/info/refs"},
+		{"primary info/refs", "/-/push_from_secondary/2/group/project.git/info/refs", "Geo primary received request to path /-/push_from_secondary/2/group/project.git/info/refs"},
+		{"primary receive-pack", "/-/push_from_secondary/2/group/project.git/git-receive-pack", "Geo primary received request to path /-/push_from_secondary/2/group/project.git/git-receive-pack"},
+	}
+
+	runTestCasesWithGeoProxyEnabled(t, testCases)
+}
+
+func TestProjectNotExistingGitSSHPullWithGeoProxy(t *testing.T) {
+	testCases := []testCase{
+		{"GitLab Shell call to authorized-keys", "/api/v4/internal/authorized_keys", "Local Rails server received request to path /api/v4/internal/authorized_keys"},
+		{"GitLab Shell call to allowed", "/api/v4/internal/allowed", "Local Rails server received request to path /api/v4/internal/allowed"},
+		{"GitLab Shell call to info/refs", "/api/v4/geo/proxy_git_ssh/info_refs_receive_pack", "Local Rails server received request to path /api/v4/geo/proxy_git_ssh/info_refs_receive_pack"},
+		{"GitLab Shell call to receive_pack", "/api/v4/geo/proxy_git_ssh/receive_pack", "Local Rails server received request to path /api/v4/geo/proxy_git_ssh/receive_pack"},
+	}
+
+	runTestCasesWithGeoProxyEnabled(t, testCases)
+}
+
+func TestProjectNotExistingGitSSHPushWithGeoProxy(t *testing.T) {
+	testCases := []testCase{
+		{"GitLab Shell call to authorized-keys", "/api/v4/internal/authorized_keys", "Local Rails server received request to path /api/v4/internal/authorized_keys"},
+		{"GitLab Shell call to allowed", "/api/v4/internal/allowed", "Local Rails server received request to path /api/v4/internal/allowed"},
+		{"GitLab Shell call to info/refs", "/api/v4/geo/proxy_git_ssh/info_refs_upload_pack", "Local Rails server received request to path /api/v4/geo/proxy_git_ssh/info_refs_upload_pack"},
+		{"GitLab Shell call to receive_pack", "/api/v4/geo/proxy_git_ssh/upload_pack", "Local Rails server received request to path /api/v4/geo/proxy_git_ssh/upload_pack"},
+	}
+
+	runTestCasesWithGeoProxyEnabled(t, testCases)
+}

workhorse/internal/upstream/upstream_test.go

@@ -88,16 +88,6 @@ func TestGeoProxyFeatureDisabledOnGeoSecondarySite(t *testing.T) {
 }
 
 func TestGeoProxyFeatureEnabledOnGeoSecondarySite(t *testing.T) {
-	remoteServer, rsDeferredClose := startRemoteServer("Geo primary")
-	defer rsDeferredClose()
-
-	geoProxyEndpointResponseBody := fmt.Sprintf(`{"geo_proxy_url":"%v"}`, remoteServer.URL)
-	railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
-	defer deferredClose()
-
-	ws, wsDeferredClose, _ := startWorkhorseServer(railsServer.URL, true)
-	defer wsDeferredClose()
-
 	testCases := []testCase{
 		{"push from secondary is forwarded", "/-/push_from_secondary/foo/bar.git/info/refs", "Geo primary received request to path /-/push_from_secondary/foo/bar.git/info/refs"},
 		{"LFS files are served locally", "/group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6", "Local Rails server received request to path /group/project.git/gitlab-lfs/objects/37446575700829a11278ad3a550f244f45d5ae4fe1552778fa4f041f9eaeecf6"},
@@ -106,7 +96,7 @@ func TestGeoProxyFeatureEnabledOnGeoSecondarySite(t *testing.T) {
 		{"unknown route is forwarded", "/anything", "Geo primary received request to path /anything"},
 	}
 
-	runTestCases(t, ws, testCases)
+	runTestCasesWithGeoProxyEnabled(t, testCases)
 }
 
 // This test can be removed when the environment variable `GEO_SECONDARY_PROXY` is removed
@@ -227,6 +217,20 @@ func runTestCases(t *testing.T, ws *httptest.Server, testCases []testCase) {
 	}
 }
 
+func runTestCasesWithGeoProxyEnabled(t *testing.T, testCases []testCase) {
+	remoteServer, rsDeferredClose := startRemoteServer("Geo primary")
+	defer rsDeferredClose()
+
+	geoProxyEndpointResponseBody := fmt.Sprintf(`{"geo_proxy_url":"%v"}`, remoteServer.URL)
+	railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
+	defer deferredClose()
+
+	ws, wsDeferredClose, _ := startWorkhorseServer(railsServer.URL, true)
+	defer wsDeferredClose()
+
+	runTestCases(t, ws, testCases)
+}
+
 func newUpstreamConfig(authBackend string) *config.Config {
 	return &config.Config{
 		Version:            "123",
@@ -284,9 +288,13 @@ func startWorkhorseServer(railsServerURL string, enableGeoProxyFeature bool) (*h
 	}
 	cfg := newUpstreamConfig(railsServerURL)
 	upstreamHandler := newUpstream(*cfg, logrus.StandardLogger(), myConfigureRoutes)
-	ws := httptest.NewServer(upstreamHandler)
+
+	// Secret should be configured before the first Geo API poll happens on server start
+	// to prevent race conditions where the first API call happens without a secret path
 	testhelper.ConfigureSecret()
 
+	ws := httptest.NewServer(upstreamHandler)
+
 	waitForNextApiPoll := func() {}
 
 	if enableGeoProxyFeature {