Add validation for Git settings in bundles (#578)

## Changes

This checks whether the Git settings are consistent with the actual Git
state of a source directory.

(This PR adds to https://github.com/databricks/cli/pull/577.) 

Previously, we would silently let users configure their Git branch to
e.g. `main` and deploy with that metadata even if they were actually on
a different branch.

With these changes, the following config would result in an error when
deployed from any other branch than `main`:

```
bundle:
  name: example

workspace:
  git:
    branch: main

environments:
  ...
```

> not on the right Git branch:
>   expected according to configuration: main
>   actual: my-feature-branch

It's not very useful to set the same branch for all environments,
though. For development, it's better to just let the CLI auto-detect the
right branch. Therefore, it's now possible to set the branch just for a
single environment:

```
bundle:
  name: example 2

environments:
  development:
    default: true

  production:
    # production can only be deployed from the 'main' branch
    git:
      branch: main
```

Adding to that, the `mode: production` option actually checks that users
explicitly set the Git branch as seen above. Setting that branch helps
avoid mistakes, where someone accidentally deploys to production from
the wrong branch. (I could see us offering an escape hatch for that in
the future.)

# Testing

Manual testing to validate the experience and error messages. Automated
unit tests.

---------

Co-authored-by: Fabian Jakobs <fabian.jakobs@databricks.com>

bundle/config/bundle.go

@@ -25,6 +25,9 @@ type Bundle struct {
 	// Lock configures locking behavior on deployment.
 	Lock Lock `json:"lock" bundle:"readonly"`
 
+	// Force-override Git branch validation.
+	Force bool `json:"force" bundle:"readonly"`
+
 	// Contains Git information like current commit, current branch and
 	// origin url. Automatically loaded by reading .git directory if not specified
 	Git Git `json:"git,omitempty"`

bundle/config/environment.go

@@ -29,6 +29,8 @@ type Environment struct {
 	// Does not permit defining new variables or redefining existing ones
 	// in the scope of an environment
 	Variables map[string]string `json:"variables,omitempty"`
+
+	Git Git `json:"git,omitempty"`
 }
 
 const (

bundle/config/git.go

@@ -4,4 +4,10 @@ type Git struct {
 	Branch    string `json:"branch,omitempty"`
 	OriginURL string `json:"origin_url,omitempty"`
 	Commit    string `json:"commit,omitempty" bundle:"readonly"`
+
+	// Inferred is set to true if the Git details were inferred and weren't set explicitly
+	Inferred bool `json:"-" bundle:"readonly"`
+
+	// The actual branch according to Git (may be different from the configured branch)
+	ActualBranch string `json:"-" bundle:"readonly"`
 }

bundle/config/mutator/load_git_details.go

@@ -31,6 +31,8 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) error {
 			log.Warnf(ctx, "failed to load current branch: %s", err)
 		} else {
 			b.Config.Bundle.Git.Branch = branch
+			b.Config.Bundle.Git.ActualBranch = branch
+			b.Config.Bundle.Git.Inferred = true
 		}
 	}
 	// load commit hash if undefined

bundle/config/mutator/process_environment_mode.go

@@ -109,6 +109,11 @@ func findIncorrectPath(b *bundle.Bundle, mode config.Mode) string {
 }
 
 func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUsed bool) error {
+	if b.Config.Bundle.Git.Inferred {
+		env := b.Config.Bundle.Environment
+		return fmt.Errorf("environment with 'mode: production' must specify an explicit 'environments.%s.git' configuration", env)
+	}
+
 	r := b.Config.Resources
 	for i := range r.Pipelines {
 		if r.Pipelines[i].Development {

bundle/config/mutator/process_environment_mode_test.go

@@ -22,6 +22,10 @@ func mockBundle(mode config.Mode) *bundle.Bundle {
 		Config: config.Root{
 			Bundle: config.Bundle{
 				Mode: mode,
+				Git: config.Git{
+					OriginURL: "http://origin",
+					Branch:    "main",
+				},
 			},
 			Workspace: config.Workspace{
 				CurrentUser: &config.User{
@@ -114,6 +118,17 @@ func TestProcessEnvironmentModeProduction(t *testing.T) {
 	assert.False(t, bundle.Config.Resources.Pipelines["pipeline1"].PipelineSpec.Development)
 }
 
+func TestProcessEnvironmentModeProductionGit(t *testing.T) {
+	bundle := mockBundle(config.Production)
+
+	// Pretend the user didn't set Git configuration explicitly
+	bundle.Config.Bundle.Git.Inferred = true
+
+	err := validateProductionMode(context.Background(), bundle, false)
+	require.ErrorContains(t, err, "git")
+	bundle.Config.Bundle.Git.Inferred = false
+}
+
 func TestProcessEnvironmentModeProductionOkForPrincipal(t *testing.T) {
 	bundle := mockBundle(config.Production)
 

bundle/config/mutator/validate_git_details.go

@@ -0,0 +1,29 @@
+package mutator
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/databricks/cli/bundle"
+)
+
+type validateGitDetails struct{}
+
+func ValidateGitDetails() *validateGitDetails {
+	return &validateGitDetails{}
+}
+
+func (m *validateGitDetails) Name() string {
+	return "ValidateGitDetails"
+}
+
+func (m *validateGitDetails) Apply(ctx context.Context, b *bundle.Bundle) error {
+	if b.Config.Bundle.Git.Branch == "" || b.Config.Bundle.Git.ActualBranch == "" {
+		return nil
+	}
+
+	if b.Config.Bundle.Git.Branch != b.Config.Bundle.Git.ActualBranch && !b.Config.Bundle.Force {
+		return fmt.Errorf("not on the right Git branch:\n  expected according to configuration: %s\n  actual: %s\nuse --force to override", b.Config.Bundle.Git.Branch, b.Config.Bundle.Git.ActualBranch)
+	}
+	return nil
+}

bundle/config/mutator/validate_git_details_test.go

@@ -0,0 +1,65 @@
+package mutator
+
+import (
+	"context"
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidateGitDetailsMatchingBranches(t *testing.T) {
+	bundle := &bundle.Bundle{
+		Config: config.Root{
+			Bundle: config.Bundle{
+				Git: config.Git{
+					Branch:       "main",
+					ActualBranch: "main",
+				},
+			},
+		},
+	}
+
+	m := ValidateGitDetails()
+	err := m.Apply(context.Background(), bundle)
+
+	assert.NoError(t, err)
+}
+
+func TestValidateGitDetailsNonMatchingBranches(t *testing.T) {
+	bundle := &bundle.Bundle{
+		Config: config.Root{
+			Bundle: config.Bundle{
+				Git: config.Git{
+					Branch:       "main",
+					ActualBranch: "feature",
+				},
+			},
+		},
+	}
+
+	m := ValidateGitDetails()
+	err := m.Apply(context.Background(), bundle)
+
+	expectedError := "not on the right Git branch:\n  expected according to configuration: main\n  actual: feature\nuse --force to override"
+	assert.EqualError(t, err, expectedError)
+}
+
+func TestValidateGitDetailsNotUsingGit(t *testing.T) {
+	bundle := &bundle.Bundle{
+		Config: config.Root{
+			Bundle: config.Bundle{
+				Git: config.Git{
+					Branch:       "main",
+					ActualBranch: "",
+				},
+			},
+		},
+	}
+
+	m := ValidateGitDetails()
+	err := m.Apply(context.Background(), bundle)
+
+	assert.NoError(t, err)
+}

bundle/config/root.go

@@ -225,5 +225,17 @@ func (r *Root) MergeEnvironment(env *Environment) error {
 		r.Bundle.ComputeID = env.ComputeID
 	}
 
+	git := &r.Bundle.Git
+	if env.Git.Branch != "" {
+		git.Branch = env.Git.Branch
+		git.Inferred = false
+	}
+	if env.Git.Commit != "" {
+		git.Commit = env.Git.Commit
+	}
+	if env.Git.OriginURL != "" {
+		git.OriginURL = env.Git.OriginURL
+	}
+
 	return nil
 }

bundle/phases/deploy.go

@@ -3,6 +3,7 @@ package phases
 import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/artifacts"
+	"github.com/databricks/cli/bundle/config/mutator"
 	"github.com/databricks/cli/bundle/deploy/files"
 	"github.com/databricks/cli/bundle/deploy/lock"
 	"github.com/databricks/cli/bundle/deploy/terraform"
@@ -15,6 +16,7 @@ func Deploy() bundle.Mutator {
 		lock.Acquire(),
 		bundle.Defer(
 			bundle.Seq(
+				mutator.ValidateGitDetails(),
 				files.Upload(),
 				libraries.MatchWithArtifacts(),
 				artifacts.CleanUp(),

bundle/tests/autoload_git/databricks.yml

@@ -1,4 +1,11 @@
 bundle:
   name: autoload git config test
-  git:
-    branch: foo
+
+environments:
+  development:
+    default: true
+
+  production:
+    # production can only be deployed from the 'main' branch
+    git:
+      branch: main

bundle/tests/autoload_git_test.go

@@ -6,10 +6,15 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestGitConfig(t *testing.T) {
+func TestAutoLoad(t *testing.T) {
 	b := load(t, "./autoload_git")
-	assert.Equal(t, "foo", b.Config.Bundle.Git.Branch)
-	sshUrl := "git@github.com:databricks/cli.git"
-	httpsUrl := "https://github.com/databricks/cli"
-	assert.Contains(t, []string{sshUrl, httpsUrl}, b.Config.Bundle.Git.OriginURL)
+	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")
 }

cmd/bundle/deploy.go

@@ -13,16 +13,18 @@ func newDeployCommand() *cobra.Command {
 		PreRunE: ConfigureBundleWithVariables,
 	}
 
-	var forceDeploy bool
+	var force bool
+	var forceLock bool
 	var computeID string
-	cmd.Flags().BoolVar(&forceDeploy, "force", false, "Force acquisition of deployment lock.")
+	cmd.Flags().BoolVar(&force, "force", false, "Force-override Git branch validation.")
+	cmd.Flags().BoolVar(&forceLock, "force-deploy", false, "Force acquisition of deployment lock.")
 	cmd.Flags().StringVarP(&computeID, "compute-id", "c", "", "Override compute in the deployment with the given compute ID.")
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		b := bundle.Get(cmd.Context())
 
-		// If `--force` is specified, force acquisition of the deployment lock.
-		b.Config.Bundle.Lock.Force = forceDeploy
+		b.Config.Bundle.Force = force
+		b.Config.Bundle.Lock.Force = forceLock
 		b.Config.Bundle.ComputeID = computeID
 
 		return bundle.Apply(cmd.Context(), b, bundle.Seq(

cmd/bundle/destroy.go

@@ -23,13 +23,13 @@ func newDestroyCommand() *cobra.Command {
 	var autoApprove bool
 	var forceDestroy bool
 	cmd.Flags().BoolVar(&autoApprove, "auto-approve", false, "Skip interactive approvals for deleting resources and files")
-	cmd.Flags().BoolVar(&forceDestroy, "force", false, "Force acquisition of deployment lock.")
+	cmd.Flags().BoolVar(&forceDestroy, "force-lock", false, "Force acquisition of deployment lock.")
 
 	cmd.RunE = func(cmd *cobra.Command, args []string) error {
 		ctx := cmd.Context()
 		b := bundle.Get(ctx)
 
-		// If `--force` is specified, force acquisition of the deployment lock.
+		// If `--force-lock` is specified, force acquisition of the deployment lock.
 		b.Config.Bundle.Lock.Force = forceDestroy
 
 		// If `--auto-approve`` is specified, we skip confirmation checks