Skip to content

Commit a8bbe79

Browse files
author
Jon McLean
committed
feat(reposerver): enable a multi-source application manifest to have same repoURL with difference revision
1 parent ae03d8f commit a8bbe79

File tree

8 files changed

+607
-30
lines changed

8 files changed

+607
-30
lines changed

reposerver/repository/repository.go

Lines changed: 126 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -766,6 +766,77 @@ type repoRef struct {
766766
commitSHA string
767767
// key is the name of the key which was used to reference this repo.
768768
key string
769+
// worktreePath is the path to a git worktree if this ref required a separate checkout.
770+
// Empty if using the main repository checkout.
771+
worktreePath string
772+
}
773+
774+
// worktreeCleanup holds references needed to clean up a git worktree.
775+
type worktreeCleanup struct {
776+
client git.Client
777+
worktreePath string
778+
repoURL string
779+
commitSHA string
780+
gitRepoPaths utilio.TempPaths
781+
}
782+
783+
// cleanup removes the worktree and cleans up the path registration.
784+
func (w *worktreeCleanup) cleanup() {
785+
if err := w.client.RemoveWorktree(w.worktreePath); err != nil {
786+
log.Warnf("Failed to remove worktree at %s: %v", w.worktreePath, err)
787+
}
788+
// Remove from gitRepoPaths
789+
w.gitRepoPaths.Remove(w.repoURL + "@" + w.commitSHA)
790+
}
791+
792+
// createAndRegisterWorktree creates a git worktree at a temporary path for the given revision,
793+
// registers it in gitRepoPaths, and returns the path along with a cleanup function.
794+
// The caller should defer the cleanup function.
795+
func (s *Service) createAndRegisterWorktree(
796+
gitClient git.Client,
797+
normalizedRepoURL string,
798+
commitSHA string,
799+
) (string, *worktreeCleanup, error) {
800+
worktreePath := filepath.Join(os.TempDir(), "argocd-worktree-"+uuid.New().String())
801+
802+
if err := gitClient.CreateWorktree(commitSHA, worktreePath); err != nil {
803+
return "", nil, fmt.Errorf("failed to create worktree at revision %s: %w", commitSHA, err)
804+
}
805+
806+
// Register the worktree path so getResolvedRefValueFile can find it
807+
s.gitRepoPaths.Add(normalizedRepoURL+"@"+commitSHA, worktreePath)
808+
809+
cleanup := &worktreeCleanup{
810+
client: gitClient,
811+
worktreePath: worktreePath,
812+
repoURL: normalizedRepoURL,
813+
commitSHA: commitSHA,
814+
gitRepoPaths: s.gitRepoPaths,
815+
}
816+
817+
return worktreePath, cleanup, nil
818+
}
819+
820+
// checkWorktreeSymlinks checks for out-of-bounds symlinks in a worktree path.
821+
func (s *Service) checkWorktreeSymlinks(worktreePath string, repo v1alpha1.Repository, revision string) error {
822+
if s.initConstants.AllowOutOfBoundsSymlinks {
823+
return nil
824+
}
825+
err := apppathutil.CheckOutOfBoundsSymlinks(worktreePath)
826+
if err != nil {
827+
oobError := &apppathutil.OutOfBoundsSymlinkError{}
828+
if errors.As(err, &oobError) {
829+
log.WithFields(log.Fields{
830+
common.SecurityField: common.SecurityHigh,
831+
"repo": repo,
832+
"revision": revision,
833+
"file": oobError.File,
834+
}).Warn("repository worktree contains out-of-bounds symlink")
835+
return fmt.Errorf("repository contains out-of-bounds symlinks. file: %s", oobError.File)
836+
}
837+
return err
838+
}
839+
return nil
769840
}
770841

771842
func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA, cacheKey string, opContextSrc operationContextSrc, q *apiclient.ManifestRequest, ch *generateManifestCh) {
@@ -816,24 +887,47 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
816887
return
817888
}
818889
normalizedRepoURL := git.NormalizeGitURL(refSourceMapping.Repo.Repo)
819-
closer, ok := repoRefs[normalizedRepoURL]
820-
if ok {
821-
if closer.revision != refSourceMapping.TargetRevision {
822-
ch.errCh <- fmt.Errorf("cannot reference multiple revisions for the same repository (%s references %q while %s references %q)", refVar, refSourceMapping.TargetRevision, closer.key, closer.revision)
823-
return
824-
}
825-
} else {
826-
gitClient, referencedCommitSHA, err := s.newClientResolveRevision(&refSourceMapping.Repo, refSourceMapping.TargetRevision, git.WithCache(s.cache, !q.NoRevisionCache && !q.NoCache))
890+
existingRef, ok := repoRefs[normalizedRepoURL]
891+
if ok && existingRef.revision == refSourceMapping.TargetRevision {
892+
// Same repo and same revision already referenced - reuse existing checkout
893+
continue
894+
}
895+
896+
// Resolve the git client and commit SHA for this ref source
897+
gitClient, referencedCommitSHA, err := s.newClientResolveRevision(&refSourceMapping.Repo, refSourceMapping.TargetRevision, git.WithCache(s.cache, !q.NoRevisionCache && !q.NoCache))
898+
if err != nil {
899+
ch.errCh <- fmt.Errorf("failed to get git client for repo %s: %w", refSourceMapping.Repo.Repo, err)
900+
return
901+
}
902+
903+
// Determine if we need a worktree (same repo at different revision)
904+
needsWorktree := ok || // Already have a ref to this repo at a different revision
905+
(git.NormalizeGitURL(q.ApplicationSource.RepoURL) == normalizedRepoURL && commitSHA != referencedCommitSHA) // Same as primary source at different revision
906+
907+
if needsWorktree {
908+
// Create a worktree for this different revision
909+
worktreePath, cleanup, err := s.createAndRegisterWorktree(gitClient, normalizedRepoURL, referencedCommitSHA)
827910
if err != nil {
828-
log.Errorf("Failed to get git client for repo %s: %v", refSourceMapping.Repo.Repo, err)
829-
ch.errCh <- fmt.Errorf("failed to get git client for repo %s", refSourceMapping.Repo.Repo)
911+
ch.errCh <- fmt.Errorf("failed to create worktree for repo %s: %w", refSourceMapping.Repo.Repo, err)
830912
return
831913
}
914+
defer cleanup.cleanup()
832915

833-
if git.NormalizeGitURL(q.ApplicationSource.RepoURL) == normalizedRepoURL && commitSHA != referencedCommitSHA {
834-
ch.errCh <- fmt.Errorf("cannot reference a different revision of the same repository (%s references %q which resolves to %q while the application references %q which resolves to %q)", refVar, refSourceMapping.TargetRevision, referencedCommitSHA, q.Revision, commitSHA)
916+
// Symlink check for the worktree
917+
if err := s.checkWorktreeSymlinks(worktreePath, refSourceMapping.Repo, refSourceMapping.TargetRevision); err != nil {
918+
ch.errCh <- err
835919
return
836920
}
921+
922+
// Use a unique key that includes the commit SHA for worktree refs
923+
repoRefs[normalizedRepoURL+"@"+referencedCommitSHA] = repoRef{
924+
revision: refSourceMapping.TargetRevision,
925+
commitSHA: referencedCommitSHA,
926+
key: refVar,
927+
worktreePath: worktreePath,
928+
}
929+
} else {
930+
// Different repo - use normal checkout flow
837931
closer, err := s.repoLock.Lock(gitClient.Root(), referencedCommitSHA, true, func() (goio.Closer, error) {
838932
return s.checkoutRevision(gitClient, referencedCommitSHA, s.initConstants.SubmoduleEnabled, q.Repo.Depth)
839933
})
@@ -850,23 +944,9 @@ func (s *Service) runManifestGenAsync(ctx context.Context, repoRoot, commitSHA,
850944
}(closer)
851945

852946
// Symlink check must happen after acquiring lock.
853-
if !s.initConstants.AllowOutOfBoundsSymlinks {
854-
err := apppathutil.CheckOutOfBoundsSymlinks(gitClient.Root())
855-
if err != nil {
856-
oobError := &apppathutil.OutOfBoundsSymlinkError{}
857-
if errors.As(err, &oobError) {
858-
log.WithFields(log.Fields{
859-
common.SecurityField: common.SecurityHigh,
860-
"repo": refSourceMapping.Repo,
861-
"revision": refSourceMapping.TargetRevision,
862-
"file": oobError.File,
863-
}).Warn("repository contains out-of-bounds symlink")
864-
ch.errCh <- fmt.Errorf("repository contains out-of-bounds symlinks. file: %s", oobError.File)
865-
return
866-
}
867-
ch.errCh <- err
868-
return
869-
}
947+
if err := s.checkWorktreeSymlinks(gitClient.Root(), refSourceMapping.Repo, refSourceMapping.TargetRevision); err != nil {
948+
ch.errCh <- err
949+
return
870950
}
871951

872952
repoRefs[normalizedRepoURL] = repoRef{revision: refSourceMapping.TargetRevision, commitSHA: referencedCommitSHA, key: refVar}
@@ -1414,7 +1494,23 @@ func getResolvedRefValueFile(
14141494
gitRepoPaths utilio.TempPaths,
14151495
) (pathutil.ResolvedFilePath, error) {
14161496
pathStrings := strings.Split(rawValueFile, "/")
1417-
repoPath := gitRepoPaths.GetPathIfExists(git.NormalizeGitURL(refSourceRepo))
1497+
normalizedRepoURL := git.NormalizeGitURL(refSourceRepo)
1498+
1499+
// First, try the standard repo path (for same-revision or non-worktree cases)
1500+
repoPath := gitRepoPaths.GetPathIfExists(normalizedRepoURL)
1501+
1502+
// If not found, check for worktree paths (keyed as "repoURL@commitSHA")
1503+
if repoPath == "" {
1504+
allPaths := gitRepoPaths.GetPaths()
1505+
for key, path := range allPaths {
1506+
// Check if this key starts with our repo URL followed by "@" (worktree pattern)
1507+
if strings.HasPrefix(key, normalizedRepoURL+"@") {
1508+
repoPath = path
1509+
break
1510+
}
1511+
}
1512+
}
1513+
14181514
if repoPath == "" {
14191515
return "", fmt.Errorf("failed to find repo %q", refSourceRepo)
14201516
}

reposerver/repository/repository_test.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4604,3 +4604,127 @@ func TestGenerateManifest_OCISourceSkipsGitClient(t *testing.T) {
46044604
// verify that newGitClient was never invoked
46054605
assert.False(t, gitCalled, "GenerateManifest should not invoke Git for OCI sources")
46064606
}
4607+
4608+
func TestGenerateManifest_MultiSource_SameRepoWithDifferentRevisions(t *testing.T) {
4609+
// This test validates that multi-source applications can reference the same
4610+
// git repository with different target revisions by using git worktrees.
4611+
root, err := filepath.Abs("../../util/helm/testdata")
4612+
require.NoError(t, err)
4613+
4614+
primarySHA := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
4615+
refSHA := "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
4616+
4617+
paths := &iomocks.TempPaths{}
4618+
helmClient := &helmmocks.Client{}
4619+
ociClient := &ocimocks.Client{}
4620+
4621+
// Create two separate git clients - one for primary, one for ref source
4622+
primaryGitClient := &gitmocks.Client{}
4623+
refGitClient := &gitmocks.Client{}
4624+
4625+
// Primary source checkout behavior
4626+
primaryGitClient.EXPECT().Init().Return(nil)
4627+
primaryGitClient.EXPECT().IsRevisionPresent(mock.Anything).Return(false)
4628+
primaryGitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Return(nil)
4629+
primaryGitClient.EXPECT().Checkout(mock.Anything, mock.Anything).Return("", nil)
4630+
primaryGitClient.EXPECT().LsRemote("main").Return(primarySHA, nil)
4631+
primaryGitClient.EXPECT().CommitSHA().Return(primarySHA, nil)
4632+
primaryGitClient.EXPECT().Root().Return(root)
4633+
primaryGitClient.EXPECT().IsAnnotatedTag(mock.Anything).Return(false)
4634+
primaryGitClient.EXPECT().VerifyCommitSignature(mock.Anything).Return("", nil)
4635+
4636+
// Ref source behavior - uses worktree since different revision
4637+
refGitClient.EXPECT().Init().Return(nil)
4638+
refGitClient.EXPECT().IsRevisionPresent(mock.Anything).Return(false)
4639+
refGitClient.EXPECT().Fetch(mock.Anything, mock.Anything).Return(nil)
4640+
refGitClient.EXPECT().LsRemote("v1.0.0").Return(refSHA, nil)
4641+
refGitClient.EXPECT().CommitSHA().Return(refSHA, nil)
4642+
refGitClient.EXPECT().CreateWorktree(refSHA, mock.AnythingOfType("string")).Return(nil)
4643+
refGitClient.EXPECT().RemoveWorktree(mock.AnythingOfType("string")).Return(nil).Maybe()
4644+
refGitClient.EXPECT().Root().Return(root)
4645+
4646+
// Track which client to return
4647+
callCount := 0
4648+
4649+
// Helm client setup
4650+
helmClient.EXPECT().GetIndex(mock.AnythingOfType("bool"), mock.Anything).Return(&helm.Index{Entries: map[string]helm.Entries{
4651+
"redis": {{Version: "1.0.0"}},
4652+
}}, nil)
4653+
helmClient.EXPECT().GetTags(mock.Anything, mock.Anything).Return(nil, nil)
4654+
4655+
// OCI client setup
4656+
ociClient.EXPECT().GetTags(mock.Anything, mock.Anything).Return(nil, nil)
4657+
ociClient.EXPECT().ResolveRevision(mock.Anything, mock.Anything, mock.Anything).Return("", nil)
4658+
4659+
// Paths mock - Add is called for both primary and worktree
4660+
paths.EXPECT().Add(mock.Anything, mock.Anything).Return()
4661+
paths.EXPECT().GetPath(mock.Anything).Return(root, nil)
4662+
paths.EXPECT().GetPathIfExists(mock.Anything).Return(root)
4663+
paths.EXPECT().GetPaths().Return(map[string]string{"fake-nonce": root})
4664+
paths.EXPECT().Remove(mock.Anything).Return()
4665+
4666+
cacheMocks := newCacheMocks()
4667+
t.Cleanup(cacheMocks.mockCache.StopRedisCallback)
4668+
4669+
service := NewService(metrics.NewMetricsServer(), cacheMocks.cache, RepoServerInitConstants{ParallelismLimit: 1}, &git.NoopCredsStore{}, root)
4670+
service.newGitClient = func(_, _ string, _ git.Creds, _, _ bool, _, _ string, _ ...git.ClientOpts) (git.Client, error) {
4671+
callCount++
4672+
if callCount == 1 {
4673+
return primaryGitClient, nil
4674+
}
4675+
return refGitClient, nil
4676+
}
4677+
service.newHelmClient = func(_ string, _ helm.Creds, _ bool, _, _ string, _ ...helm.ClientOpts) helm.Client {
4678+
return helmClient
4679+
}
4680+
service.newOCIClient = func(_ string, _ oci.Creds, _, _ string, _ []string, _ ...oci.ClientOpts) (oci.Client, error) {
4681+
return ociClient, nil
4682+
}
4683+
service.gitRepoInitializer = func(_ string) goio.Closer {
4684+
return utilio.NopCloser
4685+
}
4686+
service.gitRepoPaths = paths
4687+
4688+
// Create a request with same repo but different target revisions
4689+
req := &apiclient.ManifestRequest{
4690+
Repo: &v1alpha1.Repository{
4691+
Repo: "https://github.com/example/repo.git",
4692+
},
4693+
Revision: "main",
4694+
ApplicationSource: &v1alpha1.ApplicationSource{
4695+
RepoURL: "https://github.com/example/repo.git",
4696+
Path: "./redis",
4697+
TargetRevision: "main",
4698+
Helm: &v1alpha1.ApplicationSourceHelm{
4699+
ValueFiles: []string{"$values/redis/values-production.yaml"},
4700+
},
4701+
},
4702+
HasMultipleSources: true,
4703+
NoCache: true,
4704+
ProjectName: "test-project",
4705+
ProjectSourceRepos: []string{"*"},
4706+
// RefSources with same repo but different revision
4707+
RefSources: map[string]*v1alpha1.RefTarget{
4708+
"$values": {
4709+
TargetRevision: "v1.0.0", // Different revision than primary source
4710+
Repo: v1alpha1.Repository{
4711+
Repo: "https://github.com/example/repo.git", // Same repo as primary
4712+
},
4713+
},
4714+
},
4715+
}
4716+
4717+
// The test should succeed - worktree allows same repo with different revisions
4718+
_, err = service.GenerateManifest(t.Context(), req)
4719+
// We expect no error since worktrees should handle different revisions
4720+
// Note: The actual manifest generation may still fail due to missing files,
4721+
// but the key assertion is that we don't get the old
4722+
// "cannot reference the same repository multiple times when the" error
4723+
if err != nil {
4724+
assert.NotContains(t, err.Error(), "cannot reference the same repository multiple times")
4725+
}
4726+
4727+
// Verify that CreateWorktree was called on refGitClient
4728+
// RemoveWorktree is called via defer, which may happen after test assertions in some timing scenarios
4729+
refGitClient.AssertCalled(t, "CreateWorktree", refSHA, mock.AnythingOfType("string"))
4730+
}

util/git/client.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,11 @@ type Client interface {
145145
RemoveContents(paths []string) (string, error)
146146
// CommitAndPush commits and pushes changes to the target branch.
147147
CommitAndPush(branch, message string) (string, error)
148+
// CreateWorktree creates a new git worktree at the specified path for the given revision.
149+
// This allows checking out multiple revisions of the same repository simultaneously.
150+
CreateWorktree(revision string, path string) error
151+
// RemoveWorktree removes a git worktree at the specified path.
152+
RemoveWorktree(path string) error
148153
}
149154

150155
type EventHandlers struct {
@@ -610,6 +615,31 @@ func (m *nativeGitClient) Checkout(revision string, submoduleEnabled bool) (stri
610615
return "", nil
611616
}
612617

618+
// CreateWorktree creates a new git worktree at the specified path for the given revision.
619+
// This allows checking out multiple revisions of the same repository simultaneously.
620+
// The worktree shares the git object database with the main repository, making it efficient.
621+
func (m *nativeGitClient) CreateWorktree(revision string, path string) error {
622+
ctx := context.Background()
623+
// git worktree add --detach <path> <revision>
624+
// --detach creates the worktree with a detached HEAD at the specified revision
625+
if out, err := m.runCmd(ctx, "worktree", "add", "--detach", path, revision); err != nil {
626+
return fmt.Errorf("failed to create worktree at %s for revision %s: %s, %w", path, revision, out, err)
627+
}
628+
return nil
629+
}
630+
631+
// RemoveWorktree removes a git worktree at the specified path.
632+
// This cleans up both the worktree directory and its administrative files.
633+
func (m *nativeGitClient) RemoveWorktree(path string) error {
634+
ctx := context.Background()
635+
// git worktree remove --force <path>
636+
// --force is needed if the worktree contains untracked or modified files
637+
if out, err := m.runCmd(ctx, "worktree", "remove", "--force", path); err != nil {
638+
return fmt.Errorf("failed to remove worktree at %s: %s, %w", path, out, err)
639+
}
640+
return nil
641+
}
642+
613643
func (m *nativeGitClient) getRefs() ([]*plumbing.Reference, error) {
614644
myLockUUID, err := uuid.NewRandom()
615645
myLockId := ""

0 commit comments

Comments
 (0)