diff options
author | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-12-03 16:23:59 +0300 |
---|---|---|
committer | Patrick Steinhardt <psteinhardt@gitlab.com> | 2020-12-03 16:28:43 +0300 |
commit | 26327c3f0ca02c6f4cadf85975cff8bfbbfa8c0e (patch) | |
tree | 2ed0f2e8f42596d74baf71bee13e52a127240bf6 | |
parent | 86e4cde9aa91833d79e8d28b36946fc6198e4c4d (diff) |
repository: Remove test relying on broken behaviour
In 750b5b44c (Internal connections to itself should use token,
2020-12-02), we fixed a bug which caused us to not pass the
authentication token when connecting to the internal socket. While the
fix is perfectly sensible, unfortunately the test that was added relies
on broken behaviour.
When executing `CloneFromPoolInternal`, it will call out to
`FetchInternalRemote` which in turn executes git-fetch(1) to perform the
fetch and some other RPC calls to get information about the remote's
state. To do this, we have two tokens at play: first the credentials of
ourselves which we need to connect to our own internal socket. And
second the credentials of the remote repository. In case of the test,
remote and internal socket are in fact the same. This info is accessed
once via the config and once via context-injected Gitaly server
information.
In the test setup, we set up the server with invalid authentication
token via the config, but inject other information into the context.
The expectation now is that the test will fail with the RPCs failing.
Which is weird, as in fact we should already fail with the git-fetch(1)
as it uses gitaly-ssh as transport and thus connects to the internal
socket with invalid credentials. But it happens to work right now
because of another bug: git-fetch(1) uses both a custom environment and
`git.WithRefTxHook()`, where the latter will override the custom
environment. Thus it accidentally uses correct credentials.
This test is thus going to stop working as soon as we fix this bug. As I
wasn't able to come up with a test setup which demonstrates the same
failure, this commit just removes the test.
-rw-r--r-- | internal/gitaly/service/repository/clone_from_pool_internal_test.go | 47 |
1 files changed, 0 insertions, 47 deletions
diff --git a/internal/gitaly/service/repository/clone_from_pool_internal_test.go b/internal/gitaly/service/repository/clone_from_pool_internal_test.go index f3f14d58b..fe76c21d5 100644 --- a/internal/gitaly/service/repository/clone_from_pool_internal_test.go +++ b/internal/gitaly/service/repository/clone_from_pool_internal_test.go @@ -90,53 +90,6 @@ func TestCloneFromPoolInternal(t *testing.T) { testhelper.MustRunCommand(t, nil, "git", "-C", forkRepoPath, "show-ref", "--verify", fmt.Sprintf("refs/heads/%s", newBranch)) } -func TestCloneFromPoolInternal_bad_token(t *testing.T) { - defer func(old string) { config.Config.Auth.Token = old }(config.Config.Auth.Token) - config.Config.Auth.Token = "invalid" - - serverSocketPath, clean := runFullServer(t, config.NewLocator(config.Config)) - defer clean() - - ctxOuter, cancel := testhelper.Context() - defer cancel() - - md := testhelper.GitalyServersMetadata(t, serverSocketPath) - ctx := metadata.NewOutgoingContext(ctxOuter, md) - - client, conn := repository.NewRepositoryClient(t, serverSocketPath) - defer conn.Close() - - testRepo, testRepoPath, cleanupFn := testhelper.InitBareRepo(t) - defer cleanupFn() - - pool, poolRepo := NewTestObjectPool(t) - defer pool.Remove(ctx) - - require.NoError(t, pool.Create(ctx, testRepo)) - require.NoError(t, pool.Link(ctx, testRepo)) - - fullRepack(t, testRepoPath) - - forkedRepo, _, forkRepoCleanup := getForkDestination(t) - defer forkRepoCleanup() - - req := &gitalypb.CloneFromPoolInternalRequest{ - Repository: forkedRepo, - SourceRepository: testRepo, - Pool: &gitalypb.ObjectPool{ - Repository: poolRepo, - }, - } - - _, err := client.CloneFromPoolInternal(ctx, req) - require.Error(t, err) - require.Equal(t, - err.Error(), - "rpc error: code = Internal desc = fetch internal remote: "+ - "rpc error: code = Internal desc = FetchInternalRemote: remote default branch: getRemoteDefaultBranch: "+ - "rpc error: code = PermissionDenied desc = permission denied") -} - // fullRepack does a full repack on the repository, which means if it has a pool repository linked, it will get rid of redundant objects that are reachable in the pool func fullRepack(t *testing.T, repoPath string) { testhelper.MustRunCommand(t, nil, "git", "-C", repoPath, "repack", "-A", "-l", "-d") |