diff --git a/source/git/source.go b/source/git/source.go index 67a3b13e1afc..9fda2076f76f 100644 --- a/source/git/source.go +++ b/source/git/source.go @@ -206,11 +206,12 @@ func (gs *gitSource) mountRemote(ctx context.Context, remote string, authArgs [] type gitSourceHandler struct { *gitSource - src GitIdentifier - cacheKey string - sha256 bool - sm *session.Manager - authArgs []string + src GitIdentifier + cacheKey string + cacheCommit string + sha256 bool + sm *session.Manager + authArgs []string } func (gs *gitSourceHandler) shaToCacheKey(sha, ref string) string { @@ -379,6 +380,7 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index if gitutil.IsCommitSHA(gs.src.Ref) { cacheKey := gs.shaToCacheKey(gs.src.Ref, gs.src.Ref) gs.cacheKey = cacheKey + gs.cacheCommit = gs.src.Ref gs.sha256 = len(gs.src.Ref) == 64 // gs.src.Checksum is verified when checking out the commit return cacheKey, gs.src.Ref, nil, true, nil @@ -467,6 +469,7 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index cacheKey := gs.shaToCacheKey(shaForCacheKey, usedRef) gs.cacheKey = cacheKey gs.sha256 = len(sha) == 64 + gs.cacheCommit = sha return cacheKey, sha, nil, true, nil } @@ -548,6 +551,14 @@ func (gs *gitSourceHandler) trySnapshot(ctx context.Context, g session.Group, re } } + // local refs are needed so they would be advertised on next fetches. Force is used + // in case the ref is a branch and it now points to a different commit sha + // TODO: is there a better way to do this? + targetRef := ref + if !strings.HasPrefix(ref, "refs/tags/") { + targetRef = "tags/" + ref + } + if doFetch { // make sure no old lock files have leaked os.RemoveAll(filepath.Join(gitDir, "shallow.lock")) @@ -565,13 +576,6 @@ func (gs *gitSourceHandler) trySnapshot(ctx context.Context, g session.Group, re if gitutil.IsCommitSHA(ref) { args = append(args, ref) } else { - // local refs are needed so they would be advertised on next fetches. Force is used - // in case the ref is a branch and it now points to a different commit sha - // TODO: is there a better way to do this? - targetRef := ref - if !strings.HasPrefix(ref, "refs/tags/") { - targetRef = "tags/" + ref - } args = append(args, "--force", ref+":"+targetRef) } if _, err := git.Run(ctx, args...); err != nil { @@ -589,9 +593,49 @@ func (gs *gitSourceHandler) trySnapshot(ctx context.Context, g session.Group, re return nil, err } - _, err = git.Run(ctx, "reflog", "expire", "--all", "--expire=now") + + // verify that commit matches the cache key commit + dt, err := git.Run(ctx, "rev-parse", ref) if err != nil { - return nil, errors.Wrapf(err, "failed to expire reflog for remote %s", urlutil.RedactCredentials(gs.src.Remote)) + return nil, err + } + // if fetched ref does not match cache key, the remote side has changed the ref + // if possible we can try to force the commit that the cache key points to, otherwise we need to error + if strings.TrimSpace(string(dt)) != gs.cacheCommit { + uptRef := targetRef + if !strings.HasPrefix(uptRef, "refs/") { + uptRef = "refs/" + uptRef + } + // check if the commit still exists in the repo + if _, err := git.Run(ctx, "cat-file", "-e", gs.cacheCommit); err == nil { + // force the ref to point to the commit that the cache key points to + if _, err := git.Run(ctx, "update-ref", uptRef, gs.cacheCommit, "--no-deref"); err != nil { + return nil, err + } + } else { + // try to fetch the commit directly + args := []string{"fetch", "--tags"} + if _, err := os.Lstat(filepath.Join(gitDir, "shallow")); err == nil { + args = append(args, "--unshallow") + } + args = append(args, "origin", gs.cacheCommit) + if _, err := git.Run(ctx, args...); err != nil { + return nil, errors.Wrapf(err, "failed to fetch remote %s", urlutil.RedactCredentials(gs.src.Remote)) + } + + _, err = git.Run(ctx, "reflog", "expire", "--all", "--expire=now") + if err != nil { + return nil, errors.Wrapf(err, "failed to expire reflog for remote %s", urlutil.RedactCredentials(gs.src.Remote)) + } + if _, err := git.Run(ctx, "cat-file", "-e", gs.cacheCommit); err == nil { + // force the ref to point to the commit that the cache key points to + if _, err := git.Run(ctx, "update-ref", uptRef, gs.cacheCommit, "--no-deref"); err != nil { + return nil, err + } + } else { + return nil, errors.Errorf("fetched ref %s does not match expected commit %s and commit can not be found in the repository", ref, gs.cacheCommit) + } + } } } diff --git a/source/git/source_test.go b/source/git/source_test.go index e7b29954af7e..2f368e114cd3 100644 --- a/source/git/source_test.go +++ b/source/git/source_test.go @@ -831,6 +831,294 @@ func testFetchAnnotatedTagChecksums(t *testing.T, format string, keepGitDir bool } } +func TestFetchTagChangeRaceSHA1(t *testing.T) { + testFetchTagChangeRace(t, "sha1", false) +} + +func TestFetchTagChangeRaceKeepGitDirSHA1(t *testing.T) { + testFetchTagChangeRace(t, "sha1", true) +} + +func TestFetchTagChangeRaceSHA256(t *testing.T) { + testFetchTagChangeRace(t, "sha256", false) +} + +func TestFetchTagChangeRaceKeepGitDirSHA256(t *testing.T) { + testFetchTagChangeRace(t, "sha256", true) +} + +// testFetchTagChangeRace tests case where tag is change in between cache key and snapshot calls. +func testFetchTagChangeRace(t *testing.T, format string, keepGitDir bool) { + ctx := t.Context() + if runtime.GOOS == "windows" { + t.Skip("Depends on unimplemented containerd bind-mount support on Windows") + } + repo := setupGitRepo(t, format) + cmd := exec.Command("git", "rev-parse", "v1.2.3") + cmd.Dir = repo.mainPath + + out, err := cmd.Output() + require.NoError(t, err) + + expLen := 40 + if format == "sha256" { + expLen = 64 + } + shaTag := strings.TrimSpace(string(out)) + require.Equal(t, expLen, len(shaTag)) + + cmd = exec.Command("git", "rev-parse", "v1.2.3^{}") + cmd.Dir = repo.mainPath + + out, err = cmd.Output() + require.NoError(t, err) + shaTagCommit := strings.TrimSpace(string(out)) + require.Equal(t, expLen, len(shaTagCommit)) + + id := &GitIdentifier{Remote: repo.mainURL, Ref: "v1.2.3", KeepGitDir: keepGitDir} + gs := setupGitSource(t, t.TempDir()) + + g, err := gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + key1, pin1, _, done, err := g.CacheKey(ctx, nil, 0) + require.NoError(t, err) + require.True(t, done) + + require.Equal(t, shaTag, pin1) + if keepGitDir { + require.Equal(t, shaTag+".git#refs/tags/v1.2.3", key1) + } else { + require.Equal(t, shaTagCommit, key1) + } + + cmd = exec.Command("git", "rev-parse", "v1.2.3-special") + cmd.Dir = repo.mainPath + out, err = cmd.Output() + require.NoError(t, err) + shaTagNew := strings.TrimSpace(string(out)) + require.NotEqual(t, shaTag, shaTagNew) + + // delete tag v1.2.3 + cmd = exec.Command("git", "tag", "-d", "v1.2.3") + cmd.Dir = repo.mainPath + out, err = cmd.CombinedOutput() + require.NoError(t, err, string(out)) + + // recreate tag v1.2.3 to point to another commit + cmd = exec.Command("git", "tag", "v1.2.3", shaTagNew) + cmd.Dir = repo.mainPath + out, err = cmd.CombinedOutput() + require.NoError(t, err, string(out)) + + ref, err := g.Snapshot(ctx, nil) + require.NoError(t, err) + ref.Release(context.TODO()) + + mount, err := ref.Mount(ctx, true, nil) + require.NoError(t, err) + + lm := snapshot.LocalMounter(mount) + dir, err := lm.Mount() + require.NoError(t, err) + defer lm.Unmount() + + // bar only exists in the new commit + _, err = os.ReadFile(filepath.Join(dir, "bar")) + require.Error(t, err) + require.True(t, os.IsNotExist(err)) + + if keepGitDir { + // read current HEAD commit + cmd = exec.Command("git", "rev-parse", "HEAD") + cmd.Dir = dir + + out, err = cmd.Output() + require.NoError(t, err) + shaHead := strings.TrimSpace(string(out)) + require.Equal(t, shaTagCommit, shaHead) + } +} + +func TestFetchBranchChangeRaceSHA1(t *testing.T) { + testFetchBranchChangeRace(t, "sha1", false) +} + +func TestFetchBranchChangeRaceKeepGitDirSHA1(t *testing.T) { + testFetchBranchChangeRace(t, "sha1", true) +} + +func TestFetchBranchChangeRaceSHA256(t *testing.T) { + testFetchBranchChangeRace(t, "sha256", false) +} + +func TestFetchBranchChangeRaceKeepGitDirSHA256(t *testing.T) { + testFetchBranchChangeRace(t, "sha256", true) +} + +func testFetchBranchChangeRace(t *testing.T, format string, keepGitDir bool) { + ctx := t.Context() + if runtime.GOOS == "windows" { + t.Skip("Depends on unimplemented containerd bind-mount support on Windows") + } + repo := setupGitRepo(t, format) + cmd := exec.Command("git", "rev-parse", "master") + cmd.Dir = repo.mainPath + + out, err := cmd.Output() + require.NoError(t, err) + + expLen := 40 + if format == "sha256" { + expLen = 64 + } + shaMaster := strings.TrimSpace(string(out)) + require.Equal(t, expLen, len(shaMaster)) + + id := &GitIdentifier{Remote: repo.mainURL, KeepGitDir: keepGitDir} + gs := setupGitSource(t, t.TempDir()) + + g, err := gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + key1, pin1, _, done, err := g.CacheKey(ctx, nil, 0) + require.NoError(t, err) + require.True(t, done) + + require.Equal(t, shaMaster, pin1) + if keepGitDir { + require.Equal(t, shaMaster+".git#refs/heads/master", key1) + } else { + require.Equal(t, shaMaster, key1) + } + + cmd = exec.Command("git", "rev-parse", "feature") + cmd.Dir = repo.mainPath + out, err = cmd.Output() + require.NoError(t, err) + shaFeature := strings.TrimSpace(string(out)) + require.NotEqual(t, shaMaster, shaFeature) + + // checkout feature as master + cmd = exec.Command("git", "checkout", "-B", "master", "feature") + cmd.Dir = repo.mainPath + out, err = cmd.CombinedOutput() + require.NoError(t, err, string(out)) + + ref, err := g.Snapshot(ctx, nil) + require.NoError(t, err) + ref.Release(context.TODO()) + + mount, err := ref.Mount(ctx, true, nil) + require.NoError(t, err) + + lm := snapshot.LocalMounter(mount) + dir, err := lm.Mount() + require.NoError(t, err) + defer lm.Unmount() + + // ghi only exists in the feature branch + _, err = os.ReadFile(filepath.Join(dir, "ghi")) + require.Error(t, err) + require.True(t, os.IsNotExist(err)) + + if keepGitDir { + // read current HEAD commit + cmd = exec.Command("git", "rev-parse", "HEAD") + cmd.Dir = dir + + out, err = cmd.Output() + require.NoError(t, err) + shaHead := strings.TrimSpace(string(out)) + require.Equal(t, shaMaster, shaHead) + } +} + +func TestFetchBranchRemoveRaceSHA1(t *testing.T) { + testFetchBranchRemoveRace(t, "sha1", false) +} + +func TestFetchBranchRemoveRaceKeepGitDirSHA1(t *testing.T) { + testFetchBranchRemoveRace(t, "sha1", true) +} + +func TestFetchBranchRemoveRaceSHA256(t *testing.T) { + testFetchBranchRemoveRace(t, "sha256", false) +} + +func TestFetchBranchRemoveRaceKeepGitDirSHA256(t *testing.T) { + testFetchBranchRemoveRace(t, "sha256", true) +} + +func testFetchBranchRemoveRace(t *testing.T, format string, keepGitDir bool) { + ctx := t.Context() + if runtime.GOOS == "windows" { + t.Skip("Depends on unimplemented containerd bind-mount support on Windows") + } + repo := setupGitRepo(t, format) + cmd := exec.Command("git", "rev-parse", "feature") + cmd.Dir = repo.mainPath + + out, err := cmd.Output() + require.NoError(t, err) + + expLen := 40 + if format == "sha256" { + expLen = 64 + } + shaFeature := strings.TrimSpace(string(out)) + require.Equal(t, expLen, len(shaFeature)) + + id := &GitIdentifier{Remote: repo.mainURL, Ref: "feature", KeepGitDir: keepGitDir} + gs := setupGitSource(t, t.TempDir()) + + g, err := gs.Resolve(ctx, id, nil, nil) + require.NoError(t, err) + + key1, pin1, _, done, err := g.CacheKey(ctx, nil, 0) + require.NoError(t, err) + require.True(t, done) + + require.Equal(t, shaFeature, pin1) + if keepGitDir { + require.Equal(t, shaFeature+".git#refs/heads/feature", key1) + } else { + require.Equal(t, shaFeature, key1) + } + + cmd = exec.Command("git", "rev-parse", "master") + cmd.Dir = repo.mainPath + out, err = cmd.Output() + require.NoError(t, err) + shaMaster := strings.TrimSpace(string(out)) + require.NotEqual(t, shaMaster, shaFeature) + + // change feature to point to master + cmd = exec.Command("git", "branch", "-f", "feature", "master") + cmd.Dir = repo.mainPath + out, err = cmd.CombinedOutput() + require.NoError(t, err, string(out)) + + cmd = exec.Command("git", "reflog", "expire", "--expire-unreachable=now", "--expire=now", "--all") + cmd.Dir = repo.mainPath + out, err = cmd.CombinedOutput() + require.NoError(t, err, string(out)) + + cmd = exec.Command("git", "gc", "--prune=now", "--aggressive") + cmd.Dir = repo.mainPath + out, err = cmd.CombinedOutput() + require.NoError(t, err, string(out)) + + cmd = exec.Command("git", "cat-file", "-t", shaFeature) + cmd.Dir = repo.mainPath + out, err = cmd.CombinedOutput() + require.Error(t, err, string(out)) + + _, err = g.Snapshot(ctx, nil) + require.Error(t, err) + require.ErrorContains(t, err, "fetched ref feature does not match expected commit "+shaFeature) +} + func TestFetchMutatedTagSHA1(t *testing.T) { testFetchMutatedTag(t, "sha1", false) }