featureflag: Remove old interface to check for feature flags

Now that all callers have been converted to use receiver functions,
remove the old way of checking feature flags.

4 files changed, 77 insertions, 80 deletions

diff --git a/internal/metadata/featureflag/context_test.go b/internal/metadata/featureflag/context_test.go
index 095198032..391d15479 100644
--- a/internal/metadata/featureflag/context_test.go
+++ b/internal/metadata/featureflag/context_test.go
@@ -12,28 +12,28 @@ var mockFeatureFlag = FeatureFlag{"turn meow on", false}
 
 func TestIncomingCtxWithFeatureFlag(t *testing.T) {
 	ctx := context.Background()
-	require.False(t, IsEnabled(ctx, mockFeatureFlag))
+	require.False(t, mockFeatureFlag.IsEnabled(ctx))
 
 	ctx = IncomingCtxWithFeatureFlag(ctx, mockFeatureFlag)
-	require.True(t, IsEnabled(ctx, mockFeatureFlag))
+	require.True(t, mockFeatureFlag.IsEnabled(ctx))
 }
 
 func TestIncomingCtxWithDisabledFeatureFlag(t *testing.T) {
 	ctx := context.Background()
 
-	require.False(t, IsEnabled(ctx, mockFeatureFlag))
+	require.False(t, mockFeatureFlag.IsEnabled(ctx))
 
 	ctx = IncomingCtxWithDisabledFeatureFlag(ctx, mockFeatureFlag)
 
-	require.True(t, IsDisabled(ctx, mockFeatureFlag))
+	require.True(t, mockFeatureFlag.IsDisabled(ctx))
 }
 
 func TestOutgoingCtxWithFeatureFlag(t *testing.T) {
 	ctx := context.Background()
-	require.False(t, IsEnabled(ctx, mockFeatureFlag))
+	require.False(t, mockFeatureFlag.IsEnabled(ctx))
 
 	ctx = OutgoingCtxWithFeatureFlags(ctx, mockFeatureFlag)
-	require.False(t, IsEnabled(ctx, mockFeatureFlag))
+	require.False(t, mockFeatureFlag.IsEnabled(ctx))
 
 	// simulate an outgoing context leaving the process boundary and then
 	// becoming an incoming context in a new process boundary
@@ -41,5 +41,5 @@ func TestOutgoingCtxWithFeatureFlag(t *testing.T) {
 	require.True(t, ok)
 
 	ctx = metadata.NewIncomingContext(context.Background(), md)
-	require.True(t, IsEnabled(ctx, mockFeatureFlag))
+	require.True(t, mockFeatureFlag.IsEnabled(ctx))
 }
diff --git a/internal/metadata/featureflag/featureflag.go b/internal/metadata/featureflag/featureflag.go
index 268a6a4fa..bb2c1e8b7 100644
--- a/internal/metadata/featureflag/featureflag.go
+++ b/internal/metadata/featureflag/featureflag.go
@@ -1,6 +1,31 @@
 package featureflag
 
-import "context"
+import (
+	"context"
+	"strconv"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/env"
+	"google.golang.org/grpc/metadata"
+)
+
+var (
+	// featureFlagsOverride allows to enable all feature flags with a
+	// single environment variable. If the value of
+	// GITALY_TESTING_ENABLE_ALL_FEATURE_FLAGS is set to "true", then all
+	// feature flags will be enabled. This is only used for testing
+	// purposes such that we can run integration tests with feature flags.
+	featureFlagsOverride, _ = env.GetBool("GITALY_TESTING_ENABLE_ALL_FEATURE_FLAGS", false)
+
+	flagChecks = promauto.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "gitaly_feature_flag_checks_total",
+			Help: "Number of enabled/disabled checks for Gitaly server side feature flags",
+		},
+		[]string{"flag", "enabled"},
+	)
+)
 
 // FeatureFlag gates the implementation of new or changed functionality.
 type FeatureFlag struct {
@@ -11,12 +36,48 @@ type FeatureFlag struct {
 	OnByDefault bool `json:"on_by_default"`
 }
 
-// IsEnabled determines whether the feature flag is enabled in the incoming context.
+// IsEnabled checks if the feature flag is enabled for the passed context.
+// Only returns true if the metadata for the feature flag is set to "true"
 func (ff FeatureFlag) IsEnabled(ctx context.Context) bool {
-	return IsEnabled(ctx, ff)
+	if featureFlagsOverride {
+		return true
+	}
+
+	val, ok := getFlagVal(ctx, ff.Name)
+	if !ok {
+		return ff.OnByDefault
+	}
+
+	enabled := val == "true"
+
+	flagChecks.WithLabelValues(ff.Name, strconv.FormatBool(enabled)).Inc()
+
+	return enabled
 }
 
 // IsDisabled determines whether the feature flag is disabled in the incoming context.
 func (ff FeatureFlag) IsDisabled(ctx context.Context) bool {
 	return !ff.IsEnabled(ctx)
 }
+
+func getFlagVal(ctx context.Context, flag string) (string, bool) {
+	if flag == "" {
+		return "", false
+	}
+
+	md, ok := metadata.FromIncomingContext(ctx)
+	if !ok {
+		return "", false
+	}
+
+	val, ok := md[HeaderKey(flag)]
+	if !ok {
+		return "", false
+	}
+
+	if len(val) == 0 {
+		return "", false
+	}
+
+	return val[0], true
+}
diff --git a/internal/metadata/featureflag/grpc_header.go b/internal/metadata/featureflag/grpc_header.go
index f8b7709df..ed5422f91 100644
--- a/internal/metadata/featureflag/grpc_header.go
+++ b/internal/metadata/featureflag/grpc_header.go
@@ -3,83 +3,19 @@ package featureflag
 import (
 	"context"
 	"fmt"
-	"strconv"
 	"strings"
 
-	"github.com/prometheus/client_golang/prometheus"
-	"github.com/prometheus/client_golang/prometheus/promauto"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/helper/env"
 	"google.golang.org/grpc/metadata"
 )
 
-var (
-	// featureFlagsOverride allows to enable all feature flags with a
-	// single environment variable. If the value of
-	// GITALY_TESTING_ENABLE_ALL_FEATURE_FLAGS is set to "true", then all
-	// feature flags will be enabled. This is only used for testing
-	// purposes such that we can run integration tests with feature flags.
-	featureFlagsOverride, _ = env.GetBool("GITALY_TESTING_ENABLE_ALL_FEATURE_FLAGS", false)
+const (
+	// Delim is a delimiter used between a feature flag name and its value.
+	Delim = ":"
 
-	flagChecks = promauto.NewCounterVec(
-		prometheus.CounterOpts{
-			Name: "gitaly_feature_flag_checks_total",
-			Help: "Number of enabled/disabled checks for Gitaly server side feature flags",
-		},
-		[]string{"flag", "enabled"},
-	)
+	// ffPrefix is the prefix used for Gitaly-scoped feature flags.
+	ffPrefix = "gitaly-feature-"
 )
 
-// Delim is a delimiter used between a feature flag name and its value.
-const Delim = ":"
-
-// IsEnabled checks if the feature flag is enabled for the passed context.
-// Only returns true if the metadata for the feature flag is set to "true"
-func IsEnabled(ctx context.Context, flag FeatureFlag) bool {
-	if featureFlagsOverride {
-		return true
-	}
-
-	val, ok := getFlagVal(ctx, flag.Name)
-	if !ok {
-		return flag.OnByDefault
-	}
-
-	enabled := val == "true"
-
-	flagChecks.WithLabelValues(flag.Name, strconv.FormatBool(enabled)).Inc()
-
-	return enabled
-}
-
-func getFlagVal(ctx context.Context, flag string) (string, bool) {
-	if flag == "" {
-		return "", false
-	}
-
-	md, ok := metadata.FromIncomingContext(ctx)
-	if !ok {
-		return "", false
-	}
-
-	val, ok := md[HeaderKey(flag)]
-	if !ok {
-		return "", false
-	}
-
-	if len(val) == 0 {
-		return "", false
-	}
-
-	return val[0], true
-}
-
-// IsDisabled is the inverse of IsEnabled
-func IsDisabled(ctx context.Context, flag FeatureFlag) bool {
-	return !IsEnabled(ctx, flag)
-}
-
-const ffPrefix = "gitaly-feature-"
-
 // HeaderKey returns the feature flag key to be used in the metadata map
 func HeaderKey(flag string) string {
 	return ffPrefix + strings.ReplaceAll(flag, "_", "-")
diff --git a/internal/metadata/featureflag/grpc_header_test.go b/internal/metadata/featureflag/grpc_header_test.go
index 679d0d7ce..87baf6fa4 100644
--- a/internal/metadata/featureflag/grpc_header_test.go
+++ b/internal/metadata/featureflag/grpc_header_test.go
@@ -31,7 +31,7 @@ func TestGRPCMetadataFeatureFlag(t *testing.T) {
 			md := metadata.New(tc.headers)
 			ctx := metadata.NewIncomingContext(context.Background(), md)
 
-			require.Equal(t, tc.enabled, IsEnabled(ctx, FeatureFlag{tc.flag, tc.onByDefault}))
+			require.Equal(t, tc.enabled, FeatureFlag{tc.flag, tc.onByDefault}.IsEnabled(ctx))
 		})
 	}
 }