Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSami Hiltunen <shiltunen@gitlab.com>2023-09-27 09:06:32 +0300
committerSami Hiltunen <shiltunen@gitlab.com>2023-09-27 09:06:32 +0300
commitb7074c08d5e57806fda21f77dec30185db6043a1 (patch)
tree6902e854f840241c71a9eac4cd831afb83d18b9f
parent0d02e2dfdd429b7c5cda79c8f2af1132566ade65 (diff)
parenta56e600ead177e500c82514759291902638c7df1 (diff)
Merge branch 'smh-support-multiple-ref-updates' into 'master'
Capture reference changes into a transaction in reference transaction hook See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6383 Merged-by: Sami Hiltunen <shiltunen@gitlab.com> Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com> Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com> Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com> Reviewed-by: karthik nayak <knayak@gitlab.com>
-rw-r--r--cmd/gitaly-hooks/hooks_test.go10
-rw-r--r--internal/cli/gitaly/serve.go3
-rw-r--r--internal/cli/gitaly/subcmd_check.go3
-rw-r--r--internal/git/hooks_options.go4
-rw-r--r--internal/git/hooks_payload.go7
-rw-r--r--internal/git/hooks_payload_test.go9
-rw-r--r--internal/gitaly/hook/manager.go30
-rw-r--r--internal/gitaly/hook/postreceive_test.go14
-rw-r--r--internal/gitaly/hook/prereceive_test.go14
-rw-r--r--internal/gitaly/hook/referencetransaction.go75
-rw-r--r--internal/gitaly/hook/transactions_test.go8
-rw-r--r--internal/gitaly/hook/update_test.go10
-rw-r--r--internal/gitaly/hook/updateref/update_with_hooks.go4
-rw-r--r--internal/gitaly/hook/updateref/update_with_hooks_test.go7
-rw-r--r--internal/gitaly/server/auth_test.go3
-rw-r--r--internal/gitaly/service/dependencies.go6
-rw-r--r--internal/gitaly/service/hook/post_receive_test.go2
-rw-r--r--internal/gitaly/service/hook/pre_receive_test.go4
-rw-r--r--internal/gitaly/service/hook/reference_transaction_test.go169
-rw-r--r--internal/gitaly/service/hook/testhelper_test.go9
-rw-r--r--internal/gitaly/service/hook/update_test.go1
-rw-r--r--internal/gitaly/service/operations/merge_branch_test.go3
-rw-r--r--internal/gitaly/storage/storagemgr/transaction_manager.go116
-rw-r--r--internal/gitaly/storage/storagemgr/transaction_manager_test.go433
-rw-r--r--internal/gitaly/storage/storagemgr/transaction_registry.go51
-rw-r--r--internal/gitaly/storage/storagemgr/transaction_registry_test.go33
-rw-r--r--internal/gitaly/storage/transaction.go27
-rw-r--r--internal/gitaly/storage/transaction_test.go26
-rw-r--r--internal/testhelper/testserver/gitaly.go4
29 files changed, 918 insertions, 167 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/cli/gitaly/serve.go b/internal/cli/gitaly/serve.go
index 81f332883..67a34acf1 100644
--- a/internal/cli/gitaly/serve.go
+++ b/internal/cli/gitaly/serve.go
@@ -31,6 +31,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service/setup"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/counter"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/storagemgr"
"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"
@@ -263,7 +264,7 @@ func run(cfg config.Cfg, logger log.Logger) error {
}
prometheus.MustRegister(gitlabClient)
- hm := hook.NewManager(cfg, locator, gitCmdFactory, transactionManager, gitlabClient)
+ hm := hook.NewManager(cfg, locator, gitCmdFactory, transactionManager, gitlabClient, hook.NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
hookManager = hm
}
diff --git a/internal/cli/gitaly/subcmd_check.go b/internal/cli/gitaly/subcmd_check.go
index 36385cc2b..a8362c656 100644
--- a/internal/cli/gitaly/subcmd_check.go
+++ b/internal/cli/gitaly/subcmd_check.go
@@ -9,6 +9,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config/prometheus"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/storagemgr"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitlab"
"gitlab.com/gitlab-org/gitaly/v16/internal/log"
)
@@ -73,5 +74,5 @@ func checkAPI(cfg config.Cfg, logger log.Logger) (*gitlab.CheckInfo, error) {
}
defer cleanup()
- return hook.NewManager(cfg, config.NewLocator(cfg), gitCmdFactory, nil, gitlabAPI).Check(context.Background())
+ return hook.NewManager(cfg, config.NewLocator(cfg), gitCmdFactory, nil, gitlabAPI, hook.NewTransactionRegistry(storagemgr.NewTransactionRegistry())).Check(context.Background())
}
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/manager.go b/internal/gitaly/hook/manager.go
index b021d9909..d093412db 100644
--- a/internal/gitaly/hook/manager.go
+++ b/internal/gitaly/hook/manager.go
@@ -7,6 +7,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/git"
"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/storage/storagemgr"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitlab"
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
@@ -47,6 +48,32 @@ type Manager interface {
ReferenceTransactionHook(ctx context.Context, state ReferenceTransactionState, env []string, stdin io.Reader) error
}
+// Transaction is the interface of storagemgr.Transaction. It's used for mocking in the tests.
+type Transaction interface {
+ RecordInitialReferenceValues(context.Context, map[git.ReferenceName]git.ObjectID) error
+ UpdateReferences(storagemgr.ReferenceUpdates)
+}
+
+// TransactionRegistry is the interface of storagemgr.TransactionRegistry. It's used for mocking
+// in the tests.
+type TransactionRegistry interface {
+ Get(storage.TransactionID) (Transaction, error)
+}
+
+type transactionRegistry struct {
+ registry *storagemgr.TransactionRegistry
+}
+
+func (r *transactionRegistry) Get(id storage.TransactionID) (Transaction, error) {
+ return r.registry.Get(id)
+}
+
+// NewTransactionRegistry wraps a storagemgr.TransactionRegistry to adapt it to the interface
+// used by the manager.
+func NewTransactionRegistry(txRegistry *storagemgr.TransactionRegistry) TransactionRegistry {
+ return &transactionRegistry{registry: txRegistry}
+}
+
// GitLabHookManager is a hook manager containing Git hook business logic. It
// uses the GitLab API to authenticate and track ongoing hook calls.
type GitLabHookManager struct {
@@ -55,6 +82,7 @@ type GitLabHookManager struct {
gitCmdFactory git.CommandFactory
txManager transaction.Manager
gitlabClient gitlab.Client
+ txRegistry TransactionRegistry
}
// NewManager returns a new hook manager
@@ -64,6 +92,7 @@ func NewManager(
gitCmdFactory git.CommandFactory,
txManager transaction.Manager,
gitlabClient gitlab.Client,
+ txRegistry TransactionRegistry,
) *GitLabHookManager {
return &GitLabHookManager{
cfg: cfg,
@@ -71,5 +100,6 @@ func NewManager(
gitCmdFactory: gitCmdFactory,
txManager: txManager,
gitlabClient: gitlabClient,
+ txRegistry: txRegistry,
}
}
diff --git a/internal/gitaly/hook/postreceive_test.go b/internal/gitaly/hook/postreceive_test.go
index 9d8227979..096279196 100644
--- a/internal/gitaly/hook/postreceive_test.go
+++ b/internal/gitaly/hook/postreceive_test.go
@@ -16,6 +16,8 @@ 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/storage/storagemgr"
"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"
@@ -82,7 +84,7 @@ func TestPostReceive_customHook(t *testing.T) {
txManager := transaction.NewTrackingManager()
hookManager := NewManager(cfg, locator, gitCmdFactory, txManager, gitlab.NewMockClient(
t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive,
- ))
+ ), NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
receiveHooksPayload := &git.UserDetails{
UserID: "1234",
@@ -98,6 +100,7 @@ func TestPostReceive_customHook(t *testing.T) {
receiveHooksPayload,
git.PostReceiveHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -111,6 +114,7 @@ func TestPostReceive_customHook(t *testing.T) {
receiveHooksPayload,
git.PostReceiveHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -124,6 +128,7 @@ func TestPostReceive_customHook(t *testing.T) {
receiveHooksPayload,
git.PostReceiveHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -287,7 +292,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}
@@ -376,7 +381,7 @@ func TestPostReceive_gitlab(t *testing.T) {
},
}
- hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), &gitlabAPI)
+ hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), &gitlabAPI, NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
gittest.WriteCustomHook(t, repoPath, "post-receive", []byte("#!/bin/sh\necho hook called\n"))
@@ -414,7 +419,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,
- ))
+ ), NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
gittest.WriteCustomHook(t, repoPath, "post-receive", []byte(fmt.Sprintf(
`#!/bin/sh
@@ -438,6 +443,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..9da48e9da 100644
--- a/internal/gitaly/hook/prereceive_test.go
+++ b/internal/gitaly/hook/prereceive_test.go
@@ -15,6 +15,8 @@ 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/storage/storagemgr"
"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"
@@ -41,7 +43,7 @@ func TestPrereceive_customHooks(t *testing.T) {
txManager := transaction.NewTrackingManager()
hookManager := NewManager(cfg, locator, gitCmdFactory, txManager, gitlab.NewMockClient(
t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive,
- ))
+ ), NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
receiveHooksPayload := &git.UserDetails{
UserID: "1234",
@@ -57,6 +59,7 @@ func TestPrereceive_customHooks(t *testing.T) {
receiveHooksPayload,
git.PreReceiveHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -70,6 +73,7 @@ func TestPrereceive_customHooks(t *testing.T) {
receiveHooksPayload,
git.PreReceiveHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -83,6 +87,7 @@ func TestPrereceive_customHooks(t *testing.T) {
receiveHooksPayload,
git.PreReceiveHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -223,7 +228,7 @@ func TestPrereceive_quarantine(t *testing.T) {
hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), nil, gitlab.NewMockClient(
t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive,
- ))
+ ), NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
//nolint:gitaly-linters
gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte(fmt.Sprintf(
@@ -248,6 +253,7 @@ func TestPrereceive_quarantine(t *testing.T) {
},
git.PreReceiveHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -309,7 +315,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}
@@ -412,7 +418,7 @@ func TestPrereceive_gitlab(t *testing.T) {
},
}
- hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), &gitlabAPI)
+ hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), transaction.NewManager(cfg, backchannel.NewRegistry()), &gitlabAPI, NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
gittest.WriteCustomHook(t, repoPath, "pre-receive", []byte("#!/bin/sh\necho called\n"))
diff --git a/internal/gitaly/hook/referencetransaction.go b/internal/gitaly/hook/referencetransaction.go
index cf376e389..bd90023f0 100644
--- a/internal/gitaly/hook/referencetransaction.go
+++ b/internal/gitaly/hook/referencetransaction.go
@@ -7,8 +7,10 @@ import (
"crypto/sha1"
"fmt"
"io"
+ "strings"
"gitlab.com/gitlab-org/gitaly/v16/internal/git"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/storagemgr"
"gitlab.com/gitlab-org/gitaly/v16/internal/transaction/voting"
)
@@ -29,18 +31,53 @@ func (m *GitLabHookManager) ReferenceTransactionHook(ctx context.Context, state
return fmt.Errorf("reading stdin from request: %w", err)
}
+ var tx Transaction
+ if payload.TransactionID > 0 {
+ tx, err = m.txRegistry.Get(payload.TransactionID)
+ if err != nil {
+ return fmt.Errorf("get transaction: %w", err)
+ }
+ }
+
var phase voting.Phase
switch state {
// We're voting in prepared state as this is the only stage in Git's reference transaction
// which allows us to abort the transaction.
case ReferenceTransactionPrepared:
phase = voting.Prepared
+
+ if tx != nil {
+ updates, err := parseChanges(ctx, objectHash, bytes.NewReader(changes))
+ if err != nil {
+ return fmt.Errorf("parse changes: %w", err)
+ }
+
+ initialValues := map[git.ReferenceName]git.ObjectID{}
+ for reference, update := range updates {
+ initialValues[reference] = update.OldOID
+ }
+
+ // Only record the initial values of the reference in the prepare step as this
+ // change hasn't yet been committed.
+ if err := tx.RecordInitialReferenceValues(ctx, initialValues); err != nil {
+ return fmt.Errorf("record initial reference value: %w", err)
+ }
+ }
// We're also voting in committed state to tell Praefect we've actually persisted the
// changes. This is necessary as some RPCs fail return errors in the response body rather
// than as an error code. Praefect can't tell if these RPCs have failed. Voting on committed
// ensure Praefect sees either a missing vote or that the RPC did commit the changes.
case ReferenceTransactionCommitted:
phase = voting.Committed
+
+ if tx != nil {
+ updates, err := parseChanges(ctx, objectHash, bytes.NewReader(changes))
+ if err != nil {
+ return fmt.Errorf("parse changes: %w", err)
+ }
+
+ tx.UpdateReferences(updates)
+ }
default:
return nil
}
@@ -76,6 +113,44 @@ func (m *GitLabHookManager) ReferenceTransactionHook(ctx context.Context, state
return nil
}
+// parseChanges parses the changes from the reader. All updates to references lacking a 'refs/' prefix are ignored. These
+// are the various pseudo reference like ORIG_HEAD but also HEAD. See the documentation of the reference-transaction hook
+// for details on the format: https://git-scm.com/docs/githooks#_reference_transaction
+func parseChanges(ctx context.Context, objectHash git.ObjectHash, changes io.Reader) (storagemgr.ReferenceUpdates, error) {
+ scanner := bufio.NewScanner(changes)
+
+ updates := storagemgr.ReferenceUpdates{}
+ for scanner.Scan() {
+ line := scanner.Text()
+ components := strings.Split(line, " ")
+ if len(components) != 3 {
+ return nil, fmt.Errorf("unexpected change line: %q", line)
+ }
+
+ reference := git.ReferenceName(components[2])
+ if !strings.HasPrefix(reference.String(), "refs/") {
+ continue
+ }
+
+ update := storagemgr.ReferenceUpdate{}
+
+ var err error
+ update.OldOID, err = objectHash.FromHex(components[0])
+ if err != nil {
+ return nil, fmt.Errorf("parse old: %w", err)
+ }
+
+ update.NewOID, err = objectHash.FromHex(components[1])
+ if err != nil {
+ return nil, fmt.Errorf("parse new: %w", err)
+ }
+
+ updates[reference] = update
+ }
+
+ return updates, nil
+}
+
// isForceDeletionsOnly determines whether the given changes only consist of force-deletions.
func isForceDeletionsOnly(objectHash git.ObjectHash, changes io.Reader) bool {
// forceDeletionPrefix is the prefix of a queued reference transaction which deletes a
diff --git a/internal/gitaly/hook/transactions_test.go b/internal/gitaly/hook/transactions_test.go
index bf708b15e..ce829848c 100644
--- a/internal/gitaly/hook/transactions_test.go
+++ b/internal/gitaly/hook/transactions_test.go
@@ -13,6 +13,8 @@ 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/storage/storagemgr"
"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"
@@ -36,7 +38,7 @@ func TestHookManager_stopCalled(t *testing.T) {
var mockTxMgr transaction.MockManager
hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), &mockTxMgr, gitlab.NewMockClient(
t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive,
- ))
+ ), NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
hooksPayload, err := git.NewHooksPayload(
cfg,
@@ -50,6 +52,7 @@ func TestHookManager_stopCalled(t *testing.T) {
},
git.ReferenceTransactionHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -141,7 +144,7 @@ func TestHookManager_contextCancellationCancelsVote(t *testing.T) {
hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), &mockTxMgr, gitlab.NewMockClient(
t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive,
- ))
+ ), NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
hooksPayload, err := git.NewHooksPayload(
cfg,
@@ -153,6 +156,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..45dec2ca1 100644
--- a/internal/gitaly/hook/update_test.go
+++ b/internal/gitaly/hook/update_test.go
@@ -13,6 +13,8 @@ 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/storage/storagemgr"
"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"
@@ -39,7 +41,7 @@ func TestUpdate_customHooks(t *testing.T) {
txManager := transaction.NewTrackingManager()
hookManager := NewManager(cfg, locator, gitCmdFactory, txManager, gitlab.NewMockClient(
t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive,
- ))
+ ), NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
receiveHooksPayload := &git.UserDetails{
UserID: "1234",
@@ -55,6 +57,7 @@ func TestUpdate_customHooks(t *testing.T) {
receiveHooksPayload,
git.UpdateHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -68,6 +71,7 @@ func TestUpdate_customHooks(t *testing.T) {
receiveHooksPayload,
git.UpdateHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -81,6 +85,7 @@ func TestUpdate_customHooks(t *testing.T) {
receiveHooksPayload,
git.UpdateHook,
featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
).Env()
require.NoError(t, err)
@@ -253,7 +258,7 @@ func TestUpdate_quarantine(t *testing.T) {
hookManager := NewManager(cfg, config.NewLocator(cfg), gittest.NewCommandFactory(t, cfg), nil, gitlab.NewMockClient(
t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive,
- ))
+ ), NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
//nolint:gitaly-linters
gittest.WriteCustomHook(t, repoPath, "update", []byte(fmt.Sprintf(
@@ -278,6 +283,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/hook/updateref/update_with_hooks_test.go b/internal/gitaly/hook/updateref/update_with_hooks_test.go
index 45306cde1..13786f2cd 100644
--- a/internal/gitaly/hook/updateref/update_with_hooks_test.go
+++ b/internal/gitaly/hook/updateref/update_with_hooks_test.go
@@ -19,6 +19,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook/updateref"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service"
hookservice "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/testhelper"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testserver"
@@ -118,7 +119,11 @@ func TestUpdaterWithHooks_UpdateReference(t *testing.T) {
UserID: gittest.TestUser.GlId,
Username: gittest.TestUser.GlUsername,
Protocol: "web",
- }, git.ReceivePackHooks, featureflag.FromContext(ctx))
+ },
+ git.ReceivePackHooks,
+ featureflag.FromContext(ctx),
+ storage.ExtractTransactionID(ctx),
+ )
actualPayload, err := git.HooksPayloadFromEnv(env)
require.NoError(t, err)
diff --git a/internal/gitaly/server/auth_test.go b/internal/gitaly/server/auth_test.go
index 4ca5b2584..7ab5aad0e 100644
--- a/internal/gitaly/server/auth_test.go
+++ b/internal/gitaly/server/auth_test.go
@@ -22,6 +22,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook/updateref"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service/setup"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/storagemgr"
"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"
@@ -194,7 +195,7 @@ func runServer(t *testing.T, cfg config.Cfg) string {
gitCmdFactory := gittest.NewCommandFactory(t, cfg)
hookManager := hook.NewManager(cfg, locator, gitCmdFactory, txManager, gitlab.NewMockClient(
t, gitlab.MockAllowed, gitlab.MockPreReceive, gitlab.MockPostReceive,
- ))
+ ), hook.NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
catfileCache := catfile.NewCache(cfg)
t.Cleanup(catfileCache.Stop)
diskCache := cache.New(cfg, locator, logger)
diff --git a/internal/gitaly/service/dependencies.go b/internal/gitaly/service/dependencies.go
index e260a1465..923dcddbd 100644
--- a/internal/gitaly/service/dependencies.go
+++ b/internal/gitaly/service/dependencies.go
@@ -39,6 +39,7 @@ type Dependencies struct {
RepositoryCounter *counter.RepositoryCounter
UpdaterWithHooks *updateref.UpdaterWithHooks
HousekeepingManager housekeeping.Manager
+ TransactionRegistry *storagemgr.TransactionRegistry
PartitionManager *storagemgr.PartitionManager
BackupSink backup.Sink
BackupLocator backup.Locator
@@ -124,6 +125,11 @@ func (dc *Dependencies) GetPackObjectsLimiter() limiter.Limiter {
return dc.PackObjectsLimiter
}
+// GetTransactionRegistry returns the TransactionRegistry.
+func (dc *Dependencies) GetTransactionRegistry() *storagemgr.TransactionRegistry {
+ return dc.TransactionRegistry
+}
+
// GetPartitionManager returns the PartitionManager.
func (dc *Dependencies) GetPartitionManager() *storagemgr.PartitionManager {
return dc.PartitionManager
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..3dfeb9153 100644
--- a/internal/gitaly/service/hook/reference_transaction_test.go
+++ b/internal/gitaly/service/hook/reference_transaction_test.go
@@ -3,6 +3,7 @@ package hook
import (
"context"
"crypto/sha1"
+ "fmt"
"net"
"testing"
@@ -10,7 +11,9 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/featureflag"
"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/hook"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/storagemgr"
"gitlab.com/gitlab-org/gitaly/v16/internal/grpc/backchannel"
"gitlab.com/gitlab-org/gitaly/v16/internal/structerr"
"gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
@@ -22,6 +25,27 @@ import (
"google.golang.org/grpc/credentials/insecure"
)
+type mockTransactionRegistry struct {
+ getFunc func(storage.TransactionID) (hook.Transaction, error)
+}
+
+func (m mockTransactionRegistry) Get(id storage.TransactionID) (hook.Transaction, error) {
+ return m.getFunc(id)
+}
+
+type mockTransaction struct {
+ updateReferencesFunc func(storagemgr.ReferenceUpdates)
+ recordInitialReferenceValues func(context.Context, map[git.ReferenceName]git.ObjectID) error
+}
+
+func (m mockTransaction) UpdateReferences(updates storagemgr.ReferenceUpdates) {
+ m.updateReferencesFunc(updates)
+}
+
+func (m mockTransaction) RecordInitialReferenceValues(ctx context.Context, initialValues map[git.ReferenceName]git.ObjectID) error {
+ return m.recordInitialReferenceValues(ctx, initialValues)
+}
+
type testTransactionServer struct {
gitalypb.UnimplementedRefTransactionServer
handler func(in *gitalypb.VoteTransactionRequest) (*gitalypb.VoteTransactionResponse, error)
@@ -51,29 +75,45 @@ func TestReferenceTransactionHookInvalidArgument(t *testing.T) {
}
func TestReferenceTransactionHook(t *testing.T) {
+ stdin := []byte(fmt.Sprintf(
+ `%[1]s %[2]s refs/heads/branch-1
+%[2]s %[1]s refs/heads/branch-2
+%[1]s %[2]s HEAD
+`,
+ gittest.DefaultObjectHash.ZeroOID,
+ gittest.DefaultObjectHash.EmptyTreeOID,
+ ))
+
testCases := []struct {
- desc string
- stdin []byte
- state gitalypb.ReferenceTransactionHookRequest_State
- voteResponse gitalypb.VoteTransactionResponse_TransactionState
- expectedErr error
- expectedResponse *gitalypb.ReferenceTransactionHookResponse
- expectedReftxHash []byte
+ desc string
+ stdin []byte
+ state gitalypb.ReferenceTransactionHookRequest_State
+ voteResponse gitalypb.VoteTransactionResponse_TransactionState
+ noTransaction bool
+ expectedErr error
+ expectedResponse *gitalypb.ReferenceTransactionHookResponse
+ expectedReftxHash []byte
+ expectedReferenceUpdates storagemgr.ReferenceUpdates
+ expectedInitialValues map[git.ReferenceName]git.ObjectID
}{
{
desc: "hook triggers transaction with default state",
- stdin: []byte("foobar"),
+ stdin: stdin,
voteResponse: gitalypb.VoteTransactionResponse_COMMIT,
expectedResponse: &gitalypb.ReferenceTransactionHookResponse{
ExitStatus: &gitalypb.ExitStatus{
Value: 0,
},
},
- expectedReftxHash: []byte("foobar"),
+ expectedReftxHash: stdin,
+ expectedInitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/branch-1": gittest.DefaultObjectHash.ZeroOID,
+ "refs/heads/branch-2": gittest.DefaultObjectHash.EmptyTreeOID,
+ },
},
{
desc: "hook triggers transaction with explicit prepared state",
- stdin: []byte("foobar"),
+ stdin: stdin,
state: gitalypb.ReferenceTransactionHookRequest_PREPARED,
voteResponse: gitalypb.VoteTransactionResponse_COMMIT,
expectedResponse: &gitalypb.ReferenceTransactionHookResponse{
@@ -81,11 +121,28 @@ func TestReferenceTransactionHook(t *testing.T) {
Value: 0,
},
},
- expectedReftxHash: []byte("foobar"),
+ expectedReftxHash: stdin,
+ expectedInitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/branch-1": gittest.DefaultObjectHash.ZeroOID,
+ "refs/heads/branch-2": gittest.DefaultObjectHash.EmptyTreeOID,
+ },
+ },
+ {
+ desc: "hook triggers transaction with explicit prepared state without transaction",
+ stdin: stdin,
+ state: gitalypb.ReferenceTransactionHookRequest_PREPARED,
+ voteResponse: gitalypb.VoteTransactionResponse_COMMIT,
+ noTransaction: true,
+ expectedResponse: &gitalypb.ReferenceTransactionHookResponse{
+ ExitStatus: &gitalypb.ExitStatus{
+ Value: 0,
+ },
+ },
+ expectedReftxHash: stdin,
},
{
desc: "hook does not trigger transaction with aborted state",
- stdin: []byte("foobar"),
+ stdin: stdin,
state: gitalypb.ReferenceTransactionHookRequest_ABORTED,
expectedResponse: &gitalypb.ReferenceTransactionHookResponse{
ExitStatus: &gitalypb.ExitStatus{
@@ -95,28 +152,76 @@ func TestReferenceTransactionHook(t *testing.T) {
},
{
desc: "hook triggers transaction with committed state",
- stdin: []byte("foobar"),
+ stdin: stdin,
state: gitalypb.ReferenceTransactionHookRequest_COMMITTED,
expectedResponse: &gitalypb.ReferenceTransactionHookResponse{
ExitStatus: &gitalypb.ExitStatus{
Value: 0,
},
},
- expectedReftxHash: []byte("foobar"),
+ expectedReftxHash: stdin,
+ expectedReferenceUpdates: storagemgr.ReferenceUpdates{
+ "refs/heads/branch-1": {
+ OldOID: gittest.DefaultObjectHash.ZeroOID,
+ NewOID: gittest.DefaultObjectHash.EmptyTreeOID,
+ },
+ "refs/heads/branch-2": {
+ OldOID: gittest.DefaultObjectHash.EmptyTreeOID,
+ NewOID: gittest.DefaultObjectHash.ZeroOID,
+ },
+ },
+ },
+ {
+ desc: "hook triggers transaction with committed state without transaction",
+ stdin: stdin,
+ state: gitalypb.ReferenceTransactionHookRequest_COMMITTED,
+ noTransaction: true,
+ expectedResponse: &gitalypb.ReferenceTransactionHookResponse{
+ ExitStatus: &gitalypb.ExitStatus{
+ Value: 0,
+ },
+ },
+ expectedReftxHash: stdin,
},
{
desc: "hook fails with failed vote",
- stdin: []byte("foobar"),
+ stdin: stdin,
voteResponse: gitalypb.VoteTransactionResponse_ABORT,
expectedErr: structerr.NewAborted("reference-transaction hook: error voting on transaction: transaction was aborted"),
- expectedReftxHash: []byte("foobar"),
+ expectedReftxHash: stdin,
+ expectedInitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/branch-1": gittest.DefaultObjectHash.ZeroOID,
+ "refs/heads/branch-2": gittest.DefaultObjectHash.EmptyTreeOID,
+ },
},
{
desc: "hook fails with stopped vote",
- stdin: []byte("foobar"),
+ stdin: stdin,
voteResponse: gitalypb.VoteTransactionResponse_STOP,
expectedErr: structerr.NewFailedPrecondition("reference-transaction hook: error voting on transaction: transaction was stopped"),
- expectedReftxHash: []byte("foobar"),
+ expectedReftxHash: stdin,
+ expectedInitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/branch-1": gittest.DefaultObjectHash.ZeroOID,
+ "refs/heads/branch-2": gittest.DefaultObjectHash.EmptyTreeOID,
+ },
+ },
+ {
+ desc: "invalid change line",
+ stdin: []byte("invalid change_line"),
+ voteResponse: gitalypb.VoteTransactionResponse_STOP,
+ expectedErr: structerr.NewInternal(`reference-transaction hook: parse changes: unexpected change line: "invalid change_line"`),
+ },
+ {
+ desc: "invalid old oid",
+ stdin: []byte(fmt.Sprintf("invalid %s refs/heads/main", gittest.DefaultObjectHash.EmptyTreeOID)),
+ voteResponse: gitalypb.VoteTransactionResponse_STOP,
+ expectedErr: structerr.NewInternal(`reference-transaction hook: parse changes: parse old: invalid object ID: "invalid", expected length %d, got 7`, gittest.DefaultObjectHash.EncodedLen()),
+ },
+ {
+ desc: "invalid new oid",
+ stdin: []byte(fmt.Sprintf("%s invalid refs/heads/main", gittest.DefaultObjectHash.EmptyTreeOID)),
+ voteResponse: gitalypb.VoteTransactionResponse_STOP,
+ expectedErr: structerr.NewInternal(`reference-transaction hook: parse changes: parse new: invalid object ID: "invalid", expected length %d, got 7`, gittest.DefaultObjectHash.EncodedLen()),
},
}
@@ -155,11 +260,31 @@ func TestReferenceTransactionHook(t *testing.T) {
}, nil
}
- cfg.SocketPath = runHooksServer(t, cfg, nil, testserver.WithBackchannelRegistry(registry))
+ var actualReferenceUpdates storagemgr.ReferenceUpdates
+ var actualInitialValues map[git.ReferenceName]git.ObjectID
+ txRegistry := mockTransactionRegistry{
+ getFunc: func(storage.TransactionID) (hook.Transaction, error) {
+ return mockTransaction{
+ updateReferencesFunc: func(updates storagemgr.ReferenceUpdates) {
+ actualReferenceUpdates = updates
+ },
+ recordInitialReferenceValues: func(_ context.Context, initialValues map[git.ReferenceName]git.ObjectID) error {
+ actualInitialValues = initialValues
+ return nil
+ },
+ }, nil
+ },
+ }
+
+ cfg.SocketPath = runHooksServerWithTransactionRegistry(t, cfg, nil, txRegistry, testserver.WithBackchannelRegistry(registry))
ctx := testhelper.Context(t)
repo, _ := gittest.CreateRepository(t, ctx, cfg)
+ transactionID := storage.TransactionID(1)
+ if tc.noTransaction {
+ transactionID = 0
+ }
hooksPayload, err := git.NewHooksPayload(
cfg,
repo,
@@ -172,6 +297,7 @@ func TestReferenceTransactionHook(t *testing.T) {
nil,
git.ReferenceTransactionHook,
featureflag.FromContext(ctx),
+ transactionID,
).Env()
require.NoError(t, err)
@@ -200,10 +326,13 @@ func TestReferenceTransactionHook(t *testing.T) {
var expectedReftxHash []byte
if tc.expectedReftxHash != nil {
- hash := sha1.Sum(tc.expectedReftxHash)
+ hash := sha1.Sum(tc.stdin)
expectedReftxHash = hash[:]
}
require.Equal(t, expectedReftxHash[:], reftxHash)
+
+ require.Equal(t, tc.expectedReferenceUpdates, actualReferenceUpdates)
+ require.Equal(t, tc.expectedInitialValues, actualInitialValues)
})
}
}
diff --git a/internal/gitaly/service/hook/testhelper_test.go b/internal/gitaly/service/hook/testhelper_test.go
index f55cbea57..d3f541193 100644
--- a/internal/gitaly/service/hook/testhelper_test.go
+++ b/internal/gitaly/service/hook/testhelper_test.go
@@ -4,6 +4,7 @@ import (
"testing"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config"
+ gitalyhook "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/service/repository"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction"
@@ -48,11 +49,19 @@ func newHooksClient(tb testing.TB, serverSocketPath string) (gitalypb.HookServic
type serverOption func(*server)
func runHooksServer(tb testing.TB, cfg config.Cfg, opts []serverOption, serverOpts ...testserver.GitalyServerOpt) string {
+ return runHooksServerWithTransactionRegistry(tb, cfg, opts, nil, serverOpts...)
+}
+
+func runHooksServerWithTransactionRegistry(tb testing.TB, cfg config.Cfg, opts []serverOption, txRegistry gitalyhook.TransactionRegistry, serverOpts ...testserver.GitalyServerOpt) string {
tb.Helper()
serverOpts = append(serverOpts, testserver.WithDisablePraefect())
return testserver.RunGitalyServer(tb, cfg, func(srv *grpc.Server, deps *service.Dependencies) {
+ if txRegistry != nil {
+ deps.GitalyHookManager = gitalyhook.NewManager(deps.GetCfg(), deps.GetLocator(), deps.GetGitCmdFactory(), deps.GetTxManager(), deps.GetGitlabClient(), txRegistry)
+ }
+
hookServer := NewServer(deps)
for _, opt := range opts {
opt(hookServer.(*server))
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)
diff --git a/internal/gitaly/service/operations/merge_branch_test.go b/internal/gitaly/service/operations/merge_branch_test.go
index a74b2fafd..6bf9d04c6 100644
--- a/internal/gitaly/service/operations/merge_branch_test.go
+++ b/internal/gitaly/service/operations/merge_branch_test.go
@@ -20,6 +20,7 @@ import (
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook"
"gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/storagemgr"
"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"
@@ -1117,7 +1118,7 @@ func testUserMergeBranchAllowed(t *testing.T, ctx context.Context) {
},
gitlab.MockPreReceive,
gitlab.MockPostReceive,
- ))
+ ), hook.NewTransactionRegistry(storagemgr.NewTransactionRegistry()))
ctx, cfg, client := setupOperationsServiceWithCfg(
t, ctx, cfg,
diff --git a/internal/gitaly/storage/storagemgr/transaction_manager.go b/internal/gitaly/storage/storagemgr/transaction_manager.go
index af0174b1f..5de67ab82 100644
--- a/internal/gitaly/storage/storagemgr/transaction_manager.go
+++ b/internal/gitaly/storage/storagemgr/transaction_manager.go
@@ -90,9 +90,6 @@ func (index LogIndex) String() string {
// ReferenceUpdate describes the state of a reference's old and new tip in an update.
type ReferenceUpdate struct {
- // Force indicates this is a forced reference update. If set, the reference is pointed
- // to the new value regardless of the old value.
- Force bool
// OldOID is the old OID the reference is expected to point to prior to updating it.
// If the reference does not point to the old value, the reference verification fails.
OldOID git.ObjectID
@@ -182,6 +179,7 @@ type Transaction struct {
snapshot Snapshot
skipVerificationFailures bool
+ initialReferenceValues map[git.ReferenceName]git.ObjectID
referenceUpdates ReferenceUpdates
defaultBranchUpdate *DefaultBranchUpdate
customHooksUpdate *CustomHooksUpdate
@@ -403,10 +401,75 @@ func (txn *Transaction) SkipVerificationFailures() {
txn.skipVerificationFailures = true
}
-// UpdateReferences updates the given references as part of the transaction. If UpdateReferences is called
-// multiple times, only the changes from the latest invocation take place.
+// RecordInitialReferenceValues records the initial values of the reference if they haven't yet been recorded. If oid is
+// not a zero OID, it's used as the initial value. If oid is a zero value, the reference's actual value is resolved.
+//
+// The reference's first recorded value is used as its old OID in the final committed update. RecordInitialReferenceValues
+// can be used to record the value without staging an update in the transaction. This is useful for example generally recording
+// the initial value in the 'prepare' phase of the reference transaction hook before any changes are made without staging
+// any updates before the 'committed' phase is reached.
+func (txn *Transaction) RecordInitialReferenceValues(ctx context.Context, initialValues map[git.ReferenceName]git.ObjectID) error {
+ if txn.initialReferenceValues == nil {
+ txn.initialReferenceValues = make(map[git.ReferenceName]git.ObjectID, len(initialValues))
+ }
+
+ for reference, oid := range initialValues {
+ if _, ok := txn.initialReferenceValues[reference]; ok {
+ // If the reference's starting value has already been recorded, we don't have to record it again.
+ continue
+ }
+
+ objectHash, err := txn.stagingRepository.ObjectHash(ctx)
+ if err != nil {
+ return fmt.Errorf("object hash: %w", err)
+ }
+
+ if objectHash.IsZeroOID(oid) {
+ // If this is a zero OID, resolve the value to see if this is a force update or the
+ // reference doesn't exist.
+ if current, err := txn.stagingRepository.ResolveRevision(ctx, reference.Revision()); err != nil {
+ if !errors.Is(err, git.ErrReferenceNotFound) {
+ return fmt.Errorf("resolve revision: %w", err)
+ }
+
+ // The reference doesn't exist, leave the value as zero oid.
+ } else {
+ oid = current
+ }
+ }
+
+ txn.initialReferenceValues[reference] = oid
+ }
+
+ return nil
+}
+
+// UpdateReferences updates the given references as part of the transaction.
+//
+// If a reference is updated multiple times during a transaction, its first recorded old OID is kept
+// and the new OID is updated. This means updates like 'oid-1 -> oid-2 -> oid-3' will ultimately be
+// committed as 'oid-1 -> oid-3'. The intermediate states are not relevant when committing the write
+// to the actual repository.
func (txn *Transaction) UpdateReferences(updates ReferenceUpdates) {
- txn.referenceUpdates = updates
+ if txn.referenceUpdates == nil {
+ txn.referenceUpdates = ReferenceUpdates{}
+ }
+
+ for reference, update := range updates {
+ oldOID := update.OldOID
+ if initialValue, ok := txn.initialReferenceValues[reference]; ok {
+ oldOID = initialValue
+ }
+
+ if previousUpdate, ok := txn.referenceUpdates[reference]; ok {
+ oldOID = previousUpdate.OldOID
+ }
+
+ txn.referenceUpdates[reference] = ReferenceUpdate{
+ OldOID: oldOID,
+ NewOID: update.NewOID,
+ }
+ }
}
// DeleteRepository deletes the repository when the transaction is committed.
@@ -455,9 +518,6 @@ func (txn *Transaction) walFilesPath() string {
// - The reference verification failures can be ignored instead of aborting the entire transaction.
// If done, the references that failed verification are dropped from the transaction but the updates
// that passed verification are still performed.
-// - The reference verification may also be skipped if the write is force updating references. If
-// done, the current state of the references is ignored and they are directly updated to point
-// to the new tips.
// 2. The transaction is appended to the write-ahead log. Once the write has been logged, it is effectively
// committed and will be applied to the repository even after restarting.
// 3. The transaction is applied from the write-ahead log to the repository by actually performing the reference
@@ -1222,29 +1282,27 @@ func (mgr *TransactionManager) verifyReferences(ctx context.Context, transaction
return nil, InvalidReferenceFormatError{ReferenceName: referenceName}
}
- if !update.Force {
- actualOldTip, err := transaction.stagingRepository.ResolveRevision(ctx, referenceName.Revision())
- if errors.Is(err, git.ErrReferenceNotFound) {
- objectHash, err := transaction.stagingRepository.ObjectHash(ctx)
- if err != nil {
- return nil, fmt.Errorf("object hash: %w", err)
- }
-
- actualOldTip = objectHash.ZeroOID
- } else if err != nil {
- return nil, fmt.Errorf("resolve revision: %w", err)
+ actualOldTip, err := transaction.stagingRepository.ResolveRevision(ctx, referenceName.Revision())
+ if errors.Is(err, git.ErrReferenceNotFound) {
+ objectHash, err := transaction.stagingRepository.ObjectHash(ctx)
+ if err != nil {
+ return nil, fmt.Errorf("object hash: %w", err)
}
- if update.OldOID != actualOldTip {
- if transaction.skipVerificationFailures {
- continue
- }
+ actualOldTip = objectHash.ZeroOID
+ } else if err != nil {
+ return nil, fmt.Errorf("resolve revision: %w", err)
+ }
- return nil, ReferenceVerificationError{
- ReferenceName: referenceName,
- ExpectedOID: update.OldOID,
- ActualOID: actualOldTip,
- }
+ if update.OldOID != actualOldTip {
+ if transaction.skipVerificationFailures {
+ continue
+ }
+
+ return nil, ReferenceVerificationError{
+ ReferenceName: referenceName,
+ ExpectedOID: update.OldOID,
+ ActualOID: actualOldTip,
}
}
diff --git a/internal/gitaly/storage/storagemgr/transaction_manager_test.go b/internal/gitaly/storage/storagemgr/transaction_manager_test.go
index d06af7e2c..7575861c6 100644
--- a/internal/gitaly/storage/storagemgr/transaction_manager_test.go
+++ b/internal/gitaly/storage/storagemgr/transaction_manager_test.go
@@ -352,6 +352,22 @@ func TestTransactionManager(t *testing.T) {
ExpectedError error
}
+ // RecordInitialReferenceValues calls RecordInitialReferenceValues on a transaction.
+ type RecordInitialReferenceValues struct {
+ // TransactionID identifies the transaction to prepare the reference updates on.
+ TransactionID int
+ // InitialValues are the initial values to record.
+ InitialValues map[git.ReferenceName]git.ObjectID
+ }
+
+ // UpdateReferences calls UpdateReferences on a transaction.
+ type UpdateReferences struct {
+ // TransactionID identifies the transaction to update references on.
+ TransactionID int
+ // ReferenceUpdates are the reference updates to make.
+ ReferenceUpdates ReferenceUpdates
+ }
+
// Rollback calls Rollback on a transaction.
type Rollback struct {
// TransactionID identifies the transaction to rollback.
@@ -1379,6 +1395,318 @@ func TestTransactionManager(t *testing.T) {
},
},
{
+ desc: "update reference multiple times successfully in a transaction",
+ steps: steps{
+ StartManager{},
+ Begin{},
+ UpdateReferences{
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {OldOID: setup.ObjectHash.ZeroOID, NewOID: setup.Commits.First.OID},
+ },
+ },
+ UpdateReferences{
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {OldOID: setup.Commits.First.OID, NewOID: setup.Commits.Second.OID},
+ },
+ },
+ Commit{},
+ },
+ expectedState: StateAssertion{
+ Database: DatabaseState{
+ string(keyAppliedLogIndex(relativePath)): LogIndex(1).toProto(),
+ },
+ Repositories: RepositoryStates{
+ relativePath: {
+ References: []git.Reference{{Name: "refs/heads/main", Target: setup.Commits.Second.OID.String()}},
+ },
+ },
+ },
+ },
+ {
+ desc: "update reference multiple times fails due to wrong initial value",
+ steps: steps{
+ StartManager{},
+ Begin{},
+ UpdateReferences{
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {OldOID: setup.Commits.First.OID, NewOID: setup.Commits.Second.OID},
+ },
+ },
+ UpdateReferences{
+ ReferenceUpdates: ReferenceUpdates{
+ // The old oid should be ignored since there's already a recorded initial value for the
+ // reference.
+ "refs/heads/main": {NewOID: setup.Commits.Third.OID},
+ },
+ },
+ Commit{
+ ExpectedError: ReferenceVerificationError{
+ ReferenceName: "refs/heads/main",
+ ExpectedOID: setup.Commits.First.OID,
+ ActualOID: setup.ObjectHash.ZeroOID,
+ },
+ },
+ },
+ },
+ {
+ desc: "recording initial value of a reference stages no updates",
+ steps: steps{
+ StartManager{},
+ Begin{},
+ RecordInitialReferenceValues{
+ InitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/main": setup.Commits.First.OID,
+ },
+ },
+ Commit{},
+ },
+ expectedState: StateAssertion{
+ Database: DatabaseState{
+ string(keyAppliedLogIndex(relativePath)): LogIndex(1).toProto(),
+ },
+ },
+ },
+ {
+ desc: "update reference with non-existent initial value",
+ steps: steps{
+ StartManager{},
+ Begin{},
+ RecordInitialReferenceValues{
+ InitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/main": setup.ObjectHash.ZeroOID,
+ },
+ },
+ UpdateReferences{
+ // The old oid is ignored as the references old value was already recorded.
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {NewOID: setup.Commits.First.OID},
+ },
+ },
+ Commit{},
+ },
+ expectedState: StateAssertion{
+ Database: DatabaseState{
+ string(keyAppliedLogIndex(relativePath)): LogIndex(1).toProto(),
+ },
+ Repositories: RepositoryStates{
+ relativePath: {
+ References: []git.Reference{{Name: "refs/heads/main", Target: setup.Commits.First.OID.String()}},
+ },
+ },
+ },
+ },
+ {
+ desc: "update reference with the zero oid initial value",
+ steps: steps{
+ StartManager{},
+ Begin{
+ TransactionID: 1,
+ },
+ Commit{
+ TransactionID: 1,
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {OldOID: setup.ObjectHash.ZeroOID, NewOID: setup.Commits.First.OID},
+ },
+ },
+ Begin{
+ TransactionID: 2,
+ ExpectedSnapshot: Snapshot{
+ ReadIndex: 1,
+ },
+ },
+ RecordInitialReferenceValues{
+ TransactionID: 2,
+ InitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/main": setup.ObjectHash.ZeroOID,
+ },
+ },
+ UpdateReferences{
+ TransactionID: 2,
+ // The old oid is ignored as the references old value was already recorded.
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {NewOID: setup.Commits.Second.OID},
+ },
+ },
+ Commit{
+ TransactionID: 2,
+ },
+ },
+ expectedState: StateAssertion{
+ Database: DatabaseState{
+ string(keyAppliedLogIndex(relativePath)): LogIndex(2).toProto(),
+ },
+ Repositories: RepositoryStates{
+ relativePath: {
+ References: []git.Reference{{Name: "refs/heads/main", Target: setup.Commits.Second.OID.String()}},
+ },
+ },
+ },
+ },
+ {
+ desc: "update reference with the correct initial value",
+ steps: steps{
+ StartManager{},
+ Begin{
+ TransactionID: 1,
+ },
+ Commit{
+ TransactionID: 1,
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {OldOID: setup.ObjectHash.ZeroOID, NewOID: setup.Commits.First.OID},
+ },
+ },
+ Begin{
+ TransactionID: 2,
+ ExpectedSnapshot: Snapshot{
+ ReadIndex: 1,
+ },
+ },
+ RecordInitialReferenceValues{
+ TransactionID: 2,
+ InitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/main": setup.Commits.First.OID,
+ },
+ },
+ UpdateReferences{
+ TransactionID: 2,
+ // The old oid is ignored as the references old value was already recorded.
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {NewOID: setup.Commits.Second.OID},
+ },
+ },
+ Commit{
+ TransactionID: 2,
+ },
+ },
+ expectedState: StateAssertion{
+ Database: DatabaseState{
+ string(keyAppliedLogIndex(relativePath)): LogIndex(2).toProto(),
+ },
+ Repositories: RepositoryStates{
+ relativePath: {
+ References: []git.Reference{{Name: "refs/heads/main", Target: setup.Commits.Second.OID.String()}},
+ },
+ },
+ },
+ },
+ {
+ desc: "update reference with the incorrect initial value",
+ steps: steps{
+ StartManager{},
+ Begin{
+ TransactionID: 1,
+ },
+ Commit{
+ TransactionID: 1,
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {OldOID: setup.ObjectHash.ZeroOID, NewOID: setup.Commits.First.OID},
+ },
+ },
+ Begin{
+ TransactionID: 2,
+ ExpectedSnapshot: Snapshot{
+ ReadIndex: 1,
+ },
+ },
+ RecordInitialReferenceValues{
+ TransactionID: 2,
+ InitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/main": setup.Commits.Third.OID,
+ },
+ },
+ UpdateReferences{
+ TransactionID: 2,
+ // The old oid is ignored as the references old value was already recorded.
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {NewOID: setup.Commits.Second.OID},
+ },
+ },
+ Commit{
+ TransactionID: 2,
+ ExpectedError: ReferenceVerificationError{
+ ReferenceName: "refs/heads/main",
+ ExpectedOID: setup.Commits.Third.OID,
+ ActualOID: setup.Commits.First.OID,
+ },
+ },
+ },
+ expectedState: StateAssertion{
+ Database: DatabaseState{
+ string(keyAppliedLogIndex(relativePath)): LogIndex(1).toProto(),
+ },
+ Repositories: RepositoryStates{
+ relativePath: {
+ References: []git.Reference{{Name: "refs/heads/main", Target: setup.Commits.First.OID.String()}},
+ },
+ },
+ },
+ },
+ {
+ desc: "initial value is only recorded on the first time",
+ steps: steps{
+ StartManager{},
+ Begin{},
+ RecordInitialReferenceValues{
+ InitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/main": setup.ObjectHash.ZeroOID,
+ },
+ },
+ RecordInitialReferenceValues{
+ InitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/main": setup.Commits.Third.OID,
+ "refs/heads/branch-2": setup.ObjectHash.ZeroOID,
+ },
+ },
+ Commit{
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {NewOID: setup.Commits.First.OID},
+ "refs/heads/branch-2": {NewOID: setup.Commits.First.OID},
+ },
+ },
+ },
+ expectedState: StateAssertion{
+ Database: DatabaseState{
+ string(keyAppliedLogIndex(relativePath)): LogIndex(1).toProto(),
+ },
+ Repositories: RepositoryStates{
+ relativePath: {
+ References: []git.Reference{
+ {Name: "refs/heads/branch-2", Target: setup.Commits.First.OID.String()},
+ {Name: "refs/heads/main", Target: setup.Commits.First.OID.String()},
+ },
+ },
+ },
+ },
+ },
+ {
+ desc: "initial value is set on the first update",
+ steps: steps{
+ StartManager{},
+ Begin{},
+ UpdateReferences{
+ ReferenceUpdates: ReferenceUpdates{
+ "refs/heads/main": {OldOID: setup.ObjectHash.ZeroOID, NewOID: setup.Commits.First.OID},
+ },
+ },
+ RecordInitialReferenceValues{
+ InitialValues: map[git.ReferenceName]git.ObjectID{
+ "refs/heads/main": setup.Commits.Third.OID,
+ },
+ },
+ Commit{},
+ },
+ expectedState: StateAssertion{
+ Database: DatabaseState{
+ string(keyAppliedLogIndex(relativePath)): LogIndex(1).toProto(),
+ },
+ Repositories: RepositoryStates{
+ relativePath: {
+ References: []git.Reference{{Name: "refs/heads/main", Target: setup.Commits.First.OID.String()}},
+ },
+ },
+ },
+ },
+ {
desc: "set custom hooks successfully",
steps: steps{
StartManager{},
@@ -3445,99 +3773,6 @@ func TestTransactionManager(t *testing.T) {
},
},
{
- desc: "forced reference creation succeeds",
- steps: steps{
- StartManager{},
- Begin{},
- Commit{
- ReferenceUpdates: ReferenceUpdates{
- "refs/heads/main": {Force: true, NewOID: setup.Commits.First.OID},
- },
- },
- },
- expectedState: StateAssertion{
- Database: DatabaseState{
- string(keyAppliedLogIndex(relativePath)): LogIndex(1).toProto(),
- },
- Repositories: RepositoryStates{
- relativePath: {
- DefaultBranch: "refs/heads/main",
- References: []git.Reference{{Name: "refs/heads/main", Target: setup.Commits.First.OID.String()}},
- },
- },
- },
- },
- {
- desc: "forced reference update succeeds",
- steps: steps{
- StartManager{},
- Begin{
- TransactionID: 1,
- },
- Commit{
- TransactionID: 1,
- ReferenceUpdates: ReferenceUpdates{
- "refs/heads/main": {OldOID: setup.ObjectHash.ZeroOID, NewOID: setup.Commits.First.OID},
- },
- },
- Begin{
- TransactionID: 2,
- ExpectedSnapshot: Snapshot{
- ReadIndex: 1,
- },
- },
- Commit{
- TransactionID: 2,
- ReferenceUpdates: ReferenceUpdates{
- "refs/heads/main": {Force: true, NewOID: setup.Commits.Second.OID},
- },
- },
- },
- expectedState: StateAssertion{
- Database: DatabaseState{
- string(keyAppliedLogIndex(relativePath)): LogIndex(2).toProto(),
- },
- Repositories: RepositoryStates{
- relativePath: {
- DefaultBranch: "refs/heads/main",
- References: []git.Reference{{Name: "refs/heads/main", Target: setup.Commits.Second.OID.String()}},
- },
- },
- },
- },
- {
- desc: "forced reference deletion succeeds",
- steps: steps{
- StartManager{},
- Begin{
- TransactionID: 1,
- },
- Commit{
- TransactionID: 1,
- ReferenceUpdates: ReferenceUpdates{
- "refs/heads/main": {OldOID: setup.ObjectHash.ZeroOID, NewOID: setup.Commits.First.OID},
- },
- },
- Begin{
- TransactionID: 2,
- ExpectedSnapshot: Snapshot{
- ReadIndex: 1,
- },
- },
- Commit{
- TransactionID: 2,
- ReferenceUpdates: ReferenceUpdates{
- "refs/heads/main": {Force: true, NewOID: setup.ObjectHash.ZeroOID},
- },
- },
- },
- expectedState: StateAssertion{
- Database: DatabaseState{
- string(keyAppliedLogIndex(relativePath)): LogIndex(2).toProto(),
- },
- },
- },
- {
desc: "transaction rollbacked after already being rollbacked",
steps: steps{
StartManager{},
@@ -3699,7 +3934,7 @@ func TestTransactionManager(t *testing.T) {
Begin{},
Commit{
ReferenceUpdates: ReferenceUpdates{
- tc.referenceName: {Force: true, NewOID: setup.Commits.First.OID},
+ tc.referenceName: {OldOID: setup.ObjectHash.ZeroOID, NewOID: setup.Commits.First.OID},
},
ExpectedError: InvalidReferenceFormatError{ReferenceName: tc.referenceName},
},
@@ -3989,6 +4224,16 @@ func TestTransactionManager(t *testing.T) {
// determine that the deletion has actually been admitted, and is waiting for application to ensure the commit order is always
// as expected by the test.
<-transaction.admitted
+ case RecordInitialReferenceValues:
+ require.Contains(t, openTransactions, step.TransactionID, "test error: record initial reference value on transaction before beginning it")
+
+ transaction := openTransactions[step.TransactionID]
+ require.NoError(t, transaction.RecordInitialReferenceValues(ctx, step.InitialValues))
+ case UpdateReferences:
+ require.Contains(t, openTransactions, step.TransactionID, "test error: reference updates aborted on committed before beginning it")
+
+ transaction := openTransactions[step.TransactionID]
+ transaction.UpdateReferences(step.ReferenceUpdates)
case Rollback:
require.Contains(t, openTransactions, step.TransactionID, "test error: transaction rollbacked before beginning it")
require.Equal(t, step.ExpectedError, openTransactions[step.TransactionID].Rollback())
diff --git a/internal/gitaly/storage/storagemgr/transaction_registry.go b/internal/gitaly/storage/storagemgr/transaction_registry.go
new file mode 100644
index 000000000..b11508acc
--- /dev/null
+++ b/internal/gitaly/storage/storagemgr/transaction_registry.go
@@ -0,0 +1,51 @@
+package storagemgr
+
+import (
+ "errors"
+ "sync"
+
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
+)
+
+var errTransactionNotFound = errors.New("transaction not found")
+
+// TransactionRegistry stores references to transactions by their ID.
+type TransactionRegistry struct {
+ m sync.RWMutex
+ idSequence storage.TransactionID
+ transactions map[storage.TransactionID]*Transaction
+}
+
+// NewTransactionRegistry returns a new TransactionRegistry.
+func NewTransactionRegistry() *TransactionRegistry {
+ return &TransactionRegistry{
+ transactions: make(map[storage.TransactionID]*Transaction),
+ }
+}
+
+func (r *TransactionRegistry) register(tx *Transaction) storage.TransactionID {
+ r.m.Lock()
+ defer r.m.Unlock()
+
+ r.idSequence++
+ r.transactions[r.idSequence] = tx
+ return r.idSequence
+}
+
+func (r *TransactionRegistry) unregister(id storage.TransactionID) {
+ r.m.Lock()
+ defer r.m.Unlock()
+ delete(r.transactions, id)
+}
+
+// Get retrieves a transaction by its ID. An error when a transaction is not found.
+func (r *TransactionRegistry) Get(id storage.TransactionID) (*Transaction, error) {
+ r.m.RLock()
+ defer r.m.RUnlock()
+ tx, ok := r.transactions[id]
+ if !ok {
+ return nil, errTransactionNotFound
+ }
+
+ return tx, nil
+}
diff --git a/internal/gitaly/storage/storagemgr/transaction_registry_test.go b/internal/gitaly/storage/storagemgr/transaction_registry_test.go
new file mode 100644
index 000000000..f1f0dddaa
--- /dev/null
+++ b/internal/gitaly/storage/storagemgr/transaction_registry_test.go
@@ -0,0 +1,33 @@
+package storagemgr
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage"
+)
+
+func TestTransactionRegistry(t *testing.T) {
+ registry := NewTransactionRegistry()
+
+ expectedTX1 := &Transaction{}
+ txID1 := registry.register(expectedTX1)
+ require.Equal(t, txID1, storage.TransactionID(1))
+
+ expectedTX2 := &Transaction{}
+ txID2 := registry.register(expectedTX2)
+ require.Equal(t, txID2, storage.TransactionID(2))
+
+ actualTX, err := registry.Get(txID1)
+ require.NoError(t, err)
+ require.Same(t, expectedTX1, actualTX)
+
+ registry.unregister(txID1)
+ actualTX, err = registry.Get(txID1)
+ require.Equal(t, errTransactionNotFound, err)
+ require.Nil(t, actualTX)
+
+ actualTX, err = registry.Get(txID2)
+ require.NoError(t, err)
+ require.Same(t, expectedTX2, actualTX)
+}
diff --git a/internal/gitaly/storage/transaction.go b/internal/gitaly/storage/transaction.go
new file mode 100644
index 000000000..689b349e5
--- /dev/null
+++ b/internal/gitaly/storage/transaction.go
@@ -0,0 +1,27 @@
+package storage
+
+import (
+ "context"
+)
+
+// TransactionID is an ID that uniquely identifies a Transaction.
+type TransactionID uint64
+
+// keyTransactionID is the context key storing a TransactionID.
+type keyTransactionID struct{}
+
+// ContextWithTransactionID stores the transaction id in the context.
+func ContextWithTransactionID(ctx context.Context, id TransactionID) context.Context {
+ return context.WithValue(ctx, keyTransactionID{}, id)
+}
+
+// ExtractTransactionID extracts the transaction ID from the context. The returned ID is zero
+// if there was no transaction ID in the context.
+func ExtractTransactionID(ctx context.Context) TransactionID {
+ value := ctx.Value(keyTransactionID{})
+ if value == nil {
+ return 0
+ }
+
+ return value.(TransactionID)
+}
diff --git a/internal/gitaly/storage/transaction_test.go b/internal/gitaly/storage/transaction_test.go
new file mode 100644
index 000000000..9d3c5963c
--- /dev/null
+++ b/internal/gitaly/storage/transaction_test.go
@@ -0,0 +1,26 @@
+package storage
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/require"
+ "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper"
+)
+
+func TestContextWithTransactionID(t *testing.T) {
+ t.Run("no transaction id in context", func(t *testing.T) {
+ require.Equal(t,
+ TransactionID(0),
+ ExtractTransactionID(testhelper.Context(t)),
+ )
+ })
+
+ t.Run("transaction id in context", func(t *testing.T) {
+ require.Equal(t,
+ TransactionID(1),
+ ExtractTransactionID(
+ ContextWithTransactionID(testhelper.Context(t), 1),
+ ),
+ )
+ })
+}
diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go
index 2875d2e1d..e7b2659e7 100644
--- a/internal/testhelper/testserver/gitaly.go
+++ b/internal/testhelper/testserver/gitaly.go
@@ -303,8 +303,9 @@ func (gsd *gitalyServerDeps) createDependencies(tb testing.TB, cfg config.Cfg) *
gsd.gitCmdFactory = gittest.NewCommandFactory(tb, cfg)
}
+ transactionRegistry := storagemgr.NewTransactionRegistry()
if gsd.hookMgr == nil {
- gsd.hookMgr = hook.NewManager(cfg, gsd.locator, gsd.gitCmdFactory, gsd.txMgr, gsd.gitlabClient)
+ gsd.hookMgr = hook.NewManager(cfg, gsd.locator, gsd.gitCmdFactory, gsd.txMgr, gsd.gitlabClient, hook.NewTransactionRegistry(transactionRegistry))
}
if gsd.catfileCache == nil {
@@ -382,6 +383,7 @@ func (gsd *gitalyServerDeps) createDependencies(tb testing.TB, cfg config.Cfg) *
RepositoryCounter: gsd.repositoryCounter,
UpdaterWithHooks: gsd.updaterWithHooks,
HousekeepingManager: gsd.housekeepingManager,
+ TransactionRegistry: transactionRegistry,
PartitionManager: partitionManager,
BackupSink: gsd.backupSink,
BackupLocator: gsd.backupLocator,