diff --git a/bundle/config/bundle.go b/bundle/config/bundle.go index cf386477..f3401477 100644 --- a/bundle/config/bundle.go +++ b/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"` diff --git a/bundle/config/environment.go b/bundle/config/environment.go index c1f4f4ad..7152f791 100644 --- a/bundle/config/environment.go +++ b/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 ( diff --git a/bundle/config/git.go b/bundle/config/git.go index 7ada8dfb..760134a8 100644 --- a/bundle/config/git.go +++ b/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"` } diff --git a/bundle/config/mutator/load_git_details.go b/bundle/config/mutator/load_git_details.go index 121924c6..f22aafe0 100644 --- a/bundle/config/mutator/load_git_details.go +++ b/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 diff --git a/bundle/config/mutator/process_environment_mode.go b/bundle/config/mutator/process_environment_mode.go index 65d8a689..d2030234 100644 --- a/bundle/config/mutator/process_environment_mode.go +++ b/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 { diff --git a/bundle/config/mutator/process_environment_mode_test.go b/bundle/config/mutator/process_environment_mode_test.go index 6f53abd8..36e0396e 100644 --- a/bundle/config/mutator/process_environment_mode_test.go +++ b/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) diff --git a/bundle/config/mutator/validate_git_details.go b/bundle/config/mutator/validate_git_details.go new file mode 100644 index 00000000..116498bf --- /dev/null +++ b/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 +} diff --git a/bundle/config/mutator/validate_git_details_test.go b/bundle/config/mutator/validate_git_details_test.go new file mode 100644 index 00000000..252964ee --- /dev/null +++ b/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) +} diff --git a/bundle/config/root.go b/bundle/config/root.go index f5a4f00d..52f88737 100644 --- a/bundle/config/root.go +++ b/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 } diff --git a/bundle/phases/deploy.go b/bundle/phases/deploy.go index 8b53273c..011bb4b2 100644 --- a/bundle/phases/deploy.go +++ b/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(), diff --git a/bundle/tests/autoload_git/databricks.yml b/bundle/tests/autoload_git/databricks.yml index d0e1de60..ba4785ae 100644 --- a/bundle/tests/autoload_git/databricks.yml +++ b/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 diff --git a/bundle/tests/autoload_git_test.go b/bundle/tests/autoload_git_test.go index 87c7180e..a1075198 100644 --- a/bundle/tests/autoload_git_test.go +++ b/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") } diff --git a/cmd/bundle/deploy.go b/cmd/bundle/deploy.go index a39f1996..807bb982 100644 --- a/cmd/bundle/deploy.go +++ b/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( diff --git a/cmd/bundle/destroy.go b/cmd/bundle/destroy.go index 82d82144..22d998ab 100644 --- a/cmd/bundle/destroy.go +++ b/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