diff options
author | John Cai <jcai@gitlab.com> | 2019-12-19 19:43:27 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2019-12-19 19:43:27 +0300 |
commit | a4b6c71d4b7c1588587345e2dfe0c6bd7cc63a83 (patch) | |
tree | be199e568241be9d1c06df782bd25597caaa8a0c | |
parent | 3e4b6f2bec3337eeae250652af54cc4104b47b56 (diff) | |
parent | 2ca7bec0f685e8f01d9570ec45399ad5c94bdd18 (diff) |
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
-rw-r--r-- | internal/metadata/featureflag/feature_flags.go | 2 | ||||
-rw-r--r-- | internal/service/commit/filter_shas_with_signatures.go | 67 | ||||
-rw-r--r-- | internal/service/commit/filter_shas_with_signatures_test.go | 33 |
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) { |