Merge branch 'ps-filter-shas-with-signature-feature-flag-cleanup' into 'master'

Cleanup for the feature flag protection of the FilterShasWithSignatures method

See merge request gitlab-org/gitaly!1703

3 files changed, 10 insertions, 92 deletions

diff --git a/internal/metadata/featureflag/feature_flags.go b/internal/metadata/featureflag/feature_flags.go
index bc02a67af..ea26ca3c1 100644
--- a/internal/metadata/featureflag/feature_flags.go
+++ b/internal/metadata/featureflag/feature_flags.go
@@ -4,8 +4,6 @@ const (
 	// UploadPackFilter enables partial clones by sending uploadpack.allowFilter and uploadpack.allowAnySHA1InWant
 	// to upload-pack
 	UploadPackFilter = "upload_pack_filter"
-	// FilterShasWithSignaturesGo will cause the FilterShasWithSignatures RPC to use the go implementation when set
-	FilterShasWithSignaturesGo = "filter_shas_with_signatures_go"
 	// LinguistFileCountStats will invoke an additional git-linguist command to get the number of files per language
 	LinguistFileCountStats = "linguist_file_count_stats"
 	// HooksRPC will invoke update, pre receive, and post receive hooks by using RPCs
diff --git a/internal/service/commit/filter_shas_with_signatures.go b/internal/service/commit/filter_shas_with_signatures.go
index 3863bb649..1a6def272 100644
--- a/internal/service/commit/filter_shas_with_signatures.go
+++ b/internal/service/commit/filter_shas_with_signatures.go
@@ -4,27 +4,12 @@ import (
 	"errors"
 	"io"
 
-	"github.com/prometheus/client_golang/prometheus"
 	"gitlab.com/gitlab-org/gitaly/internal/git/catfile"
 	"gitlab.com/gitlab-org/gitaly/internal/git/log"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
-	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
 
-var filterShasWithSignaturesRequests = prometheus.NewCounterVec(
-	prometheus.CounterOpts{
-		Name: "gitaly_filter_shas_with_signatures_total",
-		Help: "Counter of go vs ruby implementation of FilterShasWithSignatures",
-	},
-	[]string{"implementation"},
-)
-
-func init() {
-	prometheus.MustRegister(filterShasWithSignaturesRequests)
-}
-
 func (s *server) FilterShasWithSignatures(bidi gitalypb.CommitService_FilterShasWithSignaturesServer) error {
 	firstRequest, err := bidi.Recv()
 	if err != nil {
@@ -49,16 +34,6 @@ func validateFirstFilterShasWithSignaturesRequest(in *gitalypb.FilterShasWithSig
 }
 
 func (s *server) filterShasWithSignatures(bidi gitalypb.CommitService_FilterShasWithSignaturesServer, firstRequest *gitalypb.FilterShasWithSignaturesRequest) error {
-	if featureflag.IsEnabled(bidi.Context(), featureflag.FilterShasWithSignaturesGo) {
-		filterShasWithSignaturesRequests.WithLabelValues("go").Inc()
-		return streamShasWithSignatures(bidi, firstRequest)
-	}
-
-	filterShasWithSignaturesRequests.WithLabelValues("ruby").Inc()
-	return filterShasWithSignaturesRuby(s.ruby, bidi, firstRequest)
-}
-
-func streamShasWithSignatures(bidi gitalypb.CommitService_FilterShasWithSignaturesServer, firstRequest *gitalypb.FilterShasWithSignaturesRequest) error {
 	c, err := catfile.New(bidi.Context(), firstRequest.GetRepository())
 	if err != nil {
 		return err
@@ -107,45 +82,3 @@ func filterCommitShasWithSignatures(c *catfile.Batch, shas [][]byte) ([][]byte,
 
 	return foundShas, nil
 }
-
-func filterShasWithSignaturesRuby(ruby *rubyserver.Server, bidi gitalypb.CommitService_FilterShasWithSignaturesServer, firstRequest *gitalypb.FilterShasWithSignaturesRequest) error {
-	ctx := bidi.Context()
-	client, err := ruby.CommitServiceClient(ctx)
-	if err != nil {
-		return err
-	}
-
-	clientCtx, err := rubyserver.SetHeaders(ctx, firstRequest.GetRepository())
-	if err != nil {
-		return err
-	}
-
-	rubyBidi, err := client.FilterShasWithSignatures(clientCtx)
-	if err != nil {
-		return err
-	}
-
-	if err := rubyBidi.Send(firstRequest); err != nil {
-		return err
-	}
-
-	return rubyserver.ProxyBidi(
-		func() error {
-			request, err := bidi.Recv()
-			if err != nil {
-				return err
-			}
-
-			return rubyBidi.Send(request)
-		},
-		rubyBidi,
-		func() error {
-			response, err := rubyBidi.Recv()
-			if err != nil {
-				return err
-			}
-
-			return bidi.Send(response)
-		},
-	)
-}
diff --git a/internal/service/commit/filter_shas_with_signatures_test.go b/internal/service/commit/filter_shas_with_signatures_test.go
index 1db1feba7..f5715f225 100644
--- a/internal/service/commit/filter_shas_with_signatures_test.go
+++ b/internal/service/commit/filter_shas_with_signatures_test.go
@@ -1,12 +1,10 @@
 package commit
 
 import (
-	"context"
 	"io"
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"google.golang.org/grpc/codes"
@@ -54,28 +52,17 @@ func TestFilterShasWithSignaturesSuccessful(t *testing.T) {
 		},
 	}
 
-	check := func(t *testing.T, ctx context.Context, testCases []testCase) {
-		for _, tc := range testCases {
-			t.Run(tc.desc, func(t *testing.T) {
-				stream, err := client.FilterShasWithSignatures(ctx)
-				require.NoError(t, err)
-				require.NoError(t, stream.Send(&gitalypb.FilterShasWithSignaturesRequest{Repository: testRepo, Shas: tc.in}))
-				require.NoError(t, stream.CloseSend())
-				recvOut, err := recvFSWS(stream)
-				require.NoError(t, err)
-				require.Equal(t, tc.out, recvOut)
-			})
-		}
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			stream, err := client.FilterShasWithSignatures(ctx)
+			require.NoError(t, err)
+			require.NoError(t, stream.Send(&gitalypb.FilterShasWithSignaturesRequest{Repository: testRepo, Shas: tc.in}))
+			require.NoError(t, stream.CloseSend())
+			recvOut, err := recvFSWS(stream)
+			require.NoError(t, err)
+			require.Equal(t, tc.out, recvOut)
+		})
 	}
-
-	t.Run("enabled_feature_FilterShasWithSignaturesGo", func(t *testing.T) {
-		featureCtx := featureflag.ContextWithFeatureFlag(ctx, featureflag.FilterShasWithSignaturesGo)
-		check(t, featureCtx, testCases)
-	})
-
-	t.Run("disabled_feature_FilterShasWithSignaturesGo", func(t *testing.T) {
-		check(t, ctx, testCases)
-	})
 }
 
 func TestFilterShasWithSignaturesValidationError(t *testing.T) {