| Age | Commit message (Collapse) | Author |
|---|
|
When we read in a new object via `ReadObject()`, then we must perform an
I/O operation to learn about its header. This operation is currently
done under the `currentObjectLock()`, but this can easily cause us to
deadlock when the I/O operation stalls. Consequentially, it's impossible
to access this object now anymore by other callers. Most importantly,
this means that both `close()` and `isDirty()` are blocked from making
any progress. But because those are used to determine whether processes
of the request queue should be killed in the first place we are now in a
deadlock-state.
Fix this issue by splitting up the lock into a read-lock for the current
object whose scope is a lot more fine-grained and only used when we
access the field or when we write to it. The second atomic lock is used
to only lock writing access across the whole lifetime of `ReadObject()`
so that no other caller can call it at the same time.
Changelog: fixed
|
|
We always return the currently pending object's dirty state in case the
request queue has an object set. In case the object has been fully read
without yet having been closed though this won't account for the queue's
outstanding requests. So ultimately, the queue may now says its clean
even though it still got requests pending.
Fix this issue by not returning early in case there is an object. This
causes us to correctly discard any such processes instead of returning
them to the cache with still-pending requests. This is an issue we have
indeed been observing in RPCs which have limits, like ListLFSPointers.
Amend one of our tests to enable cache reuse of catfile processes, which
does surface the issue previous to this fix.
Changelog: fixed
|
|
gitaly/config: Add option to ignore gitconfig files
Closes #4242
See merge request gitlab-org/gitaly!4588
|
|
Parallelize CI jobs
Closes #3123
See merge request gitlab-org/gitaly!4571
|
|
The verify step currently depends on the build step as it will fail
if the Go modules are not downloaded prior to the job executing. If
the modules are not present, golang-ci will fail with a timeout as
it exceeds the timeout while downloading the dependencies. This commit
removes the dependency on build and instead downloads the modules
prior to executing golang-ci. In the general case when the cache is
available, this will allow the verify step to execute in parallel with
the build steps.
|
|
gosec-sast is adding some packages that don't seem necessary. This
commit removes them.
|
|
secret_detection unnecessarily disables inheritance from default.
This commit removes the unnecessary config.
|
|
License scanning is adding some packages that don't seem necessary.
This commit removes them.
|
|
gosec-sast job moves the GOPATH and the cached modules outside the
project directory as the scan will otherwise also scan through the
sources of all the dependencies. This leads to the runtime of the
scan growing massively. The job is currently dependent on the cache
existing from the build step as it unconditionally moves the cache
folder, failing if it doesn't exist. This commit prevents the job
from failing if the cache didn't extract the modules, breaking the
dependency on the build jobs and adds the missing documentation for
the hack.
|
|
The CI drops privileges while running tests. This prevents creating
the required directories to store the test coverage files, thus the
test:coverage job doesn't work without the cache that already
contains the required directories.
To allow the test:coverage job to also run in parallel with the build
steps, this commit overrides the output directory of the coverage
files in the test jobs so they can be created even when the privileges
are dropped, similarly to what is being done with the test result
output with TEST_REPORT variable.
|
|
COVERAGE_DIR in our Makefile controls where the test coverage reports
are outputted. The other variables that control test artifact output
directories begin with TEST_ and are grouped together. This commit
renames COVERAGE_DIR to TEST_COVERAGE_DIR to align it with all the
other variable names. Additionally, it defines the variable with ?=
so the value is only set if it is not already defined. This allows
for the variable to be configured via environment variables.
|
|
This commit adds a regexp to parse the total coverage from the
test:coverage job so GitLab can show it gets shown in the UI and
included in the coverage history of the project.
|
|
Some of the jobs unnecessarily declare that they don't use a cache.
Jobs don't have caches by default, so let's remove the redundant
declarations.
|
|
The testing stage doesn't actually depend on any of the build stage's
artifacts. It only uses the cache which the build stage writes. The
caches are only expired if the relevant files changes like go.sum or
the Gemfile.lock. These caches merely speed up the testing jobs when
they are present. As such, we can remove the build dependency from the
test jobs. Most of the time, the files that make up the cache key are
not changed and thus the both the build step and the test step end up
using the cache. This speeds up the general case by running the steps
in parallel and making test failures visible earlier.
In the rarer case where the cache keys are changed, the test steps will
end up performing some extra work that the cache avoids. This seems like
a fair trade-off to speed up the general case.
There are three exceptions to this though:
'test:coverage' fails without the cache in place as it fails with permission
errors creating the directory for the test coverage report.
'verify' fails with a timeout if the cache is not in place.
'gosec_sast' depends on some files from the cache being moved into the right
place.
As the above jobs fail without the cache, the dependencies are still left
in place for them.
|
|
license_scanning, gemnasium-dependency_scanning and secret_detection
don't use the caches from the earlier build stage, yet they depend
currently on the build stage. Remove the unnecessary dependencies so
they can be executed earlier.
|
|
The QA jobs in our pipeline do not depend on any of the other stages.
The QA stage has been the last one which has thus delayed the execution
of jobs in that stage. Since we are now modeling the dependencies with
the 'needs', this commit removes the unneeded dependency.
|
|
GitLab CI supports a feature that allows for explicitly marking job
dependencies. This allows for visualizing the job dependency graph
and running CI jobs in parallel that do not depend on each other.
This commit adds the needs key where needed to denote job dependencies.
For now, we just model the existing dependency graph by having each stage
depend on the earlier. Later on, we can remove some of the unnecessary
dependencies to speed up the pipeline execution.
|
|
Make Git build with SHA1 and SHA256 routines in FIPS mode
Closes #4210
See merge request gitlab-org/gitaly!4570
|
|
When running our tests, we want Git to use a well-known configuration.
It is thus mandatory that it never picks up the user's gitconfig files,
or otherwise tests may fail due to misconfiguration. To fix this, we
have added a hack that checks whether the currently executing binary has
a `.test` suffix: if it does we assume to be running our tests, and thus
inject a set of environment variable to ignore the gitconfig files.
We now have a better alternative with the newly introduced configuration
to ignore gitconfig files. So let's remove the hack and set this entry
as required. This also proves that the configuration works as expected.
|
|
Fix a cyclic dependency in the gittest package's commit tests so that we
can use the `setup()` function. This change prepares us to drop the test
hack where we inject `GIT_CONFIG_SYSTEM` et al. based on whether the
suffix of the current binary is `.test`.
|
|
We're deprecating use of gitconfig files which exist in the filesystem
in favor of adding `[[git.config]]` entries to Gitaly's `config.toml`.
Add a new option that allows distributions to opt-in to this new
behaviour so that they can migrate earlier if they decide to. This will
become the default with release 16.0.
Changelog: added
|
|
We're in the process of making Gitaly gitconfig-clean: if it is told to
ignore gitconfig files, neither Git nor libgit2 should ever read any
gitconfig file except for the repository-scoped gitconfig.
Add known-broken gitconfigs to our intercepted home directory. Both Git
and libgit2 will fail to parse these files and thus return an error,
which means that we can easily detect if the gitconfig is read by
accident via our testing infrastructure.
|
|
Previous to a40ff540f (Remove HOME overriding hack from tests,
2021-12-15), we were intercepting the home directory of users so that
we didn't pick up their gitconfig. This was required so that none of our
tests change behaviour based on what the user has configured. With that
commit though we had to remove this logic though because some users
require the home directory to be set up correctly to get some auxiliary
tools like asdf work correctly.
We're currently in the process of trying to get Gitaly gitconfig-clean
though, where we want to ensure that Gitaly never picks up any gitconfig
at all if told to do so. And naturally, we want to verify that this is
the case automatically via tests.
Reintroduce intercepting the home directory. In order to not break any
user's workflow, this setup is now opt-in, where our CI will enable it
by default. This allows us to put known-broken gitconfig files into the
home directory that would cause Git invocations to fail.
|
|
gitaly: Become gitconfig-clean
See merge request gitlab-org/gitaly!4578
|
|
Changelog: added
Signed-off-by: Balasankar "Balu" C <balasankar@gitlab.com>
|
|
While we make sure to override `HOME` in our Ruby specs so that Git
wouldn't pick up the gitconfig, libgit2 in fact parses the passwd file
to obtain the user's home directory. Consequentially, overriding the
environment variable is not sufficient to keep libgit2 from reading the
user's gitconfig files.
Fix this issue by overriding Rugged's gitconfig search paths.
|
|
We're using Git2go in our tests for `gitaly-git2go` to access repos as
part of the test itself. And while we do already tell Git2go to ignore
any gitconfig files as part of `gitaly-git2go`'s main function, we don't
in our tests themselves.
Fix this by configuring the gitconfig search path in our tests.
|
|
The `gitaly-git2go` tests are using a Git2go-based test helper to write
commits. That test helper is duplicating functionality we already have
in `gittest.WriteCommit()`, and furthermore it is not configuring Git2go
to not use gitconfig found in the system.
Convert tests to instead use `gittest.WriteCommit()`. Note that commit
IDs are now different because the author and committer is different now.
|
|
libgit2 is by default reading Git configuration files from their
standard locations. This is nothing we want though: the configuration
should either be explicitly set by us, or not at all.
Fix this by explicitly overriding the gitconfig locations in both Git2go
and Rugged. There is only a single Git configuration that we care about,
which is `core.fsyncObjectfiles`: it must always be set to `true` or we
may cause repository corruption. We already do enable it in Git2go via
`git.EnableFsyncGitDir(true)`, and in Rugged we inject a configuration
that contains this key. So ultimately, this change shouldn't change the
behaviour of libgit2 in any way.
|
|
The git2go executor is not injecting Git environment variables when
spawning `gitaly-git2go`. This is a strict requirement though because
we're sometimes spawning git-apply(1) from it, and Git commands must
have the environment variables of the Git execution environment set up.
Fix this by injecting the environment variables.
|
|
Our command factory is injecting a set of environment variables that
override the location of Git's configuration files when running tests so
that we are running in a well-defined environment that doesn't depend on
what the user has configured. This workaround doesn't cover the XDG
config though, which is located in `$XDG_CONFIG_HOME/git/config`, where
`$XDG_CONFIG_HOME` is typically `$HOME/.config`.
Fix this oversight by also overriding `XDG_CONFIG_HOME`, as documented
in git(1). Unfortunately, there isn't a more direct way to override
this. On the other hand it doesn't matter much: previously, we always
filtered out this environment variable in case it was set, so there
cannot be any users that depend on it being set.
|
|
While almost all parts of spawned Git commands get assembled in our
command factory, we do have a set of Git-specific environment variables
that are instead set up by our `command` package. This is a violation of
our abstractions.
Move the assembly of Git-specific environment variables into the command
factory to fix this layering violation. We now append those environment
variables to the execution environments: all sites where we directly or
indirectly spawn Git commands must inject environment variables of the
Git execution environment anyway.
This change also prepares for an upcoming change where we override
`GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM` when a Gitaly configuration
is set that asks us to do this.
|
|
into 'master'
gitpipe: Fix deadlock on context cancellation with unflushed requests
Closes #4253
See merge request gitlab-org/gitaly!4581
|
|
Makefile: Fix compatibility with GNU Make v3.81
See merge request gitlab-org/gitaly!4582
|
|
With GNU Make v3.82, the behaviour of pattern rules was changed.
Previously, pattern rules were applied in definition order, while with
that new version they're instead applied in shortest stem first order.
Quoting release notes:
The pattern-specific variables and pattern rules are now applied in the
shortest stem first order instead of the definition order (variables
and rules with the same stem length are still applied in the definition
order). This produces the usually-desired behavior where more specific
patterns are preferred. To detect this feature search for 'shortest-stem'
in the .FEATURES special variable.
With the introduction of the intermediate build directory we have
started to depend on the newer behaviour with shortest stem first, which
is breaking builds on macOS which installs GNU Make v3.81 by default.
Fix this issue by shifting around our build rules.
|
|
linguist: Implement wrapper to ignore gitconfig in Rugged
See merge request gitlab-org/gitaly!4577
|
|
We are using queues to batch writes to git-cat-file(1) in the gitpipe
package. Given that the queue can only be used by a single user at the
same time, this queue is getting locked when acquired. But by accident,
we're unlocking the queue immediately when we have constructed both the
info and object pipelines even though it is still in use. Luckily, this
programming error is mostly harmless: we finish the tracing span too
early and mark the queue as unused. But as long as no concurrent caller
tries to use the same queue this is not much of an issue.
Fix the bug by using a refcount so that we only close the queues when
they become unused.
Changelog: fixed
|
|
In commit 8773480fd (gitpipe: Propagate context cancellation in object
data pipeline, 2022-05-04), we have fixed an error that cancellation of
the context wasn't properly propagated to callers. While fixing this we
have introduced a new deadlock though: when the context is cancelled, we
may abort the pipeline early without flushing outstanding requests. This
means that the downstream reader which tries to read object data from
the git-cat-file(1) process is blocked indefinitely in case the process
is cached given that it wouldn't be killed by the context cancellation.
Fix this deadlock by flushing any outstanding requests when the context
is cancelled.
Changelog: fixed
|
|
Add field for ZenDesk ticket to template
See merge request gitlab-org/gitaly!4574
|
|
In the past we required a workaround for Linguist so that it picked up
the correct Git executables. We have since converted the code to not use
`git-linguist` anymore, but to use our own `gitaly-linguist` wrapper.
And in fact, while `git-linguist` did use Git to verify that the repo
exists, the Linguist Gem exclusively uses Rugged to access repos. As a
consequence, `gitaly-linguist` doesn't use Git at all anymore.
Remove the workaround to clean up the code. Note that we'd detect the
case where we did unexpectedly execute Git here because we inject our
own `PATH` in the testhelper code that has broken `git` wrappers in it.
So if we did still execute it we'd see that tests failed.
|
|
We're using the `git-linguist` binary to derive programming-language
statics for a repository. This binary is using Rugged to read a given
commit and analyze all blobs referenced by the root tree so that it can
return accumulated lines of counts for every language.
Unfortunately, `git-linguist` reads in gitconfig files by default with
no escape hatch, which sabotages our efforts to get gitconfig-clean in
the Gitaly codebase. We are thus forced to implement our own wrapper
script around the Linguist Gem that allows us to ignore the gitconfig in
Rugged.
Do so and implement a new `gitaly-linguist` binary that mostly mirrors
what `git-linguist` is doing. This allows us to override the Rugged
search path and point it to `/dev/null` so that it won't read any
gitconfig files at all.
Note that we do not use e.g. Gitaly's own gitconfig as computed by the
Git command factory. Ultimately, the Linguist Gem does not read any Git
configuration that would change its behaviour, so it would be overkill
to do so.
Changelog: changed
|
|
When running Ruby commands we need to make sure to expose certain
environment variables to the spawned processes so that they have a valid
Ruby execution environment. This logic is required both by Linguist and
by the Ruby server, but they duplicate the logic.
Create a new `env.AllowedRubyEnvironment()` helper function to unify
this knowledge into a single place. While at it, remove the logic to
handle the generic environment variables `HOME` and `PATH` from
Linguist: they are already injected by our `command` package.
|
|
Makefile: Fix install target using doubly-prefixed Gitaly paths
See merge request gitlab-org/gitaly!4553
|
|
The Go build chain doesn't write a GNU build ID into executables in case
the executable format is not ELF. This is most importantly the case on
macOS, which means that our `replace-buildid` tool fails to work on that
platform because it cannot find the GNU build ID.
Let's fix this by only trying to replace the build ID on Linux.
|
|
With 17f3bf27b (Makefile: Use pattern rules to build Go binaries,
2022-05-10), we have converted the `GITALY_EXECUTABLES` variable to
contain absolute paths instead of only the executables' basenames. The
`install` target was still trying to prefix these binaries though, which
ultimately led to a path which had the binary directory's prefix twice
and thus broke installation.
Fix this bug by not reapplying the prefix.
Changelog: fixed
|
|
into 'master'""
This reverts commit 933109e3e849358daf78c4618d07091b3f36068f.
|
|
'master'
praefect: Remove logic to handle maintenance-style replication events
Closes #4185
See merge request gitlab-org/gitaly!4575
|
|
Upgrade Gitaly module to 15.0
See merge request gitlab-org/gitaly!4493
|
|
Makefile: Fix protoc compile option to disable building tests
See merge request gitlab-org/gitaly!4579
|
|
This commit changes the major version in the package name from v14 to
v15
Updating go.mod & go.sum with new module name v15
Update Makefile to bump major version to v15
Update the gitaly package name in the Makefile. Also update
gitaly-git2go-v14 -> gitaly-git2go-v15. We need to keep
gitaly-git2go-v14 for a release however, for zero downtime upgrades.
This pulls directly from a sha that is v14.
Update package name from v14->v15 for auth, client, cmd, internal packages
This commit changes the package name from v14 to v15 in go and proto
files in the internal, auth, client, cmd packages.
proto: Update major package number in package name
tools: Change major version number in package name from v14 to v15
gitaly-git2go: Change the package name from v14 to v15
update module updater for v15
Update the documentation for the module updater to reflect v15
|
