Skip to content

Commit a2ee990

Browse files
andrewnesterandrewnester
authored
Add a warning when resources with explicit permissions defined as app resources (#4876)
## Changes Add a warning when resources with explicit permissions defined as app resources ## Why When an app has resources (jobs, SQL warehouses, serving endpoints, experiments, Postgres projects, or UC securables) defined in its resources section that reference bundle-configured resources with explicit permissions, the app's service principal gets implicitly granted access on first deploy. However, on subsequent deploys the bundle overwrites the resource's permissions without the app's SP, breaking the app's access. This PR adds a warning during bundle validate when a referenced resource has permissions set but doesn't include the app's service principal. The warning recommends explicitly adding the SP to the resource's permissions to prevent this silent permission override. See #4309 for details. Fixes #4309 ## Tests Added an acceptance test <!-- If your PR needs to be included in the release notes for next release, add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->
1 parent b282372 commit a2ee990

10 files changed

Lines changed: 389 additions & 0 deletions

File tree

‎acceptance/bundle/apps/job_permissions/output.txt‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,19 @@
11

22
>>> [CLI] bundle deploy
3+
Warning: app "my_app" references jobs "my_job" which has permissions set. To prevent permission override after deploying the app, please add the app service principal to the jobs permissions
4+
at resources.apps.my_app
5+
in databricks.yml:24:7
6+
7+
Add the following section to the jobs permissions:
8+
9+
resources:
10+
jobs:
11+
my_job:
12+
permissions:
13+
- level: CAN_MANAGE_RUN
14+
service_principal_name: ${resources.apps.my_app.service_principal_client_id}
15+
16+
317
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files...
418
Deploying resources...
519
Updating deployment state...
@@ -10,6 +24,20 @@ Deployment complete!
1024

1125
=== After second deploy
1226
>>> [CLI] bundle deploy
27+
Warning: app "my_app" references jobs "my_job" which has permissions set. To prevent permission override after deploying the app, please add the app service principal to the jobs permissions
28+
at resources.apps.my_app
29+
in databricks.yml:24:7
30+
31+
Add the following section to the jobs permissions:
32+
33+
resources:
34+
jobs:
35+
my_job:
36+
permissions:
37+
- level: CAN_MANAGE_RUN
38+
service_principal_name: ${resources.apps.my_app.service_principal_client_id}
39+
40+
1341
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files...
1442
Deploying resources...
1543
Updating deployment state...

‎acceptance/bundle/apps/job_permissions_warning/app/app.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
print("hello")

‎acceptance/bundle/apps/job_permissions_warning/databricks.yml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
bundle:
2+
name: test-bundle
3+
4+
resources:
5+
jobs:
6+
my_job:
7+
name: my-job
8+
permissions:
9+
- level: CAN_MANAGE
10+
user_name: ${workspace.current_user.userName}
11+
tasks:
12+
- task_key: hello
13+
spark_python_task:
14+
python_file: ./hello.py
15+
new_cluster:
16+
num_workers: 1
17+
spark_version: 14.0.x-scala2.13
18+
node_type_id: i3.xlarge
19+
20+
apps:
21+
my_app:
22+
name: myapp
23+
source_code_path: ./app
24+
resources:
25+
- name: my-job
26+
job:
27+
id: ${resources.jobs.my_job.id}
28+
permission: CAN_MANAGE_RUN

‎acceptance/bundle/apps/job_permissions_warning/hello.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
print("hello")

‎acceptance/bundle/apps/job_permissions_warning/out.test.toml‎

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎acceptance/bundle/apps/job_permissions_warning/output.txt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
2+
>>> [CLI] bundle validate
3+
Warning: app "my_app" references jobs "my_job" which has permissions set. To prevent permission override after deploying the app, please add the app service principal to the jobs permissions
4+
at resources.apps.my_app
5+
in databricks.yml:22:7
6+
7+
Add the following section to the jobs permissions:
8+
9+
resources:
10+
jobs:
11+
my_job:
12+
permissions:
13+
- level: CAN_MANAGE_RUN
14+
service_principal_name: ${resources.apps.my_app.service_principal_client_id}
15+
16+
17+
Name: test-bundle
18+
Target: default
19+
Workspace:
20+
User: [USERNAME]
21+
Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default
22+
23+
Found 1 warning

‎acceptance/bundle/apps/job_permissions_warning/script‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
trace $CLI bundle validate

‎acceptance/bundle/apps/job_permissions_warning/test.toml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
RecordRequests = false
2+
3+
Ignore = [
4+
'.databricks',
5+
]

‎bundle/apps/validate.go‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,18 @@ package apps
33
import (
44
"context"
55
"fmt"
6+
"regexp"
67

78
"github.com/databricks/cli/bundle"
9+
"github.com/databricks/cli/bundle/config/resources"
810
"github.com/databricks/cli/libs/diag"
11+
"github.com/databricks/cli/libs/dyn"
12+
"github.com/databricks/databricks-sdk-go/service/apps"
913
)
1014

15+
// resourceReferencePattern matches ${resources.<type>.<key>.<field>} variable references.
16+
var resourceReferencePattern = regexp.MustCompile(`\$\{resources\.(\w+)\.([^.]+)\.\w+\}`)
17+
1118
type validate struct{}
1219

1320
func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
@@ -44,6 +51,130 @@ func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics
4451
})
4552
}
4653
usedSourceCodePaths[app.SourceCodePath] = key
54+
55+
diags = diags.Extend(warnForAppResourcePermissions(b, key, app))
56+
}
57+
58+
return diags
59+
}
60+
61+
// appResourceRef extracts resource references from an app resource entry.
62+
// When resourceType is empty the reference matches any bundle resource type
63+
// extracted from the variable reference pattern.
64+
func appResourceRef(r apps.AppResource) (appResourceReference, bool) {
65+
switch {
66+
case r.Job != nil:
67+
return appResourceReference{"jobs", r.Job.Id, string(r.Job.Permission)}, true
68+
case r.SqlWarehouse != nil:
69+
return appResourceReference{"sql_warehouses", r.SqlWarehouse.Id, string(r.SqlWarehouse.Permission)}, true
70+
case r.ServingEndpoint != nil:
71+
return appResourceReference{"model_serving_endpoints", r.ServingEndpoint.Name, string(r.ServingEndpoint.Permission)}, true
72+
case r.Experiment != nil:
73+
return appResourceReference{"experiments", r.Experiment.ExperimentId, string(r.Experiment.Permission)}, true
74+
case r.Postgres != nil:
75+
if r.Postgres.Branch != "" {
76+
return appResourceReference{"postgres_projects", r.Postgres.Branch, string(r.Postgres.Permission)}, true
77+
}
78+
return appResourceReference{
79+
"database_instances", r.Postgres.Database, string(r.Postgres.Permission),
80+
}, true
81+
case r.UcSecurable != nil:
82+
return appResourceReference{string(r.UcSecurable.SecurableType), r.UcSecurable.SecurableFullName, string(r.UcSecurable.Permission)}, true
83+
default:
84+
return appResourceReference{}, false
85+
}
86+
}
87+
88+
type appResourceReference struct {
89+
resourceType string
90+
refValue string
91+
permission string
92+
}
93+
94+
// hasPermissions checks if a bundle resource at the given dyn path has a non-empty permissions list.
95+
func hasPermissions(b *bundle.Bundle, resourcePath string) bool {
96+
pv, err := dyn.Get(b.Config.Value(), resourcePath+".permissions")
97+
if err != nil {
98+
return false
99+
}
100+
s, ok := pv.AsSequence()
101+
return ok && len(s) > 0
102+
}
103+
104+
// hasAppSPInPermissions checks if any permission entry for the given resource
105+
// references the app's service principal via variable interpolation.
106+
func hasAppSPInPermissions(b *bundle.Bundle, resourcePath, appKey string) bool {
107+
appSPRef := fmt.Sprintf("${resources.apps.%s.service_principal_client_id}", appKey)
108+
pv, err := dyn.Get(b.Config.Value(), resourcePath+".permissions")
109+
if err != nil {
110+
return false
111+
}
112+
s, ok := pv.AsSequence()
113+
if !ok {
114+
return false
115+
}
116+
for _, entry := range s {
117+
spn, err := dyn.Get(entry, "service_principal_name")
118+
if err != nil {
119+
continue
120+
}
121+
if str, ok := spn.AsString(); ok && str == appSPRef {
122+
return true
123+
}
124+
}
125+
return false
126+
}
127+
128+
// warnForAppResourcePermissions warns when an app references a bundle resource
129+
// that has explicit permissions but doesn't include the app's service principal.
130+
// Without the SP in the permission list, the second deploy will overwrite the
131+
// app-granted permission on the resource.
132+
// See https://github.com/databricks/cli/issues/4309
133+
func warnForAppResourcePermissions(b *bundle.Bundle, appKey string, app *resources.App) diag.Diagnostics {
134+
var diags diag.Diagnostics
135+
136+
for _, ar := range app.Resources {
137+
ref, ok := appResourceRef(ar)
138+
if !ok {
139+
continue
140+
}
141+
142+
matches := resourceReferencePattern.FindStringSubmatch(ref.refValue)
143+
if len(matches) < 3 {
144+
continue
145+
}
146+
refType, resourceKey := matches[1], matches[2]
147+
148+
resourcePath := fmt.Sprintf("resources.%s.%s", refType, resourceKey)
149+
if !hasPermissions(b, resourcePath) {
150+
continue
151+
}
152+
153+
if hasAppSPInPermissions(b, resourcePath, appKey) {
154+
continue
155+
}
156+
157+
appPath := "resources.apps." + appKey
158+
diags = append(diags, diag.Diagnostic{
159+
Severity: diag.Warning,
160+
Summary: fmt.Sprintf("app %q references %s %q which has permissions set. To prevent permission override after deploying the app, please add the app service principal to the %s permissions", appKey, refType, resourceKey, refType),
161+
Detail: fmt.Sprintf(
162+
"Add the following section to the %s permissions:\n\n"+
163+
" resources:\n"+
164+
" %s:\n"+
165+
" %s:\n"+
166+
" permissions:\n"+
167+
" - level: %s\n"+
168+
" service_principal_name: ${resources.apps.%s.service_principal_client_id}\n",
169+
refType,
170+
refType,
171+
resourceKey,
172+
ref.permission,
173+
appKey,
174+
),
175+
Paths: []dyn.Path{dyn.MustPathFromString(appPath)},
176+
Locations: b.Config.GetLocations(appPath),
177+
})
47178
}
48179

49180
return diags

0 commit comments

Comments
 (0)