Merge branch 'ps-proxyheaderwhitelist-race' into 'master'

Race condition on `ProxyHeaderWhitelist`

Closes #2614

See merge request gitlab-org/gitaly!2025

3 files changed, 10 insertions, 9 deletions

diff --git a/changelogs/unreleased/ps-proxyheaderwhitelist-race.yml b/changelogs/unreleased/ps-proxyheaderwhitelist-race.yml
new file mode 100644
index 000000000..d7a400caf
--- /dev/null
+++ b/changelogs/unreleased/ps-proxyheaderwhitelist-race.yml
@@ -0,0 +1,5 @@
+---
+title: Race condition on ProxyHeaderWhitelist
+merge_request: 2025
+author:
+type: fixed
diff --git a/internal/rubyserver/proxy.go b/internal/rubyserver/proxy.go
index 05a35bac8..87052c900 100644
--- a/internal/rubyserver/proxy.go
+++ b/internal/rubyserver/proxy.go
@@ -11,10 +11,6 @@ import (
 	"google.golang.org/grpc/metadata"
 )
 
-// ProxyHeaderWhitelist is the list of http/2 headers that will be
-// forwarded as-is to gitaly-ruby.
-var ProxyHeaderWhitelist = []string{"gitaly-servers"}
-
 // Headers prefixed with this string get whitelisted automatically
 const rubyFeaturePrefix = "gitaly-feature-ruby-"
 
@@ -65,17 +61,18 @@ func setHeaders(ctx context.Context, repo *gitalypb.Repository, mustExist bool)
 		repoAltDirsHeader, repoAltDirsCombined,
 	)
 
+	// list of http/2 headers that will be forwarded as-is to gitaly-ruby
+	proxyHeaderWhitelist := []string{"gitaly-servers"}
+
 	if inMD, ok := metadata.FromIncomingContext(ctx); ok {
 		// Automatically whitelist any Ruby-specific feature flag
 		for header := range inMD {
 			if strings.HasPrefix(header, rubyFeaturePrefix) {
-				// TODO: this changes state of the global variable without any synchronization
-				// https://gitlab.com/gitlab-org/gitaly/-/issues/2614
-				ProxyHeaderWhitelist = append(ProxyHeaderWhitelist, header)
+				proxyHeaderWhitelist = append(proxyHeaderWhitelist, header)
 			}
 		}
 
-		for _, header := range ProxyHeaderWhitelist {
+		for _, header := range proxyHeaderWhitelist {
 			for _, v := range inMD[header] {
 				md = metadata.Join(md, metadata.Pairs(header, v))
 			}
diff --git a/internal/rubyserver/proxy_test.go b/internal/rubyserver/proxy_test.go
index 03936f41c..7233a57d9 100644
--- a/internal/rubyserver/proxy_test.go
+++ b/internal/rubyserver/proxy_test.go
@@ -31,7 +31,6 @@ func TestSetHeadersPreservesWhitelistedMetadata(t *testing.T) {
 	defer cancel()
 
 	key := "gitaly-servers"
-	require.Contains(t, ProxyHeaderWhitelist, key, "sanity check")
 	value := "test-value"
 	inCtx := metadata.NewIncomingContext(ctx, metadata.Pairs(key, value))