| Age | Commit message (Collapse) | Author |
|---|
|
GitLab-Shell provides Gitaly with the client to connect to GitLab-Rails.
This sets the user agent for the HTTP requests to `GitLab-Shell`, and
thus Gitaly isn't easy to spot in logs.
This change sets the Gitaly version string as user agent, which than
also allows to differentiate versions during deploys.
|
|
git: Introduce new Revision and ReferenceName types
See merge request gitlab-org/gitaly!2989
|
|
The `GetBranch()` function is a small wrapper around `GetReference()`
which simply resolves the given string to a fully qualified reference
first. This commit implements an alternative set of helper functions to
create a Reference from a potentially unqualified branch name and get a
Reference's branch name and converts the codebase to use them instead of
`GetBranch()`.
|
|
Instead of accepting an untyped string, this commit refactors the
log interface to accept Revisions instead.
|
|
Instead of accepting untyped strings, this commit refactors the catfile
interface to accept Revisions instead.
|
|
The `ContainsRefish()` function is misnamed, as it does not act on
refishes but on revisions. This commit thus renames the function to
`HasRevision()` and converts it to accept a Revision instead.
|
|
The `ResolveRefish()` function is misnamed, as it does not act on
refishes but on revisions. This commit thus renames the function to
`ResolveRevision()` and converts it to accept a Revision instead.
|
|
Instead of accepting untyped strings, this commit refactors the
updateref interface to accept ReferenceNames instead.
|
|
Instead of returning an untyped slice of strings which represent
references, this commit refactors the `refsToRemove()` function to
return ReferenceNames instead.
|
|
Instead of returning an untyped slice of strings which represent
references, this commit refactors the `buildLookupTable()` function to
return ReferenceNames instead.
|
|
Instead of accepting an untyped string, this commit refactors the
UpdateRef() function to accept a ReferenceName instead.
|
|
Instead of accepting an untyped string, this commit refactors the
GetReference() function to accept a ReferenceName instead.
|
|
This commit converts the Reference.Name field to be of type
ReferenceName.
|
|
Our codebase has some confusion with regards to what a revision is and
what a reference is. Furthermore, it's typically not easy to see what
kind of argument a function accepts.
This commit lays the foundation for improving the situation by
introducing two new types Revision and ReferenceName. Subsequent commits
will convert much of our codebase to use these explicit types as
parameters in order to assert that we really only pass expected types to
them.
|
|
git: Consolidate SafeCmd interface
See merge request gitlab-org/gitaly!3000
|
|
The `os.Setenv()` function returns an error which is currently
unchecked. Let's verify it doesn't return an error to silence a linter
warning.
|
|
This commit documents some of the global functions exposed in order to
implement options.
|
|
Given that our "SafeCmd" API has now been renamed to be a plain
"Command" API, this commit renames the previous "command.go" file to
"unsafe.go" and "safecmd.go" to "command.go". This reflects that we
really only have one set of public APIs to spawn commands while there is
also an unsafe internal implementation.
|
|
Back when we introduced the Git DSL, we still had conflicting sets of
safe and unsafe functions. Because of this legacy, our safe set of
functions is still has the "Safe" prefix.
This commit now ends that chapter and renames `SafeCmd()`. In alginment
with the preceding renames, this is being renamed to `NewCommand()`.
|
|
Back when we introduced the Git DSL, we still had conflicting sets of
safe and unsafe functions. Because of this legacy, our safe set of
functions is still has the "Safe" prefix.
This commit now ends that chapter and renames `SafeBareCmd()`. The
previous name has been quite confusing, given that "bare" in the world
of git typically means a repository without a working directory. That
was not the intended meaning though, but instead that the command is not
run with an associated repository. It's thus renamed to
`NewCommandWithoutRepository()`.
|
|
Back when we introduced the Git DSL, we still had conflicting sets of
safe and unsafe functions. Because of this legacy, our safe set of
functions is still has the "Safe" prefix.
This commit now ends that chapter and renames `SafeBareCmdInDir()`. The
previous name has been quite confusing, given that "bare" in the world
of git typically means a repository without a working directory. That
was not the intended meaning though, but instead that the command is not
run with an associated repository and inside a specific directory. It's
thus renamed to `NewCommandWithDir()`.
|
|
The `git.SafeStdinCmd()` function is not used anymore, so let's drop it.
|
|
There's only a single invocation of `SafeStdinCmd()` in our codebase
right now. While not immediately obvious, this can simply be replaced
with `WithStdin(command.SetupStdin)`. Let's convert the callsite and
document this.
|
|
The `git.SafeCmdWithoutRepo()` function is not used anymore, so let's
drop it.
|
|
With `SafeCmdWithoutRepo()` and `SafeBareCmd()`, there are two sets of
functions which do exactly the same thing. Let's drop
`SafeCmdWithoutRepo()` -- we're about to rename the remaining one
anyway, so it doesn't quite matter which one we're dropping.
|
|
Now that all callsites use `WithEnv()`, the `env` parameter of
`SafeBareCmd()` is unused, so let's drop it.
|
|
With the newly introduced `WithEnv()` option, this commit converts all
useage of `SafeBareCmd()` with environment variables to use `WithEnv()`
instead.
|
|
The `git.SafeCmdWithEnv()` function is not used anymore, so let's drop
it.
|
|
With the newly introduced `WithEnv()` option, this commit converts all
usage of `SafeCmdWithEnv()` to use `SafeCmd()` with that option.
|
|
There's currently two sets of functions to spawn git commands either
with the default environment or with a supplied environment. In a quest
to reduce the number of variants we have to spawn SafeCmds, this commit
implements a new `WithEnv()` option which can be passed to set up the
environment.
|
|
The `git.SafeBareCmd()` function serves the purpose to run git commands
which are detached from a repository, e.g. because they don't require
one or because they create one. There's some locations though where
we're running commands which do have an associated repository with this
function.
Let's convert them to use `git.SafeCmd()` instead. This also allows us
to get rid of the manual setup of alternates, which was in fact
forgotten in one of the callsites.
|
|
While `monitorStdinCommand()` accepts an optional set of environment
variables as parameter which it passes along to the git process, none of
the callsites actually use this feature. So let's drop the unused
parameter.
|
|
When calculating the repository checksum, we first check whether the
given repository actually exists on disk and whether it is a valid
repository. We currently do so by using `git rev-parse
--is-inside-git-dir`, which is fine. The downside of it is that we
cannot use `git.SafeBareCmd` with an associated repository as it would
only cause us to pass `--git-dir` to git. Given that it doesn't change
the spawned process's working directory and that `--is-inside-git-dir`
checks whether the current working directory is inside of a git
directory, git would now always pretend to not be inside of a git
directory.
Let's fix the issue by instead switching to `--is-bare-repository`,
which only verifies whether the current repository is bare or not. We're
always going to run this function in bare repositories anyway, and this
switch doesn't require us to change the working directory. So we can now
nicely use `SafeCmdWithEnv()` instead.
|
|
operations: Fix UserFFBranch if called on an ambiguous reference
See merge request gitlab-org/gitaly!2992
|
|
Fix ResolveConflicts file limit error
See merge request gitlab-org/gitaly!3004
|
|
There has been a high occurrence of errors in the Go implementation of
ResolveConflicts in production. After close examination, it was found
that the maximum conflict file size was being inaccurately calculated.
This fix ensures that the file scanner only counts bytes that have
already been advanced, rather than the size of the remaining bytes in
the buffer. This will prevent the same bytes from being counted multuple
times.
|
|
Feature flag gitaly_distributed_reads enabled by default
See merge request gitlab-org/gitaly!2960
|
|
Enable feature flag go_user_delete_{branch,tag} by default
See merge request gitlab-org/gitaly!2994
|
|
Fix internal API errors not being passed back to UserMergeBranch
Closes #3404
See merge request gitlab-org/gitaly!2987
|
|
testhelper: Fix linting issues
See merge request gitlab-org/gitaly!2981
|
|
[ci skip]
|
|
Add information about whether tags were updated to the FetchRemote RPC
See merge request gitlab-org/gitaly!2901
|
|
These flags have been on at 100% for almost a month now without any
reported issues. See
https://gitlab.com/gitlab-org/gitaly/-/issues/3371 and
https://gitlab.com/gitlab-org/gitaly/-/issues/3370 Let's turn them on
by default.
I was told in a chat with @zj-gitlab that he'd prefer the relevant
operation_service.rb code removed as well, but due to a race condition
on deployment we couldn't do this atomically, it had to be done as a
two-phase rollout.
I.e. as we deploy the Ruby code might be in the middle of
auto-restarting, so we could remove its code before the Go code has a
chance to update with its default, and would still want to call it. So
therefore you need to do any such removal in two gitlab.com release
cycles.
|
|
When a file is locked in GitLab and a user attempts to merge a branch
that contains this locked file, the /api/v4/internal/allowed endpoint
will reject the push with an error such as:
```
GitLab: The path 'test' is locked by Administrator
```
In the Ruby implementation of `UserMergeBranch`, this error would have
been passed up as a pre-receive error, and GitLab Rails will display any
lines that are prefaced with `GitLab:`.
However, in the Go implementation, the error in the hooks are expected
to be read from stdout and stderr of a forked process, but the API
endpoint is run **before** this process is even run. Since stdout and
stderr are blank, the pre-receive error message is returned as a blank
message. As a result, Rails displays the wrong message, since it assumes
the merge failed due to a conflict.
To fix this, we raise a `NotAllowedError` if the `/internal/allowed` endpoint
fails and return that message.
Closes https://gitlab.com/gitlab-org/gitaly/-/issues/3404
|
|
The `hookErrorFromStdoutAndStderr()` function is used to generate a
proper error message when execution of hooks fails. It's thus only used
for `updateReferenceWithHooks()`. But since this function has been moved
into its own "update_with_hooks.go" file in ff5805ed0 (operations: Move
update with hook logic into own file, 2020-11-26) , both functions are
located quite far apart from each other.
Move `hookErrorFromStdoutAndStderr()` into "update_with_hooks.go" to
make code more localized and easier to understand.
|
|
If conditional blocks end with a return, then it's considered bad style
to have the `else` branch part of the condition. We've got such a case
in our testserver code, so let's fix that.
|
|
Some of the symbols in `./internal/testhelper` have no documentation
right now. Let's add it now.
|
|
We don't currently verify that we were able to correctly return data in
our testserver's handler functions. Let's inject a testing context and
check them properly.
|
|
The errorlint linter verifies that Go 1.13-style error handling is
correctly honored by both wrapping errors with "%w" and using
`errors.Is()`/`errors.As()` to unwrap them again. There's one warning in
our testhelper code whene we don't properly wrap an error. While
unimportant as the result is passed to `panic()` directly, let's still
fix it up to silence the warning.
|
|
The `GenerateTestCerts()` function currently creates certificate and key
files in a global temporary directory. Let's move it into the global
test directory instead, which is intended for all runtime test data.
|
