The UserCreateTag RPC has several conditions where it fails to create
the tag, but still returns a successful response. That response may
either contain a pre-receive error message indicating that updating the
reference has failed because of one of multiple reasons, or it may
indicate that the tag exists already.

Returning successfully in error cases is a code smell, and we have
started to convert all RPCs that do this to instead return structured
errors. With these structured errors, we are able to keep up the
semantics that failure to perform the action results in an error but
keep the client's ability to special-case some specific errors.

Convert UserCreateTag to do the same and return UserCreateTagErrors.
This both gives us better visibility into what exactly is happening on
the Gitaly side and fixes another case where we needlessly create
replication jobs because of missing transactional votes.

Changelog: fixed

The tests we have for UserCreateTag leave some things to be desired.
First, they depend on object hashes quite a lot and thus make it hard to
eventually transition the tests to support SHA256. Second, many of the
tests in fact change the underlying repository and thus lean on fragile
cleanup logic after every subtest to bring back the repository into a
well-defined state again. And third there are many small nits about how
things don't match our best practices around naming variables and
similar things.

Refactor the tests to address those issues. Most importantly, we now use
`gittest.CreateRepository()` everywhere and create required test data at
test execution time so that we don't have to depend on object hashes
anymore.

While we validate that the tag name doesn't contain any spaces in
UserCreateTag, that's as far as our validation goes. This is missing a
lot of different cases though where the tag name is invalidly formatted
like for example when it contains newlines. As a result, we don't fail
early when the tag name is invalid but will fail when git-update-ref(1)
notices the invalid format.

While not a serious error given that we do eventually detect this, the
error message we return in that case is still one of the old errors that
we have added to stay compatible with the exact errors returned by the
old Ruby implementation. Furthermore, we retun an `Unknown` error code
in that case instead of an `InvalidArgument`.

Fix this by using `git.ValidateRevision()` instead, which catches a lot
more invalid reference names than just a contained space.

Changelog: fixed

The validation we have for arguments passed to the UserCreateTag RPC is
still carrying around quite some baggage from the old Ruby days. While
this made sense back in the day to reduce friction when migrating the
implementation of this RPC from Ruby to Go, this does feel somewhat
dated nowadays.

Refactor validation of arguments to return Go-style error messagess and
wrap the error in an `InvalidArgument` error code.

Rename tag-related tests to match modern-best practices.

Improve the documentation for our Protobuf definitions of
UserCreateTags.

Introduce a structured UserCreateTagError that can be returned by the
UserCreateTag RPC. This will eventually replace error conditions where
we're currently failing to create the tag, but return successfully
anyway.

In order to demonstrate that this structured error covers all cases we
care about this commit also adds comments to all current sites where we
return success under an error condition.

Changelog: added

Revert "Change FindTag err when tag not found to NotFound"

See merge request gitlab-org/gitaly!4736

This reverts merge request !4735

[ci skip]

Merge branch '4366-findtag-returns-an-internal-error-code-when-it-cannot-find-a-specific-tag' into 'master'

Change FindTag err when tag not found to NotFound

Closes #4366

See merge request gitlab-org/gitaly!4735

What
---
Update the error message to be `Not Found` rather then `Internal` when a
tag is not found.

Why
---
We burn through our error budget when a single user sends a bunch of
requests to tags that don't exist because we classify the error as
`Internal`, instead it should be `NotFound`.

Reference: https://gitlab.com/gitlab-org/gitaly/-/issues/4366


Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>

ci: Enable use of fastzip to speed up cache handling

See merge request gitlab-org/gitaly!4729

git: Encapsulate object hash specific information

See merge request gitlab-org/gitaly!4720

command: Fix Goroutines sticking around until context cancellation

Closes #4188

See merge request gitlab-org/gitaly!4719

repository: Fix test writing into Gitaly's repository

See merge request gitlab-org/gitaly!4718

Creating and extracting caches takes up a significant amount of time in
our CI pipelines: extracting the cache takes almost 2 minutes, while
creating the cache takes roundabout 4:30min. This is quite excessive and
diminishes the returns we get from the cache.

Luckily, CI folks have realized that the current caching mechanism is
not ideal and have implemented a "fastzip" strategy [1] that can be
optionally enabled by setting the `FF_USE_FASTZIP` feature flag. This
results in a significant speedup: extracting the cache only takes about
40 seconds compared to 2 minutes, and saving the cache is sped up from
4:30min to about 50 seconds.

Let's enable the fastzip feature for Gitaly to speed up our pipelines.

[1]: https://gitlab.com/gitlab-org/gitlab-runner/-/issues/26490

updateref: Ignore failures of custom post-receive hooks

Closes #3595

See merge request gitlab-org/gitaly!4714

One of our tests for `clnoeFromURLCommand()` tries to clone into a
subdirectory of the Gitaly repository itself. This is wrong: all test
data should always end up in a temporary directory.

Fix this by properly cloning into a temporary directory. While at it,
add a call to `cmd.Wait()` to not leak the process.

Add a new funciton `DetectObjectHash()` that, given a repository, can
detect the object hash that's used by the repository by inspecting the
`extensions.objectFormat` config entry.

Split out the ObjectID-related tests into a separate package so that we
can easily make use of the gittest package, which otherwise would cause
a cyclic dependency.

The gittest package is currently re-exporting the functionality provided
by the `ObjectHash` structure. Depending on whether we are testing with
SHA1 or SHA256, we export either the implementation details of the
`ObjectHashSHA1` or `ObjectHashSHA256` structure.

Now that we have the hash-specific information neatly encapsulated in
its own structure there is no need to re-export the separate details of
that structure anymore. Instead, refactor the gittest package to only
export a single `DefaultObjectHash` variable that points to one of the
above structures, depending on what we're currently testing.

Remove the separate `sha256` package in favor of the new `ObjectHash`
structure that encapsulates SHA1- and SHA256-specific info.

Move the `ZeroOID` variable into the `ObjectHash` structure to make it
dependent on the hash function used.

Move the `EmptyTreeOID` variable into the `ObjectHash` structure to make
it dependent on the hash function used.

Move the `NewObjectIDFromHex()` function into the `ObjectHash` structure
to make it dependent on the hash function used.

Move the `ValidateObjectID()` function into the `ObjectHash` structure
to make it dependent on the hash function used.

Introduce the infrastructure to encapsulate different object ID
implementations in the form of a new `ObjectHash` structure. This
structure is supposed to host all information required to properly
handle object IDs regardless of whether one is using SHA1 or SHA256
as a hash function.

Rename the files hosting the `git.ObjectID` implementation to better
match the name of the structure they're hosting.

After starting an `exec.Cmd` the `Process` field is always set according
to the documentation of it. Given that we only ever access that field
after we have started the command already it's thus not needed to check
whether the field is set or not.

Remove the check to clean up the code.

When spawning a process via the command package we always make sure to
start a Goroutine that waits for the context to be done so that it can
reap the process's state. This is required so that we can be sure there
is no resource leak. The downside of this approach is that the Goroutine
will stay around until the context is cancelled though, and in case we
are forced to spawn multiple commands in a single RPC request this cost
may add up.

Fix this RPC-scoped resource leak by exiting the Goroutine early in case
`Wait()` has been explicitly called.

In order to spawn a command via our command package the caller has to
pass in an `exec.Cmd`. This calling convention is weird and opens up the
window for unexpected behaviour in case the caller passes in a command
that is initialized with state unexpected by the command package.

Refactor the package to instead accept a set of arguments. Like this,
the assembled `exec.Cmd` is fully controlled by the command package.

Callers of the command package that wanted to run the command in a
specific directory had to manually set the directory of the `exec.Cmd`
they passed into the command package. This is a weird calling convention
as ideally, the command package should handle the complete setup for the
caller.

Add a new option `WithDir()` to handle this usecase inside of the
command package and convert callers to use it.

Add tests to verify that command finalizers work as expected.

One of our tests verifies that we can kill commands spawned via our
`command` package by cancelling the context. The setup of this test does
not make a whole lot of sense though:

    - We pass an `exec.Cmd` to `New()` that already has an explicit
      context set by using `CommandContext()`. So even if the `command`
      package failed to correctly reap the child, the Go runtime would
      already ensure for us that the command got killed.

    - We cancel the context in a separate Goroutine without much of a
      point.

Fix these issues by passing a non-contextualized command to `New()` and
by cancelling the context synchronously.

The post-receive hook run by git-receive-pack(1) is special compared to
all the other hooks because any errors returned by it have no impact on
the end result of git-receive-pack(1). This is because the hook runs
after references have been updated already, so it cannot have any impact
on the end result anymore.

Our hooks updater, which emulates roughly the same semantics for hooks
execution as git-receive-pack(1), behaves differently though and will in
fact report any custom-hook errors to the caller. This has the result
that depending on how the post-receive hook is executed, an error code
has different impacts.

Align the behaviour of the hooks updater to match what Git is doing and
ignore errors returned by custom hooks. Any custom hooks which relied on
this exact behaviour were broken already because the error code wouldn't
have changed the outcome, and neither would the hook work correctly when
executed by git-receive-pack(1). This thus shouldn't be a breaking
change, but at most fix behaviour in context of misbehaving post-receive
hook scripts.

Changelog: fixed

updateref: Improve errors to include breadcrumbs

See merge request gitlab-org/gitaly!4713

[ci skip]

config: Don't treat EPERM as error when signalling

Closes #4355

See merge request gitlab-org/gitaly!4716

FIx Gitaly team-specific onboarding

See merge request gitlab-org/gitaly!4726