From 752cbf7c882a24e5066b8653536e6d8e5078839f Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Mon, 16 Mar 2026 14:40:53 +0100 Subject: [PATCH 1/4] Add tests for currentCommit Give currentCommit a dir parameter so the logic can be tested with arbitrary directories. --- internal/cmd/cli/apply.go | 10 ++++--- internal/cmd/cli/apply_test.go | 49 ++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/internal/cmd/cli/apply.go b/internal/cmd/cli/apply.go index 35d3530f2c..7742acd571 100644 --- a/internal/cmd/cli/apply.go +++ b/internal/cmd/cli/apply.go @@ -99,7 +99,7 @@ func (a *Apply) Run(cmd *cobra.Command, args []string) error { func (a *Apply) run(cmd *cobra.Command, args []string) error { labels := a.Label if a.Commit == "" { - a.Commit = currentCommit() + a.Commit = currentCommit(".") } if a.Commit != "" { if labels == nil { @@ -243,12 +243,14 @@ func (a *Apply) addAuthToOpts(opts *apply.Options, readFile readFile, helmBasicH return nil } -func currentCommit() string { +// currentCommit returns the HEAD commit SHA of the git repository +// containing dir, or "" if dir is not inside a git repository. +func currentCommit(dir string) string { cmd := exec.Command("git", "rev-parse", "HEAD") //nolint:noctx // TODO: refactor to use go-git's ResolveRevision + cmd.Dir = dir buf := &bytes.Buffer{} cmd.Stdout = buf - err := cmd.Run() - if err == nil { + if err := cmd.Run(); err == nil { return strings.TrimSpace(buf.String()) } return "" diff --git a/internal/cmd/cli/apply_test.go b/internal/cmd/cli/apply_test.go index 7977213afa..64023b4bd9 100644 --- a/internal/cmd/cli/apply_test.go +++ b/internal/cmd/cli/apply_test.go @@ -3,8 +3,13 @@ package cli import ( "errors" "os" + "path/filepath" + "regexp" "testing" + "time" + gogit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" "github.com/google/go-cmp/cmp" "sigs.k8s.io/yaml" @@ -226,3 +231,47 @@ func mockReadFile(name string) ([]byte, error) { return nil, errorNotFound } + +func TestCurrentCommit(t *testing.T) { + t.Run("returns empty string for non-git directory", func(t *testing.T) { + dir := t.TempDir() + got := currentCommit(dir) + if got != "" { + t.Errorf("expected empty string for non-git dir, got %q", got) + } + }) + + t.Run("returns HEAD commit SHA for git repository", func(t *testing.T) { + dir := t.TempDir() + + repo, err := gogit.PlainInit(dir, false) + if err != nil { + t.Fatalf("PlainInit: %v", err) + } + wt, err := repo.Worktree() + if err != nil { + t.Fatalf("Worktree: %v", err) + } + + if err := os.WriteFile(filepath.Join(dir, "README"), []byte("test"), 0600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + if _, err = wt.Add("README"); err != nil { + t.Fatalf("Add: %v", err) + } + expected, err := wt.Commit("initial commit", &gogit.CommitOptions{ + Author: &object.Signature{Name: "test", Email: "test@example.com", When: time.Now()}, + }) + if err != nil { + t.Fatalf("Commit: %v", err) + } + + got := currentCommit(dir) + if got != expected.String() { + t.Errorf("expected SHA %q, got %q", expected.String(), got) + } + if matched, _ := regexp.MatchString(`^[0-9a-f]{40}$`, got); !matched { + t.Errorf("expected a 40-char hex SHA, got %q", got) + } + }) +} From b6c1204984e716217ff04d2cdb1ce49730a55e47 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Mon, 16 Mar 2026 14:41:22 +0100 Subject: [PATCH 2/4] Replace git shell-out with go-git in currentCommit currentCommit called exec.Command("git", "rev-parse", "HEAD") to read the HEAD SHA of the working directory. Replace with go-git's PlainOpenWithOptions (with DetectDotGit to walk up to the .git root) and ResolveRevision, removing the only remaining os/exec call in production code. --- internal/cmd/cli/apply.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/internal/cmd/cli/apply.go b/internal/cmd/cli/apply.go index 7742acd571..0cf847bc49 100644 --- a/internal/cmd/cli/apply.go +++ b/internal/cmd/cli/apply.go @@ -4,9 +4,10 @@ import ( "bytes" "fmt" "os" - "os/exec" "strings" + gogit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" "github.com/sirupsen/logrus" "github.com/spf13/cobra" ctrl "sigs.k8s.io/controller-runtime" @@ -246,14 +247,15 @@ func (a *Apply) addAuthToOpts(opts *apply.Options, readFile readFile, helmBasicH // currentCommit returns the HEAD commit SHA of the git repository // containing dir, or "" if dir is not inside a git repository. func currentCommit(dir string) string { - cmd := exec.Command("git", "rev-parse", "HEAD") //nolint:noctx // TODO: refactor to use go-git's ResolveRevision - cmd.Dir = dir - buf := &bytes.Buffer{} - cmd.Stdout = buf - if err := cmd.Run(); err == nil { - return strings.TrimSpace(buf.String()) + repo, err := gogit.PlainOpenWithOptions(dir, &gogit.PlainOpenOptions{DetectDotGit: true}) + if err != nil { + return "" + } + hash, err := repo.ResolveRevision(plumbing.Revision("HEAD")) + if err != nil { + return "" } - return "" + return hash.String() } // writeTmpKnownHosts creates a temporary file and writes known_hosts data to it, if such data is available from From 183cff9803899ea00eab200c9950e414c86f3987 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Mon, 16 Mar 2026 15:11:26 +0100 Subject: [PATCH 3/4] Test currentCommit from nested subdirectory DetectDotGit walks parent directories to locate the .git root, matching the shell-out behavior. Add a subtest that calls currentCommit from a nested path inside the repo to confirm this. --- internal/cmd/cli/apply_test.go | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/internal/cmd/cli/apply_test.go b/internal/cmd/cli/apply_test.go index 64023b4bd9..f6eab52448 100644 --- a/internal/cmd/cli/apply_test.go +++ b/internal/cmd/cli/apply_test.go @@ -274,4 +274,43 @@ func TestCurrentCommit(t *testing.T) { t.Errorf("expected a 40-char hex SHA, got %q", got) } }) + + t.Run("returns HEAD commit SHA from nested subdirectory", func(t *testing.T) { + dir := t.TempDir() + + repo, err := gogit.PlainInit(dir, false) + if err != nil { + t.Fatalf("PlainInit: %v", err) + } + wt, err := repo.Worktree() + if err != nil { + t.Fatalf("Worktree: %v", err) + } + + if err := os.WriteFile(filepath.Join(dir, "README"), []byte("test"), 0600); err != nil { + t.Fatalf("WriteFile: %v", err) + } + if _, err = wt.Add("README"); err != nil { + t.Fatalf("Add: %v", err) + } + expected, err := wt.Commit("initial commit", &gogit.CommitOptions{ + Author: &object.Signature{Name: "test", Email: "test@example.com", When: time.Now()}, + }) + if err != nil { + t.Fatalf("Commit: %v", err) + } + + subdir := filepath.Join(dir, "some", "nested") + if err := os.MkdirAll(subdir, 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + + got := currentCommit(subdir) + if got != expected.String() { + t.Errorf("expected SHA %q from nested subdir, got %q", expected.String(), got) + } + if matched, _ := regexp.MatchString(`^[0-9a-f]{40}$`, got); !matched { + t.Errorf("expected a 40-char hex SHA from nested subdir, got %q", got) + } + }) } From 9611c9ee2530a68c67ec05cd28b9195678066d97 Mon Sep 17 00:00:00 2001 From: Tim Hardeck Date: Mon, 16 Mar 2026 15:20:30 +0100 Subject: [PATCH 4/4] Fix commit label dropped when no other labels set When a.Label is nil and a commit is detected, labels is reassigned to a new map but opts.Labels was set to a.Label (still nil). Use labels instead so the commit label is included in both the nil and non-nil a.Label cases. --- internal/cmd/cli/apply.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/cmd/cli/apply.go b/internal/cmd/cli/apply.go index 0cf847bc49..f03ff58f6f 100644 --- a/internal/cmd/cli/apply.go +++ b/internal/cmd/cli/apply.go @@ -116,7 +116,7 @@ func (a *Apply) run(cmd *cobra.Command, args []string) error { Output: writer.NewDefaultNone(a.Output), Compress: a.Compress, ServiceAccount: a.ServiceAccount, - Labels: a.Label, + Labels: labels, TargetsFile: a.TargetsFile, TargetNamespace: a.TargetNamespace, Paused: a.Paused,