diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-12-20 14:50:29 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2022-12-20 15:56:42 +0300 |
commit | 84775c1340395ab4b933131e620504ec5b0e8bff (patch) | |
tree | f73a88c079091f209e975fde8abbd75645a583b2 | |
parent | 64f30e48224306308d9b00b706d7961985cdecc8 (diff) |
git/objectpool: Remove remote configuration after creation
When creating object pools we do so by cloning the primary object pool
member. As git-clone(1) creates remote configuration the gitconfig of
the pool repository will afterwards contain the `remote.origin` remote.
We have dropped storing remote configuration in repositories a long time
ago though and clean up any such config entries during housekeeping.
Explicitly remove the remote after we have created the object pool.
Changelog: fixed
-rw-r--r-- | internal/git/objectpool/create.go | 16 | ||||
-rw-r--r-- | internal/git/objectpool/create_test.go | 5 | ||||
-rw-r--r-- | internal/git/objectpool/link_test.go | 4 | ||||
-rw-r--r-- | internal/gitaly/service/objectpool/fetch_into_object_pool_test.go | 7 |
4 files changed, 20 insertions, 12 deletions
diff --git a/internal/git/objectpool/create.go b/internal/git/objectpool/create.go index b523f8124..eb96458d9 100644 --- a/internal/git/objectpool/create.go +++ b/internal/git/objectpool/create.go @@ -77,5 +77,21 @@ func Create( return nil, err } + // git-clone(1) writes the remote configuration into the object pool. We nowadays don't have + // remote configuration in the gitconfig anymore, so let's remove it before returning. Note + // that we explicitly don't use git-remote(1) to do this, as this command would also remove + // references part of the remote. + if err := objectPool.ExecAndWait(ctx, git.Command{ + Name: "config", + Flags: []git.Option{ + git.Flag{Name: "--remove-section"}, + }, + Args: []string{ + "remote.origin", + }, + }); err != nil { + return nil, fmt.Errorf("removing origin remote config: %w", err) + } + return objectPool, nil } diff --git a/internal/git/objectpool/create_test.go b/internal/git/objectpool/create_test.go index b5c86b40b..0241b360b 100644 --- a/internal/git/objectpool/create_test.go +++ b/internal/git/objectpool/create_test.go @@ -18,7 +18,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v15/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v15/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v15/internal/structerr" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v15/internal/testhelper/testcfg" @@ -70,8 +69,8 @@ func TestCreate(t *testing.T) { // There should not be a "hooks" directory in the pool. require.NoDirExists(t, filepath.Join(poolPath, "hooks")) - // The "origin" remote of the pool points to the pool member. - require.Equal(t, repoPath, text.ChompBytes(gittest.Exec(t, cfg, "-C", poolPath, "remote", "get-url", "origin"))) + // The repository has no remote. + require.Empty(t, gittest.Exec(t, cfg, "-C", poolPath, "remote")) // The "master" branch points to the same commit as in the pool member. require.Equal(t, commitID, gittest.ResolveRevision(t, cfg, poolPath, "refs/heads/master")) // Objects exist in the pool repository. diff --git a/internal/git/objectpool/link_test.go b/internal/git/objectpool/link_test.go index 12afdf559..5a297ba70 100644 --- a/internal/git/objectpool/link_test.go +++ b/internal/git/objectpool/link_test.go @@ -41,7 +41,7 @@ func TestLink(t *testing.T) { newContent := testhelper.MustReadFile(t, altPath) require.Equal(t, content, newContent) - require.Equal(t, []byte("origin\n"), gittest.Exec(t, cfg, "-C", gittest.RepositoryPath(t, pool), "remote")) + require.Empty(t, gittest.Exec(t, cfg, "-C", gittest.RepositoryPath(t, pool), "remote")) } func TestLink_transactional(t *testing.T) { @@ -130,5 +130,5 @@ func TestLink_absoluteLinkExists(t *testing.T) { repoObjectsPath := filepath.Join(repoPath, "objects") require.Equal(t, poolObjectsPath, filepath.Join(repoObjectsPath, string(content)), "the content of the alternates file should be the relative version of the absolute pat") - require.Equal(t, []byte("origin\n"), gittest.Exec(t, cfg, "-C", poolPath, "remote")) + require.Empty(t, gittest.Exec(t, cfg, "-C", poolPath, "remote")) } diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go index 748db9899..ba9863dcf 100644 --- a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go +++ b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go @@ -129,13 +129,6 @@ func testFetchIntoObjectPooltransactional(t *testing.T, ctx context.Context) { poolProto, pool, poolPath := createObjectPool(t, ctx, cfg, client, repo) - // CreateObjectPool has a bug because it will leave the configuration of the origin remote - // in the gitconfig. This will get cleaned up at a later point by our housekeeping logic, so - // it doesn't hurt much in the first place to have it. But the cleanup logic would trigger - // another transactional vote which we want to avoid, so we simply unset the configuration - // here. - gittest.Exec(t, cfg, "-C", poolPath, "config", "--unset", "remote.origin.url") - // Inject transaction information so that FetchInotObjectPool knows to perform // transactional voting. ctx, err = txinfo.InjectTransaction(peer.NewContext(ctx, &peer.Peer{}), 1, "node", true) |