Improve git config section validation

bundle/config/mutator/load_git_details.go

@@ -2,6 +2,7 @@ package mutator
 
 import (
 	"context"
+	"fmt"
 	"path/filepath"
 
 	"github.com/databricks/cli/bundle"
@@ -33,28 +34,55 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
 		b.WorktreeRoot = vfs.MustNew(info.WorktreeRoot)
 	}
 
-	b.Config.Bundle.Git.ActualBranch = info.CurrentBranch
-	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 = info.CurrentBranch
-	}
+	config := &b.Config.Bundle.Git
 
-	// load commit hash if undefined
-	if b.Config.Bundle.Git.Commit == "" {
-		b.Config.Bundle.Git.Commit = info.LatestCommit
+	config.ActualBranch = info.CurrentBranch
+	if config.Branch == "" {
+		config.Inferred = true
 	}
 
-	// load origin url if undefined
-	if b.Config.Bundle.Git.OriginURL == "" {
-		b.Config.Bundle.Git.OriginURL = info.OriginURL
-	}
+	// The value from config takes precedence; however, we always warn if configValue and fetchedValue disagree.
+	// In case of branch and commit and absence of --force we raise severity to Error
+	diags = append(diags, checkMatch("Git branch", info.CurrentBranch, &config.Branch, b.Config.Bundle.Force)...)
+	diags = append(diags, checkMatch("Git commit", info.LatestCommit, &config.Commit, b.Config.Bundle.Force)...)
+	diags = append(diags, checkMatch("Git origin URL", info.OriginURL, &config.OriginURL, true)...)
 
 	relBundlePath, err := filepath.Rel(b.WorktreeRoot.Native(), b.BundleRoot.Native())
 	if err != nil {
 		diags = append(diags, diag.FromErr(err)...)
 	} else {
-		b.Config.Bundle.Git.BundleRootPath = filepath.ToSlash(relBundlePath)
+		config.BundleRootPath = filepath.ToSlash(relBundlePath)
 	}
+
 	return diags
 }
+
+func checkMatch(field, fetchedValue string, configValue *string, allowedToMismatch bool) []diag.Diagnostic {
+	if fetchedValue == "" {
+		return nil
+	}
+
+	if *configValue == "" {
+		*configValue = fetchedValue
+		return nil
+	}
+
+	if *configValue != fetchedValue {
+		tmpl := "not on the right %s:\n  expected according to configuration: %s\n  actual: %s%s"
+		extra := ""
+		var severity diag.Severity
+		if allowedToMismatch {
+			severity = diag.Warning
+		} else {
+			severity = diag.Error
+			extra = "\nuse --force to override"
+		}
+
+		return []diag.Diagnostic{{
+			Severity: severity,
+			Summary:  fmt.Sprintf(tmpl, field, *configValue, fetchedValue, extra),
+		}}
+	}
+
+	return nil
+}

bundle/config/mutator/load_git_details_test.go

@@ -0,0 +1,69 @@
+package mutator
+
+import (
+	"testing"
+
+	"github.com/databricks/cli/libs/diag"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCheckMatch(t *testing.T) {
+	type test struct {
+		Fetched          string
+		ConfiguredBefore string
+		ConfiguredAfter  string
+	}
+
+	tests := []test{
+		{
+			Fetched:          "main",
+			ConfiguredBefore: "main",
+			ConfiguredAfter:  "main",
+		},
+		{
+			Fetched:          "main",
+			ConfiguredBefore: "",
+			ConfiguredAfter:  "main",
+		},
+		{
+			Fetched:          "",
+			ConfiguredBefore: "main",
+			ConfiguredAfter:  "main",
+		},
+	}
+
+	for _, test := range tests {
+		name := "CheckMatch " + test.Fetched + " " + test.ConfiguredBefore + " " + test.ConfiguredAfter
+		t.Run(name, func(t *testing.T) {
+			configValue := test.ConfiguredBefore
+			diags := checkMatch("", test.Fetched, &configValue, false)
+			assert.Nil(t, diags)
+			assert.Equal(t, test.ConfiguredAfter, configValue)
+		})
+		t.Run(name+" force", func(t *testing.T) {
+			configValue := test.ConfiguredBefore
+			diags := checkMatch("", test.Fetched, &configValue, true)
+			assert.Nil(t, diags)
+			assert.Equal(t, test.ConfiguredAfter, configValue)
+		})
+	}
+}
+
+func TestCheckWarning(t *testing.T) {
+	configValue := "main"
+	diags := checkMatch("Git branch", "feature", &configValue, true)
+	require.Len(t, diags, 1)
+	assert.Equal(t, diags[0].Severity, diag.Warning)
+	assert.Equal(t, diags[0].Summary, "not on the right Git branch:\n  expected according to configuration: main\n  actual: feature")
+	assert.Equal(t, "main", configValue)
+}
+
+func TestCheckError(t *testing.T) {
+	configValue := "main"
+	diags := checkMatch("Git branch", "feature", &configValue, false)
+	require.Len(t, diags, 1)
+	assert.Equal(t, diags[0].Severity, diag.Error)
+	assert.Equal(t, diags[0].Summary, "not on the right Git branch:\n  expected according to configuration: main\n  actual: feature\nuse --force to override")
+	assert.Equal(t, "main", configValue)
+}

bundle/phases/deploy.go

@@ -7,7 +7,6 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/artifacts"
 	"github.com/databricks/cli/bundle/config"
-	"github.com/databricks/cli/bundle/config/mutator"
 	"github.com/databricks/cli/bundle/deploy"
 	"github.com/databricks/cli/bundle/deploy/files"
 	"github.com/databricks/cli/bundle/deploy/lock"
@@ -149,7 +148,6 @@ func Deploy(outputHandler sync.OutputHandler) bundle.Mutator {
 				terraform.StatePull(),
 				terraform.CheckDashboardsModifiedRemotely(),
 				deploy.StatePull(),
-				mutator.ValidateGitDetails(),
 				artifacts.CleanUp(),
 				libraries.ExpandGlobReferences(),
 				libraries.Upload(),

bundle/tests/git_test.go

@@ -36,11 +36,10 @@ func TestGitBundleBranchValidation(t *testing.T) {
 	})
 
 	b := load(t, "./git_branch_validation")
-	bundle.Apply(context.Background(), b, mutator.LoadGitDetails())
+	diags := bundle.Apply(context.Background(), b, mutator.LoadGitDetails())
 	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)
 
-	diags := bundle.Apply(context.Background(), b, mutator.ValidateGitDetails())
 	assert.ErrorContains(t, diags.Error(), "not on the right Git branch:")
 }