Skip to content

Commit 7c783aa

Browse files
bundle: warn during deploy when workspace folder permissions exceed the bundle's
ValidateFolderPermissions already compares the live workspace ACL against the declared permissions, but it only runs during `bundle validate`. This brings the same check to `bundle deploy` without adding any API latency: ApplyWorkspaceRoot- Permissions already calls SetPermissions on each workspace path prefix (root_path and, when separate, state_path), and the response carries the resulting ACL. Reusing that response, we compare against the declared permissions. Because the Set replaces the folder's direct ACL with the declared set, any principal still showing higher access is inherited from a parent folder — the broader access that actually persists after deploy, which is the scope mismatch worth surfacing. No extra GetPermissions round trip is made. The check is skipped for /Workspace/ Shared paths, consistent with the existing behavior. Co-authored-by: Shreyas Goenka <shreyas.goenka@databricks.com>
1 parent cc47397 commit 7c783aa

2 files changed

Lines changed: 99 additions & 26 deletions

File tree

bundle/permissions/workspace_root.go

Lines changed: 40 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strconv"
77

88
"github.com/databricks/cli/bundle"
9+
"github.com/databricks/cli/bundle/config/resources"
910
"github.com/databricks/cli/bundle/libraries"
1011
"github.com/databricks/cli/bundle/paths"
1112
"github.com/databricks/cli/libs/diag"
@@ -25,66 +26,85 @@ func (*workspaceRootPermissions) Name() string {
2526

2627
// Apply implements bundle.Mutator.
2728
func (*workspaceRootPermissions) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
28-
err := giveAccessForWorkspaceRoot(ctx, b)
29-
if err != nil {
30-
return diag.FromErr(err)
31-
}
32-
33-
return nil
29+
return giveAccessForWorkspaceRoot(ctx, b)
3430
}
3531

36-
func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) error {
37-
var permissions []workspace.WorkspaceObjectAccessControlRequest
32+
func giveAccessForWorkspaceRoot(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
33+
var acl []workspace.WorkspaceObjectAccessControlRequest
3834

3935
for _, p := range b.Config.Permissions {
4036
level, err := GetWorkspaceObjectPermissionLevel(string(p.Level))
4137
if err != nil {
42-
return err
38+
return diag.FromErr(err)
4339
}
4440

45-
permissions = append(permissions, workspace.WorkspaceObjectAccessControlRequest{
41+
acl = append(acl, workspace.WorkspaceObjectAccessControlRequest{
4642
GroupName: p.GroupName,
4743
UserName: p.UserName,
4844
ServicePrincipalName: p.ServicePrincipalName,
4945
PermissionLevel: level,
5046
})
5147
}
5248

53-
if len(permissions) == 0 {
49+
if len(acl) == 0 {
5450
return nil
5551
}
5652

5753
w := b.WorkspaceClient(ctx).Workspace
5854
bundlePaths := paths.CollectUniqueWorkspacePathPrefixes(b.Config.Workspace)
5955

56+
// Each goroutine writes its own slot, so the results are merged after Wait
57+
// rather than logged concurrently.
58+
results := make([]diag.Diagnostics, len(bundlePaths))
6059
g, ctx := errgroup.WithContext(ctx)
61-
for _, p := range bundlePaths {
60+
for i, p := range bundlePaths {
6261
g.Go(func() error {
63-
return setPermissions(ctx, w, p, permissions)
62+
diags, err := setPermissions(ctx, w, p, acl, b.Config.Permissions)
63+
results[i] = diags
64+
return err
6465
})
6566
}
6667

67-
return g.Wait()
68+
if err := g.Wait(); err != nil {
69+
return diag.FromErr(err)
70+
}
71+
72+
var diags diag.Diagnostics
73+
for _, r := range results {
74+
diags = diags.Extend(r)
75+
}
76+
return diags
6877
}
6978

70-
func setPermissions(ctx context.Context, w workspace.WorkspaceInterface, path string, permissions []workspace.WorkspaceObjectAccessControlRequest) error {
79+
// setPermissions applies the bundle's declared permissions to the workspace folder
80+
// and, by reusing the SetPermissions response, warns when the folder's effective
81+
// permissions are broader than the bundle declares — without an additional API call.
82+
//
83+
// The Set replaces the folder's direct ACL with the declared permissions, so any
84+
// principal that still shows higher access in the response is inherited from a
85+
// parent folder. That inherited access persists after the deploy, which is exactly
86+
// the scope mismatch worth surfacing.
87+
func setPermissions(ctx context.Context, w workspace.WorkspaceInterface, path string, acl []workspace.WorkspaceObjectAccessControlRequest, declared []resources.Permission) (diag.Diagnostics, error) {
7188
// 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.
7289
if libraries.IsWorkspaceSharedPath(path) {
73-
return nil
90+
return nil, nil
7491
}
7592

7693
obj, err := w.GetStatusByPath(ctx, path) //nolint:staticcheck // Deprecated in SDK v0.127.0. Migration to WorkspaceHierarchyService tracked separately.
7794
if err != nil {
78-
return err
95+
return nil, err
7996
}
8097

81-
_, err = w.SetPermissions(ctx, workspace.WorkspaceObjectPermissionsRequest{
98+
resp, err := w.SetPermissions(ctx, workspace.WorkspaceObjectPermissionsRequest{
8299
WorkspaceObjectId: strconv.FormatInt(obj.ObjectId, 10),
83100
WorkspaceObjectType: "directories",
84-
AccessControlList: permissions,
101+
AccessControlList: acl,
85102
})
103+
if err != nil {
104+
return nil, err
105+
}
86106

87-
return err
107+
return ObjectAclToResourcePermissions(path, resp.AccessControlList).Compare(declared), nil
88108
}
89109

90110
func GetWorkspaceObjectPermissionLevel(bundlePermission string) (workspace.WorkspaceObjectPermissionLevel, error) {

bundle/permissions/workspace_root_test.go

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"github.com/databricks/cli/bundle"
77
"github.com/databricks/cli/bundle/config"
88
"github.com/databricks/cli/bundle/config/resources"
9+
"github.com/databricks/cli/libs/diag"
910
"github.com/databricks/databricks-sdk-go/experimental/mocks"
1011
"github.com/databricks/databricks-sdk-go/service/jobs"
1112
"github.com/databricks/databricks-sdk-go/service/ml"
@@ -70,7 +71,7 @@ func TestApplyWorkspaceRootPermissions(t *testing.T) {
7071
},
7172
WorkspaceObjectId: "1234",
7273
WorkspaceObjectType: "directories",
73-
}).Return(nil, nil)
74+
}).Return(&workspace.WorkspaceObjectPermissions{}, nil)
7475

7576
diags := bundle.ApplySeq(t.Context(), b, ValidateWorkspacePermissions(), ApplyWorkspaceRootPermissions())
7677
require.Empty(t, diags)
@@ -143,7 +144,7 @@ func TestApplyWorkspaceRootPermissionsForAllPaths(t *testing.T) {
143144
},
144145
WorkspaceObjectId: "1",
145146
WorkspaceObjectType: "directories",
146-
}).Return(nil, nil)
147+
}).Return(&workspace.WorkspaceObjectPermissions{}, nil)
147148

148149
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
149150
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
@@ -153,7 +154,7 @@ func TestApplyWorkspaceRootPermissionsForAllPaths(t *testing.T) {
153154
},
154155
WorkspaceObjectId: "2",
155156
WorkspaceObjectType: "directories",
156-
}).Return(nil, nil)
157+
}).Return(&workspace.WorkspaceObjectPermissions{}, nil)
157158

158159
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
159160
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
@@ -163,7 +164,7 @@ func TestApplyWorkspaceRootPermissionsForAllPaths(t *testing.T) {
163164
},
164165
WorkspaceObjectId: "3",
165166
WorkspaceObjectType: "directories",
166-
}).Return(nil, nil)
167+
}).Return(&workspace.WorkspaceObjectPermissions{}, nil)
167168

168169
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
169170
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
@@ -173,7 +174,7 @@ func TestApplyWorkspaceRootPermissionsForAllPaths(t *testing.T) {
173174
},
174175
WorkspaceObjectId: "4",
175176
WorkspaceObjectType: "directories",
176-
}).Return(nil, nil)
177+
}).Return(&workspace.WorkspaceObjectPermissions{}, nil)
177178

178179
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
179180
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
@@ -183,8 +184,60 @@ func TestApplyWorkspaceRootPermissionsForAllPaths(t *testing.T) {
183184
},
184185
WorkspaceObjectId: "5",
185186
WorkspaceObjectType: "directories",
186-
}).Return(nil, nil)
187+
}).Return(&workspace.WorkspaceObjectPermissions{}, nil)
187188

188189
diags := bundle.Apply(t.Context(), b, ApplyWorkspaceRootPermissions())
189190
require.NoError(t, diags.Error())
190191
}
192+
193+
// TestApplyWorkspaceRootPermissionsWarnsOnBroaderPermissions verifies that the
194+
// SetPermissions response is reused to detect permissions broader than the bundle
195+
// declares. Here the folder inherits CAN_MANAGE for a group that is not in the
196+
// bundle's permissions, so a warning is expected without any extra API call.
197+
func TestApplyWorkspaceRootPermissionsWarnsOnBroaderPermissions(t *testing.T) {
198+
b := &bundle.Bundle{
199+
Config: config.Root{
200+
Workspace: config.Workspace{
201+
RootPath: "/Users/foo@bar.test",
202+
ArtifactPath: "/Users/foo@bar.test/artifacts",
203+
FilePath: "/Users/foo@bar.test/files",
204+
StatePath: "/Users/foo@bar.test/state",
205+
ResourcePath: "/Users/foo@bar.test/resources",
206+
},
207+
Permissions: []resources.Permission{
208+
{Level: CAN_MANAGE, UserName: "foo@bar.test"},
209+
},
210+
},
211+
}
212+
213+
m := mocks.NewMockWorkspaceClient(t)
214+
b.SetWorkpaceClient(m.WorkspaceClient)
215+
workspaceApi := m.GetMockWorkspaceAPI()
216+
workspaceApi.EXPECT().GetStatusByPath(mock.Anything, "/Users/foo@bar.test").Return(&workspace.ObjectInfo{
217+
ObjectId: 1234,
218+
}, nil)
219+
workspaceApi.EXPECT().SetPermissions(mock.Anything, workspace.WorkspaceObjectPermissionsRequest{
220+
AccessControlList: []workspace.WorkspaceObjectAccessControlRequest{
221+
{UserName: "foo@bar.test", PermissionLevel: "CAN_MANAGE"},
222+
},
223+
WorkspaceObjectId: "1234",
224+
WorkspaceObjectType: "directories",
225+
}).Return(&workspace.WorkspaceObjectPermissions{
226+
AccessControlList: []workspace.WorkspaceObjectAccessControlResponse{
227+
{
228+
UserName: "foo@bar.test",
229+
AllPermissions: []workspace.WorkspaceObjectPermission{{PermissionLevel: "CAN_MANAGE"}},
230+
},
231+
{
232+
GroupName: "data-engineers",
233+
AllPermissions: []workspace.WorkspaceObjectPermission{{PermissionLevel: "CAN_MANAGE", Inherited: true}},
234+
},
235+
},
236+
}, nil)
237+
238+
diags := bundle.Apply(t.Context(), b, ApplyWorkspaceRootPermissions())
239+
require.Len(t, diags, 1)
240+
require.Equal(t, diag.Warning, diags[0].Severity)
241+
require.Equal(t, "workspace folder has permissions not configured in bundle", diags[0].Summary)
242+
require.Contains(t, diags[0].Detail, "data-engineers")
243+
}

0 commit comments

Comments
 (0)