| Age | Commit message (Collapse) | Author |
|---|
|
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>
|
|
Clean up go build cache if it exceeds a threshold
Closes #4392
See merge request gitlab-org/gitaly!4765
|
|
ssh: Handle timeout waiting on packfile negotiation with proper errors
See merge request gitlab-org/gitaly!4761
|
|
Before the introduction of 3c6bc2db7 (Limit the negotiation phase for
certain Gitaly RPCs, 2019-10-15), SSHUploadPack and SSHUploadArchive had
been vulnerable to a TOCTOU race: the client could open a session to the
server and keep it up almost indefinitely without starting the packfile
negotiation yet, so that access checks have already been performed but
the client can still ask for an arbitrary set of objects to be sent to
it. So even if the client's access got rejected meanwhile, it could
still ask for arbitrary objects after the fact by starting the packfile
negotiation at whatever point the client wants to.
This TOCTOU race has been plugged by listening in on the negotiation and
putting a limit on how long that phase may span. The underlying thought
is that it's fine to keep the actual packfile streaming open for an
arbitrary amount of time because at that point the objects have already
been negotiated anyway.
We have observed some flakiness in this area though in our CI pipelines,
where the timeout would sometimes return a `codes.Canceled` instead of
our currently-assumed `codes.Internal` error. But taking a deeper look
it is actually surprising that we'd return `codes.Internal` in the first
place: it's a well-defined error case and we should provide proper
information to the client to distinguish the case where we timed out
waiting for the packfile negotiation to complete. Instead, we just
return an error that is indistinguishable from other errors.
Fix this shortcoming by properly handling the case where we cancel the
context ourselves due to the packfile negotiation timing out. We now
return a proper error message as well as an error code to the client
that makes it easy to see what's happening. Chances are that this will
also fix the flakiness in our CI, but it's hard to tell.
Changelog: fixed
|
|
We have two tests for SSHUploadPack and SSHUploadArchive that verify
that we correctly handle timeouts when waiting for the packfile
negotiation to conclude. These tests only verify the error code though
and don't care at all about the error message. We have seen some
flakiness here though where the error code didn't match the expected
error code, but because we don't have any information on the error
message it's hard to tell why exactly these tests flake.
Refactor them to instead assert the complete gRPC error to give us more
information the next time these tests are flaky. This also surfaces a
missing feature flag test.
|
|
When receiving an SSHUploadPack request we log some information included
in the request. Part of that information is the information about the
GitLab repository the request has been created for, which we laboriously
extract from the request while paying attention to not dereference any
`nil` pointers.
Simplify the code to insetad use the Protobuf's getter functions, which
already know to handle `nil` pointers for us.
|
|
Refactor some tests for SSHUploadPack to generate test data at runtime
instead of using the seed repository. This makes the test setup more
obvious and makes those tests compatible with SHA256. Last but not
least, it significantly speeds up the test from around 2.85s to 1.20s on
my own machine given that we have to handle a whole lot less data.
|
|
Add error functions for the gRPC code `DeadlineExceeded`.
|
|
proxy: Refactor tests to share less state and use `grpc_testing` test service
See merge request gitlab-org/gitaly!4756
|
|
|
|
Our build process never cleans up the go build cache and neither does
go. Users are expected to run `go clean --cache` occassionally to
clean up old files. This has so far been generally fine, given each
build didn't bloat the cache too much. With the introduction of
packed binaries in 861730ac, the build cache grows at a much higher
rate due to the embedded binaries. This has resulted the CI also
running out of space. This commit adds a configurable threshold for
cleaning up Go's build cache and sets the default at 5 GB. If the
build cache is larger than 5GB at the beginning of the build, the
build cache is automatically cleaned.
|
|
Template: Update Gitaly onboarding
See merge request gitlab-org/gitaly!4738
|
|
[ci skip]
|
|
[ci skip]
|
|
[ci skip]
|
|
Added information and references to the Gitaly onboarding issue.
|
|
Update google-protobuf Ruby gem to v3.21.3
See merge request gitlab-org/gitaly!4762
|
|
Support `make clean-build` to only clean the build directory
See merge request gitlab-org/gitaly!4744
|
|
praefect: Add -db-only flag to remove repository
Closes #4279
See merge request gitlab-org/gitaly!4715
|
|
objectpool: Remove feature flag guarding heuristical repo maintenance
Closes #4342
See merge request gitlab-org/gitaly!4757
|
|
[ci skip]
|
|
operations: Preparatory refactorings for UserCreateTag
See merge request gitlab-org/gitaly!4740
|
|
proto: Introduce structured UserCreateTagError
See merge request gitlab-org/gitaly!4741
|
|
Remove legacy go.mod and go.sum files from _build directory
Closes #4381
See merge request gitlab-org/gitaly!4758
|
|
This not only addresses CVE-2021-22569, but it also pulls in all the
big endian fixes made towards supporting s390x
(https://gitlab.com/gitlab-org/omnibus-gitlab/-/issues/6435).
This also mirrors the update in GitLab Rails:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93122
Changelog: changed
|
|
Bump all Rails-related gems to v6.1.6.1
See merge request gitlab-org/gitaly!4759
|
|
gittest: Refactorings to make the package hash-clean
See merge request gitlab-org/gitaly!4734
|
|
The tests we have for UserCreateTag leave some things to be desired.
First, they depend on object hashes quite a lot and thus make it hard to
eventually transition the tests to support SHA256. Second, many of the
tests in fact change the underlying repository and thus lean on fragile
cleanup logic after every subtest to bring back the repository into a
well-defined state again. And third there are many small nits about how
things don't match our best practices around naming variables and
similar things.
Refactor the tests to address those issues. Most importantly, we now use
`gittest.CreateRepository()` everywhere and create required test data at
test execution time so that we don't have to depend on object hashes
anymore.
|
|
While we validate that the tag name doesn't contain any spaces in
UserCreateTag, that's as far as our validation goes. This is missing a
lot of different cases though where the tag name is invalidly formatted
like for example when it contains newlines. As a result, we don't fail
early when the tag name is invalid but will fail when git-update-ref(1)
notices the invalid format.
While not a serious error given that we do eventually detect this, the
error message we return in that case is still one of the old errors that
we have added to stay compatible with the exact errors returned by the
old Ruby implementation. Furthermore, we retun an `Unknown` error code
in that case instead of an `InvalidArgument`.
Fix this by using `git.ValidateRevision()` instead, which catches a lot
more invalid reference names than just a contained space.
Changelog: fixed
|
|
The validation we have for arguments passed to the UserCreateTag RPC is
still carrying around quite some baggage from the old Ruby days. While
this made sense back in the day to reduce friction when migrating the
implementation of this RPC from Ruby to Go, this does feel somewhat
dated nowadays.
Refactor validation of arguments to return Go-style error messagess and
wrap the error in an `InvalidArgument` error code.
|
|
Rename tag-related tests to match modern-best practices.
|
|
Improve the documentation for our Protobuf definitions of
UserCreateTags.
|
|
The test service definitions used by the grpc-proxy package aren't used
by anything anymore given that we have now converted all tests to
instead use the `grpc_testing` test package.
Remove the test definitions.
|
|
Convert the client's dialling tests to use the `grpc_testing` test
service instead of the `grpc_proxy` one.
|
|
Now that we have duplicated a lot of sharedstate in our proxy tests they
have become a bit slower given that they need to setup their testing
state anew. Let's cushion the impact of this by parallelizing tests.
|
|
Convert the StreamHandlers tests to be table-driven and convert them to
use the `grpc_testing` test service.
|
|
Refactor the tests that are using the `assertingService` to instead use
the same testing infrastructure as our peeker-tests via the pinger
backend. This allows us more flexibility in writing tests and avoids
some global state as every test can easily put in its own RPC stubs.
|
|
We have multiple functions that set up the proxy in our tests. Refactor
the code to deduplicate this logic.
|
|
The extended proxy handler tests use a testing suite to group together a
set of tests that have similar setup. While this helps reduce the time
used to execute those tests, the shared state really makes it hard to
understand what exactly is going on in those tests. Furthermore, it
makes it harder than necessary to iterate on the implementation of a
specific test without also impacting the other tests.
Split out the test suite's setup into a `setupProxy()` function and
convert the tests into standalone tests that don't use a shared test
suite anymore.
|
|
We have defined our own gRPC test service in order to assert behaviour
with various kinds of RPCs. This isn't really necessary though given
that the grpc-go package already provides a test service that can be
used exactly for these purposes.
Convert the peeker tests to use the `grpc_testing.TestService`.
|
|
Modernize the test style of the peeker tests to use modern helpers like
`testhelper.ProtoEqual()` more extensively in order to more thoroughly
assert our assumptions. Also, avoid the usage of abbreviated variable
names in order to not leave the reader deciphering them all the time.
|
|
Simplify the setup of tests by using `t.Cleanup()` to finalize resources
instead of passing a cleanup function to the caller.
|
|
Move the `TestMain()` function into our test helper file and rename the
file to `testhelper_test.go` like it is customary in our codebase.
|
|
Clarify max_queue_size is per RPC, not project
See merge request gitlab-org/gitaly!4753
|
|
All versions should have been bumped in unison since they are released
at the same time, and this change mirrors the GitLab Rails changes in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92400.
Changelog: changed
|
|
A `make clean` wipes all Ruby gems, but sometimes we just want to
clean up the `_build` dir.
Relates to https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93119
|
|
The build process left a go.mod and go.sum file in the _build directory
in the past. This causes problems these days when attempting to embed
the auxiliary binaries into Gitaly binary as they are considered to be
in a different module due to this. Remove the legacy files prior to
building Gitaly.
|
|
operations: Remove CherryPickStructuredErrors feature flag
See merge request gitlab-org/gitaly!4747
|
|
In 410295810 (objectpool: Switch to use OptimizeRepository for pool
repos, 2022-07-12) we have migrated maintenance of object pools in
`FetchIntoObjectPool()` to use the same repository maintenance as we use
for normal repositories. This deduplicates the logic, makes sure we keep
more data structures up to date in object pools and also unifies nightly
and normal maintenance for pool repositories.
We have rolled out this feautre flag to production without any issues
observed so far. So let's remove the feature flag and unconditionally
enable heuristical repository maintenance for object pools.
Changelog: changed
|
