Skip to content

Commit

Permalink
Fixed adding /Workspace prefix for resource paths (#1866)
Browse files Browse the repository at this point in the history
## Changes
`/Workspace` prefix needs to be added to `resource_path` as well.

Fixes the issue mentioned here:
#1822 (comment)

Fixes #1867 

## Tests
Added regression test
  • Loading branch information
andrewnester authored Oct 30, 2024
1 parent 001a8da commit ac71d2e
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 2 deletions.
1 change: 1 addition & 0 deletions bundle/config/mutator/prepend_workspace_prefix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions bundle/config/mutator/prepend_workspace_prefix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func TestPrependWorkspacePrefix(t *testing.T) {
ArtifactPath: tc.path,
FilePath: tc.path,
StatePath: tc.path,
ResourcePath: tc.path,
},
},
}
Expand All @@ -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)
}
}

Expand All @@ -76,4 +78,5 @@ func TestPrependWorkspaceForDefaultConfig(t *testing.T) {
require.Equal(t, "/Workspace/Users/[email protected]/.bundle/test/dev/artifacts", b.Config.Workspace.ArtifactPath)
require.Equal(t, "/Workspace/Users/[email protected]/.bundle/test/dev/files", b.Config.Workspace.FilePath)
require.Equal(t, "/Workspace/Users/[email protected]/.bundle/test/dev/state", b.Config.Workspace.StatePath)
require.Equal(t, "/Workspace/Users/[email protected]/.bundle/test/dev/resources", b.Config.Workspace.ResourcePath)
}
2 changes: 2 additions & 0 deletions bundle/config/mutator/process_target_mode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ func TestValidateDevelopmentMode(t *testing.T) {
b.Config.Workspace.StatePath = "/Users/[email protected]/.bundle/x/y/state"
b.Config.Workspace.FilePath = "/Users/[email protected]/.bundle/x/y/files"
b.Config.Workspace.ArtifactPath = "/Users/[email protected]/.bundle/x/y/artifacts"
b.Config.Workspace.ResourcePath = "/Users/[email protected]/.bundle/x/y/resources"
diags = validateDevelopmentMode(b)
require.NoError(t, diags.Error())
}
Expand Down Expand Up @@ -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")
Expand Down
6 changes: 6 additions & 0 deletions bundle/config/validate/folder_permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions bundle/paths/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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
}

Expand Down
6 changes: 6 additions & 0 deletions bundle/permissions/workspace_root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit ac71d2e

Please sign in to comment.