From bd861bcdfb7740fad2cb13e9a2d718358a3e9ce2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 29 Jul 2022 12:22:23 +0200 Subject: updateref: Support repositories using SHA256 object hashes The updateref package needs to be able to verify object hashes, which mandates that it must be able to tell which object hash format a repo is using and to provide the correct zero object ID. Add object hash detection logic and enable testing the package with SHA256. --- internal/git/updateref/update_with_hooks.go | 21 ++++++++++++++------- internal/git/updateref/update_with_hooks_test.go | 2 -- internal/git/updateref/updateref.go | 19 +++++++++++++------ internal/git/updateref/updateref_test.go | 2 -- 4 files changed, 27 insertions(+), 17 deletions(-) diff --git a/internal/git/updateref/update_with_hooks.go b/internal/git/updateref/update_with_hooks.go index 4fc24de9f..f7d82a3d5 100644 --- a/internal/git/updateref/update_with_hooks.go +++ b/internal/git/updateref/update_with_hooks.go @@ -146,7 +146,7 @@ func NewUpdaterWithHooks( // repository. func (u *UpdaterWithHooks) UpdateReference( ctx context.Context, - repo *gitalypb.Repository, + repoProto *gitalypb.Repository, user *gitalypb.User, quarantineDir *quarantine.Dir, reference git.ReferenceName, @@ -160,13 +160,20 @@ func (u *UpdaterWithHooks) UpdateReference( return fmt.Errorf("getting transaction: %w", err) } + repo := u.localrepo(repoProto) + + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + if reference == "" { return fmt.Errorf("reference cannot be empty") } - if err := git.ObjectHashSHA1.ValidateHex(oldrev.String()); err != nil { + if err := objectHash.ValidateHex(oldrev.String()); err != nil { return fmt.Errorf("validating old value: %w", err) } - if err := git.ObjectHashSHA1.ValidateHex(newrev.String()); err != nil { + if err := objectHash.ValidateHex(newrev.String()); err != nil { return fmt.Errorf("validating new value: %w", err) } @@ -183,7 +190,7 @@ func (u *UpdaterWithHooks) UpdateReference( // repository, which carries information about the quarantined object directory. This is // then subsequently passed to Rails, which can use the quarantine directory to more // efficiently query which objects are new. - quarantinedRepo := repo + quarantinedRepo := repoProto if quarantineDir != nil { quarantinedRepo = quarantineDir.QuarantinedRepo() } @@ -210,7 +217,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, repo, transaction, &receiveHooksPayload, git.ReceivePackHooks, featureflag.FromContext(ctx)).Env() + hooksPayload, err = git.NewHooksPayload(u.cfg, repoProto, transaction, &receiveHooksPayload, git.ReceivePackHooks, featureflag.FromContext(ctx)).Env() if err != nil { return fmt.Errorf("constructing quarantined hooks payload: %w", err) } @@ -230,7 +237,7 @@ func (u *UpdaterWithHooks) UpdateReference( // is packed, which is obviously a bad thing as Gitaly nodes may be differently packed. We // thus continue to manually drive the reference-transaction hook here, which doesn't have // this problem. - updater, err := New(ctx, u.localrepo(repo), WithDisabledTransactions()) + updater, err := New(ctx, repo, WithDisabledTransactions()) if err != nil { return fmt.Errorf("creating updater: %w", err) } @@ -268,7 +275,7 @@ func (u *UpdaterWithHooks) UpdateReference( return fmt.Errorf("executing committing reference-transaction hook: %w", err) } - if err := u.hookManager.PostReceiveHook(ctx, repo, pushOptions, []string{hooksPayload}, strings.NewReader(changes), &stdout, &stderr); err != nil { + if err := u.hookManager.PostReceiveHook(ctx, repoProto, pushOptions, []string{hooksPayload}, strings.NewReader(changes), &stdout, &stderr); err != nil { // CustomHook errors are returned in case a custom hook has returned an error code. // The post-receive hook has special semantics though. Quoting githooks(5): // diff --git a/internal/git/updateref/update_with_hooks_test.go b/internal/git/updateref/update_with_hooks_test.go index c1649ef2a..808b06454 100644 --- a/internal/git/updateref/update_with_hooks_test.go +++ b/internal/git/updateref/update_with_hooks_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package updateref_test import ( diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go index 315ba0c3f..d99b49750 100644 --- a/internal/git/updateref/updateref.go +++ b/internal/git/updateref/updateref.go @@ -25,10 +25,11 @@ func (e *ErrAlreadyLocked) Error() string { // that allows references to be easily updated in bulk. It is not suitable for // concurrent use. type Updater struct { - repo git.RepositoryExecutor - cmd *command.Command - stdout *bufio.Reader - stderr *bytes.Buffer + repo git.RepositoryExecutor + cmd *command.Command + stdout *bufio.Reader + stderr *bytes.Buffer + objectHash git.ObjectHash // withStatusFlushing determines whether the Git version used supports proper flushing of // status messages. @@ -86,11 +87,17 @@ func New(ctx context.Context, repo git.RepositoryExecutor, opts ...UpdaterOpt) ( return nil, fmt.Errorf("determining git version: %w", err) } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return nil, fmt.Errorf("detecting object hash: %w", err) + } + updater := &Updater{ repo: repo, cmd: cmd, stderr: &stderr, stdout: bufio.NewReader(cmd), + objectHash: objectHash, withStatusFlushing: gitVersion.FlushesUpdaterefStatus(), } @@ -116,13 +123,13 @@ func (u *Updater) Update(reference git.ReferenceName, newOID, oldOID git.ObjectI // Create commands the reference to be created with the given object ID. The ref must not exist. func (u *Updater) Create(reference git.ReferenceName, oid git.ObjectID) error { - return u.Update(reference, oid, git.ObjectHashSHA1.ZeroOID) + return u.Update(reference, oid, u.objectHash.ZeroOID) } // Delete commands the reference to be removed from the repository. This command will ignore any old // state of the reference and just force-remove it. func (u *Updater) Delete(reference git.ReferenceName) error { - return u.Update(reference, git.ObjectHashSHA1.ZeroOID, "") + return u.Update(reference, u.objectHash.ZeroOID, "") } var refLockedRegex = regexp.MustCompile("cannot lock ref '(.+?)'") diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go index 90a3216e6..96cad19b2 100644 --- a/internal/git/updateref/updateref_test.go +++ b/internal/git/updateref/updateref_test.go @@ -1,5 +1,3 @@ -//go:build !gitaly_test_sha256 - package updateref import ( -- cgit v1.2.3