Merge branch 'ps-feature-flag-name-convertion' into 'master'

Gitaly feature flags are broken: convert underscores to dashes

Closes #2216

See merge request gitlab-org/gitaly!1659

3 files changed, 20 insertions, 10 deletions

diff --git a/changelogs/unreleased/ps-feature-flag-name-convertion.yml b/changelogs/unreleased/ps-feature-flag-name-convertion.yml
new file mode 100644
index 000000000..ef8cec605
--- /dev/null
+++ b/changelogs/unreleased/ps-feature-flag-name-convertion.yml
@@ -0,0 +1,5 @@
+---
+title: 'Gitaly feature flags are broken: convert underscores to dashes'
+merge_request: 1659
+author:
+type: fixed
diff --git a/internal/metadata/featureflag/grpc_header.go b/internal/metadata/featureflag/grpc_header.go
index e71f1cfe9..3d88c80d1 100644
--- a/internal/metadata/featureflag/grpc_header.go
+++ b/internal/metadata/featureflag/grpc_header.go
@@ -3,6 +3,7 @@ package featureflag
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	"google.golang.org/grpc/metadata"
 )
@@ -34,5 +35,5 @@ func IsDisabled(ctx context.Context, flag string) bool {
 
 // HeaderKey returns the feature flag key to be used in the metadata map
 func HeaderKey(flag string) string {
-	return fmt.Sprintf("gitaly-feature-%s", flag)
+	return fmt.Sprintf("gitaly-feature-%s", strings.ReplaceAll(flag, "_", "-"))
 }
diff --git a/internal/metadata/featureflag/grpc_header_test.go b/internal/metadata/featureflag/grpc_header_test.go
index 8d18c6037..4fceaf87c 100644
--- a/internal/metadata/featureflag/grpc_header_test.go
+++ b/internal/metadata/featureflag/grpc_header_test.go
@@ -13,19 +13,23 @@ func TestGRPCMetadataFeatureFlag(t *testing.T) {
 		flag    string
 		headers map[string]string
 		enabled bool
+		desc    string
 	}{
-		{"", nil, false},
-		{"flag", nil, false},
-		{"flag", map[string]string{"flag": "true"}, false},
-		{"flag", map[string]string{"gitaly-feature-flag": "TRUE"}, false},
-		{"flag", map[string]string{"gitaly-feature-flag": "true"}, true},
+		{"", nil, false, "empty name and no headers"},
+		{"flag", nil, false, "no headers"},
+		{"flag", map[string]string{"flag": "true"}, false, "no 'gitaly-feature' prefix in flag name"},
+		{"flag", map[string]string{"gitaly-feature-flag": "TRUE"}, false, "not valid header value"},
+		{"flag_under_score", map[string]string{"gitaly-feature-flag-under-score": "true"}, true, "flag name with underscores"},
+		{"flag-dash-ok", map[string]string{"gitaly-feature-flag-dash-ok": "true"}, true, "flag name with dashes"},
 	}
 
 	for _, tc := range testCases {
-		md := metadata.New(tc.headers)
-		ctx := metadata.NewIncomingContext(context.Background(), md)
+		t.Run(tc.desc, func(t *testing.T) {
+			md := metadata.New(tc.headers)
+			ctx := metadata.NewIncomingContext(context.Background(), md)
 
-		assert.Equal(t, tc.enabled, IsEnabled(ctx, tc.flag))
-		assert.NotEqual(t, tc.enabled, IsDisabled(ctx, tc.flag))
+			assert.Equal(t, tc.enabled, IsEnabled(ctx, tc.flag))
+			assert.NotEqual(t, tc.enabled, IsDisabled(ctx, tc.flag))
+		})
 	}
 }