diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2022-11-09 18:48:14 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2022-11-09 19:35:13 +0300 |
commit | a0f8302cb523424ebb246947e010c3168b21aa92 (patch) | |
tree | a8cfd28ff2a1fd371ff0560c8697d330511b3c25 | |
parent | 0a36b96b0f760a20489c07cf51ad7e7df987e962 (diff) |
Parse invalid reference name errors in Updater
Our 'git update-ref' wrapper Updater doesn't currently parse the
the error messages and report invalid reference names with a typed
error. This commit adds support for that as it will be useful in
the future to report more useful errors when Gitaly verifies
references prior to accepting a transaction and logging it.
-rw-r--r-- | internal/git/updateref/updateref.go | 20 | ||||
-rw-r--r-- | internal/git/updateref/updateref_test.go | 22 | ||||
-rw-r--r-- | internal/gitaly/service/ref/delete_refs_test.go | 2 |
3 files changed, 42 insertions, 2 deletions
diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go index 234a9c1fb..29611c1df 100644 --- a/internal/git/updateref/updateref.go +++ b/internal/git/updateref/updateref.go @@ -21,6 +21,16 @@ func (e *ErrAlreadyLocked) Error() string { return fmt.Sprintf("reference is already locked: %q", e.Ref) } +// ErrInvalidReferenceFormat indicates a reference name was invalid. +type ErrInvalidReferenceFormat struct { + // ReferenceName is the invalid reference name. + ReferenceName string +} + +func (e ErrInvalidReferenceFormat) Error() string { + return fmt.Sprintf("invalid reference format: %q", e.ReferenceName) +} + // Updater wraps a `git update-ref --stdin` process, presenting an interface // that allows references to be easily updated in bulk. It is not suitable for // concurrent use. @@ -122,7 +132,10 @@ func (u *Updater) Delete(reference git.ReferenceName) error { return u.Update(reference, u.objectHash.ZeroOID, "") } -var refLockedRegex = regexp.MustCompile("cannot lock ref '(.+?)'") +var ( + refLockedRegex = regexp.MustCompile("cannot lock ref '(.+?)'") + refInvalidFormatRegex = regexp.MustCompile(`invalid ref format: (.*)\\n"`) +) // Prepare prepares the reference transaction by locking all references and determining their // current values. The updates are not yet committed and will be rolled back in case there is no @@ -134,6 +147,11 @@ func (u *Updater) Prepare() error { return &ErrAlreadyLocked{Ref: string(matches[1])} } + matches = refInvalidFormatRegex.FindSubmatch([]byte(err.Error())) + if len(matches) > 1 { + return ErrInvalidReferenceFormat{ReferenceName: string(matches[1])} + } + return err } diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go index aec92090d..b724d70b7 100644 --- a/internal/git/updateref/updateref_test.go +++ b/internal/git/updateref/updateref_test.go @@ -124,6 +124,28 @@ func TestUpdater_prepareLocksTransaction(t *testing.T) { require.Contains(t, err.Error(), "fatal: prepared transactions can only be closed") } +func TestUpdater_invalidReferenceName(t *testing.T) { + t.Parallel() + + ctx := testhelper.Context(t) + + cfg := testcfg.Build(t) + + repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ + SkipCreationViaService: true, + }) + repo := localrepo.NewTestRepo(t, cfg, repoProto, git.WithSkipHooks()) + commitID := gittest.WriteCommit(t, cfg, repoPath) + + updater, err := New(ctx, repo) + require.NoError(t, err) + defer func() { require.ErrorContains(t, updater.Cancel(), "canceling update: exit status 128") }() + + const referenceName = "./refs/heads/master" + require.NoError(t, updater.Update(referenceName, commitID, "")) + require.Equal(t, ErrInvalidReferenceFormat{ReferenceName: referenceName}, updater.Prepare()) +} + func TestUpdater_concurrentLocking(t *testing.T) { t.Parallel() diff --git a/internal/gitaly/service/ref/delete_refs_test.go b/internal/gitaly/service/ref/delete_refs_test.go index 5b11922f7..0a27aae1d 100644 --- a/internal/gitaly/service/ref/delete_refs_test.go +++ b/internal/gitaly/service/ref/delete_refs_test.go @@ -207,7 +207,7 @@ func testDeleteRefsInvalidRefFormat(t *testing.T, ctx context.Context) { testhelper.RequireGrpcError(t, detailedErr, err) } else { require.NoError(t, err) - assert.Contains(t, response.GitError, "invalid ref format") + require.Equal(t, response.GitError, `unable to delete refs: invalid reference format: "refs invalid-ref-format"`) } } |