| Age | Commit message (Collapse) | Author |
|---|
|
Remove `Fatal` and related functions. The "fatal" level is not known to
the `slog` package and thus cannot be emulated easily. Replace all such
existing calls with a log message at "error" level followed by exiting
with an error code.
|
|
The function family `Print{,f,ln}()` is merely a wrapper of the
respective functions that log a message at info level. Despite being
duplicate, it's also not obvious what they are doing and not supported
by the `slog` package.
Remove these functions from the `Logger` interface and replace all their
usages with the respective info-level functions. As the Yamux library
expects a logger implementing these functions this commit implements a
wrapper that forwards the respective functions.
|
|
Introduce a wrapper for the logrus logging infrastructure. This is a
first step towards abstracting away the actual implementation of our
logs behind a type that we can iterate on and will thus pave the way
towards adoption of the `slog` package.
Note that this is only one step of this transition. The interface does
not yet roundtrip to itself, e.g. calling `log.Logger.WithError()` will
result in a `logrus.Entry`, not in a `log.Logger`. This will be handled
at a later point.
|
|
Stop using the public `client` package in favor of our own internal
`gitalyclient` package. This ensures we can continue to refactor our
internal API without breaking dependents on our public API.
|
|
This commit wires the tests to call SharedLogger instead of NewLogger
so they'll use the test case's shared logger. This way the log output
is printed to the same logger and is ordered.
|
|
Gitaly now has a new logger for tests which outputs the logs on test
failure only. Replace all usage of discarding logger with it to capture
logs instead of discarding them.
NewDiscardingLogger is unexported as it is no longer used except
for newServerLogger.
|
|
When configuration of the logger fails then we print a log message via
the logrus package. This has the problem that this logger is not yet
properly configured and thus may or may not be the correct thing to do.
Refactor the code to instead return an error when configuration of the
logger fails. This lets callers handle the error appropriately by
themselves.
|
|
We do not use our usual logging infrastructure in the gitaly-ssh
command. Furthermore, we use a mixture of the "log" and "logrus"
packages to perform the actual logging.
Address this shortcoming by converting the command to exclusively use
our "internal/log" package.
|
|
Introduce a structure that contains information about the generated
cetrificate in our testhelper package. This will allow us to provide
some helper functions on top of the structure.
|
|
Our internal client package is hosted in `internal/gitaly/client` even
though it isn't Gitaly-specific at all. Move it into our new gRPC
specific package to avoid confusion.
|
|
The feature flag logic is currently hosted inside of
`internal/metadata/featureflag`. And while they do have something to do
with metadata, they really are a standalone concept.
Move the package to `internal/featureflag` to reflect this.
|
|
We're about to release Gitaly v16.0. As we've landed a bunch of
previously announced removals it's thus time to bump our Go module
version from v15 to v16.
|
|
The Ruby server is never set up anymore and we don't ever use it either.
Let's remove the dependency on it.
|
|
Enable testing of the gitaly-ssh command with the SHA256 object format.
Convert the two tests relying on seed repositories to instead create
their own test data at runtime.
|
|
The `BuildWithRepo()` function builds a Gitaly configuration and at the
same time creates a seed repository. There's two issues with this:
- It is a layering violation as the configuration-related code
should not depend on the repository-related logic. This means that
e.g. the `gittest` package cannot depend on the `testcfg` package
even though it is at a conceptually higher level.
- It hides away the parameters used to create the repository. This
means that we use seed repositories by default, but also that we
skip creation via the Repository service. Both are bad and should
require the caller to explicitly state that they want this.
Convert callers of `BuildWithRepo()` to instead use `Build()` to build
the configuration and `gittest.CreateRepository()` to create the
repository. Remove the now-unused `BuildWithRepo()` function.
|
|
One of the tests for git-upload-pack(1) in our gitaly-ssh command is
manually assembling a test server. There is no reason to do this though.
Refactor the code to use the `testserver` package instead.
|
|
This commit renames Updater's Cancel method to Close so it matches
the io.Closer interface and the conventional naming of such methods
in Go.
|
|
Gitaly's update-ref wrapper closes after performing a single transaction.
This is not ideal in the context of applying a write-ahead log. With each
repository having a single goroutine writing into it, it is wasteful to
start and stop an update-ref process separately for each log entry. This
commit changes the Updater to support running multiple transaction over
it. Start transaction needs to be explicitly called prior to performing
any updates. Committing a transaction no longer automatically closes the
updater. The caller is responsible for calling Cancel once it is done
with the process.
Since the calling conventions changed, all of the call sites were updated
to explicitly start a transaction and to close the updater after use.
Cleaner was changed to instantiate the update-ref process in the method
where it is used to make it easy to ensure the process is always cleaned
up at the end.
|
|
We want to reduce the number of calls to `git.NewWithoutRepo()` to a
bare minimum. Refactor tests to use `gittest.NewCommand()` and similar
to get rid of some calls to `NewWithoutRepo()`.
|
|
The `grpc.Server.Serve()` function will return an error when the gRPC
server has either failed unexpectedly or in case it wasn't properly shut
down. We don't check this error in many places though, which both causes
the errcheck linter to complain and hides issues we have with some of
our test setups.
Fix the missing error checks via a new `testhelper.MustServe()` helper
function that does the error checking for us. This surfaces some test
errors for tests that forget to shut down the server or that manually
close the listener, which should be handled by gRPC itself. All of those
errors are fixed while at it.
Drop the corresponding linting exceptions.
|
|
Do not assume client users want the same tracing interceptors with the same configuration.
|
|
Enforce consistent naming of `testing.TB` variables, which should be
called `tb`, and adapt tests that violate this rule.
|
|
Change the updateref package to accept object IDs as old and new
target of the reference that is to be updated. This increases type
safety and better documents the intent of provided functions.
|
|
We're about to add the ability to test with SHA256 hashes. We assume
none of the tests work with this object format.
With this change we add the build constraint to not run any test when
the tag 'gitaly_test_sha256' is set.
|
|
Gitaly needs to locate auxiliary binaries in various places to pass
them as arguments and call them. Currently, the logic to do so is
spread around the code base where the cfg.BinDir is joined with the
binary name. Soon we'll have to make a distinction between binaries
that are in the BinDir and binaries that are unpacked into a runtime
directory on start up. To support this, this commit centralizes the
binary finding logic to BinaryPath helper added to Gitaly's config.
For now, it simply join BinDir with the binary name as each of the
call sites were doing. In a later commit, it will also help with
locating the unpacked binaries correctly.
|
|
Rewrite callers of `testhelper.ModifyEnviroment()`, which is about to be
removed in favor of either `t.Setenv()` or `testhelper.Unsetenv()`.
|
|
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
|
|
The function `GitalyInternalSocketPath()` exposed by Gitaly's
configuration is needlessly verbose with its `Gitaly` prefix. Rename it
to `InternalSocketPath()` instead.
|
|
Now that we are adding a second limit handle, adjust the code to allow
for multiple limit handlers to be passed into a server invocation.
|
|
A future commit will add a new middleware that will limit based on the
rate rather than concurrent calls. There is a good amount of logic
currently used by the concurrency limiter that can be reused since a
rate limiter is also operating on incoming requests based on RPC name.
To make easier to add this new limiter type in the future, refactor the
code by adding some abstractions easier to add another type of limiter.
|
|
testhelper.Context() currently return a cancellation function as a
second return value. Majority of the tests do not need to explicitly
cancel the context but they simply defer its cancellation to clean up
after the test. Given this, we can reduce the test verbosity and make
testhelper.Context easier to compose by removing the unnecessary second
return value. This adds a t.Cleanup function to automatically cancel the
context at the end of the tests and omits the returned cancellation
function.
Tests which simply `defer cancel()` have had the extra call removed. Some
tests explicitly call the cancellation, and these tests have been modified
to add context.WithCancel around the testhelper.Context call. There are a
few loctions where testing.TB was passed down to test helpers that create the
context.
|
|
Add SSHUploadPackWithSidechannel
Closes gitlab-com/gl-infra/scalability#1513
See merge request gitlab-org/gitaly!4251
|
|
If GITALY_USE_SIDECHANNEL=1 is set, gitaly-ssh will use
SSHUploadPackWithSidechannel instead of SSHUploadPack.
|
|
Prior to this change, ModifyEnvironment was returning a function to
revert the changes done. This was typically passed to defer or to
t.Cleanup().
This change moves calling t.Cleanup() to the ModifyEnvironment function
itself.
|
|
The updateref package used to require the Gitaly configuration given
that it needed to be passed when constructing Git command options. Now
that such dependencies are instead provided by the Git command factory
itself the configuration is not used anymore by this package.
Remove the dependency on the Gitaly configuration and adjust callers to
not pass it anymore.
|
|
We're about to change the ExecCommandFactory to carry state soonish,
which both requires us to return an error code and a cleanup function.
This is going to become tedious to manage in our tests given that
there's many locations where we construct the factory.
Introduce a new function `gittest.NewCommandFactory()` to be used in
tests, such that callsites don't have to handle all of this.
|
|
The Git binary path for the gitaly-git2go executor is currently resolved
by accessing the configured Git path directly. This is deprecated, where
callers should instead ask the Git command factory for the Git execution
environment.
Convert the custom hooks executor to accept a Git command factory and
query it for the binary path.
|
|
Convert tests which spawn Git commands to use the Git execution helpers
as exposed by the gittest package. Running Git directly without the use
of either those helpers or the Git command factory is not supported
anymore.
|
|
The limithandler package uses global variables to both track Prometheus
metrics and to handle its configuration. Especially the latter is really
fragile, where we need to set up state of the limithandler in various
places by calling global functions.
Fix this by making the `LimiterMiddleware` self-contained, where it
hosts all configuration as well as the Prometheus metrics.
|
|
The limit handler is currently created ad-hoc by Gitaly's server code.
We're about to change the way it hosts Prometheus metrics though such
that we can get rid of a set of global variables, and this will require
us to make sure that there's only a single instance of the limiter.
Inject the limit handler as a dependency when creating the server to
prepare for this.
|
|
Consolidate the feature flags context APIs such that we have the same
set of functions for creating feature flags in incoming and outgoing
contexts.
|
|
The testhelper has functions to create test loggers for us. These are
awkardly named though, and furthermore we have a global variable which
contains a reference to one of the functions which creates the logger
for us, which is bad design. Furthermore, this global variable isn't
modified by anything anymore, making it essentially the same as the
function it's aliasing.
Rename functions to match Go best practices and remove the global
variable.
|
|
Move build helpers from the testhelper into the testcfg package to break
the dependency on `internal/gitaly/config`. Note that packages other
than the testhelper package have no access to the global temporary test
directory, which makes it impossible to create directories shared across
tests from other packages. We thus also introduce a new helper function
`CreateGlobalDirectory()` which creates a global deterministic directory
in the shared temporary directory.
|
|
One of the connectivity tests for gitaly-ssh uses a TLS-based setup to
assert that we're able to connect to them. The self-signed TLS
certificate we're using for this setup keeps us from connecting to the
Gitaly server directly. While we don't do that right now in the first
place, it will break as soon as we start to do a health check for test
Gitaly servers because we cannot connect to it in the first place.
Fix this issue by injecting self-signed certificates into the system
store. While at it, generate certs on the fly instead of using
pre-seeded test data.
|
|
Setup of a test package's main function is quite repetitive: we call a
separate `testMain()` function such that we can use `defer` calls, this
function calls `MustHaveNoChildProcess()` and `Configure()`, and then we
return the result of `m.Run()`. This is almost always exactly the same,
with some exceptions where we need to have additional setup code.
Unify this code via a single `testhelper.Run()` function which does all
of this. It allows us greater flexibility in the setup code such that we
can easily perform additional validation after the tests without having
to modify all callsites.
Note that this change removes calls to the goleak package in some
places. These will resurface in a later commit in this patch series,
where we instead move this call into `testhelper.Run()` such that it is
executed by default.
|
|
Before we have converted the updateref package to always use
transactional semantics in 154dbc1a2 (updateref: Safeguard against
accidentally committing updates, 2020-12-15), all the `Wait()` function
used to do was to call `Wait()` on git-update-ref(1). This simply closed
stdin and then waited for the command to return.
But since above commit, semantics have changed such that we now always
use an explicit transaction, where `Wait()` is now responsible for
committing the transaction by sending "commit" to the process. In this
context, calling the function `Wait()` doesn't really convey its spirit
anymore: yes, we wait for the command, but what we are really doing is
to commit the transaction.
Rename the function to `Commit()` to better highlight its modern use.
|
|
The GitLab mock client will currently always return success for all of
its stubbed functions. In a subsequent patch, we'll want to write tests
though which return various failures instead. Prepare for this by
generalizing the mock client such that callers can pass arbitrary
implementations of its mocked functions.
|
|
Rename `ConfigureGitalySSHBin()` to `BuildGitalySSH()`: this highlights
more clearly that the intention of the function is to build a command in
contrast to configuring the command.
|
|
Rename `ConfigureGitalyHooksBin()` to `BuildGitalyHooks()`: this
highlights more clearly that the intention of the function is to build a
command in contrast to configuring the command.
|
|
The dependency github.com/golang/protobuf is deprecated and should
be replaced with google.golang.org/protobuf.
This change replaces usage of the jsonpb package with encoding/protojson.
The packages have incompatible API that is why the code changes
were done to migrate.
The exclusion rule removed from the golanci-lint tool configuration file
to prevent future use of the deprecated package.
|
