steve/databricks-cli
steve
/
databricks-cli
mirror of https://github.com/databricks/cli.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Harden code with deeper tests

This commit is contained in:
Lennart Kats 2024-08-22 21:34:09 +02:00
parent 26c3b421e8
commit f93d39430c
No known key found for this signature in database
GPG Key ID: 1EB8B57673197023
4 changed files with 94 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
bundle/permissions/permission_diagnostics.go
View File

@ -53,7 +53,7 @@ func (m *permissionDiagnostics) Apply(ctx context.Context, b *bundle.Bundle) dia
func analyzeBundlePermissions(b *bundle.Bundle) (bool, string) {
canManageBundle := false
otherManagers := set.NewSet[string]()
if b.Config.RunAs != nil && b.Config.RunAs.UserName != "" {
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)
@ -117,6 +117,9 @@ func ReportPossiblePermissionDenied(ctx context.Context, b *bundle.Bundle, path
log.Errorf(ctx, "Failed to update, encountered possible permission error: %v", path)
user := b.Config.Workspace.CurrentUser.DisplayName
if user == "" {
user = b.Config.Workspace.CurrentUser.UserName
}
canManageBundle, assistance := analyzeBundlePermissions(b)
if !canManageBundle {

44
bundle/permissions/permission_diagnostics_test.go
View File

@ -29,7 +29,7 @@ func TestPermissionDiagnosticsApplyFail(t *testing.T) {
diags := permissions.PermissionDiagnostics().Apply(context.Background(), b)
require.Equal(t, diags[0].Severity, diag.Warning)
require.Contains(t, diags[0].Summary, "testuser@databricks.com")
require.Contains(t, diags[0].Summary, "permissions section should include testuser@databricks.com or one of their groups with CAN_MANAGE permissions")
}
func TestPermissionDiagnosticsApplySuccessWithOwner(t *testing.T) {
@ -41,13 +41,32 @@ func TestPermissionDiagnosticsApplySuccessWithOwner(t *testing.T) {
require.Empty(t, diags)
}
func TestPermissionDiagnosticsPermissionDeniedWithPermission(t *testing.T) {
func TestPermissionDiagnosticsPermissionDeniedWithGroup(t *testing.T) {
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", GroupName: "testgroup"},
})
diags := permissions.ReportPossiblePermissionDenied(context.Background(), b, "testpath")
require.ErrorContains(t, diags.Error(), string(diag.CannotChangePathPermissions))
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: testgroup@databricks.com.\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 TestPermissionDiagnosticsPermissionDeniedWithOtherGroup(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 TestPermissionDiagnosticsPermissionDeniedWithoutPermission(t *testing.T) {
@ -56,14 +75,24 @@ func TestPermissionDiagnosticsPermissionDeniedWithoutPermission(t *testing.T) {
})
diags := permissions.ReportPossiblePermissionDenied(context.Background(), b, "testpath")
require.ErrorContains(t, diags.Error(), string(diag.PathPermissionDenied))
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 TestPermissionDiagnosticsPermissionDeniedNilPermission(t *testing.T) {
b := mockBundle(nil)
diags := permissions.ReportPossiblePermissionDenied(context.Background(), b, "testpath")
require.ErrorContains(t, diags.Error(), string(diag.PathPermissionDenied))
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 TestPermissionDiagnosticsFindOtherOwners(t *testing.T) {
@ -73,7 +102,10 @@ func TestPermissionDiagnosticsFindOtherOwners(t *testing.T) {
})
diags := permissions.ReportPossiblePermissionDenied(context.Background(), b, "testpath")
require.ErrorContains(t, diags.Error(), "include: alice@databricks.com")
require.ErrorContains(t, diags.Error(), "EPERM3: access denied while updating deployment permissions as testuser@databricks.com.\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.")
}
func mockBundle(permissions []resources.Permission) *bundle.Bundle {

81
bundle/permissions/terraform_errors_test.go
View File

@ -7,7 +7,6 @@ import (
"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/jobs"
"github.com/stretchr/testify/require"
)
@ -17,60 +16,82 @@ func TestTryExtendTerraformPermissionError1(t *testing.T) {
b := mockBundle([]resources.Permission{
{Level: "CAN_MANAGE", UserName: "alice@databricks.com"},
})
err := permissions.TryExtendTerraformPermissionError(ctx, b, errors.New(`Error: terraform apply: exit status 1
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()
Error: cannot update permissions: ...
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"
with databricks_pipeline.my_project_pipeline,
on bundle.tf.json line 39, in resource.databricks_pipeline.my_project_pipeline:
39: }`)).Error()
require.ErrorContains(t, err, string(diag.ResourcePermissionDenied))
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
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()
Error: cannot read pipeline: User xyz does not have View permissions on pipeline 4521dbb6-42aa-418c-b94d-b5f4859a3454.
with databricks_pipeline.my_project_pipeline,
on bundle.tf.json line 39, in resource.databricks_pipeline.my_project_pipeline:
39: }`)).Error()
require.ErrorContains(t, err, string(diag.ResourcePermissionDenied))
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: "alice@databricks.com"},
{Level: "CAN_MANAGE", UserName: "testuser@databricks.com"},
})
err := permissions.TryExtendTerraformPermissionError(ctx, b, errors.New(`Error: terraform apply: exit status 1
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()
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.
with databricks_pipeline.my_project_pipeline,
on bundle.tf.json line 39, in resource.databricks_pipeline.my_project_pipeline:
39: }`)).Error()
require.ErrorContains(t, err, string(diag.ResourcePermissionDenied))
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", UserName: "alice@databricks.com"},
{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
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()
Error: cannot read pipeline: User xyz does not have View permissions on pipeline 4521dbb6-42aa-418c-b94d-b5f4859a3454.
with databricks_pipeline.my_project_pipeline,
on bundle.tf.json line 39, in resource.databricks_pipeline.my_project_pipeline:
39: }`)).Error()
require.ErrorContains(t, err, string(diag.ResourcePermissionDenied))
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)
}

1
libs/filer/workspace_files_client.go
View File

@ -206,6 +206,7 @@ func (w *workspaceFilesClient) Write(ctx context.Context, name string, reader io
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}
}