Show actionable errors for collaborative deployment scenarios (#1386)

## Changes

This adds diagnostics for collaborative (production) deployment
scenarios, including:

- Bob deploys a bundle that is normally deployed by Alice, but this
fails because Bob can't write to `/Users/Alice/.bundle`.
- Charlie deploys a bundle that is normally deployed by Alice, but this
fails because he can't create a new pipeline where Alice would be the
owner.
- Alice deploys a bundle where she didn't list herself as one of the
CAN_MANAGE users in permissions. That can work, but is probably a
mistake.

## Tests

Unit tests, manual testing.
This commit is contained in:
Lennart Kats (databricks) 2024-10-10 13:18:23 +02:00 committed by GitHub
parent c92c67afc9
commit e885794722
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 579 additions and 77 deletions

View File

@ -30,50 +30,44 @@ func (m *setRunAs) Name() string {
return "SetRunAs" return "SetRunAs"
} }
type errUnsupportedResourceTypeForRunAs struct { func reportRunAsNotSupported(resourceType string, location dyn.Location, currentUser string, runAsUser string) diag.Diagnostics {
resourceType string return diag.Diagnostics{{
resourceLocation dyn.Location Summary: fmt.Sprintf("%s do not support a setting a run_as user that is different from the owner.\n"+
currentUser string "Current identity: %s. Run as identity: %s.\n"+
runAsUser string "See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", resourceType, currentUser, runAsUser),
Locations: []dyn.Location{location},
Severity: diag.Error,
}}
} }
func (e errUnsupportedResourceTypeForRunAs) Error() string { func validateRunAs(b *bundle.Bundle) diag.Diagnostics {
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) diags := diag.Diagnostics{}
}
type errBothSpAndUserSpecified struct { neitherSpecifiedErr := diag.Diagnostics{{
spName string Summary: "run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified",
spLoc dyn.Location Locations: []dyn.Location{b.Config.GetLocation("run_as")},
userName string Severity: diag.Error,
userLoc dyn.Location }}
}
func (e errBothSpAndUserSpecified) Error() string { // Fail fast if neither service_principal_name nor user_name are specified, but the
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 {
neitherSpecifiedErr := 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"))
// Error if neither service_principal_name nor user_name are specified, but the
// run_as section is present. // run_as section is present.
if b.Config.Value().Get("run_as").Kind() == dyn.KindNil { if b.Config.Value().Get("run_as").Kind() == dyn.KindNil {
return neitherSpecifiedErr return neitherSpecifiedErr
} }
// Error if one or both of service_principal_name and user_name are specified,
// Fail fast if one or both of service_principal_name and user_name are specified,
// but with empty values. // but with empty values.
if b.Config.RunAs.ServicePrincipalName == "" && b.Config.RunAs.UserName == "" { runAs := b.Config.RunAs
if runAs.ServicePrincipalName == "" && runAs.UserName == "" {
return neitherSpecifiedErr return neitherSpecifiedErr
} }
// Error if both service_principal_name and user_name are specified
runAs := b.Config.RunAs
if runAs.UserName != "" && runAs.ServicePrincipalName != "" { if runAs.UserName != "" && runAs.ServicePrincipalName != "" {
return errBothSpAndUserSpecified{ diags = diags.Extend(diag.Diagnostics{{
spName: runAs.ServicePrincipalName, Summary: "run_as section cannot specify both user_name and service_principal_name",
userName: runAs.UserName, Locations: []dyn.Location{b.Config.GetLocation("run_as")},
spLoc: b.Config.GetLocation("run_as.service_principal_name"), Severity: diag.Error,
userLoc: b.Config.GetLocation("run_as.user_name"), }})
}
} }
identity := runAs.ServicePrincipalName identity := runAs.ServicePrincipalName
@ -83,40 +77,40 @@ func validateRunAs(b *bundle.Bundle) error {
// All resources are supported if the run_as identity is the same as the current deployment identity. // All resources are supported if the run_as identity is the same as the current deployment identity.
if identity == b.Config.Workspace.CurrentUser.UserName { if identity == b.Config.Workspace.CurrentUser.UserName {
return nil return diags
} }
// DLT pipelines do not support run_as in the API. // DLT pipelines do not support run_as in the API.
if len(b.Config.Resources.Pipelines) > 0 { if len(b.Config.Resources.Pipelines) > 0 {
return errUnsupportedResourceTypeForRunAs{ diags = diags.Extend(reportRunAsNotSupported(
resourceType: "pipelines", "pipelines",
resourceLocation: b.Config.GetLocation("resources.pipelines"), b.Config.GetLocation("resources.pipelines"),
currentUser: b.Config.Workspace.CurrentUser.UserName, b.Config.Workspace.CurrentUser.UserName,
runAsUser: identity, identity,
} ))
} }
// Model serving endpoints do not support run_as in the API. // Model serving endpoints do not support run_as in the API.
if len(b.Config.Resources.ModelServingEndpoints) > 0 { if len(b.Config.Resources.ModelServingEndpoints) > 0 {
return errUnsupportedResourceTypeForRunAs{ diags = diags.Extend(reportRunAsNotSupported(
resourceType: "model_serving_endpoints", "model_serving_endpoints",
resourceLocation: b.Config.GetLocation("resources.model_serving_endpoints"), b.Config.GetLocation("resources.model_serving_endpoints"),
currentUser: b.Config.Workspace.CurrentUser.UserName, b.Config.Workspace.CurrentUser.UserName,
runAsUser: identity, identity,
} ))
} }
// Monitors do not support run_as in the API. // Monitors do not support run_as in the API.
if len(b.Config.Resources.QualityMonitors) > 0 { if len(b.Config.Resources.QualityMonitors) > 0 {
return errUnsupportedResourceTypeForRunAs{ diags = diags.Extend(reportRunAsNotSupported(
resourceType: "quality_monitors", "quality_monitors",
resourceLocation: b.Config.GetLocation("resources.quality_monitors"), b.Config.GetLocation("resources.quality_monitors"),
currentUser: b.Config.Workspace.CurrentUser.UserName, b.Config.Workspace.CurrentUser.UserName,
runAsUser: identity, identity,
} ))
} }
return nil return diags
} }
func setRunAsForJobs(b *bundle.Bundle) { func setRunAsForJobs(b *bundle.Bundle) {
@ -187,8 +181,9 @@ func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics {
} }
// Assert the run_as configuration is valid in the context of the bundle // Assert the run_as configuration is valid in the context of the bundle
if err := validateRunAs(b); err != nil { diags := validateRunAs(b)
return diag.FromErr(err) if diags.HasError() {
return diags
} }
setRunAsForJobs(b) setRunAsForJobs(b)

View File

@ -188,11 +188,8 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) {
Config: *r, Config: *r,
} }
diags := bundle.Apply(context.Background(), b, SetRunAs()) diags := bundle.Apply(context.Background(), b, SetRunAs())
assert.Equal(t, diags.Error().Error(), errUnsupportedResourceTypeForRunAs{ assert.Contains(t, diags.Error().Error(), "do not support a setting a run_as user that is different from the owner.\n"+
resourceType: rt, "Current identity: alice. Run as identity: bob.\n"+
resourceLocation: dyn.Location{}, "See https://docs.databricks.com/dev-tools/bundles/run-as.html to learn more about the run_as property.", rt)
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)
} }
} }

View File

@ -2,9 +2,12 @@ package files
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"io/fs"
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/permissions"
"github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/cmdio"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/log"
@ -35,6 +38,9 @@ func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
b.Files, err = sync.RunOnce(ctx) b.Files, err = sync.RunOnce(ctx)
if err != nil { if err != nil {
if errors.Is(err, fs.ErrPermission) {
return permissions.ReportPossiblePermissionDenied(ctx, b, b.Config.Workspace.FilePath)
}
return diag.FromErr(err) return diag.FromErr(err)
} }

View File

@ -3,8 +3,10 @@ package lock
import ( import (
"context" "context"
"errors" "errors"
"io/fs"
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/permissions"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/filer"
"github.com/databricks/cli/libs/locker" "github.com/databricks/cli/libs/locker"
@ -51,12 +53,17 @@ func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
if err != nil { if err != nil {
log.Errorf(ctx, "Failed to acquire deployment lock: %v", err) log.Errorf(ctx, "Failed to acquire deployment lock: %v", err)
if errors.Is(err, fs.ErrPermission) {
return permissions.ReportPossiblePermissionDenied(ctx, b, b.Config.Workspace.StatePath)
}
notExistsError := filer.NoSuchDirectoryError{} notExistsError := filer.NoSuchDirectoryError{}
if errors.As(err, &notExistsError) { if errors.As(err, &notExistsError) {
// If we get a "doesn't exist" error from the API this indicates // If we get a "doesn't exist" error from the API this indicates
// we either don't have permissions or the path is invalid. // 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) return permissions.ReportPossiblePermissionDenied(ctx, b, b.Config.Workspace.StatePath)
} }
return diag.FromErr(err) return diag.FromErr(err)
} }

View File

@ -4,6 +4,7 @@ import (
"context" "context"
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/permissions"
"github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/log" "github.com/databricks/cli/libs/log"
"github.com/hashicorp/terraform-exec/tfexec" "github.com/hashicorp/terraform-exec/tfexec"
@ -34,6 +35,10 @@ func (w *apply) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
// Apply terraform according to the computed plan // Apply terraform according to the computed plan
err := tf.Apply(ctx, tfexec.DirOrPlan(b.Plan.Path)) err := tf.Apply(ctx, tfexec.DirOrPlan(b.Plan.Path))
if err != nil { if err != nil {
diags := permissions.TryExtendTerraformPermissionError(ctx, b, err)
if diags != nil {
return diags
}
return diag.Errorf("terraform apply: %v", err) return diag.Errorf("terraform apply: %v", err)
} }

View File

@ -13,6 +13,7 @@ import (
const CAN_MANAGE = "CAN_MANAGE" const CAN_MANAGE = "CAN_MANAGE"
const CAN_VIEW = "CAN_VIEW" const CAN_VIEW = "CAN_VIEW"
const CAN_RUN = "CAN_RUN" const CAN_RUN = "CAN_RUN"
const IS_OWNER = "IS_OWNER"
var allowedLevels = []string{CAN_MANAGE, CAN_VIEW, CAN_RUN} var allowedLevels = []string{CAN_MANAGE, CAN_VIEW, CAN_RUN}
var levelsMap = map[string](map[string]string){ var levelsMap = map[string](map[string]string){

View File

@ -0,0 +1,110 @@
package permissions
import (
"context"
"fmt"
"sort"
"strings"
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/set"
)
type permissionDiagnostics struct{}
func PermissionDiagnostics() bundle.Mutator {
return &permissionDiagnostics{}
}
func (m *permissionDiagnostics) Name() string {
return "CheckPermissions"
}
func (m *permissionDiagnostics) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
if len(b.Config.Permissions) == 0 {
// Only warn if there is an explicit top-level permissions section
return nil
}
canManageBundle, _ := analyzeBundlePermissions(b)
if canManageBundle {
return nil
}
return diag.Diagnostics{{
Severity: diag.Warning,
Summary: fmt.Sprintf("permissions section should include %s or one of their groups with CAN_MANAGE permissions", b.Config.Workspace.CurrentUser.UserName),
Locations: []dyn.Location{b.Config.GetLocation("permissions")},
ID: diag.PermissionNotIncluded,
}}
}
// analyzeBundlePermissions analyzes the top-level permissions of the bundle.
// This permission set is important since it determines the permissions of the
// target workspace folder.
//
// Returns:
// - isManager: true if the current user is can manage the bundle resources.
// - assistance: advice on who to contact as to manage this project
func analyzeBundlePermissions(b *bundle.Bundle) (bool, string) {
canManageBundle := false
otherManagers := set.NewSet[string]()
if b.Config.RunAs != nil && b.Config.RunAs.UserName != "" && b.Config.RunAs.UserName != b.Config.Workspace.CurrentUser.UserName {
// The run_as user is another human that could be contacted
// about this bundle.
otherManagers.Add(b.Config.RunAs.UserName)
}
currentUser := b.Config.Workspace.CurrentUser.UserName
targetPermissions := b.Config.Permissions
for _, p := range targetPermissions {
if p.Level != CAN_MANAGE {
continue
}
if p.UserName == currentUser || p.ServicePrincipalName == currentUser {
canManageBundle = true
continue
}
if isGroupOfCurrentUser(b, p.GroupName) {
canManageBundle = true
continue
}
// Permission doesn't apply to current user; add to otherManagers
otherManager := p.UserName
if otherManager == "" {
otherManager = p.GroupName
}
if otherManager == "" {
// Skip service principals
continue
}
otherManagers.Add(otherManager)
}
assistance := "For assistance, contact the owners of this project."
if otherManagers.Size() > 0 {
list := otherManagers.Values()
sort.Strings(list)
assistance = fmt.Sprintf(
"For assistance, users or groups with appropriate permissions may include: %s.",
strings.Join(list, ", "),
)
}
return canManageBundle, assistance
}
func isGroupOfCurrentUser(b *bundle.Bundle, groupName string) bool {
currentUserGroups := b.Config.Workspace.CurrentUser.User.Groups
for _, g := range currentUserGroups {
if g.Display == groupName {
return true
}
}
return false
}

View File

@ -0,0 +1,52 @@
package permissions_test
import (
"context"
"testing"
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/permissions"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/databricks-sdk-go/service/iam"
"github.com/stretchr/testify/require"
)
func TestPermissionDiagnosticsApplySuccess(t *testing.T) {
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", UserName: "testuser@databricks.com"},
})
diags := permissions.PermissionDiagnostics().Apply(context.Background(), b)
require.NoError(t, diags.Error())
}
func TestPermissionDiagnosticsApplyFail(t *testing.T) {
b := mockBundle([]resources.Permission{
{Level: "CAN_VIEW", UserName: "testuser@databricks.com"},
})
diags := permissions.PermissionDiagnostics().Apply(context.Background(), b)
require.Equal(t, diags[0].Severity, diag.Warning)
require.Contains(t, diags[0].Summary, "permissions section should include testuser@databricks.com or one of their groups with CAN_MANAGE permissions")
}
func mockBundle(permissions []resources.Permission) *bundle.Bundle {
return &bundle.Bundle{
Config: config.Root{
Workspace: config.Workspace{
CurrentUser: &config.User{
User: &iam.User{
UserName: "testuser@databricks.com",
DisplayName: "Test User",
Groups: []iam.ComplexValue{
{Display: "testgroup"},
},
},
},
},
Permissions: permissions,
},
}
}

View File

@ -0,0 +1,51 @@
package permissions
import (
"context"
"fmt"
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/auth"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/log"
)
// ReportPossiblePermissionDenied generates a diagnostic message when a permission denied error is encountered.
//
// Note that since the workspace API doesn't always distinguish between permission denied and path errors,
// we must treat this as a "possible permission error". See acquire.go for more about this.
func ReportPossiblePermissionDenied(ctx context.Context, b *bundle.Bundle, path string) diag.Diagnostics {
log.Errorf(ctx, "Failed to update, encountered possible permission error: %v", path)
user := b.Config.Workspace.CurrentUser.UserName
if auth.IsServicePrincipal(user) {
user = b.Config.Workspace.CurrentUser.DisplayName
}
canManageBundle, assistance := analyzeBundlePermissions(b)
if !canManageBundle {
return diag.Diagnostics{{
Summary: fmt.Sprintf("unable to deploy to %s as %s.\n"+
"Please make sure the current user or one of their groups is listed under the permissions of this bundle.\n"+
"%s\n"+
"They may need to redeploy the bundle to apply the new permissions.\n"+
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions.",
path, user, assistance),
Severity: diag.Error,
ID: diag.PathPermissionDenied,
}}
}
// According databricks.yml, the current user has the right permissions.
// But we're still seeing permission errors. So someone else will need
// to redeploy the bundle with the right set of permissions.
return diag.Diagnostics{{
Summary: fmt.Sprintf("unable to deploy to %s as %s. Cannot apply local deployment permissions.\n"+
"%s\n"+
"They can redeploy the project to apply the latest set of permissions.\n"+
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions.",
path, user, assistance),
Severity: diag.Error,
ID: diag.CannotChangePathPermissions,
}}
}

View File

@ -0,0 +1,76 @@
package permissions_test
import (
"context"
"testing"
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/permissions"
"github.com/stretchr/testify/require"
)
func TestPermissionsReportPermissionDeniedWithGroup(t *testing.T) {
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", GroupName: "testgroup"},
})
diags := permissions.ReportPossiblePermissionDenied(context.Background(), b, "testpath")
expected := "EPERM3: unable to deploy to testpath as testuser@databricks.com. Cannot apply local deployment permissions.\n" +
"For assistance, contact the owners of this project.\n" +
"They can redeploy the project to apply the latest set of permissions.\n" +
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions."
require.ErrorContains(t, diags.Error(), expected)
}
func TestPermissionsReportPermissionDeniedWithOtherGroup(t *testing.T) {
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", GroupName: "othergroup"},
})
diags := permissions.ReportPossiblePermissionDenied(context.Background(), b, "testpath")
expected := "EPERM1: unable to deploy to testpath as testuser@databricks.com.\n" +
"Please make sure the current user or one of their groups is listed under the permissions of this bundle.\n" +
"For assistance, users or groups with appropriate permissions may include: othergroup.\n" +
"They may need to redeploy the bundle to apply the new permissions.\n" +
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions."
require.ErrorContains(t, diags.Error(), expected)
}
func TestPermissionsReportPermissionDeniedWithoutPermission(t *testing.T) {
b := mockBundle([]resources.Permission{
{Level: "CAN_VIEW", UserName: "testuser@databricks.com"},
})
diags := permissions.ReportPossiblePermissionDenied(context.Background(), b, "testpath")
expected := "EPERM1: unable to deploy to testpath as testuser@databricks.com.\n" +
"Please make sure the current user or one of their groups is listed under the permissions of this bundle.\n" +
"For assistance, contact the owners of this project.\n" +
"They may need to redeploy the bundle to apply the new permissions.\n" +
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions."
require.ErrorContains(t, diags.Error(), expected)
}
func TestPermissionsReportPermissionDeniedNilPermission(t *testing.T) {
b := mockBundle(nil)
diags := permissions.ReportPossiblePermissionDenied(context.Background(), b, "testpath")
expected := "EPERM1: unable to deploy to testpath as testuser@databricks.com.\n" +
"Please make sure the current user or one of their groups is listed under the permissions of this bundle.\n" +
"For assistance, contact the owners of this project.\n" +
"They may need to redeploy the bundle to apply the new permissions.\n" +
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions"
require.ErrorContains(t, diags.Error(), expected)
}
func TestPermissionsReportFindOtherOwners(t *testing.T) {
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", GroupName: "testgroup"},
{Level: "CAN_MANAGE", UserName: "alice@databricks.com"},
})
diags := permissions.ReportPossiblePermissionDenied(context.Background(), b, "testpath")
require.ErrorContains(t, diags.Error(), "EPERM3: unable to deploy to testpath as testuser@databricks.com. Cannot apply local deployment permissions.\n"+
"For assistance, users or groups with appropriate permissions may include: alice@databricks.com.\n"+
"They can redeploy the project to apply the latest set of permissions.\n"+
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions.")
}

View File

@ -0,0 +1,47 @@
package permissions
import (
"context"
"fmt"
"regexp"
"strings"
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/log"
)
func TryExtendTerraformPermissionError(ctx context.Context, b *bundle.Bundle, err error) diag.Diagnostics {
_, assistance := analyzeBundlePermissions(b)
// In a best-effort attempt to provide actionable error messages, we match
// against a few specific error messages that come from the Jobs and Pipelines API.
// For matching errors we provide a more specific error message that includes
// details on how to resolve the issue.
if !strings.Contains(err.Error(), "cannot update permissions") &&
!strings.Contains(err.Error(), "permissions on pipeline") &&
!strings.Contains(err.Error(), "cannot read permissions") &&
!strings.Contains(err.Error(), "cannot set run_as to user") {
return nil
}
log.Errorf(ctx, "Terraform error during deployment: %v", err.Error())
// Best-effort attempt to extract the resource name from the error message.
re := regexp.MustCompile(`databricks_(\w*)\.(\w*)`)
match := re.FindStringSubmatch(err.Error())
resource := "resource"
if len(match) > 1 {
resource = match[2]
}
return diag.Diagnostics{{
Summary: fmt.Sprintf("permission denied creating or updating %s.\n"+
"%s\n"+
"They can redeploy the project to apply the latest set of permissions.\n"+
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions.",
resource, assistance),
Severity: diag.Error,
ID: diag.ResourcePermissionDenied,
}}
}

View File

@ -0,0 +1,97 @@
package permissions_test
import (
"context"
"errors"
"testing"
"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/bundle/permissions"
"github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/stretchr/testify/require"
)
func TestTryExtendTerraformPermissionError1(t *testing.T) {
ctx := context.Background()
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", UserName: "alice@databricks.com"},
})
err := permissions.TryExtendTerraformPermissionError(ctx, b, errors.New("Error: terraform apply: exit status 1\n"+
"\n"+
"Error: cannot update permissions: ...\n"+
"\n"+
" with databricks_pipeline.my_project_pipeline,\n"+
" on bundle.tf.json line 39, in resource.databricks_pipeline.my_project_pipeline:\n"+
" 39: }")).Error()
expected := "EPERM2: permission denied creating or updating my_project_pipeline.\n" +
"For assistance, users or groups with appropriate permissions may include: alice@databricks.com.\n" +
"They can redeploy the project to apply the latest set of permissions.\n" +
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions"
require.ErrorContains(t, err, expected)
}
func TestTryExtendTerraformPermissionError2(t *testing.T) {
ctx := context.Background()
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", UserName: "alice@databricks.com"},
{Level: "CAN_MANAGE", UserName: "bob@databricks.com"},
})
err := permissions.TryExtendTerraformPermissionError(ctx, b, errors.New("Error: terraform apply: exit status 1\n"+
"\n"+
"Error: cannot read pipeline: User xyz does not have View permissions on pipeline 4521dbb6-42aa-418c-b94d-b5f4859a3454.\n"+
"\n"+
" with databricks_pipeline.my_project_pipeline,\n"+
" on bundle.tf.json line 39, in resource.databricks_pipeline.my_project_pipeline:\n"+
" 39: }")).Error()
expected := "EPERM2: permission denied creating or updating my_project_pipeline.\n" +
"For assistance, users or groups with appropriate permissions may include: alice@databricks.com, bob@databricks.com.\n" +
"They can redeploy the project to apply the latest set of permissions.\n" +
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions."
require.ErrorContains(t, err, expected)
}
func TestTryExtendTerraformPermissionError3(t *testing.T) {
ctx := context.Background()
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", UserName: "testuser@databricks.com"},
})
err := permissions.TryExtendTerraformPermissionError(ctx, b, errors.New("Error: terraform apply: exit status 1\n"+
"\n"+
"Error: cannot read permissions: 1706906c-c0a2-4c25-9f57-3a7aa3cb8b90 does not have Owner permissions on Job with ID: ElasticJobId(28263044278868). Please contact the owner or an administrator for access.\n"+
"\n"+
" with databricks_pipeline.my_project_pipeline,\n"+
" on bundle.tf.json line 39, in resource.databricks_pipeline.my_project_pipeline:\n"+
" 39: }")).Error()
expected := "EPERM2: permission denied creating or updating my_project_pipeline.\n" +
"For assistance, contact the owners of this project.\n" +
"They can redeploy the project to apply the latest set of permissions.\n" +
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions."
require.ErrorContains(t, err, expected)
}
func TestTryExtendTerraformPermissionErrorNotOwner(t *testing.T) {
ctx := context.Background()
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", GroupName: "data_team@databricks.com"},
})
b.Config.RunAs = &jobs.JobRunAs{
UserName: "testuser@databricks.com",
}
err := permissions.TryExtendTerraformPermissionError(ctx, b, errors.New("Error: terraform apply: exit status 1\n"+
"\n"+
"Error: cannot read pipeline: User xyz does not have View permissions on pipeline 4521dbb6-42aa-418c-b94d-b5f4859a3454.\n"+
"\n"+
" with databricks_pipeline.my_project_pipeline,\n"+
" on bundle.tf.json line 39, in resource.databricks_pipeline.my_project_pipeline:\n"+
" 39: }")).Error()
expected := "EPERM2: permission denied creating or updating my_project_pipeline.\n" +
"For assistance, users or groups with appropriate permissions may include: data_team@databricks.com.\n" +
"They can redeploy the project to apply the latest set of permissions.\n" +
"Please refer to https://docs.databricks.com/dev-tools/bundles/permissions.html for more on managing permissions."
require.ErrorContains(t, err, expected)
}

View File

@ -62,6 +62,8 @@ func Initialize() bundle.Mutator {
"workspace", "workspace",
"variables", "variables",
), ),
// Provide permission config errors & warnings after initializing all variables
permissions.PermissionDiagnostics(),
mutator.SetRunAs(), mutator.SetRunAs(),
mutator.OverrideCompute(), mutator.OverrideCompute(),
mutator.ProcessTargetMode(), mutator.ProcessTargetMode(),

View File

@ -3,7 +3,6 @@ package config_tests
import ( import (
"context" "context"
"fmt" "fmt"
"path/filepath"
"testing" "testing"
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
@ -113,8 +112,9 @@ func TestRunAsErrorForPipelines(t *testing.T) {
diags := bundle.Apply(ctx, b, mutator.SetRunAs()) diags := bundle.Apply(ctx, b, mutator.SetRunAs())
err := diags.Error() err := diags.Error()
configPath := filepath.FromSlash("run_as/not_allowed/pipelines/databricks.yml") assert.ErrorContains(t, err, "pipelines do not support a setting a run_as user that is different from the owner.\n"+
assert.EqualError(t, err, fmt.Sprintf("pipelines 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:14:5. Current identity: jane@doe.com. Run as identity: my_service_principal", configPath)) "Current identity: jane@doe.com. Run as identity: my_service_principal.\n"+
"See https://docs")
} }
func TestRunAsNoErrorForPipelines(t *testing.T) { func TestRunAsNoErrorForPipelines(t *testing.T) {
@ -152,8 +152,9 @@ func TestRunAsErrorForModelServing(t *testing.T) {
diags := bundle.Apply(ctx, b, mutator.SetRunAs()) diags := bundle.Apply(ctx, b, mutator.SetRunAs())
err := diags.Error() err := diags.Error()
configPath := filepath.FromSlash("run_as/not_allowed/model_serving/databricks.yml") assert.ErrorContains(t, err, "model_serving_endpoints do not support a setting a run_as user that is different from the owner.\n"+
assert.EqualError(t, err, fmt.Sprintf("model_serving_endpoints 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:14:5. Current identity: jane@doe.com. Run as identity: my_service_principal", configPath)) "Current identity: jane@doe.com. Run as identity: my_service_principal.\n"+
"See https://docs")
} }
func TestRunAsNoErrorForModelServingEndpoints(t *testing.T) { func TestRunAsNoErrorForModelServingEndpoints(t *testing.T) {
@ -191,8 +192,7 @@ func TestRunAsErrorWhenBothUserAndSpSpecified(t *testing.T) {
diags := bundle.Apply(ctx, b, mutator.SetRunAs()) diags := bundle.Apply(ctx, b, mutator.SetRunAs())
err := diags.Error() err := diags.Error()
configPath := filepath.FromSlash("run_as/not_allowed/both_sp_and_user/databricks.yml") assert.ErrorContains(t, err, "run_as section cannot specify both user_name and service_principal_name")
assert.EqualError(t, err, fmt.Sprintf("run_as section must specify exactly one identity. A service_principal_name \"my_service_principal\" is specified at %s:6:27. A user_name \"my_user_name\" is defined at %s:7:14", configPath, configPath))
} }
func TestRunAsErrorNeitherUserOrSpSpecified(t *testing.T) { func TestRunAsErrorNeitherUserOrSpSpecified(t *testing.T) {
@ -202,19 +202,19 @@ func TestRunAsErrorNeitherUserOrSpSpecified(t *testing.T) {
}{ }{
{ {
name: "empty_run_as", name: "empty_run_as",
err: fmt.Sprintf("run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified at %s:4:8", filepath.FromSlash("run_as/not_allowed/neither_sp_nor_user/empty_run_as/databricks.yml")), err: "run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified",
}, },
{ {
name: "empty_sp", name: "empty_sp",
err: fmt.Sprintf("run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified at %s:5:3", filepath.FromSlash("run_as/not_allowed/neither_sp_nor_user/empty_sp/databricks.yml")), err: "run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified",
}, },
{ {
name: "empty_user", name: "empty_user",
err: fmt.Sprintf("run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified at %s:5:3", filepath.FromSlash("run_as/not_allowed/neither_sp_nor_user/empty_user/databricks.yml")), err: "run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified",
}, },
{ {
name: "empty_user_and_sp", name: "empty_user_and_sp",
err: fmt.Sprintf("run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified at %s:5:3", filepath.FromSlash("run_as/not_allowed/neither_sp_nor_user/empty_user_and_sp/databricks.yml")), err: "run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified",
}, },
} }
@ -257,8 +257,7 @@ func TestRunAsErrorNeitherUserOrSpSpecifiedAtTargetOverride(t *testing.T) {
diags := bundle.Apply(ctx, b, mutator.SetRunAs()) diags := bundle.Apply(ctx, b, mutator.SetRunAs())
err := diags.Error() err := diags.Error()
configPath := filepath.FromSlash("run_as/not_allowed/neither_sp_nor_user/override/override.yml") assert.EqualError(t, err, "run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified")
assert.EqualError(t, err, fmt.Sprintf("run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified at %s:4:12", configPath))
} }
func TestLegacyRunAs(t *testing.T) { func TestLegacyRunAs(t *testing.T) {

View File

@ -1,6 +1,7 @@
package diag package diag
import ( import (
"errors"
"fmt" "fmt"
"github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn"
@ -24,6 +25,9 @@ type Diagnostic struct {
// Paths are paths to the values in the configuration tree that the diagnostic is associated with. // Paths are paths to the values in the configuration tree that the diagnostic is associated with.
// It may be nil if there are no associated paths. // It may be nil if there are no associated paths.
Paths []dyn.Path Paths []dyn.Path
// A diagnostic ID. Only used for select diagnostic messages.
ID ID
} }
// Errorf creates a new error diagnostic. // Errorf creates a new error diagnostic.
@ -69,7 +73,7 @@ func Infof(format string, args ...any) Diagnostics {
} }
} }
// Diagsnostics holds zero or more instances of [Diagnostic]. // Diagnostics holds zero or more instances of [Diagnostic].
type Diagnostics []Diagnostic type Diagnostics []Diagnostic
// Append adds a new diagnostic to the end of the list. // Append adds a new diagnostic to the end of the list.
@ -96,7 +100,14 @@ func (ds Diagnostics) HasError() bool {
func (ds Diagnostics) Error() error { func (ds Diagnostics) Error() error {
for _, d := range ds { for _, d := range ds {
if d.Severity == Error { if d.Severity == Error {
return fmt.Errorf(d.Summary) message := d.Detail
if message == "" {
message = d.Summary
}
if d.ID != "" {
message = string(d.ID) + ": " + message
}
return errors.New(message)
} }
} }
return nil return nil

16
libs/diag/id.go Normal file
View File

@ -0,0 +1,16 @@
package diag
type ID string
// For select diagnostic messages we use IDs to identify them
// for support or tooling purposes.
// It is a non-goal to have an exhaustive list of IDs.
const (
// We have many subtly different permission errors.
// These are numbered for easy reference and tooling support.
PathPermissionDenied ID = "EPERM1"
ResourcePermissionDenied ID = "EPERM2"
CannotChangePathPermissions ID = "EPERM3"
RunAsDenied ID = "EPERM4"
PermissionNotIncluded ID = "EPERM5"
)

View File

@ -103,6 +103,18 @@ func (err CannotDeleteRootError) Is(other error) bool {
return other == fs.ErrInvalid return other == fs.ErrInvalid
} }
type PermissionError struct {
path string
}
func (err PermissionError) Error() string {
return fmt.Sprintf("access denied: %s", err.path)
}
func (err PermissionError) Is(other error) bool {
return other == fs.ErrPermission
}
// Filer is used to access files in a workspace. // Filer is used to access files in a workspace.
// It has implementations for accessing files in WSFS and in DBFS. // It has implementations for accessing files in WSFS and in DBFS.
type Filer interface { type Filer interface {

View File

@ -178,6 +178,9 @@ func (w *workspaceFilesClient) Write(ctx context.Context, name string, reader io
// Create parent directory. // Create parent directory.
err = w.workspaceClient.Workspace.MkdirsByPath(ctx, path.Dir(absPath)) err = w.workspaceClient.Workspace.MkdirsByPath(ctx, path.Dir(absPath))
if err != nil { if err != nil {
if errors.As(err, &aerr) && aerr.StatusCode == http.StatusForbidden {
return PermissionError{absPath}
}
return fmt.Errorf("unable to mkdir to write file %s: %w", absPath, err) return fmt.Errorf("unable to mkdir to write file %s: %w", absPath, err)
} }
@ -203,6 +206,11 @@ func (w *workspaceFilesClient) Write(ctx context.Context, name string, reader io
return FileAlreadyExistsError{absPath} return FileAlreadyExistsError{absPath}
} }
// This API returns StatusForbidden when you have read access but don't have write access to a file
if aerr.StatusCode == http.StatusForbidden {
return PermissionError{absPath}
}
return err return err
} }
@ -295,11 +303,11 @@ func (w *workspaceFilesClient) ReadDir(ctx context.Context, name string) ([]fs.D
return nil, err return nil, err
} }
// This API returns a 404 if the specified path does not exist. // NOTE: This API returns a 404 if the specified path does not exist,
// but can also do so if we don't have read access.
if aerr.StatusCode == http.StatusNotFound { if aerr.StatusCode == http.StatusNotFound {
return nil, NoSuchDirectoryError{path.Dir(absPath)} return nil, NoSuchDirectoryError{path.Dir(absPath)}
} }
return nil, err return nil, err
} }

View File

@ -14,6 +14,11 @@ type Set[T any] struct {
data map[string]T data map[string]T
} }
// Values returns a slice of the set's values
func (s *Set[T]) Values() []T {
return maps.Values(s.data)
}
// NewSetFromF initialise a new set with initial values and a hash function // NewSetFromF initialise a new set with initial values and a hash function
// to define uniqueness of value // to define uniqueness of value
func NewSetFromF[T any](values []T, f hashFunc[T]) *Set[T] { func NewSetFromF[T any](values []T, f hashFunc[T]) *Set[T] {
@ -69,6 +74,11 @@ func (s *Set[T]) Has(item T) bool {
return ok return ok
} }
// Size returns the number of elements in the set
func (s *Set[T]) Size() int {
return len(s.data)
}
// Returns an iterable slice of values from set // Returns an iterable slice of values from set
func (s *Set[T]) Iter() []T { func (s *Set[T]) Iter() []T {
return maps.Values(s.data) return maps.Values(s.data)