mirror of https://github.com/databricks/cli.git
Encourage the use of root_path in production to ensure single deployment (#1712)
## Changes This updates `mode: production` to allow `root_path` to indicate uniqueness. Historically, we required `run_as` for this, which isn't actually very effective for that purpose. `run_as` also had the problem that it doesn't work for pipelines. This is a cherry-pick from https://github.com/databricks/cli/pull/1387 --------- Co-authored-by: Pieter Noordhuis <pcnoordhuis@gmail.com>
This commit is contained in:
parent
f8f804fe17
commit
3e40a0c2f1
|
@ -57,6 +57,9 @@ type Bundle struct {
|
||||||
// It is loaded from the bundle configuration files and mutators may update it.
|
// It is loaded from the bundle configuration files and mutators may update it.
|
||||||
Config config.Root
|
Config config.Root
|
||||||
|
|
||||||
|
// Target stores a snapshot of the Root.Bundle.Target configuration when it was selected by SelectTarget.
|
||||||
|
Target *config.Target `json:"target_config,omitempty" bundle:"internal"`
|
||||||
|
|
||||||
// Metadata about the bundle deployment. This is the interface Databricks services
|
// Metadata about the bundle deployment. This is the interface Databricks services
|
||||||
// rely on to integrate with bundles when they need additional information about
|
// rely on to integrate with bundles when they need additional information about
|
||||||
// a bundle deployment.
|
// a bundle deployment.
|
||||||
|
|
|
@ -2,6 +2,7 @@ package mutator
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"strings"
|
"strings"
|
||||||
|
|
||||||
"github.com/databricks/cli/bundle"
|
"github.com/databricks/cli/bundle"
|
||||||
|
@ -146,8 +147,21 @@ func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUs
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if !isPrincipalUsed && !isRunAsSet(r) {
|
// We need to verify that there is only a single deployment of the current target.
|
||||||
return diag.Errorf("'run_as' must be set for all jobs when using 'mode: production'")
|
// The best way to enforce this is to explicitly set root_path.
|
||||||
|
advice := fmt.Sprintf(
|
||||||
|
"set 'workspace.root_path' to make sure only one copy is deployed. A common practice is to use a username or principal name in this path, i.e. root_path: /Workspace/Users/%s/.bundle/${bundle.name}/${bundle.target}",
|
||||||
|
b.Config.Workspace.CurrentUser.UserName,
|
||||||
|
)
|
||||||
|
if !isExplicitRootSet(b) {
|
||||||
|
if isRunAsSet(r) || isPrincipalUsed {
|
||||||
|
// Just setting run_as is not enough to guarantee a single deployment,
|
||||||
|
// and neither is setting a principal.
|
||||||
|
// We only show a warning for these cases since we didn't historically
|
||||||
|
// report an error for them.
|
||||||
|
return diag.Recommendationf("target with 'mode: production' should %s", advice)
|
||||||
|
}
|
||||||
|
return diag.Errorf("target with 'mode: production' must %s", advice)
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
@ -164,6 +178,10 @@ func isRunAsSet(r config.Resources) bool {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func isExplicitRootSet(b *bundle.Bundle) bool {
|
||||||
|
return b.Target != nil && b.Target.Workspace != nil && b.Target.Workspace.RootPath != ""
|
||||||
|
}
|
||||||
|
|
||||||
func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
|
func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
|
||||||
switch b.Config.Bundle.Mode {
|
switch b.Config.Bundle.Mode {
|
||||||
case config.Development:
|
case config.Development:
|
||||||
|
|
|
@ -321,7 +321,7 @@ func TestProcessTargetModeProduction(t *testing.T) {
|
||||||
b := mockBundle(config.Production)
|
b := mockBundle(config.Production)
|
||||||
|
|
||||||
diags := validateProductionMode(context.Background(), b, false)
|
diags := validateProductionMode(context.Background(), b, false)
|
||||||
require.ErrorContains(t, diags.Error(), "run_as")
|
require.ErrorContains(t, diags.Error(), "target with 'mode: production' must set 'workspace.root_path' to make sure only one copy is deployed. A common practice is to use a username or principal name in this path, i.e. root_path: /Workspace/Users/lennart@company.com/.bundle/${bundle.name}/${bundle.target}")
|
||||||
|
|
||||||
b.Config.Workspace.StatePath = "/Shared/.bundle/x/y/state"
|
b.Config.Workspace.StatePath = "/Shared/.bundle/x/y/state"
|
||||||
b.Config.Workspace.ArtifactPath = "/Shared/.bundle/x/y/artifacts"
|
b.Config.Workspace.ArtifactPath = "/Shared/.bundle/x/y/artifacts"
|
||||||
|
@ -329,7 +329,7 @@ func TestProcessTargetModeProduction(t *testing.T) {
|
||||||
b.Config.Workspace.ResourcePath = "/Shared/.bundle/x/y/resources"
|
b.Config.Workspace.ResourcePath = "/Shared/.bundle/x/y/resources"
|
||||||
|
|
||||||
diags = validateProductionMode(context.Background(), b, false)
|
diags = validateProductionMode(context.Background(), b, false)
|
||||||
require.ErrorContains(t, diags.Error(), "production")
|
require.ErrorContains(t, diags.Error(), "target with 'mode: production' must set 'workspace.root_path' to make sure only one copy is deployed. A common practice is to use a username or principal name in this path, i.e. root_path: /Workspace/Users/lennart@company.com/.bundle/${bundle.name}/${bundle.target}")
|
||||||
|
|
||||||
permissions := []resources.Permission{
|
permissions := []resources.Permission{
|
||||||
{
|
{
|
||||||
|
@ -375,6 +375,23 @@ func TestProcessTargetModeProductionOkForPrincipal(t *testing.T) {
|
||||||
require.NoError(t, diags.Error())
|
require.NoError(t, diags.Error())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestProcessTargetModeProductionOkWithRootPath(t *testing.T) {
|
||||||
|
b := mockBundle(config.Production)
|
||||||
|
|
||||||
|
// Our target has all kinds of problems when not using service principals ...
|
||||||
|
diags := validateProductionMode(context.Background(), b, false)
|
||||||
|
require.Error(t, diags.Error())
|
||||||
|
|
||||||
|
// ... but we're okay if we specify a root path
|
||||||
|
b.Target = &config.Target{
|
||||||
|
Workspace: &config.Workspace{
|
||||||
|
RootPath: "some-root-path",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
diags = validateProductionMode(context.Background(), b, false)
|
||||||
|
require.NoError(t, diags.Error())
|
||||||
|
}
|
||||||
|
|
||||||
// Make sure that we have test coverage for all resource types
|
// Make sure that we have test coverage for all resource types
|
||||||
func TestAllResourcesMocked(t *testing.T) {
|
func TestAllResourcesMocked(t *testing.T) {
|
||||||
b := mockBundle(config.Development)
|
b := mockBundle(config.Development)
|
||||||
|
|
|
@ -15,6 +15,7 @@ type selectTarget struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
// SelectTarget merges the specified target into the root configuration.
|
// SelectTarget merges the specified target into the root configuration.
|
||||||
|
// After merging, it removes the 'Targets' section from the configuration.
|
||||||
func SelectTarget(name string) bundle.Mutator {
|
func SelectTarget(name string) bundle.Mutator {
|
||||||
return &selectTarget{
|
return &selectTarget{
|
||||||
name: name,
|
name: name,
|
||||||
|
@ -31,7 +32,7 @@ func (m *selectTarget) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnosti
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get specified target
|
// Get specified target
|
||||||
_, ok := b.Config.Targets[m.name]
|
target, ok := b.Config.Targets[m.name]
|
||||||
if !ok {
|
if !ok {
|
||||||
return diag.Errorf("%s: no such target. Available targets: %s", m.name, strings.Join(maps.Keys(b.Config.Targets), ", "))
|
return diag.Errorf("%s: no such target. Available targets: %s", m.name, strings.Join(maps.Keys(b.Config.Targets), ", "))
|
||||||
}
|
}
|
||||||
|
@ -43,13 +44,15 @@ func (m *selectTarget) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnosti
|
||||||
}
|
}
|
||||||
|
|
||||||
// Store specified target in configuration for reference.
|
// Store specified target in configuration for reference.
|
||||||
|
b.Target = target
|
||||||
b.Config.Bundle.Target = m.name
|
b.Config.Bundle.Target = m.name
|
||||||
|
|
||||||
// We do this for backward compatibility.
|
// We do this for backward compatibility.
|
||||||
// TODO: remove when Environments section is not supported anymore.
|
// TODO: remove when Environments section is not supported anymore.
|
||||||
b.Config.Bundle.Environment = b.Config.Bundle.Target
|
b.Config.Bundle.Environment = b.Config.Bundle.Target
|
||||||
|
|
||||||
// Clear targets after loading.
|
// Cleanup the original targets and environments sections since they
|
||||||
|
// show up in the JSON output of the 'summary' and 'validate' commands.
|
||||||
b.Config.Targets = nil
|
b.Config.Targets = nil
|
||||||
b.Config.Environments = nil
|
b.Config.Environments = nil
|
||||||
|
|
||||||
|
|
|
@ -47,8 +47,8 @@ type Root struct {
|
||||||
|
|
||||||
// Targets can be used to differentiate settings and resources between
|
// Targets can be used to differentiate settings and resources between
|
||||||
// bundle deployment targets (e.g. development, staging, production).
|
// bundle deployment targets (e.g. development, staging, production).
|
||||||
// If not specified, the code below initializes this field with a
|
// Note that this field is set to 'nil' by the SelectTarget mutator;
|
||||||
// single default-initialized target called "default".
|
// use bundle.Bundle.Target to access the selected target configuration.
|
||||||
Targets map[string]*Target `json:"targets,omitempty"`
|
Targets map[string]*Target `json:"targets,omitempty"`
|
||||||
|
|
||||||
// DEPRECATED. Left for backward compatibility with Targets
|
// DEPRECATED. Left for backward compatibility with Targets
|
||||||
|
|
|
@ -86,6 +86,16 @@ func Infof(format string, args ...any) Diagnostics {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Recommendationf creates a new recommendation diagnostic.
|
||||||
|
func Recommendationf(format string, args ...any) Diagnostics {
|
||||||
|
return []Diagnostic{
|
||||||
|
{
|
||||||
|
Severity: Recommendation,
|
||||||
|
Summary: fmt.Sprintf(format, args...),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Diagnostics holds zero or more instances of [Diagnostic].
|
// Diagnostics holds zero or more instances of [Diagnostic].
|
||||||
type Diagnostics []Diagnostic
|
type Diagnostics []Diagnostic
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue