From faa604606f6a99d83e046c06883d841d80f55d54 Mon Sep 17 00:00:00 2001 From: Kyounghoon Jang Date: Tue, 2 Sep 2025 13:33:42 +0000 Subject: [PATCH 1/2] feat(git-clone): add support for submodules Signed-off-by: Kyounghoon Jang --- pkg/controller/git/mock_repo.go | 5 ++ pkg/controller/git/work_tree.go | 9 +++ pkg/promotion/runner/builtin/git_cloner.go | 6 ++ .../runner/builtin/git_cloner_test.go | 71 +++++++++++++++++++ .../builtin/schemas/git-clone-config.json | 4 ++ .../runner/builtin/zz_config_types.go | 2 + ui/src/gen/directives/git-clone-config.json | 4 ++ 7 files changed, 101 insertions(+) diff --git a/pkg/controller/git/mock_repo.go b/pkg/controller/git/mock_repo.go index 957b6dc583..40277fe399 100644 --- a/pkg/controller/git/mock_repo.go +++ b/pkg/controller/git/mock_repo.go @@ -30,6 +30,7 @@ type MockRepo struct { RemoteBranchExistsFn func(branch string) (bool, error) ResetHardFn func() error URLFn func() string + UpdateSubmodulesFn func() error } func (m *MockRepo) AddAll() error { @@ -146,3 +147,7 @@ func (m *MockRepo) ResetHard() error { func (m *MockRepo) URL() string { return m.URLFn() } + +func (m *MockRepo) UpdateSubmodules() error { + return m.UpdateSubmodulesFn() +} diff --git a/pkg/controller/git/work_tree.go b/pkg/controller/git/work_tree.go index 2f2db41cef..fe58fc4e31 100644 --- a/pkg/controller/git/work_tree.go +++ b/pkg/controller/git/work_tree.go @@ -87,6 +87,8 @@ type WorkTree interface { ResetHard() error // URL returns the remote URL of the repository. URL() string + // UpdateSubmodules updates the submodules in the working tree. + UpdateSubmodules() error } // workTree is an implementation of the WorkTree interface for interacting with @@ -634,3 +636,10 @@ func (w *workTree) ResetHard() error { } return nil } + +func (w *workTree) UpdateSubmodules() error { + if _, err := libExec.Exec(w.buildGitCommand("submodule", "update", "--init", "--recursive")); err != nil { + return fmt.Errorf("error updating submodules: %w", err) + } + return nil +} \ No newline at end of file diff --git a/pkg/promotion/runner/builtin/git_cloner.go b/pkg/promotion/runner/builtin/git_cloner.go index 650c8c1e4f..ebad346077 100644 --- a/pkg/promotion/runner/builtin/git_cloner.go +++ b/pkg/promotion/runner/builtin/git_cloner.go @@ -186,6 +186,12 @@ func (g *gitCloner) run( "error adding work tree %s to repo %s: %w", checkout.Path, cfg.RepoURL, err, ) + } + if cfg.RecurseSubmodules{ + if err := worktree.UpdateSubmodules(); err != nil { + return promotion.StepResult{Status: kargoapi.PromotionStepStatusErrored}, + fmt.Errorf("error updating submodules for worktree at %s: %w", path, err) + } } key := checkout.Path if checkout.As != "" { diff --git a/pkg/promotion/runner/builtin/git_cloner_test.go b/pkg/promotion/runner/builtin/git_cloner_test.go index 9d7f44d337..970b089f30 100644 --- a/pkg/promotion/runner/builtin/git_cloner_test.go +++ b/pkg/promotion/runner/builtin/git_cloner_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http/httptest" "os" + "os/exec" "path/filepath" "testing" @@ -393,3 +394,73 @@ func Test_gitCloner_run(t *testing.T) { res.Output["commits"], ) } + +func Test_gitCloner_run_with_submodules(t *testing.T) { + // Set up a test Git server in-process + service := gitkit.New( + gitkit.Config{ + Dir: t.TempDir(), + AutoCreate: true, + }, + ) + require.NoError(t, service.Setup()) + server := httptest.NewServer(service) + defer server.Close() + + // Create submodule remote repo and push a file + subRepoURL := fmt.Sprintf("%s/sub.git", server.URL) + subRepo, err := git.Clone(subRepoURL, nil, nil) + require.NoError(t, err) + defer subRepo.Close() + err = os.WriteFile(filepath.Join(subRepo.Dir(), "sub.txt"), []byte("sub"), 0600) + require.NoError(t, err) + err = subRepo.AddAllAndCommit("Initial commit sub", nil) + require.NoError(t, err) + err = subRepo.Push(nil) + require.NoError(t, err) + + // Create main repo and add the submodule + mainRepoURL := fmt.Sprintf("%s/main.git", server.URL) + mainRepo, err := git.Clone(mainRepoURL, nil, nil) + require.NoError(t, err) + defer mainRepo.Close() + + // Use git submodule add to create proper submodule metadata + cmd := exec.Command("git", "submodule", "add", subRepoURL, "sub") + cmd.Dir = mainRepo.Dir() + out, err := cmd.CombinedOutput() + require.NoErrorf(t, err, "git submodule add failed: %s", string(out)) + + // Commit and push the submodule addition + err = mainRepo.AddAllAndCommit("Add submodule", nil) + require.NoError(t, err) + err = mainRepo.Push(nil) + require.NoError(t, err) + + mainCommitID, err := mainRepo.LastCommitID() + require.NoError(t, err) + + // Run git-cloner with recurseSubmodules = true + r := newGitCloner(&credentials.FakeDB{}) + runner, ok := r.(*gitCloner) + require.True(t, ok) + + stepCtx := &promotion.StepContext{ + WorkDir: t.TempDir(), + } + + res, err := runner.run( + context.Background(), + stepCtx, + builtin.GitCloneConfig{ + RepoURL: mainRepoURL, + Checkout: []builtin.Checkout{{Commit: mainCommitID, Path: "src"}}, + RecurseSubmodules: true, + }, + ) + require.NoError(t, err) + require.Equal(t, kargoapi.PromotionStepStatusSucceeded, res.Status) + + // Assert submodule file was populated inside worktree + require.FileExists(t, filepath.Join(stepCtx.WorkDir, "src", "sub", "sub.txt")) +} \ No newline at end of file diff --git a/pkg/promotion/runner/builtin/schemas/git-clone-config.json b/pkg/promotion/runner/builtin/schemas/git-clone-config.json index 4715441059..3479bf49af 100644 --- a/pkg/promotion/runner/builtin/schemas/git-clone-config.json +++ b/pkg/promotion/runner/builtin/schemas/git-clone-config.json @@ -9,6 +9,10 @@ "type": "boolean", "description": "Indicates whether to skip TLS verification when cloning the repository. Default is false." }, + "recurseSubmodules": { + "type": "boolean", + "description": "Indicates whether to recursively clone submodules. Default is false." + }, "repoURL": { "type": "string", "description": "The URL of a remote Git repository to clone. Required.", diff --git a/pkg/x/promotion/runner/builtin/zz_config_types.go b/pkg/x/promotion/runner/builtin/zz_config_types.go index 1c196e0980..b4008d134a 100644 --- a/pkg/x/promotion/runner/builtin/zz_config_types.go +++ b/pkg/x/promotion/runner/builtin/zz_config_types.go @@ -114,6 +114,8 @@ type GitCloneConfig struct { Checkout []Checkout `json:"checkout"` // Indicates whether to skip TLS verification when cloning the repository. Default is false. InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"` + // Indicates whether to recursively clone submodules. Default is false. + RecurseSubmodules bool `json:"recurseSubmodules,omitempty"` // The URL of a remote Git repository to clone. Required. RepoURL string `json:"repoURL"` } diff --git a/ui/src/gen/directives/git-clone-config.json b/ui/src/gen/directives/git-clone-config.json index 108aa48ce1..51e34dd491 100644 --- a/ui/src/gen/directives/git-clone-config.json +++ b/ui/src/gen/directives/git-clone-config.json @@ -8,6 +8,10 @@ "type": "boolean", "description": "Indicates whether to skip TLS verification when cloning the repository. Default is false." }, + "recurseSubmodules": { + "type": "boolean", + "description": "Indicates whether to recursively clone submodules. Default is false." + }, "repoURL": { "type": "string", "description": "The URL of a remote Git repository to clone. Required.", From e2a4cbe275ecf19ffa40be4ea5d9414336740412 Mon Sep 17 00:00:00 2001 From: Kyounghoon Jang Date: Tue, 2 Sep 2025 14:14:50 +0000 Subject: [PATCH 2/2] feat(git-clone): add support for submodules Signed-off-by: Kyounghoon Jang --- pkg/controller/git/work_tree.go | 2 +- pkg/promotion/runner/builtin/git_cloner.go | 13 +- .../runner/builtin/git_cloner_test.go | 120 +++++++++--------- .../builtin/schemas/git-clone-config.json | 2 +- .../runner/builtin/zz_config_types.go | 3 +- ui/src/gen/directives/git-clone-config.json | 2 +- 6 files changed, 73 insertions(+), 69 deletions(-) diff --git a/pkg/controller/git/work_tree.go b/pkg/controller/git/work_tree.go index fe58fc4e31..697f95b7d8 100644 --- a/pkg/controller/git/work_tree.go +++ b/pkg/controller/git/work_tree.go @@ -642,4 +642,4 @@ func (w *workTree) UpdateSubmodules() error { return fmt.Errorf("error updating submodules: %w", err) } return nil -} \ No newline at end of file +} diff --git a/pkg/promotion/runner/builtin/git_cloner.go b/pkg/promotion/runner/builtin/git_cloner.go index ebad346077..ad5ca5f017 100644 --- a/pkg/promotion/runner/builtin/git_cloner.go +++ b/pkg/promotion/runner/builtin/git_cloner.go @@ -186,12 +186,13 @@ func (g *gitCloner) run( "error adding work tree %s to repo %s: %w", checkout.Path, cfg.RepoURL, err, ) - } - if cfg.RecurseSubmodules{ - if err := worktree.UpdateSubmodules(); err != nil { - return promotion.StepResult{Status: kargoapi.PromotionStepStatusErrored}, - fmt.Errorf("error updating submodules for worktree at %s: %w", path, err) - } + } + if cfg.RecurseSubmodules { + err = worktree.UpdateSubmodules() + if err != nil { + return promotion.StepResult{Status: kargoapi.PromotionStepStatusErrored}, + fmt.Errorf("error updating submodules for worktree at %s: %w", path, err) + } } key := checkout.Path if checkout.As != "" { diff --git a/pkg/promotion/runner/builtin/git_cloner_test.go b/pkg/promotion/runner/builtin/git_cloner_test.go index 970b089f30..bc231eed28 100644 --- a/pkg/promotion/runner/builtin/git_cloner_test.go +++ b/pkg/promotion/runner/builtin/git_cloner_test.go @@ -396,71 +396,73 @@ func Test_gitCloner_run(t *testing.T) { } func Test_gitCloner_run_with_submodules(t *testing.T) { - // Set up a test Git server in-process - service := gitkit.New( - gitkit.Config{ - Dir: t.TempDir(), - AutoCreate: true, - }, - ) - require.NoError(t, service.Setup()) - server := httptest.NewServer(service) - defer server.Close() + // Set up a test Git server in-process + service := gitkit.New( + gitkit.Config{ + Dir: t.TempDir(), + AutoCreate: true, + }, + ) + require.NoError(t, service.Setup()) + server := httptest.NewServer(service) + defer server.Close() - // Create submodule remote repo and push a file - subRepoURL := fmt.Sprintf("%s/sub.git", server.URL) - subRepo, err := git.Clone(subRepoURL, nil, nil) - require.NoError(t, err) - defer subRepo.Close() - err = os.WriteFile(filepath.Join(subRepo.Dir(), "sub.txt"), []byte("sub"), 0600) - require.NoError(t, err) - err = subRepo.AddAllAndCommit("Initial commit sub", nil) - require.NoError(t, err) - err = subRepo.Push(nil) - require.NoError(t, err) + // Create submodule remote repo and push a file + subRepoURL := fmt.Sprintf("%s/sub.git", server.URL) + subRepo, err := git.Clone(subRepoURL, nil, nil) + require.NoError(t, err) + defer subRepo.Close() + err = os.WriteFile(filepath.Join(subRepo.Dir(), "sub.txt"), []byte("sub"), 0600) + require.NoError(t, err) + err = subRepo.AddAllAndCommit("Initial commit sub", nil) + require.NoError(t, err) + err = subRepo.Push(nil) + require.NoError(t, err) - // Create main repo and add the submodule - mainRepoURL := fmt.Sprintf("%s/main.git", server.URL) - mainRepo, err := git.Clone(mainRepoURL, nil, nil) - require.NoError(t, err) - defer mainRepo.Close() + // Create main repo and add the submodule + mainRepoURL := fmt.Sprintf("%s/main.git", server.URL) + mainRepo, err := git.Clone(mainRepoURL, nil, nil) + require.NoError(t, err) + defer mainRepo.Close() - // Use git submodule add to create proper submodule metadata - cmd := exec.Command("git", "submodule", "add", subRepoURL, "sub") - cmd.Dir = mainRepo.Dir() - out, err := cmd.CombinedOutput() - require.NoErrorf(t, err, "git submodule add failed: %s", string(out)) + // Use git submodule add to create proper submodule metadata + cmd := exec.Command("git", "submodule", "add", subRepoURL, "sub") + cmd.Dir = mainRepo.Dir() + out, err := cmd.CombinedOutput() + require.NoErrorf(t, err, "git submodule add failed: %s", string(out)) - // Commit and push the submodule addition - err = mainRepo.AddAllAndCommit("Add submodule", nil) - require.NoError(t, err) - err = mainRepo.Push(nil) - require.NoError(t, err) + // Commit and push the submodule addition + err = mainRepo.AddAllAndCommit("Add submodule", nil) + require.NoError(t, err) + err = mainRepo.Push(nil) + require.NoError(t, err) - mainCommitID, err := mainRepo.LastCommitID() - require.NoError(t, err) + mainCommitID, err := mainRepo.LastCommitID() + require.NoError(t, err) - // Run git-cloner with recurseSubmodules = true - r := newGitCloner(&credentials.FakeDB{}) - runner, ok := r.(*gitCloner) - require.True(t, ok) + // Run git-cloner with recurseSubmodules = true + r := newGitCloner(promotion.StepRunnerCapabilities{ + CredsDB: &credentials.FakeDB{}, + }) + runner, ok := r.(*gitCloner) + require.True(t, ok) - stepCtx := &promotion.StepContext{ - WorkDir: t.TempDir(), - } + stepCtx := &promotion.StepContext{ + WorkDir: t.TempDir(), + } - res, err := runner.run( - context.Background(), - stepCtx, - builtin.GitCloneConfig{ - RepoURL: mainRepoURL, - Checkout: []builtin.Checkout{{Commit: mainCommitID, Path: "src"}}, - RecurseSubmodules: true, - }, - ) - require.NoError(t, err) - require.Equal(t, kargoapi.PromotionStepStatusSucceeded, res.Status) + res, err := runner.run( + context.Background(), + stepCtx, + builtin.GitCloneConfig{ + RepoURL: mainRepoURL, + Checkout: []builtin.Checkout{{Commit: mainCommitID, Path: "src"}}, + RecurseSubmodules: true, + }, + ) + require.NoError(t, err) + require.Equal(t, kargoapi.PromotionStepStatusSucceeded, res.Status) - // Assert submodule file was populated inside worktree - require.FileExists(t, filepath.Join(stepCtx.WorkDir, "src", "sub", "sub.txt")) -} \ No newline at end of file + // Assert submodule file was populated inside worktree + require.FileExists(t, filepath.Join(stepCtx.WorkDir, "src", "sub", "sub.txt")) +} diff --git a/pkg/promotion/runner/builtin/schemas/git-clone-config.json b/pkg/promotion/runner/builtin/schemas/git-clone-config.json index 3479bf49af..7dc1424751 100644 --- a/pkg/promotion/runner/builtin/schemas/git-clone-config.json +++ b/pkg/promotion/runner/builtin/schemas/git-clone-config.json @@ -11,7 +11,7 @@ }, "recurseSubmodules": { "type": "boolean", - "description": "Indicates whether to recursively clone submodules. Default is false." + "description": "Indicates whether to recursively clone submodules. Default is false. Note that any provided credentials must also be valid for the submodules." }, "repoURL": { "type": "string", diff --git a/pkg/x/promotion/runner/builtin/zz_config_types.go b/pkg/x/promotion/runner/builtin/zz_config_types.go index b4008d134a..3f35960be5 100644 --- a/pkg/x/promotion/runner/builtin/zz_config_types.go +++ b/pkg/x/promotion/runner/builtin/zz_config_types.go @@ -114,7 +114,8 @@ type GitCloneConfig struct { Checkout []Checkout `json:"checkout"` // Indicates whether to skip TLS verification when cloning the repository. Default is false. InsecureSkipTLSVerify bool `json:"insecureSkipTLSVerify,omitempty"` - // Indicates whether to recursively clone submodules. Default is false. + // Indicates whether to recursively clone submodules. Default is false. Note that any + // provided credentials must also be valid for the submodules. RecurseSubmodules bool `json:"recurseSubmodules,omitempty"` // The URL of a remote Git repository to clone. Required. RepoURL string `json:"repoURL"` diff --git a/ui/src/gen/directives/git-clone-config.json b/ui/src/gen/directives/git-clone-config.json index 51e34dd491..968faabc73 100644 --- a/ui/src/gen/directives/git-clone-config.json +++ b/ui/src/gen/directives/git-clone-config.json @@ -10,7 +10,7 @@ }, "recurseSubmodules": { "type": "boolean", - "description": "Indicates whether to recursively clone submodules. Default is false." + "description": "Indicates whether to recursively clone submodules. Default is false. Note that any provided credentials must also be valid for the submodules." }, "repoURL": { "type": "string",