git2go: Refactor featureflag subcommand to not use raw flags

The featureflag subcommand is only used for testing purposes to assert
that we can indeed pass feature flags from the `git2go.Executor()` to
the process we're about to spawn. We're using raw feature flags for this
purpose, but we're about to remove them in favor of using the type-safe
`FeatureFlag` structure.

Convert the subcommand to return a new `FeatureFlag` structure that we
can use to more thoroughly check whether things behave as expected and
remove usage of `RawFromContext()` to prepare for its removal.

Note that while this is a breaking change to the calling interface of
the `gitaly-git2go` executable, this command is only ever used in our
tests. So in practice, this doesn't matter.

2 files changed, 27 insertions, 8 deletions

diff --git a/internal/git2go/featureflags.go b/internal/git2go/featureflags.go
index cabc60e6f..bfde6867b 100644
--- a/internal/git2go/featureflags.go
+++ b/internal/git2go/featureflags.go
@@ -1,11 +1,21 @@
 package git2go
 
+// FeatureFlag is a feature flag state as seen by the `gitaly-git2go featureflag` test subcommand.
+type FeatureFlag struct {
+	// MetadataKey is the metadata key of the feature flag.
+	MetadataKey string
+	// Name is the name of the feature flag.
+	Name string
+	// Value is the value of the feature flag.
+	Value bool
+}
+
 // FeatureFlags is a struct only used by tests to confirm that feature flags are
 // being properly propagated from the git2go Executor to the gitaly-git2go
 // binary
 type FeatureFlags struct {
-	// Raw is a map of feature flags and their corresponding values
-	Raw map[string]string
+	// Flags is the set of feature flags observed by the command.
+	Flags []FeatureFlag
 	// Err is set if an error occurred. Err must exist on all gob serialized
 	// results so that any error can be returned.
 	Err error
diff --git a/internal/git2go/featureflags_test.go b/internal/git2go/featureflags_test.go
index 312bc40ea..61ebd6d31 100644
--- a/internal/git2go/featureflags_test.go
+++ b/internal/git2go/featureflags_test.go
@@ -3,7 +3,6 @@ package git2go
 import (
 	"context"
 	"encoding/gob"
-	"strconv"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -16,7 +15,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg"
 )
 
-func (b *Executor) FeatureFlags(ctx context.Context, repo repository.GitRepo) (featureflag.Raw, error) {
+func (b *Executor) FeatureFlags(ctx context.Context, repo repository.GitRepo) ([]FeatureFlag, error) {
 	output, err := b.run(ctx, repo, nil, "feature-flags")
 	if err != nil {
 		return nil, err
@@ -31,7 +30,7 @@ func (b *Executor) FeatureFlags(ctx context.Context, repo repository.GitRepo) (f
 		return nil, result.Err
 	}
 
-	return result.Raw, err
+	return result.Flags, err
 }
 
 var (
@@ -53,9 +52,19 @@ func testExecutorFeatureFlags(t *testing.T, ctx context.Context) {
 
 	executor := NewExecutor(cfg, gittest.NewCommandFactory(t, cfg), config.NewLocator(cfg))
 
-	raw, err := executor.FeatureFlags(ctx, repo)
+	flags, err := executor.FeatureFlags(ctx, repo)
 	require.NoError(t, err)
 
-	require.Equal(t, strconv.FormatBool(featureA.IsEnabled(ctx)), raw["gitaly-feature-feature-a"])
-	require.Equal(t, strconv.FormatBool(featureB.IsEnabled(ctx)), raw["gitaly-feature-feature-b"])
+	require.Subset(t, flags, []FeatureFlag{
+		{
+			Name:        "feature_a",
+			MetadataKey: "gitaly-feature-feature-a",
+			Value:       featureA.IsEnabled(ctx),
+		},
+		{
+			Name:        "feature_b",
+			MetadataKey: "gitaly-feature-feature-b",
+			Value:       featureB.IsEnabled(ctx),
+		},
+	}, flags)
 }