| Age | Commit message (Collapse) | Author |
|---|
|
By default, git-fetch(1) will compute for every reference it's about to
fetch whether the update is a forced update or a normal one. This info
is used in two different ways:
1. To compute whether the branch update should be allowed in the
first place. This is pointless though in the case where we ask
git-fetch(1) to force all updates anyway.
2. To provide information via stdout whether the reference has been
force-updated or not. This is pointless in the case where we ask
git-fetch(1) to be quiet.
And while this check can indeed be quite expensive and take dozens of
seconds, git-fetch(1) will still perform it even when asked to be quiet
and to force-update any references.
Skip this computation when fetching into localrepos when forcing the
update and asking Git to be quiet.
Changelog: fixed
|
|
By default, git-fetch(1) will compute for every reference it's about to
fetch whether the update is a forced update or a normal one. This info
is used in two different ways:
1. To compute whether the branch update should be allowed in the
first place. This is pointless though in the case where we ask
git-fetch(1) to force all updates anyway.
2. To provide information via stdout whether the reference has been
force-updated or not. This is pointless in the case where we ask
git-fetch(1) to be quiet.
And while this check can indeed be quite expensive and take dozens of
seconds, git-fetch(1) will still perform it even when asked to be quiet
and to force-update any references.
Skip this computation when fetching into object pools to speed up these
fetches.
Changelog: fixed
|
|
ref: Fix `Internal` errors in `FindTag()` when tag doesn't exist
See merge request gitlab-org/gitaly!4771
|
|
Update nokogiri gem to v1.13.8
See merge request gitlab-org/gitaly!4776
|
|
go: Update gRPC dependencies
See merge request gitlab-org/gitaly!4781
|
|
|
|
Using require.Equal does generally not work with grpc messages. We've
written the ProtoEqual testhelper, which we are now using instead.
|
|
Stop installing packed binaries
See merge request gitlab-org/gitaly!4673
|
|
refs: Return structured errors for DeleteRefs
See merge request gitlab-org/gitaly!4554
|
|
This is done to match the changes in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93412 and
to ensure we don't duplicate versions of this gem.
Changes: https://nokogiri.org/CHANGELOG.html
Changelog: changed
|
|
In the cases where we get an error trying to update the ref, return a
structured error with the git error embedded in it so the rails side can
interpret the error and raise a GitError.
Changelog: changed
|
|
A common error is when a reference is already locked. Detect this by
parsing the error string and emit a typed error so callers don't need to
parse the error string themselves.
|
|
localrepo: Support testing with SHA256
Closes #4354
See merge request gitlab-org/gitaly!4769
|
|
The logic of gitSupportsStatusFlushing is useful beyond the updateref
package. Extract this to its own method in the gittest package.
|
|
We can extract which reference caused an error. Add Ref field to
ReferencesLockedError so we can pass this information back to the
caller.
|
|
Enable testing of most parts of our localrepo tests with SHA256 as
object hash. There are two notable exclusions:
- The remote extra tests depend on the `CreateRepository()` RPC that
doesn't yet support creating SHA256 repositories.
- The `ReadCommit()` function depends on the `catfile` package,
which doesn't yet support parsing SHA256 commits.
We thus skip testing those two parts of the package.
|
|
Over time many of the places in our production-facing code will require
to detect object hashes, often even multiple times per RPC call. Because
we need to spawn a Git command to detect the object hash this cost may
add up over time, so ideally we'd run this detection logic infrequently
only.
We implement a new `ObjectHash()` function for `RepositoryExecutor`s
that allows us to at cache the detected object hash on the `localrepo`
level. Assuming that `localrepo`s get constructed once only per RPC call
this at least allows us to limit the frequency to detect the object hash
to once per RPC call.
While we might eventually want to use an LRU cache to do cache recent
detections, this feels quite a lot like premature optimization. We
should first see how things behave and whether this is going to put some
strain on our command factory before we grow more complexity.
|
|
operations: Introduce structured errors for UserCreateTag
Closes #4137
See merge request gitlab-org/gitaly!4743
|
|
Replace hardcoded uses of SHA1 object hashes in our tests to instead use
the default object hash as indicated by the gittest package.
|
|
Similar as the preceding commit this converts remaining tests that use
seeded repositories to stop doing so and instead write their required
test data at runtime.
|
|
The `setupRepo()` helper function used in the localrepo tests is using a
seeded repository. Refactor these tests to instead use an empty repo and
create the test data at runtime so that we can more readily support
SHA256 in those tests.
|
|
We have already disabled the use of CreateRepository with seeded
repositories in 9ef8ff076 (gittest: Disallow use of seeded repositories
with SHA256, 2022-07-21) when SHA256 is in use. This isn't broad enough
though given that this function uses the `CreateRepository()` RPC behind
the scenes which doesn't yet support SHA256 at all, so it doesn't even
work to use this function with unseeded repositories.
Let's add another test failure that is specific to the unseeded-repo
case to avoid accidental use of this function in SHA256 tests.
|
|
The shared repository suite that is used to verify both the localrepo
and remoterepo implementations currently uses a toggle to tell the
implementation-specific setup function whether the repository should be
seeded or not. This doesn't make a whole lot of sense: setup of the test
data really should be handled by the suite itself.
Refactor the code and create the test data at runtime. This also enables
us to easily test against SHA256.
|
|
Lots of our tests verify `GitCommit`s of commits that have been written
via the `WriteCommit()` test helper and thus need to compare whether the
`CommitAuthor` matches the default commit author. This is quite verbose
and duplicates information across lots of our tests.
Provide a global structure as part of the `gittest` package that holds
this information to reduce the duplication. Note that this requires us
to change some locations to use `testhelper.ProtoEqual()` instead of the
previous `require.Equal()` because Protobuf messages cannot necessarily
be directly compared with each other.
|
|
Many of our tests will require the ability to tell how long the current
object hash is supposed to be in its hex-encoded length. Add a new
function `EncodedLen()` that returns this information.
|
|
Use custom image for FIPS jobs
See merge request gitlab-org/gitaly!4687
|
|
Return context error from requestFinalizer
Closes #4391
See merge request gitlab-org/gitaly!4755
|
|
|
|
With the auxiliary binaries packed into the gitaly binary and deployed
as a single unit, we no longer need to maintain backwards compatibility
between them and Gitaly. This commit drops the now unnecessary major
version suffix from the gitaly-git2go binary which was needed due to the
major version upgrades causing the type paths to change and thus breaking
the Gob protocol between the binaries.
|
|
Makefile contains a target for building a gitaly-git2go-v14 executable.
This is an older version of the gitaly-git2go-v15 so the old executable
stays around for backwards compatibility reasons throughout a major
version upgrade. Since we are now deploying the binaries as a single unit
packed inside the main gitaly binary, we no longer need to maintain backwards
compatibility. Remove the unnecessary target.
|
|
Since Gitaly now unpacks the auxiliary binaries from itself into a
temporary directory and runs them from there, we can stop installing
the packed binaries.
|
|
Makefile: Check if $GOCACHE exists
See merge request gitlab-org/gitaly!4774
|
|
cli: Relocated check subcommand to Gitaly binary
Closes #4320
See merge request gitlab-org/gitaly!4763
|
|
With 6641d296(Clean up go build cache if it exceeds a threshold,
2022-07-28) we now check if the build cache is larger than 4.7 GiB with
`du -sk`.
However, if `make clean` has just been executed then $GOCACHE does
not exist and `make` will emit error:
du: /Users/wchandler/devel/gdk/gitaly/_build/cache: No such file or directory
bash: line 1: [: -gt: unary operator expected
To avoid this, check if $GOCACHE is a directory before running `du`.
|
|
ssh: Fix silent errors when SSHReceivePack fails
Closes #4136
See merge request gitlab-org/gitaly!4766
|
|
The `check` subcommand has been relocated from `gitaly-hooks` to the
main `gitaly` binary. References to the subcommand were updated to
reflect this change.
|
|
The gitaly-hooks check subcommand is used by admins to check whether
the internal API of Rails is accessible. Since in a running system
gitaly-hooks itself does not need to call the internal API this check
makes more sense to reside in the Gitaly binary and has been moved
there accordingly. This will validate whether or not Gitaly can
actually call the internal API.
|
|
[ci skip]
|
|
The `FindTag()` RPC is currently returning an `Internal` error in case
the requested tag doesn't exist. This is bad due to two reasons:
- It impacts our SLOs as `Internal` errors are typically unexpected
error cases that shouldn't happen during normal operations. A tag
that doesn't exist is an entirely valid outcome though and should
thus have a proper error code.
- The Rails callsite needs to always handle `Internal` errors as if
the tag doesn't exist, even though the RPC call could have due to
an entirely different reason.
Fix both issues by returning a structured `FindTagError` error which
allows us to indicate that the tag indeed wasn't found. Like this,
callers can special-case this scenario and don't have to parse error
messages and we can start to return a proper gRPC error code, which is
`NotFound` in our case here.
Changelog: fixed
|
|
[ci skip]
|
|
proto: Fix definition of FindTagError
See merge request gitlab-org/gitaly!4770
|
|
git: Remove feature flag to hide refs in git-upload-pack(1)
Closes #4390
See merge request gitlab-org/gitaly!4768
|
|
The definition of the FindTagError we have just introduced in c6e1eb148
(proto: Introduce FindTagError to distinguish non-existent tags,
2022-07-29) was mistakenly missing the `oneof` definition. The error is
not yet used by anything, so let's correct it before we do grow any
callers.
|
|
objectpool: Fix conflicting references when fetching into pools
Closes #4373
See merge request gitlab-org/gitaly!4745
|
|
[ci skip]
|
|
With commit 4a789524c (git: Don't advertise internal references via
git-upload-pack(1), 2022-07-19) we have started to hide a subset of
internal references in git-upload-pack(1) so that these aren't
advertised at all anymore. When this change was rolled out to canary
initially we observed a short CPU burst and increase in writes on that
Gitaly node. This was totally unexpected given that the change was
supposed to only mirror what we have already configured in Omnibus
anyway, so it shouldn't have caused any visible change in behaviour. We
thus decided to retroactively introduce a feature flag for the change
via bc34b47ac (git: Introduce feature flag for hiding refs in
git-upload-pack(1), 2022-07-26).
As it turns out though, the observed behaviour was completely unrelated
to the change at hand, and we have subsequently rolled out the flag to
production without any observed weirdnesses as initially expected. So
let's remove the feature flag again.
|
|
go: Update module github.com/sirupsen/logrus to v1.9.0
See merge request gitlab-org/gitaly!4752
|
|
proto: Introduce FindTagError to distinguish non-existent tags
See merge request gitlab-org/gitaly!4767
|
|
Our tests that exercise pushing objects to SSHReceivePack are using the
helper funciton `sshPushCommand()` to construct a git-push(1) process.
As part of this setup we also inject various environment variables that
are required for the `gitaly-ssh` auxiliary binary so that it knows how
to connect back to the Gitaly server. The setup of those environment
variables is accidentally overriding the complete environment that was
returned by `gittest.NewCommand()` though, which also means that any
setup done by the Git command factory is lost.
Part of this setup also includes the `GIT_EXEC_PATH` environment
variable that tells Git where to find its binaries. This essentially
means that when running tests with bundled Git, we have by accident been
picking up the wrong Git executables. While this has worked just fine
until now, this breaks with the introduction of FIPS-enabled UBI images
which don't have Git installed in `/usr`, but in `/usr/local`. As a
consequence, we fail to find the correct Git executable and thus fail
the test.
Fix this bug by appending to the environment instead of overriding it
completely.
|
|
Signed-off-by: Balasankar "Balu" C <balasankar@gitlab.com>
|
