diff --git a/bundle/permissions/permission_diagnostics.go b/bundle/permissions/permission_diagnostics.go index 5b19dd026..c0677c65d 100644 --- a/bundle/permissions/permission_diagnostics.go +++ b/bundle/permissions/permission_diagnostics.go @@ -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 { diff --git a/bundle/permissions/permission_diagnostics_test.go b/bundle/permissions/permission_diagnostics_test.go index 8660e9574..fa913df91 100644 --- a/bundle/permissions/permission_diagnostics_test.go +++ b/bundle/permissions/permission_diagnostics_test.go @@ -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 { diff --git a/bundle/permissions/terraform_errors_test.go b/bundle/permissions/terraform_errors_test.go index 1a4ef6c43..c9b5b1786 100644 --- a/bundle/permissions/terraform_errors_test.go +++ b/bundle/permissions/terraform_errors_test.go @@ -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) } diff --git a/libs/filer/workspace_files_client.go b/libs/filer/workspace_files_client.go index 1f76be832..43b8ddab1 100644 --- a/libs/filer/workspace_files_client.go +++ b/libs/filer/workspace_files_client.go @@ -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} }