| Age | Commit message (Collapse) | Author |
|---|
|
|
|
Stop seeding the repositories in test from an existing repository.
|
|
config/locator: Improve error reporting for invalid repositories
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5891
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Pavlo Strokov <pstrokov@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Pavlo Strokov <pstrokov@gitlab.com>
Reviewed-by: Justin Tobler <jtobler@gitlab.com>
|
|
The config.Locator structure optionally verifies whether a repository
path maps to a valid Git repository, which helps us to get better error
reporting when trying to execute Git commands in repositories that don't
actually exist or which are corrupt.
But the error reporting in this function is quite lacking as we used to
call `IsGitRepository()`, which only gave us a boolean to indicate
whether the path corresponds to a Git repository that is indeed a valid.
Consequentially, all we were able to report in this case was that the
repository is missing.
We have since replaced `IsGitRepository()` with `ValidateRepository()`,
which provides more detailed error reporting in the case where the path
does not point to a valid repository. We can thus now report detailed
information to the caller why exactly we think that a repository is not
valid.
Refactor the function to bubble up this error. This should significantly
improve our ability to diagnose why we think a specific repository is
invalid and gives callers the ability to act accordingly. As an added
benefit, the refactoring removes paths from error messages in favor of
error metadata, which should help us to further deduplicate errors in
Sentry.
|
|
The `IsGitDirectory()` function accepts a path and then checks whether
it corresponds to a valid Git repository or not. This information is
returned via a boolean, but that is indeed quite lacking given that the
caller cannot figure out _why_ a specific path would not be a valid
repository. This is noticable in our error reporting, where we don't
discern between a missing and a corrupt repository.
Refactor the function to instead return an error, renaming it to
`ValidateRepository()` to better match its new semantics. The newly
introduced errors are not yet being used to improve our error reporting
as it would be seen in our logs.
|
|
The commonerr package bundles a set of common errors that we use across
the project. Right now the current set only includes storage-related
errors though that relate to repositories being missing or invalid. And
while we use these common errors only in the Praefect codebase right
now, it would make sense to have Praefect and Gitaly share these errors
so that we ideally have a single source of truth for these.
Move the errors into the storage package so that it becomes less awkward
to share them between Gitaly and Praefect. This feels like a natural
place for these errors given that the errors can be directly attributed
to the storage level.
|
|
The `storage.Locator` interface provides two functions `GetPath()` and
`GetRepoPath()` which both retrieve the path of a repository. It isn't
exactly easy to tell which one does what though. To shed some light on
it: the only difference is that `GetPath()` returns the repository path
without verifying that the repository is valid, while `GetRepoPath()`
does verify the repository location.
To improve upon this confusing interface we have introduced options for
`GetRepoPath()` that allow the caller to skip verification in 854b62c45
(storage: Introduce `GetRepoPathConfig`, 2023-03-27). Let's thus perform
the migration and use this function exclusively now, dropping the old
`GetPath()` function.
|
|
|
|
Add configuration validation docs
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5667
Merged-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Evan Read <eread@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: Evan Read <eread@gitlab.com>
Co-authored-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: Raditya Surya <radityasurya1911@gmail.com>
|
|
|
|
git: Validate pseudo-revisions via `git.ValidateRevisions()`
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5878
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
|
|
Update FindDefaultBranchName to add HeadOnly option
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5880
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>
|
|
Moves all the common test setup to outside the test loop so that it is
only run once.
|
|
Previously only a single field was checked, but using ProtoEqual allows
us to differentiate valid and invalid responses.
|
|
repository: Disable `fsck` during replication
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5879
Merged-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
[ci skip]
|
|
[ci skip]
|
|
The `ReplicateRepository()` RPC uses `git-fetch(1)` to retrieve objects
from preexisting internal repositories. Currently the `fetch` also
performs consistency checks on the newly retrieved objects. This is
problematic because preexisting objects should alway be able to be
replicated regardless of the consistency check. This change sets
`fetch.fsckObjects` to `false` for the performed `fetch` to disable
`fsck` consistency checks.
|
|
go: Update module github.com/stretchr/testify to v1.8.4
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5887
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
proto: Add documentation to `smarthttp.proto`
Closes #5120
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5858
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Reviewed-by: Toon Claes <toon@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Karthik Nayak <knayak@gitlab.com>
|
|
go: Update module github.com/beevik/ntp to v1
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5865
Merged-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: James Fargher <proglottis@gmail.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
go: Update module github.com/urfave/cli/v2 to v2.25.5
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5876
Merged-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: GitLab Renovate Bot <gitlab-bot@gitlab.com>
|
|
|
|
ci: Update to macOS M1 runners
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5881
Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
|
|
mod: Update logrus to v1.9.3
Closes #5092
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5877
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
|
|
storage: Refactor code to contain knowledge of repo paths in a single package
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5863
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Pavlo Strokov <pstrokov@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
|
|
objectpool: Refactor alternates file parsing
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5871
Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>
|
|
Fixed: UserMergeBranch loses error messages from the merge operation
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5777
Merged-by: James Fargher <proglottis@gmail.com>
Approved-by: James Fargher <proglottis@gmail.com>
Reviewed-by: James Fargher <proglottis@gmail.com>
Reviewed-by: OK_MF <oski.kervinen@m-files.com>
Co-authored-by: Oski Kervinen <oski.kervinen@m-files.com>
|
|
'5036-introduce-structured-error-definitions-for-gettreeentries' into 'master'
proto: Add new message `GetTreeEntriesError`
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5827
Merged-by: karthik nayak <knayak@gitlab.com>
Approved-by: Christian Couder <chriscool@tuxfamily.org>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
|
|
GitLab has recently deprecated the Intel-based macOS runners. As we are
still using them as part of our pipeline the consequence is that the
pipelines are now broken.
Fix this by updating to the new M1 runners.
|
|
With the move to M1-based macOS runners the Postgres database directory
is about to be moved. Instead of relying on any hardcoded locations,
let's convert the CI job to initialize our own database at a location
controlled by us.
|
|
The latest release of logrus (v1.9.3) contains a fix for the panic in
the Writer. Let us update to the new version.
|
|
The `smarthttp.proto` protobuf is missing documentation. Let's amend
the existing documentation and add missing documenatation.
Closes https://gitlab.com/gitlab-org/gitaly/-/issues/5120
|
|
|
|
These tests were written in an out-of-date style. This commit updates
them to all be table driven.
|
|
We want to progressively migrate gitlab-rails away from the heuristic
that this RPC usually uses. There were too many failures to do this
using a feature flag. So instead here we introduce a flag to control the
behaviour.
Updates RPC documentation while in the area.
Changelog: added
|
|
|
|
|
|
Currently `Disconnect()` implements its own means to parse a Git
alternates file. This change refactors `Disconnect()` to use the already
existing `stats.AlternatesInfoForRepository()`, which is much more
thorough, to handle the parsing instead.
|
|
The `GetTagSignatures()` RPC has an open-coded version of revision
validation. This validation is both too lenient and too restrictive as
it does not reject known-bad patterns while also restricting
pseudo-revisions which would be perfectly fine.
Refactor the RPC to use `ValidateRevision()` as our new single source of
truth for revision validation. Note that we do not allow path-scoped
revisions here given that tags cannot ever live at a path anyway.
|
|
The `ListCommits()` RPC has an open-coded version of revision
validation. This validation is both too lenient and too restrictive as
it does not reject known-bad patterns while also restricting
pseudo-revisions which would be perfectly fine.
Refactor the RPC to use `ValidateRevision()` as our new single source of
truth for revision validation. Note that we do not allow path-scoped
revisions here given that commits cannot ever live at a path anyway.
|
|
The `ListLFSPointers()` RPC has an open-coded version of revision
validation. This validation is both too lenient and too restrictive as
it does not reject known-bad patterns while also restricting
pseudo-revisions which would be perfectly fine.
Refactor the RPC to use `ValidateRevision()` as our new single source of
truth for revision validation.
|
|
The `ListBlobs()` RPC has an open-coded version of revision validation.
This validation is both too lenient and too restrictive as it does not
reject known-bad patterns while also restricting pseudo-revisions which
would be perfectly fine.
Refactor the RPC to use `ValidateRevision()` as our new single source of
truth for revision validation.
|
|
As a safety guard against command-line options injection we disallow
passing any revisions that have a dash as leading character. There are
valid usecases where a caller may want to pass these though, namely in
the form of pseudo-revisions like `--all`, `--not` or `--glob=`.
Add a new option `AllowPseudoRevisions()` that puts known
pseudo-revisions onto an allowed-list.
|
|
The `ValidateRevision()` helper function rejects all revisions that
contain a colon. This also restricts this function from being used with
path-scoped revisions like `HEAD:README.md`, which require the caller to
use a colon.
Introduce a new option `AllowPathScopedRevision()` that lets callers
accept these kinds of revisions.
|
|
We're about to extend `ValidateRevision()` so that it can optionally
handle pseudo-revisions like `--all` and `--not` as well as path-scoped
revisions. As not all existing callers will want to make use of these we
need to allow callers to opt-in to this new behaviour.
Merge `ValidateRevision()` as well as `ValidateRevisionAllowEmpty()`
into a single function that can be passed an option. As a first example,
we also introduce the `AllowEmptyRevision()` option that gets passed
from all callers which were calling `ValidateRevisionAllowEmpty()`.
|
|
|
|
Implement restores in backup.Repository
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5844
Merged-by: Toon Claes <toon@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Co-authored-by: James Fargher <jfargher@gitlab.com>
|
|
praefect: Migrate track-repositories sub-command
Closes #5001
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5834
Merged-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Sami Hiltunen <shiltunen@gitlab.com>
Approved-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Sami Hiltunen <shiltunen@gitlab.com>
Co-authored-by: Pavlo Strokov <pstrokov@gitlab.com>
|
|
|
