| Age | Commit message (Collapse) | Author |
|---|
|
Before this when running with my nascent UserCreateTag in Go code we'd
die in auth_test.go with:
TestAuthBeforeLimit: auth_test.go:380:
Error Trace: auth_test.go:380
Error: Received unexpected error:
rpc error: code = Internal desc =
panic: runtime error: invalid memory
address or nil pointer dereference
Test: TestAuthBeforeLimit
Now we'll emit the better error message of:
rpc error: code = Internal desc = panic: The hookManager must be
set up already! Did you migrate non-hook-supporting code from Ruby
to Go?
To reproduce run:
go test -v ./internal/gitaly/server/ -run 'TestAuth' -count=1
See [1] for more background details & further explanation, add a
comment pointing to it to aid future debugging.
1. https://gitlab.com/gitlab-org/gitaly/-/merge_requests/2911#note_468836913
|
|
git: Allow ConfigPair to be used as GlobalOption
See merge request gitlab-org/gitaly!2936
|
|
Handle start and target repositoires being the same in UserCommitFiles
See merge request gitlab-org/gitaly!2941
|
|
log: Fix semantic merge conflicts caused by `GetCommit()` change
See merge request gitlab-org/gitaly!2944
|
|
ref: Refactor the way we find references which are to be deleted
See merge request gitlab-org/gitaly!2939
|
|
With commit ef065f1e5 (Break dependency of the catfile on global
config.Config, 2020-12-15), the signature of `log.GetCommit()` has
changed to include a locator such that it doesn't implicitly rely on the
global config object anymore. There's been a semantical merge conflict
though, where other MRs introduced new usages of this function which
still used the old signature.
Fix the conflict by properly passing the locator.
|
|
Removal of the helper.GetRepoPath
See merge request gitlab-org/gitaly!2935
|
|
updateref: Safeguard against accidentally committing updates
See merge request gitlab-org/gitaly!2930
|
|
Gitaly receives requests in UserCommitFiles where the start and the
target repositories are the same. In such cases, it doesn't have to
resolve the branches from start repository as it is the same. This
commit checks if the repositories are the same and only operates on
the local repository if that is the case.
|
|
operations: Fix Go UserMergeBranch failing with ambiguous references
See merge request gitlab-org/gitaly!2921
|
|
Support repository specific primaries and host assignments in dataloss
Closes #3301 and #3199
See merge request gitlab-org/gitaly!2890
|
|
This commit adds back testing with dataloss testing with the SQL
elector as well. While the query itself was tested using both variants
before, the condition setting the boolean switch was not. This commit
thus tests with both electors to ensure the switch works correctly.
|
|
This commit changes `praefect dataloss` to use repository specific
primaries in the tests to cover assert they are correctly printed
out per repository. Assignments are also set for the repositories
in order to test omitting the 'assigned host' message when a storage
is not assigned.
|
|
Adds support for repository specific primaries and variable replication
factor in dataloss. When 'per_repository' elector is in use, the dataloss
returns repository specific primaries from the database. Assignments are
taken into account if they are set for the repository. If the repository
has no assignments, all configured storages are considered assigned as
a fallback to Praefect's behavior before assignments were introduced.
As before, dataloss by default only returns read-only repositories.
When fetching also partially replicated writable repositories, only
repositories which have at least one outdated assigned node are printed
out. Having an outdated copy of the repository on an unassigned node does
not indicate the repository's replication factor has not been met, only
that there is an extra copy lying around.
|
|
When determining which references to delete for the DeleteRefs RPC, we
shell out to git-for-each-ref(1) to determine which references match a
given set of prefixes. We do have a higher-level abstraction around to
list references via git-for-each-ref(1) though in our LocalRepository
interface.
Convert the code to use `LocalReposiory.GetReferences()` to have one
less moving part where we manually invoke git.
|
|
When computing which references we are about to delete in the DeleteRefs
RPC, we allocate two arrays into which we put the final reference names.
We're doing this quite inefficiently though by just calling `append`,
even though we know the size of both arrays beforehand.
Improve our code to instead preallocate arrays to avoid repeated
reallocations.
|
|
Stub out hook manager when hooks are disabled in tests
See merge request gitlab-org/gitaly!2938
|
|
This reverts commit bca59804605b4afd0bebacbaa0952c5b4ca16141, reversing
changes made to 97b0280d97d847e9c09948d17587cc20398907c0.
|
|
Gitaly has a `GITALY_TESTING_NO_GIT_HOOKS` environment variable
to disable git hooks during tests. Currently, it only skips configuring
the interal API client but leaves the hook manager in place. This causes
nil pointer dereferencing panics when executing code that would invoke
hooks when the environment variable is set.
This commit fixes the problem by stubbing out the whole hook manager
when the environment variable is set.
|
|
helper.GetPath is not used anymore in the production
code so we remove it and refactor all usages of it.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
helper.GetRepoPath is not used anymore in the production
code so we remove it and all outdated dependencies of it.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
The catfile package directly or indirectly uses config.Config
value. And as we are about to remove config.Config from the
code we need to break this dependency and substitute with
other abstractions.
For now those are storage.Locator and alternates.Env instead
of the old alternates.PathAndEnv that depends on the global var.
It was decided to extend constructor function of the catfile
to accept storage.Locator as a dependency as it is already
injected in most of the "server"s which handle requests.
The other option could be creation of the "factory", but it will
require a new dependency to be injected into all existing
services which is a lot more changes and adds currently unnecessary
abstraction that needs to be managed.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
Revert "Merge branch 'smh-enable-user-commit-files' into 'master'"
See merge request gitlab-org/gitaly!2937
|
|
This reverts commit c6d679391328796f4679542ab651b39de75e7a23, reversing
changes made to 993f4ccd104882e0766bd1377f24642603bd558f.
|
|
Now that we have a better mechanism to pass configuration to git
commands via the GlobalOption support for `ConfigPair`, this commit
converts commands to use it instead of `ValueFlag`s.
|
|
Using the ConfigPair option as a GlobalOption is currently not possible
because in the past, it was only used as parameter for git-config(1).
This means that every SafeCmd which uses configuration parameters needs
to assemble its own format which is understood by git via a normal
ValueFlag. Which is not ideal given that it distributes knowledge on how
the format looks like, makes it harder than necessary to find any
configuration and that it cannot verify the format.
This commit thus extends the ConfigPair to also implement the
GlobalOption interface. Like this, we can easily verify the format of
the configuration key to look as expected, namely that we have the
typical `section.optional_subsection.key` format which git expects.
Furthermore, we can now verify that the key doesn't contain any equals
signs. This is important, as git always interprets the first equals sign
it sees as the separator between key and value. If the key already
contains an equals sign, this would mean that the configuration key is
misparsed.
|
|
While the Options interface has its fair share of tests, GlobalOptions
don't yet. This is mostly as it has only been introduced fairly recently
and because its implementation was the same as that for Options anyway.
So adding tests back when the GlobalOptions interface was created didn't
make a lot of tests.
This is about to change though as we're going to add the first option
which has different behaviour based on whether it's invoked as Option or
GlobalOption. So let's prepare for this by adding some testcases for our
existing GlobalOption implementations.
|
|
Document Praefect replication of forks
See merge request gitlab-org/gitaly!2912
|
|
|
|
'master'
User{Branch,Tag,Submodule}: ferry update-ref errors upwards
See merge request gitlab-org/gitaly!2926
|
|
Cleanup redundant data from notification events
Closes #3309
See merge request gitlab-org/gitaly!2893
|
|
Update resolve conflict command to use gob over stdin
See merge request gitlab-org/gitaly!2903
|
|
|
|
Variable replication factor demo script
See merge request gitlab-org/gitaly!2892
|
|
Makefile: Fix test execution in newly cloned repository
See merge request gitlab-org/gitaly!2924
|
|
Repository importer has been successfully demoed and has finished
in both production and staging. This commit removes the demo script
as it no longer is relevant.
|
|
|
|
Auth tests: do not hardcode port 9999
See merge request gitlab-org/gitaly!2928
|
|
Revert "Merge branch 'po-user-update-submodule-ff-default' into 'master'"
See merge request gitlab-org/gitaly!2931
|
|
This reverts commit 61c631b3e50b2a4db5c6c821d1d22906260f2f28, reversing
changes made to 64d6e3dc24637c9755c5ca5220c1330d260689e5.
Because of the bug noted in my
https://gitlab.com/gitlab-org/gitaly/-/merge_requests/2926 of not
ferrying errors upwards. The feature has been stopped already, my MR
fixes it, but as discussed in Slack we'd like to have master in a
known-good state for an upcoming release.
|
|
The triggers may be called with no actual change done by the
operation, like updating of the non-existing rows. In this case
existing trigger will generate a notification with empty payload.
This payload can't be properly handled by the listener and will
cause cache disabling.
To deal with it we add check in the trigger logic to
verify if there are any actual changes done to the state.
This change also includes structural changes into the query but
preserve the same logic and format of the payload.
Closes: https://gitlab.com/gitlab-org/gitaly/-/issues/3309
|
|
git: Improve SafeCmd interface
See merge request gitlab-org/gitaly!2891
|
|
SubSubCmds are currently implemented as an option, which is in fact
quite weird when using them as they're now intermingled with all the
other options. With all the preceding refactorings, we can now do a lot
better though and instead implement SubSubCmd as a Cmd.
|
|
Assembling arguments for subcommands is non-trivial logic as it requires
handling names, options, positional arguments and post-separator
arguments. As we're about to introduce a second user which requires this
logic by converting `SubSubCmd` from an option to a command, let's pull
out common code into a separate function which can be called by both
SubCmd and SubSubCmd types.
|
|
In order to disambiguate implementations of the Option and Cmd
interfaces, we currently use both a `IsCmd()` and an `IsOption()`
function which simply does nothing. This is bad style, which is why this
commit instead refactors the code to have type-specific functions
`CommandArgs()` and `OptionArgs()`. This is more idiomatic and also
makes code easier to read when arguments are assembled.
|
|
Currently, we use the Option type both for global and command-specific
options. This use is confusing though as it is possible to pass
non-global options as global ones and vice versa. Two examples exist
already where the interface is lacking with the `ConfigPair` and
`SubSubCmd` types, which both make no sense at all as a global option
and would break the git command.
Let's disambiguate both usages by introducing a new `GlobalOption` type.
While it has the same implementation for most options, above examples
don't implement it. As a result, they cannot be used in global context
anymore.
|
|
Now that all external users of `CmdStream` have been converted to
instead use command options, we can now make the `CmdStream` type
internal to the "git" package.
|
|
Before we had variable command options, we refactored some of our
functions to accept a `CmdStream` parameter. This is confusing nowaday
as there is now multiple ways to pass standard streams to commands, once
via the parameter and once via `WithStdout()` and the likes.
Let's drop the parameter in favor of using options to avoid the
confusion and to unify interfaces.
|
|
Before we had variable command options, we refactored some of our
functions to accept a `CmdStream` parameter. This is confusing nowaday
as there is now multiple ways to pass standard streams to commands, once
via the parameter and once via `WithStdout()` and the likes.
Let's drop the parameter in favor of using options to avoid the
confusion and to unify interfaces.
|
|
Before we had variable command options, we refactored some of our
functions to accept a `CmdStream` parameter. This is confusing nowaday
as there is now multiple ways to pass standard streams to commands, once
via the parameter and once via `WithStdout()` and the likes.
Let's drop the parameter in favor of using options to avoid the
confusion and to unify interfaces.
|
