From ac71d2e5ce0eea242ebffe01aa06c69f38e2a790 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 30 Oct 2024 18:34:11 +0100 Subject: [PATCH] Fixed adding /Workspace prefix for resource paths (#1866) ## Changes `/Workspace` prefix needs to be added to `resource_path` as well. Fixes the issue mentioned here: https://github.com/databricks/cli/pull/1822#issuecomment-2447697498 Fixes #1867 ## Tests Added regression test --- bundle/config/mutator/prepend_workspace_prefix.go | 1 + bundle/config/mutator/prepend_workspace_prefix_test.go | 3 +++ bundle/config/mutator/process_target_mode_test.go | 2 ++ bundle/config/validate/folder_permissions.go | 6 ++++++ bundle/paths/paths.go | 4 ++-- bundle/permissions/workspace_root.go | 6 ++++++ 6 files changed, 20 insertions(+), 2 deletions(-) diff --git a/bundle/config/mutator/prepend_workspace_prefix.go b/bundle/config/mutator/prepend_workspace_prefix.go index dd467344..de71bf7f 100644 --- a/bundle/config/mutator/prepend_workspace_prefix.go +++ b/bundle/config/mutator/prepend_workspace_prefix.go @@ -32,6 +32,7 @@ func (m *prependWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) di dyn.NewPattern(dyn.Key("workspace"), dyn.Key("file_path")), dyn.NewPattern(dyn.Key("workspace"), dyn.Key("artifact_path")), dyn.NewPattern(dyn.Key("workspace"), dyn.Key("state_path")), + dyn.NewPattern(dyn.Key("workspace"), dyn.Key("resource_path")), } err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { diff --git a/bundle/config/mutator/prepend_workspace_prefix_test.go b/bundle/config/mutator/prepend_workspace_prefix_test.go index 287c694d..6fbadec5 100644 --- a/bundle/config/mutator/prepend_workspace_prefix_test.go +++ b/bundle/config/mutator/prepend_workspace_prefix_test.go @@ -41,6 +41,7 @@ func TestPrependWorkspacePrefix(t *testing.T) { ArtifactPath: tc.path, FilePath: tc.path, StatePath: tc.path, + ResourcePath: tc.path, }, }, } @@ -51,6 +52,7 @@ func TestPrependWorkspacePrefix(t *testing.T) { require.Equal(t, tc.expected, b.Config.Workspace.ArtifactPath) require.Equal(t, tc.expected, b.Config.Workspace.FilePath) require.Equal(t, tc.expected, b.Config.Workspace.StatePath) + require.Equal(t, tc.expected, b.Config.Workspace.ResourcePath) } } @@ -76,4 +78,5 @@ func TestPrependWorkspaceForDefaultConfig(t *testing.T) { require.Equal(t, "/Workspace/Users/jane@doe.com/.bundle/test/dev/artifacts", b.Config.Workspace.ArtifactPath) require.Equal(t, "/Workspace/Users/jane@doe.com/.bundle/test/dev/files", b.Config.Workspace.FilePath) require.Equal(t, "/Workspace/Users/jane@doe.com/.bundle/test/dev/state", b.Config.Workspace.StatePath) + require.Equal(t, "/Workspace/Users/jane@doe.com/.bundle/test/dev/resources", b.Config.Workspace.ResourcePath) } diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 4346e88f..d76d2d8f 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -283,6 +283,7 @@ func TestValidateDevelopmentMode(t *testing.T) { b.Config.Workspace.StatePath = "/Users/lennart@company.com/.bundle/x/y/state" b.Config.Workspace.FilePath = "/Users/lennart@company.com/.bundle/x/y/files" b.Config.Workspace.ArtifactPath = "/Users/lennart@company.com/.bundle/x/y/artifacts" + b.Config.Workspace.ResourcePath = "/Users/lennart@company.com/.bundle/x/y/resources" diags = validateDevelopmentMode(b) require.NoError(t, diags.Error()) } @@ -311,6 +312,7 @@ func TestProcessTargetModeProduction(t *testing.T) { b.Config.Workspace.StatePath = "/Shared/.bundle/x/y/state" b.Config.Workspace.ArtifactPath = "/Shared/.bundle/x/y/artifacts" b.Config.Workspace.FilePath = "/Shared/.bundle/x/y/files" + b.Config.Workspace.ResourcePath = "/Shared/.bundle/x/y/resources" diags = validateProductionMode(context.Background(), b, false) require.ErrorContains(t, diags.Error(), "production") diff --git a/bundle/config/validate/folder_permissions.go b/bundle/config/validate/folder_permissions.go index 88502ec8..505e82a1 100644 --- a/bundle/config/validate/folder_permissions.go +++ b/bundle/config/validate/folder_permissions.go @@ -6,6 +6,7 @@ import ( "path" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/bundle/paths" "github.com/databricks/cli/bundle/permissions" "github.com/databricks/cli/libs/diag" @@ -47,6 +48,11 @@ func (f *folderPermissions) Apply(ctx context.Context, b bundle.ReadOnlyBundle) } func checkFolderPermission(ctx context.Context, b bundle.ReadOnlyBundle, folderPath string) diag.Diagnostics { + // If the folder is shared, then we don't need to check permissions as it was already checked in the other mutator before. + if libraries.IsWorkspaceSharedPath(folderPath) { + return nil + } + w := b.WorkspaceClient().Workspace obj, err := getClosestExistingObject(ctx, w, folderPath) if err != nil { diff --git a/bundle/paths/paths.go b/bundle/paths/paths.go index 50b75a6c..e3a6b0ae 100644 --- a/bundle/paths/paths.go +++ b/bundle/paths/paths.go @@ -10,7 +10,7 @@ import ( func CollectUniqueWorkspacePathPrefixes(workspace config.Workspace) []string { rootPath := workspace.RootPath paths := []string{} - if !libraries.IsVolumesPath(rootPath) && !libraries.IsWorkspaceSharedPath(rootPath) { + if !libraries.IsVolumesPath(rootPath) { paths = append(paths, rootPath) } @@ -24,7 +24,7 @@ func CollectUniqueWorkspacePathPrefixes(workspace config.Workspace) []string { workspace.StatePath, workspace.ResourcePath, } { - if libraries.IsWorkspaceSharedPath(p) || libraries.IsVolumesPath(p) { + if libraries.IsVolumesPath(p) { continue } diff --git a/bundle/permissions/workspace_root.go b/bundle/permissions/workspace_root.go index 4ab8198b..de4f3a7f 100644 --- a/bundle/permissions/workspace_root.go +++ b/bundle/permissions/workspace_root.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/bundle/paths" "github.com/databricks/cli/libs/diag" "github.com/databricks/databricks-sdk-go/service/workspace" @@ -67,6 +68,11 @@ func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error { } func setPermissions(ctx context.Context, w workspace.WorkspaceInterface, path string, permissions []workspace.WorkspaceObjectAccessControlRequest) error { + // If the folder is shared, then we don't need to set permissions since it's always set for all users and it's checked in mutators before. + if libraries.IsWorkspaceSharedPath(path) { + return nil + } + obj, err := w.GetStatusByPath(ctx, path) if err != nil { return err