This commit is contained in:
Andrew Nester 2024-12-30 12:44:09 +01:00
parent 86630b61dd
commit 4b4cc42e5d
No known key found for this signature in database
GPG Key ID: 12BC628A44B7DA57
9 changed files with 182 additions and 150 deletions

View File

@ -4,6 +4,7 @@ import (
"context" "context"
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/dyn/dynvar" "github.com/databricks/cli/libs/dyn/dynvar"
@ -19,19 +20,9 @@ func (i *interpolateVariables) Apply(ctx context.Context, b *bundle.Bundle) diag
dyn.Key("config"), dyn.Key("config"),
) )
tfToConfigMap := map[string]string{ tfToConfigMap := map[string]string{}
"databricks_pipeline": "pipelines", for k, r := range config.SupportedResources() {
"databricks_job": "jobs", tfToConfigMap[r.TerraformResourceName] = k
"databricks_mlflow_model": "models",
"databricks_mlflow_experiment": "experiments",
"databricks_model_serving": "model_serving_endpoints",
"databricks_registered_model": "registered_models",
"databricks_quality_monitor": "quality_monitors",
"databricks_schema": "schemas",
"databricks_volume": "volumes",
"databricks_cluster": "clusters",
"databricks_dashboard": "dashboards",
"databricks_app": "apps",
} }
err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) {

View File

@ -45,7 +45,7 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos
} }
errGroup.Go(func() error { errGroup.Go(func() error {
err = f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists) err := f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists)
if err != nil { if err != nil {
mu.Lock() mu.Lock()
diags = append(diags, diag.Diagnostic{ diags = append(diags, diag.Diagnostic{

View File

@ -9,8 +9,11 @@ import (
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/mutator"
"github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/internal/bundletest"
mockfiler "github.com/databricks/cli/internal/mocks/libs/filer" mockfiler "github.com/databricks/cli/internal/mocks/libs/filer"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/vfs" "github.com/databricks/cli/libs/vfs"
"github.com/databricks/databricks-sdk-go/service/apps" "github.com/databricks/databricks-sdk-go/service/apps"
@ -25,6 +28,7 @@ func TestAppUploadConfig(t *testing.T) {
b := &bundle.Bundle{ b := &bundle.Bundle{
BundleRootPath: root, BundleRootPath: root,
SyncRootPath: root,
SyncRoot: vfs.MustNew(root), SyncRoot: vfs.MustNew(root),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
@ -64,6 +68,8 @@ env:
}, },
} }
diags := bundle.Apply(context.Background(), b, &u) bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(root, "databricks.yml")}})
diags := bundle.Apply(context.Background(), b, bundle.Seq(mutator.TranslatePaths(), &u))
require.NoError(t, diags.Error()) require.NoError(t, diags.Error())
} }

View File

@ -29,7 +29,8 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
usedSourceCodePaths[app.SourceCodePath] = key usedSourceCodePaths[app.SourceCodePath] = key
for _, configFile := range possibleConfigFiles { for _, configFile := range possibleConfigFiles {
cf := path.Join(app.SourceCodePath, configFile) appPath := strings.TrimPrefix(app.SourceCodePath, b.Config.Workspace.FilePath)
cf := path.Join(appPath, configFile)
if _, err := b.SyncRoot.Stat(cf); err == nil { if _, err := b.SyncRoot.Stat(cf); err == nil {
diags = append(diags, diag.Diagnostic{ diags = append(diags, diag.Diagnostic{
Severity: diag.Error, Severity: diag.Error,
@ -38,15 +39,6 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
}) })
} }
} }
if !strings.HasPrefix(app.SourceCodePath, b.Config.Workspace.FilePath) {
diags = append(diags, diag.Diagnostic{
Severity: diag.Error,
Summary: "App source code invalid",
Detail: fmt.Sprintf("App source code path %s is not within file path %s", app.SourceCodePath, b.Config.Workspace.FilePath),
Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s.source_code_path", key)),
})
}
} }
return diags return diags

View File

@ -11,7 +11,6 @@ import (
"github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/internal/bundletest" "github.com/databricks/cli/bundle/internal/bundletest"
"github.com/databricks/cli/internal/testutil" "github.com/databricks/cli/internal/testutil"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/vfs" "github.com/databricks/cli/libs/vfs"
"github.com/databricks/databricks-sdk-go/service/apps" "github.com/databricks/databricks-sdk-go/service/apps"
@ -28,6 +27,9 @@ func TestAppsValidate(t *testing.T) {
SyncRootPath: tmpDir, SyncRootPath: tmpDir,
SyncRoot: vfs.MustNew(tmpDir), SyncRoot: vfs.MustNew(tmpDir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{
FilePath: "/foo/bar/",
},
Resources: config.Resources{ Resources: config.Resources{
Apps: map[string]*resources.App{ Apps: map[string]*resources.App{
"app1": { "app1": {
@ -64,6 +66,9 @@ func TestAppsValidateSameSourcePath(t *testing.T) {
SyncRootPath: tmpDir, SyncRootPath: tmpDir,
SyncRoot: vfs.MustNew(tmpDir), SyncRoot: vfs.MustNew(tmpDir),
Config: config.Root{ Config: config.Root{
Workspace: config.Workspace{
FilePath: "/foo/bar/",
},
Resources: config.Resources{ Resources: config.Resources{
Apps: map[string]*resources.App{ Apps: map[string]*resources.App{
"app1": { "app1": {
@ -90,36 +95,3 @@ func TestAppsValidateSameSourcePath(t *testing.T) {
require.Equal(t, "Duplicate app source code path", diags[0].Summary) require.Equal(t, "Duplicate app source code path", diags[0].Summary)
require.Contains(t, diags[0].Detail, "has the same source code path as app resource") require.Contains(t, diags[0].Detail, "has the same source code path as app resource")
} }
func TestAppsValidateIncorrectSourceCodePath(t *testing.T) {
tmpDir := t.TempDir()
b := &bundle.Bundle{
BundleRootPath: tmpDir,
SyncRootPath: tmpDir,
SyncRoot: vfs.MustNew(tmpDir),
Config: config.Root{
Workspace: config.Workspace{
FilePath: "/Workspace/Users/foo@bar.com/files",
},
Resources: config.Resources{
Apps: map[string]*resources.App{
"app1": {
App: &apps.App{
Name: "app1",
},
SourceCodePath: "/Workspace/Random/app1",
},
},
},
},
}
bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(tmpDir, "databricks.yml")}})
diags := bundle.Apply(context.Background(), b, bundle.Seq(Validate()))
require.Len(t, diags, 1)
require.Equal(t, diag.Error, diags[0].Severity)
require.Equal(t, "App source code invalid", diags[0].Summary)
require.Contains(t, diags[0].Detail, "App source code path /Workspace/Random/app1 is not within file path /Workspace/Users/foo@bar.com/files")
}

View File

@ -105,48 +105,47 @@ func TestRunAsWorksForAllowedResources(t *testing.T) {
} }
} }
func TestRunAsErrorForUnsupportedResources(t *testing.T) { // Bundle "run_as" has two modes of operation, each with a different set of
// Bundle "run_as" has two modes of operation, each with a different set of // resources that are supported.
// resources that are supported. // Cases:
// Cases: // 1. When the bundle "run_as" identity is same as the current deployment
// 1. When the bundle "run_as" identity is same as the current deployment // identity. In this case all resources are supported.
// identity. In this case all resources are supported. // 2. When the bundle "run_as" identity is different from the current
// 2. When the bundle "run_as" identity is different from the current // deployment identity. In this case only a subset of resources are
// deployment identity. In this case only a subset of resources are // supported. This subset of resources are defined in the allow list below.
// supported. This subset of resources are defined in the allow list below. //
// // To be a part of the allow list, the resource must satisfy one of the following
// To be a part of the allow list, the resource must satisfy one of the following // two conditions:
// two conditions: // 1. The resource supports setting a run_as identity to a different user
// 1. The resource supports setting a run_as identity to a different user // from the owner/creator of the resource. For example, jobs.
// from the owner/creator of the resource. For example, jobs. // 2. Run as semantics do not apply to the resource. We do not plan to add
// 2. Run as semantics do not apply to the resource. We do not plan to add // platform side support for `run_as` for these resources. For example,
// platform side support for `run_as` for these resources. For example, // experiments or registered models.
// experiments or registered models. //
// // Any resource that is not on the allow list cannot be used when the bundle
// Any resource that is not on the allow list cannot be used when the bundle // run_as is different from the current deployment user. "bundle validate" must
// run_as is different from the current deployment user. "bundle validate" must // return an error if such a resource has been defined, and the run_as identity
// return an error if such a resource has been defined, and the run_as identity // is different from the current deployment identity.
// is different from the current deployment identity. //
// // Action Item: If you are adding a new resource to DABs, please check in with
// Action Item: If you are adding a new resource to DABs, please check in with // the relevant owning team whether the resource should be on the allow list or (implicitly) on
// the relevant owning team whether the resource should be on the allow list or (implicitly) on // the deny list. Any resources that could have run_as semantics in the future
// the deny list. Any resources that could have run_as semantics in the future // should be on the deny list.
// should be on the deny list. // For example: Teams for pipelines, model serving endpoints or Lakeview dashboards
// For example: Teams for pipelines, model serving endpoints or Lakeview dashboards // are planning to add platform side support for `run_as` for these resources at
// are planning to add platform side support for `run_as` for these resources at // some point in the future. These resources are (implicitly) on the deny list, since
// some point in the future. These resources are (implicitly) on the deny list, since // they are not on the allow list below.
// they are not on the allow list below. var allowList = []string{
allowList := []string{ "clusters",
"clusters", "jobs",
"jobs", "models",
"models", "registered_models",
"registered_models", "experiments",
"experiments", "schemas",
"schemas", "volumes",
"volumes", }
"apps",
}
func TestRunAsErrorForUnsupportedResources(t *testing.T) {
base := config.Root{ base := config.Root{
Workspace: config.Workspace{ Workspace: config.Workspace{
CurrentUser: &config.User{ CurrentUser: &config.User{
@ -199,3 +198,54 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) {
"See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", rt) "See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", rt)
} }
} }
func TestRunAsNoErrorForSupportedResources(t *testing.T) {
base := config.Root{
Workspace: config.Workspace{
CurrentUser: &config.User{
User: &iam.User{
UserName: "alice",
},
},
},
RunAs: &jobs.JobRunAs{
UserName: "bob",
},
}
v, err := convert.FromTyped(base, dyn.NilValue)
require.NoError(t, err)
// Define top level resources key in the bundle configuration.
// This is not part of the typed configuration, so we need to add it manually.
v, err = dyn.Set(v, "resources", dyn.V(map[string]dyn.Value{}))
require.NoError(t, err)
for _, rt := range allResourceTypes(t) {
// Skip unsupported resources
if !slices.Contains(allowList, rt) {
continue
}
// Add an instance of the resource type that is not on the allow list to
// the bundle configuration.
nv, err := dyn.SetByPath(v, dyn.NewPath(dyn.Key("resources"), dyn.Key(rt)), dyn.V(map[string]dyn.Value{
"foo": dyn.V(map[string]dyn.Value{
"name": dyn.V("bar"),
}),
}))
require.NoError(t, err)
// Get back typed configuration from the newly created invalid bundle configuration.
r := &config.Root{}
err = convert.ToTyped(r, nv)
require.NoError(t, err)
// Assert this configuration passes validation.
b := &bundle.Bundle{
Config: *r,
}
diags := bundle.Apply(context.Background(), b, SetRunAs())
require.NoError(t, diags.Error())
}
}

View File

@ -135,82 +135,96 @@ type ResourceDescription struct {
// Singular and plural title when used in summaries / terminal UI. // Singular and plural title when used in summaries / terminal UI.
SingularTitle string SingularTitle string
PluralTitle string PluralTitle string
TerraformResourceName string
} }
// The keys of the map corresponds to the resource key in the bundle configuration. // The keys of the map corresponds to the resource key in the bundle configuration.
func SupportedResources() map[string]ResourceDescription { func SupportedResources() map[string]ResourceDescription {
return map[string]ResourceDescription{ return map[string]ResourceDescription{
"jobs": { "jobs": {
SingularName: "job", SingularName: "job",
PluralName: "jobs", PluralName: "jobs",
SingularTitle: "Job", SingularTitle: "Job",
PluralTitle: "Jobs", PluralTitle: "Jobs",
TerraformResourceName: "databricks_job",
}, },
"pipelines": { "pipelines": {
SingularName: "pipeline", SingularName: "pipeline",
PluralName: "pipelines", PluralName: "pipelines",
SingularTitle: "Pipeline", SingularTitle: "Pipeline",
PluralTitle: "Pipelines", PluralTitle: "Pipelines",
TerraformResourceName: "databricks_pipeline",
}, },
"models": { "models": {
SingularName: "model", SingularName: "model",
PluralName: "models", PluralName: "models",
SingularTitle: "Model", SingularTitle: "Model",
PluralTitle: "Models", PluralTitle: "Models",
TerraformResourceName: "databricks_mlflow_model",
}, },
"experiments": { "experiments": {
SingularName: "experiment", SingularName: "experiment",
PluralName: "experiments", PluralName: "experiments",
SingularTitle: "Experiment", SingularTitle: "Experiment",
PluralTitle: "Experiments", PluralTitle: "Experiments",
TerraformResourceName: "databricks_mlflow_experiment",
}, },
"model_serving_endpoints": { "model_serving_endpoints": {
SingularName: "model_serving_endpoint", SingularName: "model_serving_endpoint",
PluralName: "model_serving_endpoints", PluralName: "model_serving_endpoints",
SingularTitle: "Model Serving Endpoint", SingularTitle: "Model Serving Endpoint",
PluralTitle: "Model Serving Endpoints", PluralTitle: "Model Serving Endpoints",
TerraformResourceName: "databricks_model_serving_endpoint",
}, },
"registered_models": { "registered_models": {
SingularName: "registered_model", SingularName: "registered_model",
PluralName: "registered_models", PluralName: "registered_models",
SingularTitle: "Registered Model", SingularTitle: "Registered Model",
PluralTitle: "Registered Models", PluralTitle: "Registered Models",
TerraformResourceName: "databricks_registered_model",
}, },
"quality_monitors": { "quality_monitors": {
SingularName: "quality_monitor", SingularName: "quality_monitor",
PluralName: "quality_monitors", PluralName: "quality_monitors",
SingularTitle: "Quality Monitor", SingularTitle: "Quality Monitor",
PluralTitle: "Quality Monitors", PluralTitle: "Quality Monitors",
TerraformResourceName: "databricks_quality_monitor",
}, },
"schemas": { "schemas": {
SingularName: "schema", SingularName: "schema",
PluralName: "schemas", PluralName: "schemas",
SingularTitle: "Schema", SingularTitle: "Schema",
PluralTitle: "Schemas", PluralTitle: "Schemas",
TerraformResourceName: "databricks_schema",
}, },
"clusters": { "clusters": {
SingularName: "cluster", SingularName: "cluster",
PluralName: "clusters", PluralName: "clusters",
SingularTitle: "Cluster", SingularTitle: "Cluster",
PluralTitle: "Clusters", PluralTitle: "Clusters",
TerraformResourceName: "databricks_cluster",
}, },
"dashboards": { "dashboards": {
SingularName: "dashboard", SingularName: "dashboard",
PluralName: "dashboards", PluralName: "dashboards",
SingularTitle: "Dashboard", SingularTitle: "Dashboard",
PluralTitle: "Dashboards", PluralTitle: "Dashboards",
TerraformResourceName: "databricks_dashboard",
}, },
"volumes": { "volumes": {
SingularName: "volume", SingularName: "volume",
PluralName: "volumes", PluralName: "volumes",
SingularTitle: "Volume", SingularTitle: "Volume",
PluralTitle: "Volumes", PluralTitle: "Volumes",
TerraformResourceName: "databricks_volume",
}, },
"apps": { "apps": {
SingularName: "app", SingularName: "app",
PluralName: "apps", PluralName: "apps",
SingularTitle: "App", SingularTitle: "App",
PluralTitle: "Apps", PluralTitle: "Apps",
TerraformResourceName: "databricks_app",
}, },
} }
} }

View File

@ -33,8 +33,12 @@ type stateResourceInstance struct {
} }
type stateInstanceAttributes struct { type stateInstanceAttributes struct {
ID string `json:"id"` ID string `json:"id"`
Name string `json:"name,omitempty"` // Some resources such as Apps do not have an ID, so we use the name instead.
// Some resources such as Apps do not have an ID, so we use the name instead.
// We need this for cases when such resource is removed from bundle config but
// exists in the workspace still so we can correctly display its summary.
Name string `json:"name,omitempty"`
ETag string `json:"etag,omitempty"` ETag string `json:"etag,omitempty"`
} }

View File

@ -107,4 +107,7 @@ env:
require.NoError(t, err) require.NoError(t, err)
require.NotNil(t, app) require.NotNil(t, app)
require.Equal(t, apps.ApplicationStateRunning, app.AppStatus.State) require.Equal(t, apps.ApplicationStateRunning, app.AppStatus.State)
// Redeploy it again just to check that it can be redeployed
deployBundle(t, ctx, root)
} }