remote: Accept remotes with other refs ending in HEAD

José M. Requena Plensrequested to merge
gitlab-community/gitlab-org/gitaly:jmrp-find-remote-repository-head-lines into master

What does this MR do?

FindRemoteRepository now reads every line of git ls-remote <remote> HEAD and reports the remote as existing when the line for HEAD itself carries an object ID of a known length. Until now it required the whole output to be that single line.

Why

git ls-remote matches its pattern against the tail of each reference name, so asking for HEAD also lists every reference whose last component is HEAD. The repository from the issue answers like this today:

$ git ls-remote https://github.com/cee-dubb/test-head.git HEAD
001da7c96918d5fcd7fcc8ae2bcfe4fec441b77a	HEAD
001da7c96918d5fcd7fcc8ae2bcfe4fec441b77a	refs/remotes/origin/HEAD

bytes.Cut on the first tab leaves everything after it as the reference name, which is not HEAD\n, so the RPC answers exists: false. Rails calls this RPC through Gitlab::GitalyClient::RemoteService.exists? from Import::ValidateRemoteGitEndpointService, which validates the URL of the import-by-URL page, of project creation and POST /projects/:id/import/git when they are given an import_url (and of PUT /projects/:id on EE), and of pull mirror settings behind the validate_pull_mirror_url feature flag, which is off by default. Those repositories are therefore refused as not being valid Git repositories, although they are public and valid. The GitHub importer uses the same call to decide whether to import a wiki.

The RPC has required a single line since it was added in 7cb29b42, and be3e1464 kept that when it added SHA256 support.

I did not use bytes.Contains as the issue suggests: a remote whose HEAD is unborn lists no line for HEAD, and it would then be accepted because of a reference such as refs/remotes/origin/HEAD. The behavior for that case does not change, and a test pins it.

The MR has two commits: the first moves the parsing into remoteHeadExists() without changing it and adds a table test of what it accepts today, and the second is the fix.

Closes gitlab#596393 (closed)

How was this tested?

  • TestFindRemoteRepository gains two cases against a real HTTP remote served by gittest.HTTPServer, both with refs/heads/feature/HEAD and refs/remotes/origin/HEAD: one where they sit next to a born HEAD (expects exists: true), and one where HEAD is unborn and they are the only references (expects exists: false).
  • TestRemoteHeadExists covers SHA1 and SHA256 object IDs, HEAD before and after other references ending in HEAD, only such references, an object ID of unknown length and empty output.
  • With the new tests run against the code of the first commit, they fail as expected: other references ending in HEAD returns exists: false, and the two table cases with a HEAD line next to other references return false. The unborn case passes on both commits.
  • With the check changed to the bytes.Contains the issue suggests, the unborn case and the table case with only other references ending in HEAD fail, because both are then accepted.
  • Through the RPC itself against the three repositories from the issue: before the fix, cee-dubb/test-head and lttng/lttng-tools answer exists: false and lttng/lttng-ust answers exists: true; with the fix, all three answer exists: true.
  • With the fix, TEST_PACKAGES=./internal/gitaly/service/remote passes under make test-go, make test-with-sha256 and make test-with-reftable (57 tests each), with Go 1.26 and Go 1.27. make format leaves both files unchanged, and make lint restricted to the package reports no issues.
Failures with the new tests against the code of the first commit 
=== FAIL: internal/gitaly/service/remote TestRemoteHeadExists/HEAD_after_other_references_ending_in_HEAD
        expected: true
        actual  : false
=== FAIL: internal/gitaly/service/remote TestRemoteHeadExists/HEAD_followed_by_other_references_ending_in_HEAD
        expected: true
        actual  : false
=== FAIL: internal/gitaly/service/remote TestFindRemoteRepository/other_references_ending_in_HEAD
        Protobufs not equal
        - "exists": bool(true),
What the test HTTP remote lists for HEAD

A throwaway test printed git ls-remote <url> HEAD against the two remotes the new RPC cases set up:

born:
549090fbeacc6607bc70648d3ba554c355e670c5	HEAD
549090fbeacc6607bc70648d3ba554c355e670c5	refs/heads/feature/HEAD
549090fbeacc6607bc70648d3ba554c355e670c5	refs/remotes/origin/HEAD

unborn:
549090fbeacc6607bc70648d3ba554c355e670c5	refs/heads/feature/HEAD
549090fbeacc6607bc70648d3ba554c355e670c5	refs/remotes/origin/HEAD

Merge request reports