Skip to content
Snippets Groups Projects
Unverified Commit 9fd7dfd5 authored by Will Chandler's avatar Will Chandler
Browse files

repository: No storage check for fork source repo

With 79e94764 (repository: Refactor repository validation to verify
existence, 2023-06-12) we started checking that the `SourceRepository`
storage name passed to `CreateFork` is valid and exists on the receiving
Gitaly node. Previously we confirmed this value was not empty, but
did not actually check that storage existed in the config or was present
on disk.

This field is unusual in that Praefect intentionally does not rewrite it
when proxying, which caused fork requests using the new validation
checks to fail with:

  validating source repository: storage name not found

Use the newly added `WithSkipStorageExistenceCheck` option to elide this
check.
parent d88d2059
No related merge requests found
...@@ -15,7 +15,7 @@ import ( ...@@ -15,7 +15,7 @@ import (
func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest) (*gitalypb.CreateForkResponse, error) { func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest) (*gitalypb.CreateForkResponse, error) {
// We don't validate existence of the source repository given that we may connect to a different Gitaly host in // We don't validate existence of the source repository given that we may connect to a different Gitaly host in
// order to fetch from it. So it may or may not exist locally. // order to fetch from it. So it may or may not exist locally.
if err := s.locator.ValidateRepository(req.GetSourceRepository(), storage.WithSkipRepositoryExistenceCheck()); err != nil { if err := s.locator.ValidateRepository(req.GetSourceRepository(), storage.WithSkipStorageExistenceCheck()); err != nil {
return nil, structerr.NewInvalidArgument("validating source repository: %w", err) return nil, structerr.NewInvalidArgument("validating source repository: %w", err)
} }
......
...@@ -261,6 +261,10 @@ func TestCreateFork_validate(t *testing.T) { ...@@ -261,6 +261,10 @@ func TestCreateFork_validate(t *testing.T) {
cfg, cli := setupRepositoryServiceWithoutRepo(t) cfg, cli := setupRepositoryServiceWithoutRepo(t)
repo, _ := gittest.CreateRepository(t, ctx, cfg) repo, _ := gittest.CreateRepository(t, ctx, cfg)
// Praefect does not rewrite the SourceRepository storage name, confirm
// we accept a storage name unknown to Gitaly.
repo.StorageName = "RailsStorageName"
for _, tc := range []struct { for _, tc := range []struct {
desc string desc string
req *gitalypb.CreateForkRequest req *gitalypb.CreateForkRequest
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment