| Age | Commit message (Collapse) | Author |
|---|
|
Git v2.33.0 had a bug in git-update-ref(1) where it didn't know to flush
its output correctly. As a result we cannot be sure that references have
been locked when calling `updateref.Prepare()` because we don't get a
confirmation from Git. We have since upstreamed a fix for this bug which
works as expected, but one of our tests is still frequently failing
because of exactly that bug when running with Git v2.33.0.
Skip this test to reduce flakiness of our pipelines. We know it's an
issue, we have a fix for it, and we want to upgrade the minimum required
Git version anyway. So there is not much of a point to continue hitting
this flake.
|
|
'master'
testcfg: Fix building binaries as unprivileged user with Go 1.18+
See merge request gitlab-org/gitaly!4689
|
|
featureflag: Remove raw feature flags
See merge request gitlab-org/gitaly!4675
|
|
Makefile: Add target to debug go tests in Delve
See merge request gitlab-org/gitaly!4662
|
|
ci: Upgrade build images to most recent ones
See merge request gitlab-org/gitaly!4688
|
|
We have changed the Postgres client version due to our update to a more
recent GitLab Build Image, which caused some minor changes in the
Praefect schema. Update the schema to match.
|
|
Our CI pipelines execute tests as unprivileged user so that we can
verify that all tests pass without any additional capabilities and that
tests don't write any data into the source tree. This broke for our
nightly jobs though with the recent update to Go 1.18 because we now
fail to build the auxiliary Go binaries.
The root cause is a combination of two things:
- Go 1.18 started to query the repository for VCS information so
that it can embed that information into the resulting Go binaries.
- Git has addressed CVE-2022-24765 and won't open repositories by
default anymore that aren't owned by the current user.
So when testing with recent Go and Git versions as unprivileged user
then Go will try to extract the VCS information, but Git will refuse to
operate.
Fix this by declaring the source directory as "safe". While this is not
necessarily the case, it doesn't compromise our security any more than
before: if an adversary was able to write to the `.git/config` file,
then the very same adversary would also able to just touch up the source
code of Gitaly itself. So that adversary could obtain arbitrary code
execution by just changing whatever is executed as part of our tests.
|
|
Allow manually executing nightly jobs that test against Git's `master`
and `next` branches to easily allow testing changes against these Git
versions.
|
|
go: Update module gocloud.dev to v0.25.0
See merge request gitlab-org/gitaly!4683
|
|
The GitLab Build Images created for Gitaly have been simplified to
neither include Postgres nor PgBouncer given that both are not used by
our CI pipelines anymore. Furthermore, while not really important to us,
the Git version was bumped to v2.36.0.
Update our default build image to switch to the most recent ones.
|
|
|
|
go: Update module google.golang.org/grpc to v1.47.0
See merge request gitlab-org/gitaly!4651
|
|
repository: Verify behaviour when fetching into pooled repos
Closes #4304
See merge request gitlab-org/gitaly!4671
|
|
gittest: Convert most tests that use worktrees
See merge request gitlab-org/gitaly!4666
|
|
Adjust infrastructure to use Go v1.17
See merge request gitlab-org/gitaly!4684
|
|
The difference between raw and non-raw feature flags has been really
confusing at times: while a non-raw flag has the format "some_a", a raw
flag has the format "gitaly-feature-some-a". "Raw" in this context thus
refers to the metadata as used in the gRPC context.
To avoid this confusion and make the difference more distinguished and
to improve type safety the preceding commits have replaced all usage of
raw feature flags with actual `FeatureFlag` structures. In case one
needs access to the raw flag one would thus now use `MetadataKey()` to
obtain the raw metadata key.
Hopefully, this should clarify things when we use `FeatureFlags` in all
of our codebase in contrast to using raw ones in some places and non-raw
ones in others.
|
|
We're currently injecting raw feature flags into the hooks payload so
that these flags can be set in the outgoing context that's used to
connect back to Gitaly again. We're reducing the use of raw feature
flags though in favor of using the typed `FeatureFlag` structure.
Convert the hooks payload to instead inject `FeatureFlag`s directly with
their respective value so that we can get rid of raw feature flags.
This is technically-seen a breaking change because any old server that
is running would still inject the old set of feature flags which the new
binary doesn't understand anymore. As a consequence, it wouldn't use any
feature flags at all. This is fine though: none of our hook RPCs is
using feature flags right now, so this doesn't change anything.
|
|
When injecting default values for feature flags into the outgoing gRPC
context of Gitaly peers we're using `RawFlags()` to extract currently
set feature flags. This function is going away in favor of the type-safe
`FromContext()` function.
Migrate the coordinator to use `FromContext()`.
|
|
In order to guarantee a persistent view of enabled and disabled feature
flags the coordinator makes sure to inject all feature flags that aren't
explicitly set in the gRPC context with their default flags as seen by
Praefect. This fixes a race where the default of any feature flag may
change during an upgrade that can lead to the flag being default-enabled
on some Gitaly nodes, while it's default-disabled on others.
This mechanism has a bug though: we first convert the incoming context
to an outgoing one and thus retain all metadata. We then compute the
full set of feature flags we wish to inject, including already-set ones.
And then finally amend this metadata to the outgoing context. The end
result is that any feature flag that has been explicitly set in the
incoming context will be set twice in the outgoing context.
Fortunately, this bug is benign because we always use the same value in
both cases, and our feature flagging code only ever checks the first
value. So this bug doesn't cause any adverse effects.
Fix it regardless by only setting feature flags in the outgoing context
that haven't yet been set before.
Changelog: fixed
|
|
Convert the logic to inject and extract feature flags in `gitaly-git2go`
to not use raw feature flags anymore. Instead, use `FromMetadataKey()`
and `FromContext()` to work on typed `FeatureFlag`s.
This prepares for the removal of `featureflag.Raw`.
|
|
The featureflag subcommand is only used for testing purposes to assert
that we can indeed pass feature flags from the `git2go.Executor()` to
the process we're about to spawn. We're using raw feature flags for this
purpose, but we're about to remove them in favor of using the type-safe
`FeatureFlag` structure.
Convert the subcommand to return a new `FeatureFlag` structure that we
can use to more thoroughly check whether things behave as expected and
remove usage of `RawFromContext()` to prepare for its removal.
Note that while this is a breaking change to the calling interface of
the `gitaly-git2go` executable, this command is only ever used in our
tests. So in practice, this doesn't matter.
|
|
Extract the logic to construct a feature flag from its gRPC metadata key
from `FromContext()` into a new `FromMetadataKey()` function. This will
be used at a later point.
|
|
To extract feature flags from the current context you can call the
`AllFlags()` function. Just from looking at that function's name it's
not clear though what it returns. As it turns out, it returns the flag
as plain strings in the format of "${flag_name}:${flag_value}". This
format is quite inaccessible to callers given that they'd have to
manually split name and value and then also know to parse the value.
This makes for an easy-to-misuse interface.
Replace `AllFlags()` with a new type-safe `FromContext()` function that
returns a map of `FeatureFlag`s mapped to their respective value. Like
this it's immediately clear what you obtain and enjoy all the comfort of
using the various functions on the feature flag itself.
Refactor callers to use this new function.
|
|
Feature flags must follow a specific format that is of the form
`some_feature_flag`. Most importantly, flags must not contain some
special characters:
- They must not contain dashes because we need to replace dashes
with underscores and the other way round when converting between
the gRPC metadata key and the actual feature flag name. So if the
name contained a dash, then the name wouldn't round-trip.
- The must not contain colons because we use colons to separate the
feature flag name from its value.
Disallow creating feature flags like this and convert tests that violate
this rule.
|
|
The list of available feature flags is currently a globally accessible
variable that can be changed at will by other packages. Let's change
this to instead use an accessor function `DefinedFlags()`: this forbids
any change to the feature flags and allows us to iterate on the way we
store flags.
|
|
Move functions whihc inject feature flags into the context into
"context.go" to group together related functionality.
|
|
Rename test for the `git2go.Executor` exercising the feature flags infra
to match modern best practices.
|
|
Disallow use of `os.Setenv()` and `os.Unsetenv()` in tests. Callers
should instead use `testhelper.ModifyEnvironment()`. Adjust existing
callers to do so. Note: the log tests cannot use the testhelper package
due to a cyclic import and thus use `t.Setenv()` directly.
|
|
When using `os.Setenv()`, we change the environment variable for the
full process. While not surprising, this means that we must avoid this
in parallel tests given that the environment variable would also be set
for any other, currently running tests.
With Go v1.17 a new `t.Setenv()` helper function has been introduced
that detects this case and that instead causes us to panic. Convert
`testhelper.ModifyEnvironment()` to use that function and fix a test
that is using `t.Parallel()`.
|
|
We have recently started to require Go 1.17 and later. Interestingly
enough, our CI has not been updated to use that new version yet. Because
of whatever reason this seems to be fine, and Go doesn't complain about
it when running with v1.16.
Let's fix this regardless and start testing with Go v1.17 and v1.18,
only.
|
|
The tags generated for GitLab build images have changed a bit due to an
upstream refactoring of how these images are built [1]. Migrate to the
new format.
[1]: https://gitlab.com/gitlab-org/gitlab-build-images/-/merge_requests/460
|
|
go: Update module gitlab.com/gitlab-org/labkit to v1.15.0
See merge request gitlab-org/gitaly!4682
|
|
go: Update module github.com/prometheus/client_golang to v1.12.2
See merge request gitlab-org/gitaly!4680
|
|
git: Add a gitversion label to command related metrics
See merge request gitlab-org/gitaly!4460
|
|
featureflags: Require version and rollout issue URL for flags
Closes #4312
See merge request gitlab-org/gitaly!4676
|
|
[ci skip]
|
|
go: Update module github.com/uber/jaeger-client-go to v2.30.0
See merge request gitlab-org/gitaly!4681
|
|
We have bumped the minimum required Go version to v1.17 recently, but
didn't update our asdf tool versions. Let's update them to the latest
available Go v1.17 and v1.18 releases.
|
|
We sometimes want to know the git version of the spawned command
Add gitversion label to metrics to increase the visibility of
git version.
Changelog: added
|
|
|
|
|
|
|
|
go: Update module github.com/kelseyhightower/envconfig to v1.4.0
See merge request gitlab-org/gitaly!4649
|
|
When creating a new feature flag it is mandatory to create a feature
flag rollout issue that keeps track of the current status of any flag.
This issue is used both to not forget about rolling out a flag, and also
for the sake of documenting whole rollout process. We have nothing in
our process though that enforces creation of any such issue, and thus it
happens from time to time that we forget to create it.
Add two new parameters to `NewFeatureFlag()` that document the version a
flag has been introduced in and the URL of the issue that's tracking the
rollout. This brings us several benefits:
- It's hard to forget about creating the issue given that you now
have to specify the link when creating the flag.
- Reviewers immediately spot a missing link and can verify that it's
referring to the correct flag. Furthermore, they may review the
rollout issue itself to see whether it documents important
details.
- When checking up on a feature flag after it has been merged it
becomes trivial to find the rollout issue as a developer.
Furthermore, this commit also adds a new version parameter to keep track
of the version a flag has been introduced in. This also makes it easier
for reviewers to see whether a flag can really already be removed or
not.
Adjust all existing feature flag definitions to have this information.
|
|
Add FullPath RPC
See merge request gitlab-org/gitaly!4642
|
|
FullPath reads the path from the repository's gitconfig under the key
"gitlab.fullpath".
|
|
FullPath will allow fetching the value previously set by the SetFullPath
RPC. FullPath assists admins to determine which repo on disk is
associated with which repository on gitlab.
Changelog: added
|
|
|
|
|
|
build: drop support for Go 1.16
See merge request gitlab-org/gitaly!4522
|
