Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
2cea265
Diagnostics for collaborative deployment scenarios
lennartkats-db Apr 19, 2024
7cdfc8d
Merge remote-tracking branch 'databricks/main' into cp-better-errors
lennartkats-db Apr 25, 2024
fc07725
Merge remote-tracking branch 'databricks/main' into cp-better-errors
lennartkats-db Jun 2, 2024
4aa38b8
Add missing file
lennartkats-db Jun 2, 2024
c46ecde
Add Lakehouse monitoring
lennartkats-db Jun 2, 2024
0b9feab
Cleanup
lennartkats-db Jun 2, 2024
7fff4bc
Cleanup
lennartkats-db Jul 6, 2024
c222ba3
Refactor
lennartkats-db Jul 6, 2024
b384b36
Merge remote-tracking branch 'databricks/main' into cp-better-errors
lennartkats-db Jul 6, 2024
3ba3c17
Address reviewer comments
lennartkats-db Jul 28, 2024
9981f07
Cleanup
lennartkats-db Jul 31, 2024
5ed6fc4
Merge remote-tracking branch 'databricks/main' into cp-better-errors
lennartkats-db Aug 22, 2024
26c3b42
Restore original 404 handling logic
lennartkats-db Aug 22, 2024
f93d394
Harden code with deeper tests
lennartkats-db Aug 22, 2024
a237dfa
Fix linter error
lennartkats-db Aug 22, 2024
49598cc
Fix copy/paste error
lennartkats-db Aug 22, 2024
8abae34
Update comment
lennartkats-db Sep 9, 2024
b11917e
Fix path
lennartkats-db Sep 9, 2024
4c06a34
Cleanup
lennartkats-db Sep 9, 2024
e533dd0
Extract function to separate module
lennartkats-db Sep 9, 2024
3b33d8c
Add comment
lennartkats-db Sep 9, 2024
bfe36fc
Rework error now that we can no longer detect permission errors
lennartkats-db Sep 12, 2024
bde209c
Process PR feedback
lennartkats-db Sep 22, 2024
a39457e
Show email addresses for regular users
lennartkats-db Sep 23, 2024
c850cfb
Merge remote-tracking branch 'databricks/main' into cp-better-errors
lennartkats-db Sep 23, 2024
763bcd5
Make sure list of managers is deterministic
lennartkats-db Oct 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 33 additions & 47 deletions bundle/config/mutator/run_as.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,44 +30,30 @@ func (m *setRunAs) Name() string {
return "SetRunAs"
}

type errUnsupportedResourceTypeForRunAs struct {
resourceType string
resourceLocation dyn.Location
currentUser string
runAsUser string
func reportRunAsNotSupported(resourceType string, location dyn.Location, currentUser string, runAsUser string) diag.Diagnostics {
return diag.Diagnostics{{
Summary: fmt.Sprintf("%s do not support a setting a run_as user that is different from the owner.\n"+
Comment thread
lennartkats-db marked this conversation as resolved.
"Current identity: %s. Run as identity: %s.\n"+
"See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", resourceType, currentUser, runAsUser),
Location: location,
}}
}

func (e errUnsupportedResourceTypeForRunAs) Error() string {
return fmt.Sprintf("%s are not supported when the current deployment user is different from the bundle's run_as identity. Please deploy as the run_as identity. Please refer to the documentation at https://docs.databricks.com/dev-tools/bundles/run-as.html for more details. Location of the unsupported resource: %s. Current identity: %s. Run as identity: %s", e.resourceType, e.resourceLocation, e.currentUser, e.runAsUser)
}

type errBothSpAndUserSpecified struct {
spName string
spLoc dyn.Location
userName string
userLoc dyn.Location
}

func (e errBothSpAndUserSpecified) Error() string {
return fmt.Sprintf("run_as section must specify exactly one identity. A service_principal_name %q is specified at %s. A user_name %q is defined at %s", e.spName, e.spLoc, e.userName, e.userLoc)
}

func validateRunAs(b *bundle.Bundle) error {
func validateRunAs(b *bundle.Bundle) diag.Diagnostics {
runAs := b.Config.RunAs

// Error if neither service_principal_name nor user_name are specified
if runAs.ServicePrincipalName == "" && runAs.UserName == "" {
return fmt.Errorf("run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified at %s", b.Config.GetLocation("run_as"))
return diag.Errorf("run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified at %s", b.Config.GetLocation("run_as"))
}

// Error if both service_principal_name and user_name are specified
if runAs.UserName != "" && runAs.ServicePrincipalName != "" {
return errBothSpAndUserSpecified{
spName: runAs.ServicePrincipalName,
userName: runAs.UserName,
spLoc: b.Config.GetLocation("run_as.service_principal_name"),
userLoc: b.Config.GetLocation("run_as.user_name"),
}
return diag.Diagnostics{{
Summary: "run_as section cannot specify both user_name and service_principal_name",
Location: b.Config.GetLocation("run_as"),
Severity: diag.Error,
}}
}

identity := runAs.ServicePrincipalName
Expand All @@ -82,32 +68,32 @@ func validateRunAs(b *bundle.Bundle) error {

// DLT pipelines do not support run_as in the API.
if len(b.Config.Resources.Pipelines) > 0 {
return errUnsupportedResourceTypeForRunAs{
resourceType: "pipelines",
resourceLocation: b.Config.GetLocation("resources.pipelines"),
currentUser: b.Config.Workspace.CurrentUser.UserName,
runAsUser: identity,
}
return reportRunAsNotSupported(
"pipelines",
b.Config.GetLocation("resources.pipelines"),
b.Config.Workspace.CurrentUser.UserName,
identity,
)
}

// Model serving endpoints do not support run_as in the API.
if len(b.Config.Resources.ModelServingEndpoints) > 0 {
return errUnsupportedResourceTypeForRunAs{
resourceType: "model_serving_endpoints",
resourceLocation: b.Config.GetLocation("resources.model_serving_endpoints"),
currentUser: b.Config.Workspace.CurrentUser.UserName,
runAsUser: identity,
}
return reportRunAsNotSupported(
"model_serving_endpoints",
b.Config.GetLocation("resources.model_serving_endpoints"),
b.Config.Workspace.CurrentUser.UserName,
identity,
)
}

// Monitors do not support run_as in the API.
if len(b.Config.Resources.QualityMonitors) > 0 {
return errUnsupportedResourceTypeForRunAs{
resourceType: "quality_monitors",
resourceLocation: b.Config.GetLocation("resources.quality_monitors"),
currentUser: b.Config.Workspace.CurrentUser.UserName,
runAsUser: identity,
}
return reportRunAsNotSupported(
"quality_monitors",
b.Config.GetLocation("resources.quality_monitors"),
b.Config.Workspace.CurrentUser.UserName,
identity,
)
Comment thread
lennartkats-db marked this conversation as resolved.
Outdated
}

return nil
Expand Down Expand Up @@ -183,7 +169,7 @@ func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {

// Assert the run_as configuration is valid in the context of the bundle
if err := validateRunAs(b); err != nil {
return diag.FromErr(err)
return err
}
Comment thread
pietern marked this conversation as resolved.

setRunAsForJobs(b)
Expand Down
7 changes: 1 addition & 6 deletions bundle/config/mutator/run_as_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,6 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) {
Config: *r,
}
diags := bundle.Apply(context.Background(), b, SetRunAs())
assert.Equal(t, diags.Error().Error(), errUnsupportedResourceTypeForRunAs{
resourceType: rt,
resourceLocation: dyn.Location{},
currentUser: "alice",
runAsUser: "bob",
}.Error(), "expected run_as with a different identity than the current deployment user to not supported for resources of type: %s", rt)
assert.Contains(t, diags.Error().Error(), "identity", rt)
}
}
16 changes: 10 additions & 6 deletions bundle/config/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package config

import (
"context"
"encoding/json"
"fmt"

"github.com/databricks/cli/bundle/config/resources"
Expand All @@ -25,6 +26,15 @@ type UniqueResourceIdTracker struct {
ConfigPath map[string]string
}

type ConfigResource interface {
Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error)
TerraformResourceName() string
Validate() error

json.Marshaler
json.Unmarshaler
Comment thread
lennartkats-db marked this conversation as resolved.
Outdated
}

// verifies merging is safe by checking no duplicate identifiers exist
func (r *Resources) VerifySafeMerge(other *Resources) error {
rootTracker, err := r.VerifyUniqueResourceIdentifiers()
Expand Down Expand Up @@ -211,12 +221,6 @@ func (r *Resources) ConfigureConfigFilePath() {
}
}

type ConfigResource interface {
Exists(ctx context.Context, w *databricks.WorkspaceClient, id string) (bool, error)
TerraformResourceName() string
Validate() error
}

func (r *Resources) FindResourceByConfigKey(key string) (ConfigResource, error) {
found := make([]ConfigResource, 0)
for k := range r.Jobs {
Expand Down
7 changes: 7 additions & 0 deletions bundle/deploy/files/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ package files

import (
"context"
"errors"
"fmt"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/permissions"
"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/log"
)

Expand All @@ -25,6 +28,10 @@ func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {

err = sync.RunOnce(ctx)
if err != nil {
permissionError := filer.PermissionError{}
if errors.As(err, &permissionError) {
return permissions.ReportPermissionDenied(ctx, b, b.Config.Workspace.StatePath)
}
Comment thread
lennartkats-db marked this conversation as resolved.
return diag.FromErr(err)
}

Expand Down
10 changes: 5 additions & 5 deletions bundle/deploy/lock/acquire.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/permissions"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/locker"
Expand Down Expand Up @@ -51,12 +52,11 @@ func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
if err != nil {
log.Errorf(ctx, "Failed to acquire deployment lock: %v", err)

notExistsError := filer.NoSuchDirectoryError{}
if errors.As(err, &notExistsError) {
// If we get a "doesn't exist" error from the API this indicates
// we either don't have permissions or the path is invalid.
return diag.Errorf("cannot write to deployment root (this can indicate a previous deploy was done with a different identity): %s", b.Config.Workspace.RootPath)
permissionError := filer.PermissionError{}
if errors.As(err, &permissionError) {
return permissions.ReportPermissionDenied(ctx, b, b.Config.Workspace.StatePath)
Comment thread
lennartkats-db marked this conversation as resolved.
Outdated
}
Comment thread
lennartkats-db marked this conversation as resolved.

return diag.FromErr(err)
}

Expand Down
5 changes: 5 additions & 0 deletions bundle/deploy/terraform/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/permissions"
"github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/log"
Expand Down Expand Up @@ -31,6 +32,10 @@ func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {

err = tf.Apply(ctx)
if err != nil {
diagnosis := permissions.TryReportTerraformPermissionError(ctx, b, err)
if diagnosis != nil {
return diagnosis
}
return diag.Errorf("terraform apply: %v", err)
}

Expand Down
1 change: 1 addition & 0 deletions bundle/permissions/mutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
const CAN_MANAGE = "CAN_MANAGE"
const CAN_VIEW = "CAN_VIEW"
const CAN_RUN = "CAN_RUN"
const IS_OWNER = "IS_OWNER"
Comment thread
lennartkats-db marked this conversation as resolved.

var allowedLevels = []string{CAN_MANAGE, CAN_VIEW, CAN_RUN}
var levelsMap = map[string](map[string]string){
Expand Down
Loading