From d6f626912f056a0a01d68312b59075ee70adebe5 Mon Sep 17 00:00:00 2001
From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>
Date: Mon, 7 Aug 2023 19:29:02 +0200
Subject: [PATCH] Fix bundle git branch validation (#645)

## Changes
This PR:
1. Fixes the computation logic for `ActualBranch`. An error in the
earlier logic caused the validation mutator to be a no-op.
2. Makes the `.git` string a global var. This is useful to configure in
tests.
3. Adds e2e test for the validation mutator.

## Tests
Unit test
---
 bundle/config/mutator/load_git_details.go     | 19 +++++----
 bundle/tests/autoload_git_test.go             | 20 ----------
 .../git_branch_validation/.mock-git/HEAD      |  1 +
 .../git_branch_validation/databricks.yml      |  4 ++
 bundle/tests/git_test.go                      | 39 +++++++++++++++++++
 libs/git/repository.go                        | 10 +++--
 6 files changed, 61 insertions(+), 32 deletions(-)
 delete mode 100644 bundle/tests/autoload_git_test.go
 create mode 100644 bundle/tests/git_branch_validation/.mock-git/HEAD
 create mode 100644 bundle/tests/git_branch_validation/databricks.yml
 create mode 100644 bundle/tests/git_test.go

diff --git a/bundle/config/mutator/load_git_details.go b/bundle/config/mutator/load_git_details.go
index f22aafe0..ab47677d 100644
--- a/bundle/config/mutator/load_git_details.go
+++ b/bundle/config/mutator/load_git_details.go
@@ -24,17 +24,20 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) error {
 	if err != nil {
 		return err
 	}
-	// load branch name if undefined
-	if b.Config.Bundle.Git.Branch == "" {
-		branch, err := repo.CurrentBranch()
-		if err != nil {
-			log.Warnf(ctx, "failed to load current branch: %s", err)
-		} else {
-			b.Config.Bundle.Git.Branch = branch
-			b.Config.Bundle.Git.ActualBranch = branch
+
+	// Read branch name of current checkout
+	branch, err := repo.CurrentBranch()
+	if err == nil {
+		b.Config.Bundle.Git.ActualBranch = branch
+		if b.Config.Bundle.Git.Branch == "" {
+			// Only load branch if there's no user defined value
 			b.Config.Bundle.Git.Inferred = true
+			b.Config.Bundle.Git.Branch = branch
 		}
+	} else {
+		log.Warnf(ctx, "failed to load current branch: %s", err)
 	}
+
 	// load commit hash if undefined
 	if b.Config.Bundle.Git.Commit == "" {
 		commit, err := repo.LatestCommit()
diff --git a/bundle/tests/autoload_git_test.go b/bundle/tests/autoload_git_test.go
deleted file mode 100644
index a1075198..00000000
--- a/bundle/tests/autoload_git_test.go
+++ /dev/null
@@ -1,20 +0,0 @@
-package config_tests
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestAutoLoad(t *testing.T) {
-	b := load(t, "./autoload_git")
-	assert.True(t, b.Config.Bundle.Git.Inferred)
-	assert.Contains(t, b.Config.Bundle.Git.OriginURL, "/cli")
-}
-
-func TestManuallySetBranch(t *testing.T) {
-	b := loadEnvironment(t, "./autoload_git", "production")
-	assert.False(t, b.Config.Bundle.Git.Inferred)
-	assert.Equal(t, "main", b.Config.Bundle.Git.Branch)
-	assert.Contains(t, b.Config.Bundle.Git.OriginURL, "/cli")
-}
diff --git a/bundle/tests/git_branch_validation/.mock-git/HEAD b/bundle/tests/git_branch_validation/.mock-git/HEAD
new file mode 100644
index 00000000..6c83ec9d
--- /dev/null
+++ b/bundle/tests/git_branch_validation/.mock-git/HEAD
@@ -0,0 +1 @@
+ref: refs/heads/feature-b
diff --git a/bundle/tests/git_branch_validation/databricks.yml b/bundle/tests/git_branch_validation/databricks.yml
new file mode 100644
index 00000000..8c7b96ef
--- /dev/null
+++ b/bundle/tests/git_branch_validation/databricks.yml
@@ -0,0 +1,4 @@
+bundle:
+  name: "Dancing Feet"
+  git:
+    branch: "feature-a"
diff --git a/bundle/tests/git_test.go b/bundle/tests/git_test.go
new file mode 100644
index 00000000..daab4d30
--- /dev/null
+++ b/bundle/tests/git_test.go
@@ -0,0 +1,39 @@
+package config_tests
+
+import (
+	"context"
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config/mutator"
+	"github.com/databricks/cli/libs/git"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGitAutoLoad(t *testing.T) {
+	b := load(t, "./autoload_git")
+	assert.True(t, b.Config.Bundle.Git.Inferred)
+	assert.Contains(t, b.Config.Bundle.Git.OriginURL, "/cli")
+}
+
+func TestGitManuallySetBranch(t *testing.T) {
+	b := loadEnvironment(t, "./autoload_git", "production")
+	assert.False(t, b.Config.Bundle.Git.Inferred)
+	assert.Equal(t, "main", b.Config.Bundle.Git.Branch)
+	assert.Contains(t, b.Config.Bundle.Git.OriginURL, "/cli")
+}
+
+func TestGitBundleBranchValidation(t *testing.T) {
+	git.GitDirectoryName = ".mock-git"
+	t.Cleanup(func() {
+		git.GitDirectoryName = ".git"
+	})
+
+	b := load(t, "./git_branch_validation")
+	assert.False(t, b.Config.Bundle.Git.Inferred)
+	assert.Equal(t, "feature-a", b.Config.Bundle.Git.Branch)
+	assert.Equal(t, "feature-b", b.Config.Bundle.Git.ActualBranch)
+
+	err := bundle.Apply(context.Background(), b, mutator.ValidateGitDetails())
+	assert.ErrorContains(t, err, "not on the right Git branch:")
+}
diff --git a/libs/git/repository.go b/libs/git/repository.go
index 3b93669a..2f19cff9 100644
--- a/libs/git/repository.go
+++ b/libs/git/repository.go
@@ -12,6 +12,8 @@ import (
 
 const gitIgnoreFileName = ".gitignore"
 
+var GitDirectoryName = ".git"
+
 // Repository represents a Git repository or a directory
 // that could later be initialized as Git repository.
 type Repository struct {
@@ -45,7 +47,7 @@ func (r *Repository) Root() string {
 
 func (r *Repository) CurrentBranch() (string, error) {
 	// load .git/HEAD
-	ref, err := LoadReferenceFile(filepath.Join(r.rootPath, ".git", "HEAD"))
+	ref, err := LoadReferenceFile(filepath.Join(r.rootPath, GitDirectoryName, "HEAD"))
 	if err != nil {
 		return "", err
 	}
@@ -62,7 +64,7 @@ func (r *Repository) CurrentBranch() (string, error) {
 
 func (r *Repository) LatestCommit() (string, error) {
 	// load .git/HEAD
-	ref, err := LoadReferenceFile(filepath.Join(r.rootPath, ".git", "HEAD"))
+	ref, err := LoadReferenceFile(filepath.Join(r.rootPath, GitDirectoryName, "HEAD"))
 	if err != nil {
 		return "", err
 	}
@@ -81,7 +83,7 @@ func (r *Repository) LatestCommit() (string, error) {
 	if err != nil {
 		return "", err
 	}
-	branchHeadRef, err := LoadReferenceFile(filepath.Join(r.rootPath, ".git", branchHeadPath))
+	branchHeadRef, err := LoadReferenceFile(filepath.Join(r.rootPath, GitDirectoryName, branchHeadPath))
 	if err != nil {
 		return "", err
 	}
@@ -186,7 +188,7 @@ func NewRepository(path string) (*Repository, error) {
 	}
 
 	real := true
-	rootPath, err := folders.FindDirWithLeaf(path, ".git")
+	rootPath, err := folders.FindDirWithLeaf(path, GitDirectoryName)
 	if err != nil {
 		if !os.IsNotExist(err) {
 			return nil, err