Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed adding /Workspace prefix for resource paths #1866

Merged
merged 3 commits into from
Oct 30, 2024
Merged

Fixed adding /Workspace prefix for resource paths #1866

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
489e98d
Fixed adding /Workspace prefix for resource paths
andrewnester Oct 30, 2024
94ea143
don't call set permissions on shared folder
andrewnester Oct 30, 2024
f57ed7d
Update workspace_root.go
andrewnester Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bundle/config/mutator/prepend_workspace_prefix.go
View file
Open in desktop
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
View file
Open in desktop
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/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)
}
2 changes: 2 additions & 0 deletions bundle/config/mutator/process_target_mode_test.go
View file
Open in desktop
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/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())
}
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
View file
Open in desktop
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
}

andrewnester marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
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
View file
Open in desktop
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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per discussion here #1821 (comment)

return nil
}

obj, err := w.GetStatusByPath(ctx, path)
if err != nil {
return err
Expand Down
Loading