| Age | Commit message (Collapse) | Author |
|---|
|
|
|
[ci skip]
|
|
Praefect: Cancel node voter error message
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5019
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
Currently if `CancelTransactionNodeVoter()` errors the resulting message
is not decorated with relavant context. This change updates the returned
error message to make the surrounding context of the failure more clear.
|
|
git/stats: Support SHA256 object format
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5017
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
go: Update module github.com/prometheus/client_golang to v1.13.1
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5011
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
|
|
ci: Refactor fragile setup of unprivileged tests
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4999
Merged-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
remote: Fix tests that rely on resolving external URLs
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5015
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
Remove the build tags that keep us from running tests with the SHA256
object format and convert remaining hard-coded use of SHA1 to instead
use `gittest.DefaultObjectHash`.
|
|
Same as the preceding commit, refactor tests for `stats.LogObjectInfo()`
to not use seeded repositories.
|
|
Adapt the tests in the `git/stats` package to use unseeded repositories
with test data generated at runtime. Like this, we can easily run these
tests with both SHA1 and SHA256 as object hash.
|
|
The stats package has a blackbox tester for HTTP-based pushes that allow
the user to observe timings for preconfigured pushes. In order for this
to be configurable, we had to reimplement parts of the Git protocol so
that we can easily drive this.
One part that our implementation does not currently support is the
`object-format` capability that advertises the object format that the
client expects the remote repository to be in. As this capability will
default to "sha1" in case it's not explicitly set, this means that we
don't support SHA256-based pushes.
Add a new configuration that allows the user to specify the object
format that shall be used for the push probe and report the object
format capability depending on that. This is sufficient for enabling
SHA256-based pushes.
Changelog: added
|
|
Extract a function that allows the caller to retrieve an object hash by
the format name, e.g. "sha1" or "sha256". This functionality will be
required by the `stats` package in a subsequent commit.
|
|
PROCESS: Instruct gradual rollout for feature flags
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4997
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
We have recently added a set of tests in 8e8d639d4 (remote: Use
`GetURLAndResolveConfig()`, 2022-09-28) that exercise the new
`ResolveAddress` Protobuf field. These tets rely on resolving specific
URLs, which causes them to hang on my system.
Refactor the tests to run with a "real" HTTP server that we're resolving
the original remote URL to. Like this we avoid hitting any external URLs
and furthermore can verify that the command runs successfully as it has
a proper Git server to clone from.
|
|
The `gittest.HTTPServer()` function sets up an HTTP server that is
getting served by git-http-backend(1) so that clients can clone from it.
While this function returns a cleanup function, none of its callers use
it in any special way, but only ever close it in a deferred function
call.
Refactor the code to not return the close function anymore and use
`testing.TB.Cleanup` instead.
|
|
Praefect: Fix transaction voter state race
Closes #4602
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5012
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
When creating a new subtransaction previous voter state needs to be
propagated forward so the new subtransaction can be aware of previously
canceled voters. While reading the voter result state of the previous
subtransaction, the subtransaction could also have its voter state
updated through a call to `updateVoterState()`.
This change moves voter propagation logic to a new subtransaction method
`getPropagatedVoters()`. This method locks its associated subtransaction
to prevent concurrent writes to its voter result state.
|
|
Our macOS job overrides the paths where test reports are written to.
This is not needed though: we already inherit `test_variables`, which
sets the location of these files to something sensible.
Drop the useless overrides.
|
|
Now that we have converted our CI to both build and test Gitaly as the
same unprivileged user there is no need for the `UNPRIVILEGED_CI_SKIP`
variable anymore that skips building binaries. Remove it.
|
|
GitLab Runner by default runs CI jobs as the root user. This is creating
several problems for us when we want to test behaviour that relates to
file permissions as the root user has special capabilities that allow it
to just ignore those permissions altogether. To fix this, we run tests
as an unprivileged user that lacks these capabilities.
The way this works is that we clone and build the code as root and then
run the tests as unprivileged user. This both fixes above issues while
also making sure that our tests never write into Gitaly's source tree
directly, which we used to do some time ago.
This process is really fragile though: while we're reusing the Go mod
and build cache, these caches have been populated by the root user and
are thus only readable by us. So if the tests need to write anything to
those caches then they will fail because the unprivileged user lacks the
permission to do so. And while this has somehow worked until now, this
does break with Go 1.19. We could try to do introduce more magic here,
or make the caches writeable. But this only adds more workarounds on top
of the already-complicated build process, so this doesn't feel like the
right thing to do.
Instead, refactor our CI job to both build and test as the unprivileged
user. While sources are still owned by the root user, we manually create
the `_build` directory with the unprivileged user as its owner and adapt
a few variables so that all build artifacts are created inside of that
directory. This ensures compatibility with Go 1.19 as we don't rely on
any fragile caching logic in Go anymore and retains our ability to run
tests without any special permissions.
|
|
The `.ruby-bundle` file is used to control when we need to run `bundle
install`: when older than either `Gemfile` or `Gemfile.lock` then we
know we need to re-run the command. This mechanism is also used by CNG
and Omnibus to control execution of this command. They both touch(1) the
file so that it is newer than its dependencies as a workaround so that
`make build` won't re-install any Ruby Gems that have already been
installed anyway.
This is definitely an awful workaround and should ideally be changed so
that we instead provide a variable that lets callers disable installing
Ruby Gems instead of having to reach into some internal details of our
build system. But fixing this now would be quite pointless as we are in
the process of retiring the Ruby sidecar.
The current way this works is about to cause problems though: we need to
adapt the way our unprivileged CI testing works to both build and run
tests as the unprivileged user, who cannot modify the source directory
at all. But as the `.ruby-bundle` file is located directly in the source
directory this refactor would break installing the Ruby Gem.
Let's work around this issue by making the location of `.ruby-bundle`
configurable via a Makefile variable. While ugly, we'll get rid of this
in a few releases anyway.
|
|
Our CI job that builds Gitaly's binaries will build and install a Git
distribution when not instructed to use bundled Git binaries. While we
indeed want to build the Git distribution so that we can cache it,
installing it into `/usr/local` is entirely unnecessary. Furthermore,
we're about to convert the build to run as an unprivileged user who
cannot install into `/usr/local`, and thus the build would break.
Expose a new build target `build-git` that allows us to easily build the
Git distribution without installing it and convert the CI job to use it
instead of the old `git` target.
|
|
The testcfg package has some helper functions that build Go binaries
required by Gitaly at runtime. As Go 1.18 has started to run Git as part
of the build process to embed VCS information into the resulting Git
binaries we had two employ two workarounds:
- We intercept Git commands and thus had to use a Git command
factory to set up a proper environment.
- Git refuses to run in repositories not owned by the current user,
and because CI runs tests as an unprivileged user this safety
mechanism kicked in. We thus had to inject the `safe.directory`
Git configuration into the process.
With the introduction of Go 1.19, we furthermore have to change how we
set up the unprivileged user. As a consequence, we also need to start
building binaries outside of our tests as an unprivileged user and thus
the second issue also hits us during the build process.
Now that we have dropped Go 1.17 though we can just bail and do the easy
thing, which is to pass `-buildvcs=false` to Go and be done. This allows
us to get rid of the nasty build hacks and prepares us for Go 1.19.
|
|
In accordance with our policy GitLab only ever supports the latest two
Go versions. Right now we still support Go 1.17 an 1.18 though, where
the former doesn't even receive updates anymore.
Bump the required Go version to 1.18 and drop all CI jobs for the old Go
1.17. Note that we cannot yet introduce jobs for Go 1.19: the way we set
up unprivileged builds breaks with Go 1.19, so we need to address this
issue first.
Changelog: deprecated
|
|
The macOS job installs an unspecified version of Go that gets determined
by the build image. At the point of writing, this is Go 1.17, which we
are about to drop support for.
Update the build instructions to install the default Go version we're
using in our CI so that we have full control over it. This prepares us
for upgrading to Go 1.18+.
|
|
Update pg_query to 2.2.0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5007
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: Stan Hu <stanhu@gmail.com>
|
|
Merge with `security/master` after v15.3.5 security release
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5010
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>
Co-authored-by: John Skarbek <jskarbek@gitlab.com>
Co-authored-by: Karthik Nayak <knayak@gitlab.com>
|
|
While we've already merged security/master into our tree via 08c88a2c8
(Merge branch 'sync-canonical-with-security-changes' into 'master',
2022-11-02), that commit merged with 702f036ed (Update changelog for
15.3.5, 2022-11-02). This is not the commit that ultimately landed in
the security/master branch though, but it instead is ef59e59f1 (Update
changelog for 15.3.5, 2022-11-02), which is a rebased version of the
former commit.
Because we have merged the wrong commit we now still see merge conflicts
in the Auto-Update bot for Gitaly. Let's fix this by merging the correct
commit, even though this doesn't bring any changes into our tree.
|
|
[ci skip]
|
|
Add git 2.38 as a build target in Makefile
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4943
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>
|
|
This fixes build issues with macOS SDK 13.0:
https://github.com/pganalyze/pg_query/issues/263
Changelog: fixed
|
|
Syncing master into gitaly
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5006
Merged-by: Stan Hu <stanhu@gmail.com>
Approved-by: John Skarbek <jskarbek@gitlab.com>
Co-authored-by: GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com>
Co-authored-by: Karthik Nayak <knayak@gitlab.com>
|
|
[ci skip]
|
|
[ci skip]
|
|
[ci skip]
|
|
The `CreateRepositoryFromSnapshot` RPC gets the tar file from an HTTP
URL. It receives the file by doing a simple HTTP GET call using the Go's
standard library client.
To avoid DNS rebinding issues here, we use the provided resolved address
and do a custom DNS mapping by overriding the `DialContext` function of
the HTTP client.
Let's also add a test wherein we provide a random hostname but the
correct resolved address (IP). With HTTP, this should work.
Signed-off-by: Karthik Nayak <knayak@gitlab.com>
|
|
In `FetchRemote()` use `GetURLResolveConfig()` to get the configPair and
modified URL to avoid DNS rebinding. Unfortunately, because of the
structure of the code, the `git.Cmd` is not exposed on this level, which
means the added code is not testable.
|
|
In `updateRemoteMirror()` use `GetURLAndResolveConfig()` to get the
configPair and modified URL to avoid DNS rebinding. Unfortunately,
because of the structure of the code, the `git.Cmd` is not exposed on
this level, which means the added code is not testable.
|
|
In the `cloneFromURLCommand()` function lets use the
`GetURLAndResolveConfig()` function to obtain configPair and the
modified URL to prevent DNS rebinding. Add tests for the same.
|
|
Use the `GetURLAndResolveConfig()` to get modified URL and any
configPair if necessary to avoid DNS rebinding. Use this modified URL
instead and add the configPair to the git subcommand.
To make it easier to test, move the command instantiation part to a new
separate function `findRemoteRootRefCmd`. This makes it easier to test
the changes to the `git.Cmd`.
|
|
To facilitate the creation of the ConfigPair for the
`http.curloptResolve` flag or modification of the URL for avoiding DNS
rebinding we introduce the `GetURLAndCurloptResolveConfig()` function.
This function takes in the remoteURL and the resolvedAddress and then
provides the modified URL and configPair to be passed onto Git.
For HTTP/HTTPS URLs we leave the remoteURL as is and use the
`http.curloptResolve` flag which was introduced in Git. This is done by
providing a new ConfigPair with the value in the format of
`HOST:PORT:ADDRESS[,ADDRESS]`.
For SSH/Git protocol URLs we simply replace the hostname with the
provided resolved address.
The function does basic validation of the provided remoteURL and
resolved address and throws errors if any validation failed.
|
|
This field was introduced to handle DNS rebinding issues. But it was
never utilized by rails.
From the previous commit c21d6b31ab (proto: Add resolved_address along
remote URLs) we introduced a new field `resolved_address` which will be
used for DNS rebinding issues.
So mark the old `http_host` field as deprecated.
|
|
For the following protobuf definitions:
1. UpdateRemoteMirrorRequest
2. FindRemoteRootRefRequest
3. CreateRepositoryFromURLRequest
4. Remote
add a new field `resolved_address` which would hold the corresponding IP
address of the remote URL provided. This helps us prevent DNS rebinding.
Changelog: security
|
|
Add a git execution environment for v2.38, as well as a feature flag to
enable it.
|
|
ci: Introduce "analyze" stage
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5001
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
Every feature flag rollout should be done gradually. Enforce this
through the feature flag issue template.
|
|
When we roll out feature flags, we always want to roll out gradually
rather than all at once. Additionally, when turning on feature flags, we
should use certain Slack channels for maximum visibility. Explicitly
include these instructions in the PROCESS.md doc.
|
|
|
