1
0
mirror of https://github.com/moby/buildkit.git synced 2025-08-01 02:04:26 +03:00

git: ensure that pin matches checked-out commit

Previously, it was very possible for the CacheKey function to return a
sha key that was *not* the checked out commit.

There are two cases that I've encountered where this can happen:
- An annotated tag will have the pin of the tag, and not the underlying
  commit, which will be HEAD after the checkout.
- If multiple tags have the same path component (e.g. "mytag" and
  "abc/mytag") then the first alphabetical tag will be selected when (in
  this case "abc/mytag").

To avoid this kind of case, we can't just search for a single match in
the results for ls-remote. There's no way to filter for just an exact
match, so we need to scan through the output ourselves. Additionally, we
need to dereference the annotated tags by also selecting refs ending in
"^{}" - which have the commit that the tag points at.

Finally, I've improved the test suite around this to check that:
- The cache-key pin is equivalent to the checked out commit
- We can check out non-master branches
- That full ref syntax like "refs/heads/<branch-name>" and
  "refs/tags/<tag-name>" (or even "refs/<anything>") can be used.

Signed-off-by: Justin Chadwell <me@jedevc.com>
This commit is contained in:
Justin Chadwell
2023-12-07 16:29:07 +00:00
parent 6997850e16
commit 6a8d2ca2bd
2 changed files with 72 additions and 10 deletions

View File

@ -365,20 +365,45 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
// TODO: should we assume that remote tag is immutable? add a timer?
buf, err := git.Run(ctx, "ls-remote", "origin", ref)
buf, err := git.Run(ctx, "ls-remote", "origin", ref, ref+"^{}")
if err != nil {
return "", "", nil, false, errors.Wrapf(err, "failed to fetch remote %s", urlutil.RedactCredentials(remote))
}
out := string(buf)
idx := strings.Index(out, "\t")
if idx == -1 {
return "", "", nil, false, errors.Errorf("repository does not contain ref %s, output: %q", ref, string(out))
lines := strings.Split(string(buf), "\n")
var (
partialRef = "refs/" + strings.TrimPrefix(ref, "refs/")
headRef = "refs/heads/" + strings.TrimPrefix(ref, "refs/heads/")
tagRef = "refs/tags/" + strings.TrimPrefix(ref, "refs/tags/")
annotatedTagRef = tagRef + "^{}"
)
var sha, headSha, tagSha string
for _, line := range lines {
lineSha, lineRef, _ := strings.Cut(line, "\t")
switch lineRef {
case headRef:
headSha = lineSha
case tagRef, annotatedTagRef:
tagSha = lineSha
case partialRef:
sha = lineSha
}
}
sha := string(out[:idx])
// git-checkout prefers branches in case of ambiguity
if sha == "" {
sha = headSha
}
if sha == "" {
sha = tagSha
}
if sha == "" {
return "", "", nil, false, errors.Errorf("repository does not contain ref %s, output: %q", ref, string(buf))
}
if !isCommitSHA(sha) {
return "", "", nil, false, errors.Errorf("invalid commit sha %q", sha)
}
cacheKey := gs.shaToCacheKey(sha)
gs.cacheKey = cacheKey
return cacheKey, sha, nil, true, nil

View File

@ -225,6 +225,10 @@ func TestFetchByTagKeepGitDir(t *testing.T) {
testFetchByTag(t, "lightweight-tag", "third", false, true, true)
}
func TestFetchByTagFull(t *testing.T) {
testFetchByTag(t, "refs/tags/lightweight-tag", "third", false, true, true)
}
func TestFetchByAnnotatedTag(t *testing.T) {
testFetchByTag(t, "v1.2.3", "second", true, false, false)
}
@ -233,6 +237,34 @@ func TestFetchByAnnotatedTagKeepGitDir(t *testing.T) {
testFetchByTag(t, "v1.2.3", "second", true, false, true)
}
func TestFetchByAnnotatedTagFull(t *testing.T) {
testFetchByTag(t, "refs/tags/v1.2.3", "second", true, false, true)
}
func TestFetchByBranch(t *testing.T) {
testFetchByTag(t, "feature", "withsub", false, true, false)
}
func TestFetchByBranchKeepGitDir(t *testing.T) {
testFetchByTag(t, "feature", "withsub", false, true, true)
}
func TestFetchByBranchFull(t *testing.T) {
testFetchByTag(t, "refs/heads/feature", "withsub", false, true, true)
}
func TestFetchByRef(t *testing.T) {
testFetchByTag(t, "test", "feature", false, true, false)
}
func TestFetchByRefKeepGitDir(t *testing.T) {
testFetchByTag(t, "test", "feature", false, true, true)
}
func TestFetchByRefFull(t *testing.T) {
testFetchByTag(t, "refs/test", "feature", false, true, true)
}
func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotatedTag, hasFoo13File, keepGitDir bool) {
if runtime.GOOS == "windows" {
t.Skip("Depends on unimplemented containerd bind-mount support on Windows")
@ -296,15 +328,18 @@ func testFetchByTag(t *testing.T, tag, expectedCommitSubject string, isAnnotated
gitutil.WithWorkTree(dir),
)
// get current commit sha
headCommit, err := git.Run(ctx, "rev-parse", "HEAD")
require.NoError(t, err)
// ensure that we checked out the same commit as was in the cache key
require.Equal(t, strings.TrimSpace(string(headCommit)), pin1)
if isAnnotatedTag {
// get commit sha that the annotated tag points to
annotatedTagCommit, err := git.Run(ctx, "rev-list", "-n", "1", tag)
require.NoError(t, err)
// get current commit sha
headCommit, err := git.Run(ctx, "rev-parse", "HEAD")
require.NoError(t, err)
// HEAD should match the actual commit sha (and not the sha of the annotated tag,
// since it's not possible to checkout a non-commit object)
require.Equal(t, string(annotatedTagCommit), string(headCommit))
@ -582,6 +617,7 @@ func setupGitRepo(t *testing.T) gitRepoFixture {
"echo foo > abc",
"git add abc",
"git commit -m initial",
"git tag --no-sign a/v1.2.3",
"echo bar > def",
"git add def",
"git commit -m second",
@ -594,6 +630,7 @@ func setupGitRepo(t *testing.T) gitRepoFixture {
"echo baz > ghi",
"git add ghi",
"git commit -m feature",
"git update-ref refs/test $(git rev-parse HEAD)",
"git submodule add "+fixture.subURL+" sub",
"git add -A",
"git commit -m withsub",