diff options
-rw-r--r-- | cmd/gitaly-hooks/hooks.go | 2 | ||||
-rw-r--r-- | cmd/gitaly-hooks/hooks_test.go | 68 | ||||
-rw-r--r-- | internal/git/hooks_options.go | 3 | ||||
-rw-r--r-- | internal/git/hooks_payload.go | 6 | ||||
-rw-r--r-- | internal/git/hooks_payload_test.go | 14 | ||||
-rw-r--r-- | internal/gitaly/hook/postreceive_test.go | 13 | ||||
-rw-r--r-- | internal/gitaly/hook/prereceive_test.go | 13 | ||||
-rw-r--r-- | internal/gitaly/hook/transactions_test.go | 9 | ||||
-rw-r--r-- | internal/gitaly/hook/update_test.go | 11 | ||||
-rw-r--r-- | internal/gitaly/service/hook/post_receive_test.go | 20 | ||||
-rw-r--r-- | internal/gitaly/service/hook/pre_receive_test.go | 56 | ||||
-rw-r--r-- | internal/gitaly/service/hook/reference_transaction_test.go | 1 | ||||
-rw-r--r-- | internal/gitaly/service/hook/update_test.go | 24 | ||||
-rw-r--r-- | internal/gitaly/service/operations/update_with_hooks.go | 3 | ||||
-rw-r--r-- | internal/gitaly/service/operations/update_with_hooks_test.go | 3 | ||||
-rw-r--r-- | internal/gitaly/service/smarthttp/receive_pack_test.go | 32 | ||||
-rw-r--r-- | internal/gitaly/service/smarthttp/testhelper_test.go | 23 |
17 files changed, 227 insertions, 74 deletions
diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go index 70ea5d83e..ffd61ead9 100644 --- a/cmd/gitaly-hooks/hooks.go +++ b/cmd/gitaly-hooks/hooks.go @@ -17,6 +17,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/stream" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/streamio" @@ -120,6 +121,7 @@ func main() { } hookClient := gitalypb.NewHookServiceClient(conn) + ctx = featureflag.OutgoingWithRaw(ctx, payload.FeatureFlags) returnCode, err := hookCommand.exec(ctx, payload, hookClient, os.Args) if err != nil { logger.Fatal(err) diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index d2e0ffa8f..de1633e95 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "context" "encoding/json" "fmt" "io/ioutil" @@ -13,6 +14,7 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/backchannel" "gitlab.com/gitlab-org/gitaly/internal/command" @@ -25,6 +27,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/hook" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" gitalylog "gitlab.com/gitlab-org/gitaly/internal/log" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" @@ -41,13 +44,22 @@ type proxyValues struct { HTTPProxy, HTTPSProxy, NoProxy string } +var enabledFeatureFlag = featureflag.FeatureFlag{Name: "enabled-feature-flag", OnByDefault: false} +var disabledFeatureFlag = featureflag.FeatureFlag{Name: "disabled-feature-flag", OnByDefault: true} + +func rawFeatureFlags() featureflag.Raw { + ctx := featureflag.IncomingCtxWithFeatureFlag(context.Background(), enabledFeatureFlag) + ctx = featureflag.IncomingCtxWithDisabledFeatureFlag(ctx, disabledFeatureFlag) + return featureflag.RawFromContext(ctx) +} + // envForHooks generates a set of environment variables for gitaly hooks func envForHooks(t testing.TB, cfg config.Cfg, repo *gitalypb.Repository, glHookValues glHookValues, proxyValues proxyValues, gitPushOptions ...string) []string { payload, err := git.NewHooksPayload(cfg, repo, nil, nil, &git.ReceiveHooksPayload{ UserID: glHookValues.GLID, Username: glHookValues.GLUsername, Protocol: glHookValues.GLProtocol, - }, git.AllHooks).Env() + }, git.AllHooks, rawFeatureFlags()).Env() require.NoError(t, err) env := append(os.Environ(), []string{ @@ -344,9 +356,6 @@ func TestHooksPostReceiveFailed(t *testing.T) { gitlabAPI, err := gitalyhook.NewGitlabAPI(cfg.Gitlab, cfg.TLS) require.NoError(t, err) - stop := runHookServiceServerWithAPI(t, cfg, gitlabAPI) - defer stop() - customHookOutputPath := gittest.WriteEnvToCustomHook(t, repoPath, "post-receive") var stdout, stderr bytes.Buffer @@ -389,6 +398,9 @@ func TestHooksPostReceiveFailed(t *testing.T) { for _, tc := range testcases { t.Run(tc.desc, func(t *testing.T) { + stop := runHookServiceServerWithAPI(t, cfg, gitlabAPI) + defer stop() + hooksPayload, err := git.NewHooksPayload( cfg, repo, @@ -407,6 +419,7 @@ func TestHooksPostReceiveFailed(t *testing.T) { Protocol: glProtocol, }, git.PostReceiveHook, + rawFeatureFlags(), ).Env() require.NoError(t, err) @@ -572,6 +585,41 @@ func runHookServiceServer(t *testing.T, cfg config.Cfg) func() { return runHookServiceServerWithAPI(t, cfg, gitalyhook.GitlabAPIStub) } +type featureFlagAsserter struct { + t testing.TB + wrapped gitalypb.HookServiceServer +} + +func (svc featureFlagAsserter) assertFlags(ctx context.Context) { + assert.True(svc.t, featureflag.IsEnabled(ctx, enabledFeatureFlag)) + assert.True(svc.t, featureflag.IsDisabled(ctx, disabledFeatureFlag)) +} + +func (svc featureFlagAsserter) PreReceiveHook(stream gitalypb.HookService_PreReceiveHookServer) error { + svc.assertFlags(stream.Context()) + return svc.wrapped.PreReceiveHook(stream) +} + +func (svc featureFlagAsserter) PostReceiveHook(stream gitalypb.HookService_PostReceiveHookServer) error { + svc.assertFlags(stream.Context()) + return svc.wrapped.PostReceiveHook(stream) +} + +func (svc featureFlagAsserter) UpdateHook(request *gitalypb.UpdateHookRequest, stream gitalypb.HookService_UpdateHookServer) error { + svc.assertFlags(stream.Context()) + return svc.wrapped.UpdateHook(request, stream) +} + +func (svc featureFlagAsserter) ReferenceTransactionHook(stream gitalypb.HookService_ReferenceTransactionHookServer) error { + svc.assertFlags(stream.Context()) + return svc.wrapped.ReferenceTransactionHook(stream) +} + +func (svc featureFlagAsserter) PackObjectsHook(stream gitalypb.HookService_PackObjectsHookServer) error { + svc.assertFlags(stream.Context()) + return svc.wrapped.PackObjectsHook(stream) +} + func runHookServiceServerWithAPI(t *testing.T, cfg config.Cfg, gitlabAPI gitalyhook.GitlabAPI) func() { registry := backchannel.NewRegistry() txManager := transaction.NewManager(cfg, registry) @@ -579,7 +627,9 @@ func runHookServiceServerWithAPI(t *testing.T, cfg config.Cfg, gitlabAPI gitalyh gitCmdFactory := git.NewExecCommandFactory(cfg) server := testhelper.NewServerWithAuth(t, nil, nil, cfg.Auth.Token, registry, testhelper.WithInternalSocket(cfg)) - gitalypb.RegisterHookServiceServer(server.GrpcServer(), hook.NewServer(cfg, hookManager, gitCmdFactory)) + gitalypb.RegisterHookServiceServer(server.GrpcServer(), featureFlagAsserter{ + t: t, wrapped: hook.NewServer(cfg, hookManager, gitCmdFactory), + }) reflection.Register(server.GrpcServer()) server.Start(t) @@ -622,8 +672,6 @@ func TestGitalyHooksPackObjects(t *testing.T) { testhelper.ConfigureGitalyHooksBin(t, cfg) testhelper.ConfigureGitalySSHBin(t, cfg) - defer runHookServiceServer(t, cfg)() - env := envForHooks(t, cfg, repo, glHookValues{}, proxyValues{}) baseArgs := []string{ @@ -647,6 +695,8 @@ func TestGitalyHooksPackObjects(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { + defer runHookServiceServer(t, cfg)() + tempDir, cleanTempDir := testhelper.TempDir(t) defer cleanTempDir() @@ -675,7 +725,7 @@ func TestRequestedHooks(t *testing.T) { testhelper.ConfigureGitalyHooksBin(t, cfg) testhelper.ConfigureGitalySSHBin(t, cfg) - payload, err := git.NewHooksPayload(cfg, &gitalypb.Repository{}, nil, nil, nil, git.AllHooks&^hook).Env() + payload, err := git.NewHooksPayload(cfg, &gitalypb.Repository{}, nil, nil, nil, git.AllHooks&^hook, nil).Env() require.NoError(t, err) cmd := exec.Command(filepath.Join(cfg.BinDir, "gitaly-hooks"), hookName) @@ -688,7 +738,7 @@ func TestRequestedHooks(t *testing.T) { testhelper.ConfigureGitalyHooksBin(t, cfg) testhelper.ConfigureGitalySSHBin(t, cfg) - payload, err := git.NewHooksPayload(cfg, &gitalypb.Repository{}, nil, nil, nil, hook).Env() + payload, err := git.NewHooksPayload(cfg, &gitalypb.Repository{}, nil, nil, nil, hook, nil).Env() require.NoError(t, err) cmd := exec.Command(filepath.Join(cfg.BinDir, "gitaly-hooks"), hookName) diff --git a/internal/git/hooks_options.go b/internal/git/hooks_options.go index bad8d4645..84af8d627 100644 --- a/internal/git/hooks_options.go +++ b/internal/git/hooks_options.go @@ -10,6 +10,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/repository" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/log" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -88,7 +89,7 @@ func (cc *cmdCfg) configureHooks( return err } - payload, err := NewHooksPayload(cfg, repo, transaction, praefect, receiveHooksPayload, requestedHooks).Env() + payload, err := NewHooksPayload(cfg, repo, transaction, praefect, receiveHooksPayload, requestedHooks, featureflag.RawFromContext(ctx)).Env() if err != nil { return err } diff --git a/internal/git/hooks_payload.go b/internal/git/hooks_payload.go index 97e33ca80..f98fffadd 100644 --- a/internal/git/hooks_payload.go +++ b/internal/git/hooks_payload.go @@ -9,6 +9,7 @@ import ( "github.com/golang/protobuf/jsonpb" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -56,6 +57,9 @@ type HooksPayload struct { // RequestedHooks is a bitfield of requested Hooks. Hooks which // were not requested will not get executed. RequestedHooks Hook `json:"requested_hooks"` + // FeatureFlags contains feature flags with their values. They are set + // into the outgoing context when calling HookService. + FeatureFlags featureflag.Raw `json:"feature_flags,omitempty"` // Repo is the repository in which the hook is running. Repo *gitalypb.Repository `json:"-"` @@ -110,6 +114,7 @@ func NewHooksPayload( praefect *metadata.PraefectServer, receiveHooksPayload *ReceiveHooksPayload, requestedHooks Hook, + featureFlags featureflag.Raw, ) HooksPayload { return HooksPayload{ Repo: repo, @@ -121,6 +126,7 @@ func NewHooksPayload( Praefect: praefect, ReceiveHooksPayload: receiveHooksPayload, RequestedHooks: requestedHooks, + FeatureFlags: featureFlags, } } diff --git a/internal/git/hooks_payload_test.go b/internal/git/hooks_payload_test.go index 351f37f19..243f80df5 100644 --- a/internal/git/hooks_payload_test.go +++ b/internal/git/hooks_payload_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" ) @@ -28,13 +29,13 @@ func TestHooksPayload(t *testing.T) { } t.Run("envvar has proper name", func(t *testing.T) { - env, err := git.NewHooksPayload(cfg, repo, nil, nil, nil, git.AllHooks).Env() + env, err := git.NewHooksPayload(cfg, repo, nil, nil, nil, git.AllHooks, nil).Env() require.NoError(t, err) require.True(t, strings.HasPrefix(env, git.EnvHooksPayload+"=")) }) t.Run("roundtrip succeeds", func(t *testing.T) { - env, err := git.NewHooksPayload(cfg, repo, nil, nil, nil, git.PreReceiveHook).Env() + env, err := git.NewHooksPayload(cfg, repo, nil, nil, nil, git.PreReceiveHook, featureflag.Raw{"flag-key": "flag-value"}).Env() require.NoError(t, err) payload, err := git.HooksPayloadFromEnv([]string{ @@ -51,11 +52,12 @@ func TestHooksPayload(t *testing.T) { GitPath: cfg.Git.BinPath, InternalSocket: cfg.GitalyInternalSocketPath(), RequestedHooks: git.PreReceiveHook, + FeatureFlags: featureflag.Raw{"flag-key": "flag-value"}, }, payload) }) t.Run("roundtrip with transaction succeeds", func(t *testing.T) { - env, err := git.NewHooksPayload(cfg, repo, &tx, &praefect, nil, git.UpdateHook).Env() + env, err := git.NewHooksPayload(cfg, repo, &tx, &praefect, nil, git.UpdateHook, nil).Env() require.NoError(t, err) payload, err := git.HooksPayloadFromEnv([]string{env}) @@ -84,7 +86,7 @@ func TestHooksPayload(t *testing.T) { }) t.Run("payload with missing Praefect", func(t *testing.T) { - env, err := git.NewHooksPayload(cfg, repo, &tx, nil, nil, git.AllHooks).Env() + env, err := git.NewHooksPayload(cfg, repo, &tx, nil, nil, git.AllHooks, nil).Env() require.NoError(t, err) _, err = git.HooksPayloadFromEnv([]string{env}) @@ -96,7 +98,7 @@ func TestHooksPayload(t *testing.T) { UserID: "1234", Username: "user", Protocol: "ssh", - }, git.PostReceiveHook).Env() + }, git.PostReceiveHook, nil).Env() require.NoError(t, err) payload, err := git.HooksPayloadFromEnv([]string{ @@ -126,7 +128,7 @@ func TestHooksPayload(t *testing.T) { cfg, repo, _ := testcfg.BuildWithRepo(t) cfg.Git.BinPath = "" - env, err := git.NewHooksPayload(cfg, repo, nil, nil, nil, git.ReceivePackHooks).Env() + env, err := git.NewHooksPayload(cfg, repo, nil, nil, nil, git.ReceivePackHooks, nil).Env() require.NoError(t, err) payload, err := git.HooksPayloadFromEnv([]string{ diff --git a/internal/gitaly/hook/postreceive_test.go b/internal/gitaly/hook/postreceive_test.go index 17dd2642f..d4ae1d00e 100644 --- a/internal/gitaly/hook/postreceive_test.go +++ b/internal/gitaly/hook/postreceive_test.go @@ -15,6 +15,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" @@ -72,7 +73,10 @@ func TestPostReceive_customHook(t *testing.T) { Protocol: "web", } - payload, err := git.NewHooksPayload(cfg, repo, nil, nil, receiveHooksPayload, git.PostReceiveHook).Env() + ctx, cleanup := testhelper.Context() + defer cleanup() + + payload, err := git.NewHooksPayload(cfg, repo, nil, nil, receiveHooksPayload, git.PostReceiveHook, featureflag.RawFromContext(ctx)).Env() require.NoError(t, err) primaryPayload, err := git.NewHooksPayload( @@ -87,6 +91,7 @@ func TestPostReceive_customHook(t *testing.T) { }, receiveHooksPayload, git.PostReceiveHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) @@ -102,6 +107,7 @@ func TestPostReceive_customHook(t *testing.T) { }, receiveHooksPayload, git.PostReceiveHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) @@ -201,9 +207,6 @@ func TestPostReceive_customHook(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - ctx, cleanup := testhelper.Context() - defer cleanup() - gittest.WriteCustomHook(t, repoPath, "post-receive", []byte(tc.hook)) var stdout, stderr bytes.Buffer @@ -248,7 +251,7 @@ func TestPostReceive_gitlab(t *testing.T) { UserID: "1234", Username: "user", Protocol: "web", - }, git.PostReceiveHook).Env() + }, git.PostReceiveHook, nil).Env() require.NoError(t, err) standardEnv := []string{payload} diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go index 48537a048..ee0054093 100644 --- a/internal/gitaly/hook/prereceive_test.go +++ b/internal/gitaly/hook/prereceive_test.go @@ -15,6 +15,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" @@ -31,7 +32,10 @@ func TestPrereceive_customHooks(t *testing.T) { Protocol: "web", } - payload, err := git.NewHooksPayload(cfg, repo, nil, nil, receiveHooksPayload, git.PreReceiveHook).Env() + ctx, cleanup := testhelper.Context() + defer cleanup() + + payload, err := git.NewHooksPayload(cfg, repo, nil, nil, receiveHooksPayload, git.PreReceiveHook, featureflag.RawFromContext(ctx)).Env() require.NoError(t, err) primaryPayload, err := git.NewHooksPayload( @@ -46,6 +50,7 @@ func TestPrereceive_customHooks(t *testing.T) { }, receiveHooksPayload, git.PreReceiveHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) @@ -61,6 +66,7 @@ func TestPrereceive_customHooks(t *testing.T) { }, receiveHooksPayload, git.PreReceiveHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) @@ -162,9 +168,6 @@ func TestPrereceive_customHooks(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - ctx, cleanup := testhelper.Context() - defer cleanup() - gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(tc.hook)) var stdout, stderr bytes.Buffer @@ -210,7 +213,7 @@ func TestPrereceive_gitlab(t *testing.T) { UserID: "1234", Username: "user", Protocol: "web", - }, git.PreReceiveHook).Env() + }, git.PreReceiveHook, nil).Env() require.NoError(t, err) standardEnv := []string{payload} diff --git a/internal/gitaly/hook/transactions_test.go b/internal/gitaly/hook/transactions_test.go index b7c8f3707..837c1e6f7 100644 --- a/internal/gitaly/hook/transactions_test.go +++ b/internal/gitaly/hook/transactions_test.go @@ -14,6 +14,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" @@ -34,6 +35,9 @@ func TestHookManager_stopCalled(t *testing.T) { var mockTxMgr transaction.MockManager hookManager := NewManager(config.NewLocator(cfg), &mockTxMgr, GitlabAPIStub, cfg) + ctx, cleanup := testhelper.Context() + defer cleanup() + hooksPayload, err := git.NewHooksPayload( cfg, repo, @@ -45,12 +49,10 @@ func TestHookManager_stopCalled(t *testing.T) { Protocol: "web", }, git.ReferenceTransactionHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) - ctx, cleanup := testhelper.Context() - defer cleanup() - for _, hook := range []string{"pre-receive", "update", "post-receive"} { gittest.WriteCustomHook(t, repoPath, hook, []byte("#!/bin/sh\nexit 1\n")) } @@ -138,6 +140,7 @@ func TestHookManager_contextCancellationCancelsVote(t *testing.T) { }, nil, git.ReferenceTransactionHook, + nil, ).Env() require.NoError(t, err) diff --git a/internal/gitaly/hook/update_test.go b/internal/gitaly/hook/update_test.go index 896f57b22..63b212080 100644 --- a/internal/gitaly/hook/update_test.go +++ b/internal/gitaly/hook/update_test.go @@ -12,6 +12,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" @@ -28,7 +29,10 @@ func TestUpdate_customHooks(t *testing.T) { Protocol: "web", } - payload, err := git.NewHooksPayload(cfg, repo, nil, nil, receiveHooksPayload, git.UpdateHook).Env() + ctx, cleanup := testhelper.Context() + defer cleanup() + + payload, err := git.NewHooksPayload(cfg, repo, nil, nil, receiveHooksPayload, git.UpdateHook, featureflag.RawFromContext(ctx)).Env() require.NoError(t, err) primaryPayload, err := git.NewHooksPayload( @@ -43,6 +47,7 @@ func TestUpdate_customHooks(t *testing.T) { }, receiveHooksPayload, git.UpdateHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) @@ -58,6 +63,7 @@ func TestUpdate_customHooks(t *testing.T) { }, receiveHooksPayload, git.UpdateHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) @@ -188,9 +194,6 @@ func TestUpdate_customHooks(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - ctx, cleanup := testhelper.Context() - defer cleanup() - gittest.WriteCustomHook(t, repoPath, "update", []byte(tc.hook)) var stdout, stderr bytes.Buffer diff --git a/internal/gitaly/service/hook/post_receive_test.go b/internal/gitaly/service/hook/post_receive_test.go index 00b5af129..cc062f95f 100644 --- a/internal/gitaly/service/hook/post_receive_test.go +++ b/internal/gitaly/service/hook/post_receive_test.go @@ -12,6 +12,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" @@ -112,6 +113,7 @@ func TestHooksMissingStdin(t *testing.T) { Protocol: "protocol", }, git.PostReceiveHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) @@ -230,11 +232,19 @@ To create a merge request for okay, visit: stream, err := client.PostReceiveHook(ctx) require.NoError(t, err) - hooksPayload, err := git.NewHooksPayload(cfg, repo, nil, nil, &git.ReceiveHooksPayload{ - UserID: "key_id", - Username: "username", - Protocol: "protocol", - }, git.PostReceiveHook).Env() + hooksPayload, err := git.NewHooksPayload( + cfg, + repo, + nil, + nil, + &git.ReceiveHooksPayload{ + UserID: "key_id", + Username: "username", + Protocol: "protocol", + }, + git.PostReceiveHook, + featureflag.RawFromContext(ctx), + ).Env() require.NoError(t, err) envVars := []string{ diff --git a/internal/gitaly/service/hook/pre_receive_test.go b/internal/gitaly/service/hook/pre_receive_test.go index aa7f919d3..39b5b6e6f 100644 --- a/internal/gitaly/service/hook/pre_receive_test.go +++ b/internal/gitaly/service/hook/pre_receive_test.go @@ -17,6 +17,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" gitalyhook "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" @@ -142,11 +143,19 @@ func TestPreReceiveHook_GitlabAPIAccess(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - hooksPayload, err := git.NewHooksPayload(cfg, repo, nil, nil, &git.ReceiveHooksPayload{ - UserID: glID, - Username: "username", - Protocol: protocol, - }, git.PreReceiveHook).Env() + hooksPayload, err := git.NewHooksPayload( + cfg, + repo, + nil, + nil, + &git.ReceiveHooksPayload{ + UserID: glID, + Username: "username", + Protocol: protocol, + }, + git.PreReceiveHook, + featureflag.RawFromContext(ctx), + ).Env() require.NoError(t, err) stdin := bytes.NewBufferString(changes) @@ -251,11 +260,19 @@ func TestPreReceive_APIErrors(t *testing.T) { ctx, cancel := testhelper.Context() defer cancel() - hooksPayload, err := git.NewHooksPayload(cfg, repo, nil, nil, &git.ReceiveHooksPayload{ - UserID: "key-123", - Username: "username", - Protocol: "web", - }, git.PreReceiveHook).Env() + hooksPayload, err := git.NewHooksPayload( + cfg, + repo, + nil, + nil, + &git.ReceiveHooksPayload{ + UserID: "key-123", + Username: "username", + Protocol: "web", + }, + git.PreReceiveHook, + featureflag.RawFromContext(ctx), + ).Env() require.NoError(t, err) stream, err := client.PreReceiveHook(ctx) @@ -317,11 +334,19 @@ exit %d ctx, cancel := testhelper.Context() defer cancel() - hooksPayload, err := git.NewHooksPayload(cfg, repo, nil, nil, &git.ReceiveHooksPayload{ - UserID: "key-123", - Username: "username", - Protocol: "web", - }, git.PreReceiveHook).Env() + hooksPayload, err := git.NewHooksPayload( + cfg, + repo, + nil, + nil, + &git.ReceiveHooksPayload{ + UserID: "key-123", + Username: "username", + Protocol: "web", + }, + git.PreReceiveHook, + featureflag.RawFromContext(ctx), + ).Env() require.NoError(t, err) stream, err := client.PreReceiveHook(ctx) @@ -456,6 +481,7 @@ func TestPreReceiveHook_Primary(t *testing.T) { Protocol: "web", }, git.PreReceiveHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) diff --git a/internal/gitaly/service/hook/reference_transaction_test.go b/internal/gitaly/service/hook/reference_transaction_test.go index f2b602b7d..5ea5a49a7 100644 --- a/internal/gitaly/service/hook/reference_transaction_test.go +++ b/internal/gitaly/service/hook/reference_transaction_test.go @@ -156,6 +156,7 @@ func TestReferenceTransactionHook(t *testing.T) { praefectServer, nil, git.ReferenceTransactionHook, + featureflag.RawFromContext(ctx), ).Env() require.NoError(t, err) diff --git a/internal/gitaly/service/hook/update_test.go b/internal/gitaly/service/hook/update_test.go index 8daeb702d..8a1f6dc6c 100644 --- a/internal/gitaly/service/hook/update_test.go +++ b/internal/gitaly/service/hook/update_test.go @@ -12,6 +12,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/internal/helper/text" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -37,19 +38,28 @@ func TestUpdateInvalidArgument(t *testing.T) { func TestUpdate_CustomHooks(t *testing.T) { cfg, repo, repoPath, client := setupHookService(t) - hooksPayload, err := git.NewHooksPayload(cfg, repo, nil, nil, &git.ReceiveHooksPayload{ - UserID: "key-123", - Username: "username", - Protocol: "web", - }, git.UpdateHook).Env() + ctx, cancel := testhelper.Context() + defer cancel() + + hooksPayload, err := git.NewHooksPayload( + cfg, + repo, + nil, + nil, + &git.ReceiveHooksPayload{ + UserID: "key-123", + Username: "username", + Protocol: "web", + }, + git.UpdateHook, + featureflag.RawFromContext(ctx), + ).Env() require.NoError(t, err) envVars := []string{ hooksPayload, } - ctx, cancel := testhelper.Context() - defer cancel() req := gitalypb.UpdateHookRequest{ Repository: repo, Ref: []byte("master"), diff --git a/internal/gitaly/service/operations/update_with_hooks.go b/internal/gitaly/service/operations/update_with_hooks.go index 08e38733b..446a44694 100644 --- a/internal/gitaly/service/operations/update_with_hooks.go +++ b/internal/gitaly/service/operations/update_with_hooks.go @@ -11,6 +11,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/git/updateref" "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/praefect/metadata" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" ) @@ -59,7 +60,7 @@ func (s *Server) updateReferenceWithHooks( UserID: user.GetGlId(), Username: user.GetGlUsername(), Protocol: "web", - }, git.ReceivePackHooks).Env() + }, git.ReceivePackHooks, featureflag.RawFromContext(ctx)).Env() if err != nil { return err } diff --git a/internal/gitaly/service/operations/update_with_hooks_test.go b/internal/gitaly/service/operations/update_with_hooks_test.go index d86d3d3df..de2dbf531 100644 --- a/internal/gitaly/service/operations/update_with_hooks_test.go +++ b/internal/gitaly/service/operations/update_with_hooks_test.go @@ -17,6 +17,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" hookservice "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/hook" "gitlab.com/gitlab-org/gitaly/internal/gitaly/transaction" + "gitlab.com/gitlab-org/gitaly/internal/metadata/featureflag" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "gitlab.com/gitlab-org/gitaly/internal/testhelper/testcfg" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -141,7 +142,7 @@ func TestUpdateReferenceWithHooks(t *testing.T) { UserID: "1234", Username: "Username", Protocol: "web", - }, git.ReceivePackHooks).Env() + }, git.ReceivePackHooks, featureflag.RawFromContext(ctx)).Env() require.NoError(t, err) expectedEnv := []string{ diff --git a/internal/gitaly/service/smarthttp/receive_pack_test.go b/internal/gitaly/service/smarthttp/receive_pack_test.go index 735da4b2d..e63aa3b05 100644 --- a/internal/gitaly/service/smarthttp/receive_pack_test.go +++ b/internal/gitaly/service/smarthttp/receive_pack_test.go @@ -33,8 +33,6 @@ import ( "gitlab.com/gitlab-org/gitaly/streamio" "google.golang.org/grpc" "google.golang.org/grpc/codes" - "google.golang.org/grpc/health" - healthpb "google.golang.org/grpc/health/grpc_health_v1" "google.golang.org/grpc/reflection" ) @@ -104,6 +102,7 @@ func TestSuccessfulReceivePackRequest(t *testing.T) { Protocol: "http", }, RequestedHooks: git.ReceivePackHooks, + FeatureFlags: featureflag.RawFromContext(ctx), }, payload) } @@ -553,6 +552,12 @@ func (t *testTransactionServer) VoteTransaction(ctx context.Context, in *gitalyp } func TestPostReceiveWithReferenceTransactionHook(t *testing.T) { + testhelper.NewFeatureSets([]featureflag.FeatureFlag{ + featureflag.BackchannelVoting, + }).Run(t, testPostReceiveWithReferenceTransactionHook) +} + +func testPostReceiveWithReferenceTransactionHook(t *testing.T, ctx context.Context) { cfg := testcfg.Build(t) testhelper.ConfigureGitalyHooksBin(t, cfg) @@ -560,16 +565,18 @@ func TestPostReceiveWithReferenceTransactionHook(t *testing.T) { refTransactionServer := &testTransactionServer{} locator := config.NewLocator(cfg) - txManager := transaction.NewManager(cfg, backchannel.NewRegistry()) + registry := backchannel.NewRegistry() + txManager := transaction.NewManager(cfg, registry) hookManager := gitalyhook.NewManager(locator, txManager, gitalyhook.GitlabAPIStub, cfg) gitCmdFactory := git.NewExecCommandFactory(cfg) - gitalyServer := testhelper.NewTestGrpcServer(t, nil, nil) + gitalyServer := testhelper.NewServerWithAuth(t, nil, nil, cfg.Auth.Token, registry).GrpcServer() gitalypb.RegisterSmartHTTPServiceServer(gitalyServer, NewServer(cfg, locator, gitCmdFactory)) gitalypb.RegisterHookServiceServer(gitalyServer, hook.NewServer(cfg, hookManager, gitCmdFactory)) - gitalypb.RegisterRefTransactionServer(gitalyServer, refTransactionServer) - healthpb.RegisterHealthServer(gitalyServer, health.NewServer()) reflection.Register(gitalyServer) + if featureflag.IsDisabled(ctx, featureflag.BackchannelVoting) { + gitalypb.RegisterRefTransactionServer(gitalyServer, refTransactionServer) + } gitalySocketPath := testhelper.GetTemporaryGitalySocketFileName(t) listener, err := net.Listen("unix", gitalySocketPath) @@ -582,8 +589,14 @@ func TestPostReceiveWithReferenceTransactionHook(t *testing.T) { go gitalyServer.Serve(internalListener) defer gitalyServer.Stop() - client, conn := newSmartHTTPClient(t, "unix://"+gitalySocketPath, cfg.Auth.Token) - defer conn.Close() + client := newMuxedSmartHTTPClient(t, ctx, "unix://"+gitalySocketPath, cfg.Auth.Token, func() backchannel.Server { + srv := grpc.NewServer() + if featureflag.IsEnabled(ctx, featureflag.BackchannelVoting) { + gitalypb.RegisterRefTransactionServer(srv, refTransactionServer) + } + + return srv + }) // As we ain't got a Praefect server setup, we instead hooked up the // RefTransaction server for Gitaly itself. As this is the only Praefect @@ -594,12 +607,11 @@ func TestPostReceiveWithReferenceTransactionHook(t *testing.T) { }) require.NoError(t, err) - ctx, cancel := testhelper.Context() - defer cancel() ctx, err = metadata.InjectTransaction(ctx, 1234, "primary", true) require.NoError(t, err) ctx, err = praefectServer.Inject(ctx) require.NoError(t, err) + ctx = helper.IncomingToOutgoing(ctx) t.Run("update", func(t *testing.T) { diff --git a/internal/gitaly/service/smarthttp/testhelper_test.go b/internal/gitaly/service/smarthttp/testhelper_test.go index c64879929..daf5b3a68 100644 --- a/internal/gitaly/service/smarthttp/testhelper_test.go +++ b/internal/gitaly/service/smarthttp/testhelper_test.go @@ -1,6 +1,7 @@ package smarthttp import ( + "context" "os" "testing" @@ -9,6 +10,7 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/backchannel" diskcache "gitlab.com/gitlab-org/gitaly/internal/cache" "gitlab.com/gitlab-org/gitaly/internal/git" + "gitlab.com/gitlab-org/gitaly/internal/gitaly/client" "gitlab.com/gitlab-org/gitaly/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/internal/gitaly/hook" hookservice "gitlab.com/gitlab-org/gitaly/internal/gitaly/service/hook" @@ -40,17 +42,20 @@ func testMain(m *testing.M) int { func runSmartHTTPServer(t *testing.T, cfg config.Cfg, serverOpts ...ServerOpt) (string, func()) { locator := config.NewLocator(cfg) keyer := diskcache.NewLeaseKeyer(locator) - txManager := transaction.NewManager(cfg, backchannel.NewRegistry()) + registry := backchannel.NewRegistry() + txManager := transaction.NewManager(cfg, registry) hookManager := hook.NewManager(locator, txManager, hook.GitlabAPIStub, cfg) gitCmdFactory := git.NewExecCommandFactory(cfg) - srv := testhelper.NewServer(t, + srv := testhelper.NewServerWithAuth(t, []grpc.StreamServerInterceptor{ cache.StreamInvalidator(keyer, protoregistry.GitalyProtoPreregistered), }, []grpc.UnaryServerInterceptor{ cache.UnaryInvalidator(keyer, protoregistry.GitalyProtoPreregistered), }, + cfg.Auth.Token, + registry, testhelper.WithInternalSocket(cfg)) gitalypb.RegisterSmartHTTPServiceServer(srv.GrpcServer(), NewServer(cfg, locator, gitCmdFactory, serverOpts...)) @@ -72,3 +77,17 @@ func newSmartHTTPClient(t *testing.T, serverSocketPath, token string) (gitalypb. return gitalypb.NewSmartHTTPServiceClient(conn), conn } + +func newMuxedSmartHTTPClient(t *testing.T, ctx context.Context, serverSocketPath, token string, serverFactory backchannel.ServerFactory) gitalypb.SmartHTTPServiceClient { + t.Helper() + + conn, err := client.Dial( + ctx, + serverSocketPath, + []grpc.DialOption{grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(token))}, + backchannel.NewClientHandshaker(testhelper.DiscardTestEntry(t), serverFactory), + ) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, conn.Close()) }) + return gitalypb.NewSmartHTTPServiceClient(conn) +} |