diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2021-04-15 18:00:40 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2021-04-21 14:42:49 +0300 |
commit | 77eab50b01c9e7c92dcd18e0fb3ac9d09492355c (patch) | |
tree | 39935582a000afaba96bdb078ffee2db5298a81b | |
parent | 6350ec5591df7ec905e8e0a7952425d8c8f94aa8 (diff) |
Propagate feature flags through gitaly-hooks
Hooks currently do not receive nor propagate the feature flags. This
is due to hooks being called through Git, which breaks the context
chain and thus the propagation. This commits propagate the feature
flags through the hooks to Gitaly's HookService. The feature flags
are added to the hook payload in order to pass them to the hooks.
The provided feature flags are then propagated via the outgoing context
to the HookService.
-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) +} |