diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2023-07-28 19:43:28 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2023-09-26 20:10:28 +0300 |
commit | 4edd816483173fc31503c57c767f9b3e30ba5395 (patch) | |
tree | d1575d8e5f2ea51b95d8dfd61551c1427a20a3cf | |
parent | 8c1599b6468757739e4114f2e0e3ace212b34c86 (diff) |
Include transaction ID in the hook payload
The reference transaction hook needs the transaction ID in order
to stage the reference changes in the transaction. This commit
includes the transaction ID in the hook payload.
-rw-r--r-- | cmd/gitaly-hooks/hooks_test.go | 10 | ||||
-rw-r--r-- | internal/git/hooks_options.go | 4 | ||||
-rw-r--r-- | internal/git/hooks_payload.go | 7 | ||||
-rw-r--r-- | internal/git/hooks_payload_test.go | 9 | ||||
-rw-r--r-- | internal/gitaly/hook/postreceive_test.go | 9 | ||||
-rw-r--r-- | internal/gitaly/hook/prereceive_test.go | 7 | ||||
-rw-r--r-- | internal/gitaly/hook/transactions_test.go | 3 | ||||
-rw-r--r-- | internal/gitaly/hook/update_test.go | 5 | ||||
-rw-r--r-- | internal/gitaly/hook/updateref/update_with_hooks.go | 4 | ||||
-rw-r--r-- | internal/gitaly/service/hook/post_receive_test.go | 2 | ||||
-rw-r--r-- | internal/gitaly/service/hook/pre_receive_test.go | 4 | ||||
-rw-r--r-- | internal/gitaly/service/hook/reference_transaction_test.go | 1 | ||||
-rw-r--r-- | internal/gitaly/service/hook/update_test.go | 1 |
13 files changed, 57 insertions, 9 deletions
diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index 1503e7077..c8886caac 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -22,6 +22,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config/prometheus" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service/hook" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v16/internal/gitlab" "gitlab.com/gitlab-org/gitaly/v16/internal/grpc/metadata" @@ -79,7 +80,11 @@ func envForHooks(tb testing.TB, ctx context.Context, cfg config.Cfg, repo *gital Username: glHookValues.GLUsername, Protocol: glHookValues.GLProtocol, RemoteIP: glHookValues.RemoteIP, - }, git.AllHooks, featureFlags(ctx)).Env() + }, + git.AllHooks, + featureFlags(ctx), + storage.ExtractTransactionID(ctx), + ).Env() require.NoError(tb, err) env := append(command.AllowedEnvironment(os.Environ()), []string{ @@ -453,6 +458,7 @@ func TestHooksPostReceiveFailed(t *testing.T) { }, git.PostReceiveHook, featureFlags(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -557,6 +563,7 @@ func TestRequestedHooks(t *testing.T) { nil, git.AllHooks&^hook, nil, + 0, ).Env() require.NoError(t, err) @@ -579,6 +586,7 @@ func TestRequestedHooks(t *testing.T) { nil, hook, nil, + 0, ).Env() require.NoError(t, err) diff --git a/internal/git/hooks_options.go b/internal/git/hooks_options.go index 43991e765..f0c0bb32f 100644 --- a/internal/git/hooks_options.go +++ b/internal/git/hooks_options.go @@ -152,7 +152,9 @@ func (cc *cmdCfg) configureHooks( transaction, userDetails, requestedHooks, - featureflag.FromContext(ctx)).Env() + featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), + ).Env() if err != nil { return err } diff --git a/internal/git/hooks_payload.go b/internal/git/hooks_payload.go index 2dfd8f15d..c1528d0f6 100644 --- a/internal/git/hooks_payload.go +++ b/internal/git/hooks_payload.go @@ -9,6 +9,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/transaction/txinfo" "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb" "google.golang.org/protobuf/encoding/protojson" @@ -86,6 +87,10 @@ type HooksPayload struct { // UserDetails contains information required when executing // git-receive-pack or git-upload-pack UserDetails *UserDetails `json:"user_details"` + + // TransactionID identifies the storage transaction this hooks call runs in. It's + // used to access the transaction in the hook manager. + TransactionID storage.TransactionID `json:"transaction_id,omitempty"` } // UserDetails contains all information which is required for hooks @@ -121,6 +126,7 @@ func NewHooksPayload( userDetails *UserDetails, requestedHooks Hook, featureFlagsWithValue map[featureflag.FeatureFlag]bool, + transactionID storage.TransactionID, ) HooksPayload { flags := make([]FeatureFlagWithValue, 0, len(featureFlagsWithValue)) for flag, enabled := range featureFlagsWithValue { @@ -140,6 +146,7 @@ func NewHooksPayload( UserDetails: userDetails, RequestedHooks: requestedHooks, FeatureFlagsWithValue: flags, + TransactionID: transactionID, } } diff --git a/internal/git/hooks_payload_test.go b/internal/git/hooks_payload_test.go index e73e755a6..dfe5bea6a 100644 --- a/internal/git/hooks_payload_test.go +++ b/internal/git/hooks_payload_test.go @@ -38,6 +38,7 @@ func TestHooksPayload(t *testing.T) { nil, git.AllHooks, nil, + 0, ).Env() require.NoError(t, err) require.True(t, strings.HasPrefix(env, git.EnvHooksPayload+"=")) @@ -53,7 +54,9 @@ func TestHooksPayload(t *testing.T) { git.PreReceiveHook, map[featureflag.FeatureFlag]bool{ {Name: "flag_key"}: true, - }).Env() + }, + 1, + ).Env() require.NoError(t, err) payload, err := git.HooksPayloadFromEnv([]string{ @@ -76,6 +79,7 @@ func TestHooksPayload(t *testing.T) { Enabled: true, }, }, + TransactionID: 1, }, payload) }) @@ -88,6 +92,7 @@ func TestHooksPayload(t *testing.T) { nil, git.UpdateHook, nil, + 0, ).Env() require.NoError(t, err) @@ -125,7 +130,7 @@ func TestHooksPayload(t *testing.T) { UserID: "1234", Username: "user", Protocol: "ssh", - }, git.PostReceiveHook, nil).Env() + }, git.PostReceiveHook, nil, 0).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 9d8227979..957114e3f 100644 --- a/internal/gitaly/hook/postreceive_test.go +++ b/internal/gitaly/hook/postreceive_test.go @@ -16,6 +16,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v16/internal/git/quarantine" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v16/internal/gitlab" "gitlab.com/gitlab-org/gitaly/v16/internal/grpc/backchannel" @@ -98,6 +99,7 @@ func TestPostReceive_customHook(t *testing.T) { receiveHooksPayload, git.PostReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -111,6 +113,7 @@ func TestPostReceive_customHook(t *testing.T) { receiveHooksPayload, git.PostReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -124,6 +127,7 @@ func TestPostReceive_customHook(t *testing.T) { receiveHooksPayload, git.PostReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -287,7 +291,7 @@ func TestPostReceive_gitlab(t *testing.T) { UserID: "1234", Username: "user", Protocol: "web", - }, git.PostReceiveHook, nil).Env() + }, git.PostReceiveHook, nil, storage.ExtractTransactionID(ctx)).Env() require.NoError(t, err) standardEnv := []string{payload} @@ -414,7 +418,7 @@ func TestPostReceive_quarantine(t *testing.T) { hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), nil, gitlab.NewMockClient( t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive, - )) + ), storagemgr.NewTransactionRegistry()) gittest.WriteCustomHook(t, repoPath, "post-receive", []byte(fmt.Sprintf( `#!/bin/sh @@ -438,6 +442,7 @@ func TestPostReceive_quarantine(t *testing.T) { }, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) diff --git a/internal/gitaly/hook/prereceive_test.go b/internal/gitaly/hook/prereceive_test.go index 1525e096b..81cda3bf4 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/v16/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v16/internal/git/quarantine" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v16/internal/gitlab" "gitlab.com/gitlab-org/gitaly/v16/internal/grpc/backchannel" @@ -57,6 +58,7 @@ func TestPrereceive_customHooks(t *testing.T) { receiveHooksPayload, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -70,6 +72,7 @@ func TestPrereceive_customHooks(t *testing.T) { receiveHooksPayload, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -83,6 +86,7 @@ func TestPrereceive_customHooks(t *testing.T) { receiveHooksPayload, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -248,6 +252,7 @@ func TestPrereceive_quarantine(t *testing.T) { }, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -309,7 +314,7 @@ func TestPrereceive_gitlab(t *testing.T) { UserID: "1234", Username: "user", Protocol: "web", - }, git.PreReceiveHook, nil).Env() + }, git.PreReceiveHook, nil, storage.ExtractTransactionID(ctx)).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 bf708b15e..84b51364d 100644 --- a/internal/gitaly/hook/transactions_test.go +++ b/internal/gitaly/hook/transactions_test.go @@ -13,6 +13,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v16/internal/gitlab" "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper" @@ -50,6 +51,7 @@ func TestHookManager_stopCalled(t *testing.T) { }, git.ReferenceTransactionHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -153,6 +155,7 @@ func TestHookManager_contextCancellationCancelsVote(t *testing.T) { nil, git.ReferenceTransactionHook, nil, + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) diff --git a/internal/gitaly/hook/update_test.go b/internal/gitaly/hook/update_test.go index 898317fe9..2dd31027f 100644 --- a/internal/gitaly/hook/update_test.go +++ b/internal/gitaly/hook/update_test.go @@ -13,6 +13,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v16/internal/git/quarantine" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" "gitlab.com/gitlab-org/gitaly/v16/internal/gitlab" "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper" @@ -55,6 +56,7 @@ func TestUpdate_customHooks(t *testing.T) { receiveHooksPayload, git.UpdateHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -68,6 +70,7 @@ func TestUpdate_customHooks(t *testing.T) { receiveHooksPayload, git.UpdateHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -81,6 +84,7 @@ func TestUpdate_customHooks(t *testing.T) { receiveHooksPayload, git.UpdateHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -278,6 +282,7 @@ func TestUpdate_quarantine(t *testing.T) { }, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) diff --git a/internal/gitaly/hook/updateref/update_with_hooks.go b/internal/gitaly/hook/updateref/update_with_hooks.go index a1fb7175a..d2dbf7f33 100644 --- a/internal/gitaly/hook/updateref/update_with_hooks.go +++ b/internal/gitaly/hook/updateref/update_with_hooks.go @@ -202,7 +202,7 @@ func (u *UpdaterWithHooks) UpdateReference( quarantinedRepo = quarantineDir.QuarantinedRepo() } - hooksPayload, err := git.NewHooksPayload(u.cfg, quarantinedRepo, objectHash, transaction, &receiveHooksPayload, git.ReceivePackHooks, featureflag.FromContext(ctx)).Env() + hooksPayload, err := git.NewHooksPayload(u.cfg, quarantinedRepo, objectHash, transaction, &receiveHooksPayload, git.ReceivePackHooks, featureflag.FromContext(ctx), storage.ExtractTransactionID(ctx)).Env() if err != nil { return fmt.Errorf("constructing hooks payload: %w", err) } @@ -224,7 +224,7 @@ func (u *UpdaterWithHooks) UpdateReference( // We only need to update the hooks payload to the unquarantined repo in case we // had a quarantine environment. Otherwise, the initial hooks payload is for the // real repository anyway. - hooksPayload, err = git.NewHooksPayload(u.cfg, repoProto, objectHash, transaction, &receiveHooksPayload, git.ReceivePackHooks, featureflag.FromContext(ctx)).Env() + hooksPayload, err = git.NewHooksPayload(u.cfg, repoProto, objectHash, transaction, &receiveHooksPayload, git.ReceivePackHooks, featureflag.FromContext(ctx), storage.ExtractTransactionID(ctx)).Env() if err != nil { return fmt.Errorf("constructing quarantined hooks payload: %w", err) } diff --git a/internal/gitaly/service/hook/post_receive_test.go b/internal/gitaly/service/hook/post_receive_test.go index d84a0f3b8..5d2af9eba 100644 --- a/internal/gitaly/service/hook/post_receive_test.go +++ b/internal/gitaly/service/hook/post_receive_test.go @@ -125,6 +125,7 @@ func TestHooksMissingStdin(t *testing.T) { }, git.PostReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -259,6 +260,7 @@ To create a merge request for okay, visit: }, git.PostReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) diff --git a/internal/gitaly/service/hook/pre_receive_test.go b/internal/gitaly/service/hook/pre_receive_test.go index 6131e909f..921e19c50 100644 --- a/internal/gitaly/service/hook/pre_receive_test.go +++ b/internal/gitaly/service/hook/pre_receive_test.go @@ -157,6 +157,7 @@ func TestPreReceiveHook_GitlabAPIAccess(t *testing.T) { }, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -278,6 +279,7 @@ func TestPreReceive_APIErrors(t *testing.T) { }, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -352,6 +354,7 @@ exit %d }, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) @@ -505,6 +508,7 @@ func TestPreReceiveHook_Primary(t *testing.T) { }, git.PreReceiveHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(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 6cf47050f..ad2628a6f 100644 --- a/internal/gitaly/service/hook/reference_transaction_test.go +++ b/internal/gitaly/service/hook/reference_transaction_test.go @@ -172,6 +172,7 @@ func TestReferenceTransactionHook(t *testing.T) { nil, git.ReferenceTransactionHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(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 0be292b15..2eb7bfb3e 100644 --- a/internal/gitaly/service/hook/update_test.go +++ b/internal/gitaly/service/hook/update_test.go @@ -53,6 +53,7 @@ func TestUpdate_CustomHooks(t *testing.T) { }, git.UpdateHook, featureflag.FromContext(ctx), + storage.ExtractTransactionID(ctx), ).Env() require.NoError(t, err) |