| Age | Commit message (Collapse) | Author |
|---|
|
Remove the `testhelper.ModifyEnvironment()` helper. One should instead
either use `t.Setenv()` or `testhelper.Unsetenv()`.
|
|
Rewrite callers of `testhelper.ModifyEnviroment()`, which is about to be
removed in favor of either `t.Setenv()` or `testhelper.Unsetenv()`.
|
|
The tests for our environment helpers aren't quite matching our modern
best practices. Furthermore, they're missing tests for the case where
environment variables are not set at all.
Refactor these tests to adapt. While at it, convert them to use the new
`t.Setenv()` and `testhelper.Unsetenv()` helper functions.
|
|
With Go 1.17 a new `t.Setenv()` helper was introduced that automatically
restores the previous environment variable and that verifies that the
current test is not labelled as parallel. We're about to migrate all
callers to use it instead of `testhelper.ModifyEnvironment()`.
One part that `testhelper.ModifyEnvironment()` does though is to unset
an environment variable in case the given value is the empty string. And
unfortunately, Go didn't introduce a `t.Unsetenv()` helper at the same
time.
Implement a new function `testhelper.Unsetenv()` that behaves the same
as `t.Setenv()`, except that it unsets the environment variable.
|
|
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
|
|
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
|
|
|
|
|
|
We don't have any tests right now that verify that fetching into a
pooled repository is doing the right thing. Most imporantly, we don't
assert that we ignore alternate references present in the object pool.
This missing test coverage had been biting us because we were indeed
including alternate references in the reference negotiation phase before
this was fixed via 2383696d1 (git: Always disable use of alternate refs
for git-fetch(1), 2022-06-21).
Add a test to verify that we indeed don't use alternate refs in pooled
repositories to avoid any future regressions.
|
|
If not specified, the `CreateRepo()` test helper falls back to compute a
default repository path for our hashed storage. This default path is
wrong though: it's using `newDiskHash()` directly and is thus missing
the `@hashed` prefix.
Fix this issue by using `NewRepositoryName()` instead. While this should
not really impact anything, it's still sensible to do the right thing
and match our production setup.
|
|
This is a small sot of cleanups for FetchRemote tests to fix up test
names and use the correct setup helpers.
|
|
The tests we use to verify that we can correctly detect changed tags in
FetchRemote are a bit lacking. We don't verify that not fetching any
tags causes the response to say that nothing has changed, and neither do
we verify that not fetching any tags with checking disabled does the
right thing.
Refactor the test to use a set of subtests for each of the scenarios to
make it easier to understand what exactly we're testing right now and
extend test coverage.
|
|
One of our tests verifies that we can call `CreateFork()` with a TLS
enabled server. The setup is currently done manually by assembling all
the necessary prerequisites, but this is indeed not necessary: the
`testserver` package handles the setup of TLS-enabled servers just fine
when the `TLSAddress` is set in the Gitaly configuration.
Use `runRepositoryService()` with a TLS-enabled configuration instead to
reduce the boilerplate. This also allows us to run these tests with
Praefect enabled.
|
|
Many tests in the `repository` manually assemble the service server via
`testserver.RunGitalyServer()`, which also requires them to pass down
required dependencies. This is quite boilerplate-y and ultimately not
needed: we can just convert most of them to use `runRepositoryService()`
to handle the setup for us.
Do so to reduce the test setup noise.
|
|
Both `runRepositoryServerWithConfig()` and `runRepositoryService()` do
essentially the same and indeed have the same function signature. The
only difference is that the former one will only return the address,
while the latter one also returns a gRPC client. This is misleading
though given the `WithConfig()` suffix, which seems to indicate that it
can accept a configuration while other functions can't.
Fix this confusion by removing the former function and converting all
current callers to use `runRepositoryService()` instead. This also
allows us to get rid of constructing the gRPC client in many cases.
|
|
Add helper function to resolve revisions to an object ID, which is a
frequently used pattern in our tests.
|
|
[ci skip]
|
|
[ci skip]
|
|
[ci skip]
|
|
'master'
command: Export environment variable to force-enable all feature flags
Closes #4318
See merge request gitlab-org/gitaly!4672
|
|
repository: Fix clone credentials leaking via command line arguments
See merge request gitlab-org/gitaly!4668
|
|
In Rails we're setting an environment variable that force-enables all
feature flags in Gitaly so that we can ensure that newly introduced code
hidden behind any flag is still compatible with the old way of doing
things in Rails. With a recent change though we have started to export
feature flags to the `gitaly-git2go` child process so that it knows
which flags have explicitly been enabled and/or disabled. In combination
both these features result to inconsistent state: we don't export the
environment variable to the child process, and neither do we inject the
force-enabled feature flags. This means that we'll now see all feature
flags as enabled in Gitaly, while we see the default values in the child
process.
Fix this by exporting the environment variable to child processes. It
should only ever be set for tests, so that should be a fine thing to do.
And by exporting it we now have the same and thus consistent view of
overridden feature flags.
Changelog: fixed
|
|
Keep context cancel suppression logic consistent
See merge request gitlab-org/gitaly!4656
|
|
operations: Honor feature flag for structured errors in UserCherryPick
See merge request gitlab-org/gitaly!4669
|
|
The gitaly-git2go command knows to return two different styles of
conflict errors depending on whether the `CherryPickStructuderErrors`
feature flag is set or not. This is done to help zero-downtime upgrades:
we can't yet return the new-style error when the old Gitaly server is
still running.
Because the server controls the binary it shouldn't ever happen that the
binary doesn't see the feature flag while the server does. It's thus
pointless to handle the old-style `HasConflictsError` on the server side
if the flag is enabled: this rather indicates an unexpected programming
error, and we should treat it as such.
Remove special handling of this error condition to clarify this.
|
