| Age | Commit message (Collapse) | Author |
|---|
|
The test setup fucntion `setupOperationsService()` both set up the
service and a seeded repository. We have replaced all usages of it with
its variant though that don't create a repository, making this function
unused.
Remove it and rename `setupOperationsServiceWithoutRepo()` to fill in
its role.
|
|
Implement SHA256 support for UserUpdateSubmodules and start testing with
this object format.
Changelog: added
|
|
TestUserUpdateSubmodule is asserting the repository's path in an
error message. This should be avoided as the repository path may change.
Fix the test to not assert the exact repository path.
|
|
praefect/coordinator: Unify ErrRepositoryNotSet errors with Gitaly
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5975
Merged-by: Pavlo Strokov <pstrokov@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
Now that the submodules implementation without Git2Go has been running
in production for several weeks without issue, let's remove the feature
flag.
|
|
The coordinator prefixes most of its errors with "repo scoped", which
makes the errors mismatch those returned by Gitaly. Consequentially,
Praefect does not act as an actually-transparent proxy. This can be
painfully felt in our tests, where we need to distinguish between both
errors.
Drop the prefix and return the error directly to unify the errors that
both Gitaly and Praefect would return.
|
|
Teach the localrepo WriteCommit API to sign commits if a signing key is
provided.
Also adjust merge and submodule tests to test with the feature flag.
|
|
The `ValidateRepository()` function of the `config.Locator` doesn't
explicitly check whether the passed-in repository is set. And right now
it doesn't need to because all RPCs use `service.ValidateRepository()`
anyway to verify that the repository is indeed set. We're about to
remove this helper in favor of `locator.ValidateRepository()` in a
future patch series, at which point the locator would become exposed to
any unset repositories.
Explicitly handle unset repositories in the `config.Locator` and return
the same error as `service.ValidateRepository()`. Refactor tests to
check for `ErrRepositoryNotSet` explicitly and rename the error message
in order to make sure that there cannot be any tests left that check for
the old error message.
|
|
Now that we have helpers for modifying tree entries, replace our
existing git plumbing implementation with the TreeEntry Modify() helper
behind a feature flag.
|
|
localrepo: Add ability to read and write trees in memory
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5807
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: John Cai <jcai@gitlab.com>
|
|
Currently, a TreeEntry is a flat data structure in the sense that it
represents a single entry but does not have any children. A git tree
object has children though.
Change this by adding an Entries member to the TreeEntry struct. This
way, we can represent a real tree data structure.
Also, add a GetTree() function that can get an entire tree. We can
consolidate ListEntries() into GetTree().
|
|
WithInterceptedMetadata() uses testproto.ErrorMetadata, which is in a test-only package. This function is only used in tests so move it to testhelper and interceptors into testserver
|
|
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.
|
|
Remove the SubmoduleInGit feature flag, and associated call sites.
|
|
The lstree package contains a helper for reading a tree. There is no
reason why this code needs to be in its own package. In fact, this makes
it harder when for instance, we might want to add code in the
`localrepo` package that calls `ListEntries()` since it would result in
a dependency cycle because `lstree` depends on `localrepo`.
Move the code under `lstree` under `localrepo`, since that is the one
stop shop for operations on local repositories.
|
|
This commit fixes manually quoted string interpolation with '%s' and
"%s". Quoting this way doesn't escape special characters such as endline
and makes debugging harder later. We encourage to use %q verb instead.
|
|
Updating a submodule is simply updating the oid of the commit at the
path. We can do this by manually rewriting trees and writing a new
commit. If we impelement it this way, we don't need to rely on Git2Go.
|
|
This reverts merge request !5448
|
|
Updating a submodule is simply updating the oid of the commit at the
path. We can do this by manually rewriting trees and writing a new
commit. If we impelement it this way, we don't need to rely on Git2Go.
|
|
There is no reason to keep two structs around that both represent a tree
entry object. Now that we have `localrepo.TreeEntry`, added by 9fac9132,
let's unify `lstree` to use `localrepo.TreeEntry` everywhere.
To do so, we need to add a `Type` field to `localrepo.TreeEntry`.
|
|
This reverts merge request !5046
|
|
Updating a submodule is simply updating the oid of the commit at the
path. We can do this by manually rewriting trees and writing a new
commit. If we impelement it this way, we don't need to rely on Git2Go.
|
|
Previously `ValidateHex` returned a generic error message when an
invalid string representation of a SHA1 or SHA256 oid was found. This
gave no indication as to why the oid was rejected.
Now that we are iterating over characters manually rather than using a
regexp, we are in a position to provide users with more detail on what
is invalid about the object ID. Introduce new error types to bubble this
information up to users.
|
|
If the clients provide the ExpectedOldOid in the request of
UserUpdateSubmodule, we need to use this as the ref's current OID for
updating the reference.
This is directly fed to git-update-ref(1) where it'll exit with an error
code if the current OID of the ref doesn't match the OID provided by us.
This allows us to avoid any race conditions wherein the branch was
updated concurrently by another process.
|
|
There are a few Test*UserUpdateSubmodule* tests which do similar things,
but are outdated and use seeded repos. In preparation of the upcoming
additions to UserUpdateSubmodule, let's merge these tests.
This makes it easy to add new tests and also no longer uses seeded
repos, making it easier to also migrate to SHA256.
|
|
The helper function `GitalyOrPraefectMessage()` will return either one
of two messages depending on whether we execute with Praefect as proxy
or not. It is more generally useful than that though as we may also want
to check other object types. And now that we can use generics it is
needlessly tied to a specific object type.
Convert the function to accept any objects as input by using generics
and rename it to `GitalyOrPraefect()`.
|
|
Replace usage of status.Errorf() with helper.Err<StatusCode>f() to
make error creation more standard.
Replace fmt.Errorf() with errors.New() if used without a reason.
Return Internal code in case of an unexpected operation
failure instead of Undefined by using helper.ErrInternal().
Replace old %v with new %w for proper error wrapping.
Check error type to convert it to the corresponding error code.
Remove redundant prefix from error returned by RPC call.
|
|
GitalyOrPraefect now renamed to GitalyOrPraefectMessage
as it is more descriptive.
|
|
Gitaly should return the same error for all RPCs where the
Repository input parameter is missing.
Input validation code extracted into separate function where
it makes sense.
The test coverage extended to cover changed code.
The error verification is done not only for the code, but for
the message as well. It gives us confidence that a proper path
is tested.
|
|
A few weeks ago we have decided that `testing.TB` arguments should be
first, `context.Context` second. This is rather a matter of perssnal
taste instead of correctness, but we should strive for consistency.
Reorder all arguments to match the agreed-upon style.
|
|
Inject the object hash used to parse tree entries so that the tree entry
parser can easily support parsing both SHA1 and SHA256 tree entries. For
the time being all callers just inject SHA1. They will be converted over
time to inject the correct value.
|
|
Move the `ZeroOID` variable into the `ObjectHash` structure to make it
dependent on the hash function used.
|
|
Move the `NewObjectIDFromHex()` function into the `ObjectHash` structure
to make it dependent on the hash function used.
|
|
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.
|
|
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
|
|
This commit amends most of the operations service's test to create the
test state through the API. This better matches the production scenario
and allows for Praefect to create the metadata it would create in real
setups. Most of the replacements are straight forward but some tests rely
on working trees. As Gitaly does not support creating worktrees through the
API, the worktrees are created after creating the repo through the API.
TestUserCommitFiles was simplified a bit and its branch was changed as new
repositories nowadays have main as their default branch.
|
|
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.
|
|
Custom hook scripts used in tests which use Git currently have the fully
qualified Git path injected into them such that we are sure that the
correct Git version is used. This was required until now given that we
didn't inject a PATH environment variable which had the correct Git
version prepended to it. As a result, we'd have executed the wrong Git
executables in these scripts, which would typically be our intercepting
Git wrappers in `internal/testhelper/testdata/home/bin`.
Now that we do properly set up PATH, this workaround isn't needed
anymore. Quite the reverse: the fact that we injected the full Git path
into these scripts only obscured the fact that we didn't set up the
environment as expected.
Adapt tests to always resolve Git via PATH to assert that we pick up the
correct executables.
|
|
The `RequireGrpcError()` helper function misleadingly doesn't check the
error for equality, but instead only verifies that the error has the
expected gRPC code.
Rename the function to `RequireGrpcCode()` to clarify this.
|
|
Tree entries host the object ID of the entry as a normal string even
though we nowadays try to use `git.ObjectID`s instead. Convert the
struct to use the latter to improve type safety and make available
helper functions to convert these IDs as required.
|
|
When we have a quarantine directory in place, then we migrate its
objects into the main object database after we have successfully voted
on the ref update via the reference-transaction hook. Most notably, we
do so _after_ we have executed the update hook. This is different from
what Git is doing, which will migrate objects after the pre-receive has
successfully executed. The result is that the update hook runs with a
wrong environment.
This hasn't been much of an issue until now: we don't really use the
update hook ourselves, and custom hooks would have envvars set up such
that it can find quarantined objects. It will become an issue when we
fix our locking: we want to lock the ref update before doing the
"prepared" vote, but do not yet have all objects available in the main
object database.
Fix the issue by aligning our semantics with what Git is doing, which is
to migrate objects directly after the pre-receive hook.
Changelog: fixed
|
|
When any of the hooks fails to execute in our `UpdaterWithHooks`, then
we return a `PreReceiveError` to the caller. This is quite misleading
though: we not only execute the pre-receive hook, but also others, and
they all return the same error.
Refactor the error to instead be a `HookError`. While at it, we also
embed stdout and stderr in the error such that it can handle generation
of the error message itself instead of using `hookErrorMessage()`, and
implement `Unwrap()` such that we can use `errors.As()` to retrieve the
embedded error. This will be used at a later point to distinguish
`HookError`s and `NotAllowedError`s.
|
|
Quarantine directories have been introduced for two reasons: first they
are a correctness fix, where new objects don't end up in the target repo
in case the request fails. And second, they allow us to speed up access
checks enumerating new objects direcytl via the quarantine directory.
Quarantine directories have been tested since August 9th in production
now without any issues and can thus be deemed stable.
Remove the feature flags to make them generally available.
Changelog: performance
|
|
Implement support for object quarantine directories in
UserUpdateSubmodule.
|
|
There's two different functions to initialize empty repositories:
`InitBareRepoAt()` and `InitRepoWithWorktreeAtStorage()`. Both do the
same, except that they either create a bare repository or not.
Merge both into a single function `InitRepo()`. In order to serve both
purposes, they take an optional `InitRepoOpts` structure with a single
`WithWorktree` parameter.
|
|
The dependency github.com/golang/protobuf is deprecated and should
be replaced with google.golang.org/protobuf.
This change replaces usage of the ptypes package. The replacement
packages has a different structure and it requires code changes
to be done to migrate.
The exclusion rules removed from the golanci-lint tool configuration
file to prevent future use of the deprecated package.
|
|
Parallel run of the tests
See merge request gitlab-org/gitaly!3500
|
|
Previously, the time of Web GUI Git operations is set by `time.Now()` or `ptypes.Timestamp`, and the time zone (system local or UTC) is not what we expected.
This commit corrects the timezone of the operation according to the User's time zone.
Changelog: fixed
|
|
To speed up execution of the tests we mark them allowed to
run in parallel by calling t.Parallel(). As a first step we
mark for parallel execution only upper-level test and no sub-tests.
|
|
The new "v14" version of the Gitaly module is named to match
the next GitLab release. The module versioning is needed in
order to pull gitaly as a dependency in other projects. The
change updates all imports to include v14 version. The go.mod
file was modified as well after go mod tidy execution. And
the changes in dependency licenses are reflected in the NOTICE
file.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/3177
|
