Skip to content

direct: Limit server_side_default to regular fields #4038

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

Merged
merged 2 commits into from
Dec 1, 2025
Merged

direct: Limit server_side_default to regular fields #4038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0fe1738
direct: Limit server_side_default to regular fields
denik Dec 1, 2025
3d18057
add a comment
denik Dec 1, 2025
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
15 changes: 15 additions & 0 deletions acceptance/bundle/resources/permissions/jobs/added_remotely/databricks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
bundle:
name: test-bundle

resources:
jobs:
job_with_permissions:
name: job permissions added remotely
tasks:
- task_key: main
notebook_task:
notebook_path: /Workspace/Users/user@example.com/notebook
source: WORKSPACE
permissions:
- level: CAN_VIEW
user_name: viewer@example.com
102 changes: 102 additions & 0 deletions acceptance/bundle/resources/permissions/jobs/added_remotely/out.plan.direct.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
{
"plan": {
"resources.jobs.job_with_permissions": {
"action": "skip",
"remote_state": {
"created_time": [UNIX_TIME_MILLIS],
"creator_user_name": "[USERNAME]",
"job_id": [JOB_ID],
"run_as_user_name": "[USERNAME]",
"settings": {
"deployment": {
"kind": "BUNDLE",
"metadata_file_path": "/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/state/metadata.json"
},
"edit_mode": "UI_LOCKED",
"email_notifications": {},
"format": "MULTI_TASK",
"max_concurrent_runs": 1,
"name": "job permissions added remotely",
"queue": {
"enabled": true
},
"tasks": [
{
"notebook_task": {
"notebook_path": "/Workspace/Users/user@example.com/notebook",
"source": "WORKSPACE"
},
"task_key": "main"
}
],
"timeout_seconds": 0,
"webhook_notifications": {}
}
},
"changes": {
"remote": {
"email_notifications": {
"action": "skip",
"reason": "server_side_default"
},
"timeout_seconds": {
"action": "skip",
"reason": "server_side_default"
},
"webhook_notifications": {
"action": "skip",
"reason": "server_side_default"
}
}
}
},
"resources.jobs.job_with_permissions.permissions": {
"depends_on": [
{
"node": "resources.jobs.job_with_permissions",
"label": "${resources.jobs.job_with_permissions.id}"
}
],
"action": "update",
"new_state": {
"value": {
"object_id": "/jobs/[JOB_ID]",
"permissions": [
{
"permission_level": "CAN_VIEW",
"user_name": "viewer@example.com"
},
{
"permission_level": "IS_OWNER",
"user_name": "[USERNAME]"
}
]
}
},
"remote_state": {
"object_id": "/jobs/[JOB_ID]",
"permissions": [
{
"permission_level": "IS_OWNER",
"user_name": "[USERNAME]"
},
{
"permission_level": "CAN_VIEW",
"user_name": "viewer@example.com"
},
{
"group_name": "admin-team",
"permission_level": "CAN_MANAGE"
}
]
},
"changes": {
"remote": {
"permissions[group_name='admin-team']": {
"action": "update"
}
}
}
}
}
}
10 changes: 10 additions & 0 deletions acceptance/bundle/resources/permissions/jobs/added_remotely/out.plan.terraform.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"plan": {
"resources.jobs.job_with_permissions": {
"action": "skip"
},
"resources.jobs.job_with_permissions.permissions": {
"action": "update"
}
}
}
5 changes: 5 additions & 0 deletions acceptance/bundle/resources/permissions/jobs/added_remotely/out.test.toml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

161 changes: 161 additions & 0 deletions acceptance/bundle/resources/permissions/jobs/added_remotely/output.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@

>>> [CLI] bundle plan
create jobs.job_with_permissions
create jobs.job_with_permissions.permissions

Plan: 2 to add, 0 to change, 0 to delete, 0 unchanged

>>> [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
Deploying resources...
Updating deployment state...
Deployment complete!

>>> [CLI] bundle plan
Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged

>>> [CLI] permissions get jobs [JOB_ID]
{
"access_control_list": [
{
"all_permissions": [
{
"inherited":false,
"permission_level":"CAN_VIEW"
}
],
"display_name":"viewer@example.com",
"user_name":"viewer@example.com"
},
{
"all_permissions": [
{
"inherited":false,
"permission_level":"IS_OWNER"
}
],
"display_name":"[USERNAME]",
"user_name":"[USERNAME]"
},
{
"all_permissions": [
{
"inherited":true,
"inherited_from_object": [
"/jobs/"
],
"permission_level":"CAN_MANAGE"
}
],
"group_name":"admins"
}
],
"object_id":"/jobs/[JOB_ID]",
"object_type":"job"
}

=== Add permissions out of band
>>> [CLI] permissions set jobs [JOB_ID] --json @remote_add.json
{
"access_control_list": [
{
"all_permissions": [
{
"inherited":false,
"permission_level":"IS_OWNER"
}
],
"display_name":"[USERNAME]",
"user_name":"[USERNAME]"
},
{
"all_permissions": [
{
"inherited":false,
"permission_level":"CAN_VIEW"
}
],
"display_name":"viewer@example.com",
"user_name":"viewer@example.com"
},
{
"all_permissions": [
{
"inherited":false,
"permission_level":"CAN_MANAGE"
}
],
"group_name":"admin-team"
},
{
"all_permissions": [
{
"inherited":true,
"inherited_from_object": [
"/jobs/"
],
"permission_level":"CAN_MANAGE"
}
],
"group_name":"admins"
}
],
"object_id":"/jobs/[JOB_ID]",
"object_type":"job"
}

>>> [CLI] bundle plan
update jobs.job_with_permissions.permissions

Plan: 0 to add, 1 to change, 0 to delete, 1 unchanged

>>> [CLI] bundle plan -o json

>>> [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files...
Deploying resources...
Updating deployment state...
Deployment complete!

>>> [CLI] bundle plan
Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged

>>> [CLI] permissions get jobs [JOB_ID]
{
"access_control_list": [
{
"all_permissions": [
{
"inherited":false,
"permission_level":"CAN_VIEW"
}
],
"display_name":"viewer@example.com",
"user_name":"viewer@example.com"
},
{
"all_permissions": [
{
"inherited":false,
"permission_level":"IS_OWNER"
}
],
"display_name":"[USERNAME]",
"user_name":"[USERNAME]"
},
{
"all_permissions": [
{
"inherited":true,
"inherited_from_object": [
"/jobs/"
],
"permission_level":"CAN_MANAGE"
}
],
"group_name":"admins"
}
],
"object_id":"/jobs/[JOB_ID]",
"object_type":"job"
}
7 changes: 7 additions & 0 deletions acceptance/bundle/resources/permissions/jobs/added_remotely/remote_add.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"access_control_list": [
{"permission_level": "IS_OWNER", "user_name": "tester@databricks.com"},
{"permission_level": "CAN_VIEW", "user_name": "viewer@example.com"},
{"permission_level": "CAN_MANAGE", "group_name": "admin-team"}
]
}
18 changes: 18 additions & 0 deletions acceptance/bundle/resources/permissions/jobs/added_remotely/script
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
trace $CLI bundle plan
trace $CLI bundle deploy
trace $CLI bundle plan

job_id="$(read_id.py jobs job_with_permissions)"
echo "$job_id:JOB_ID" >> ACC_REPLS

trace $CLI permissions get jobs "$job_id"

title "Add permissions out of band"
trace $CLI permissions set jobs "$job_id" --json @remote_add.json

trace $CLI bundle plan
trace $CLI bundle plan -o json > out.plan.$DATABRICKS_BUNDLE_ENGINE.json
trace $CLI bundle deploy
trace $CLI bundle plan

trace $CLI permissions get jobs "$job_id"
1 change: 1 addition & 0 deletions acceptance/bundle/resources/permissions/jobs/added_remotely/test.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
RecordRequests = false
4 changes: 3 additions & 1 deletion bundle/direct/bundle_plan.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,12 @@ func interpretOldStateVsRemoteState(ctx context.Context, adapter *dresources.Ada
m := make(map[string]deployplan.Trigger)

for _, ch := range diff {
if ch.Old == nil {
if ch.Old == nil && ch.Path.IsStringKey() {
Copy link
Contributor

@pietern pietern Dec 1, 2025

Choose a reason for hiding this comment

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

Can you include a comment that explains the reasoning?

This check enforces that server side defaults are only allowed for paths that end with a struct/map key. Is that correct?

Copy link
Contributor Author

@denik denik Dec 1, 2025

Choose a reason for hiding this comment

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

added 3d18057

// The field was not set by us, but comes from the remote state.
// This could either be server-side default or a policy.
// In any case, this is not a change we should react to.
// Note, we only consider StringKeys here, because indexes and key-value pairs refer to slices and we want to react to new element in slices.
Copy link
Contributor

@shreyas-goenka shreyas-goenka Dec 1, 2025

Choose a reason for hiding this comment

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

Why do key-value pairs refer to slices? Can you extend the comment to explain?

Copy link
Contributor Author

@denik denik Dec 1, 2025

Choose a reason for hiding this comment

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

Key value pairs in paths were added to index slices by key in #4014

// Note, IsStringKey is also too broad - it currently covers struct fields and map keys, we don't want to include map keys here.
Copy link
Contributor

@shreyas-goenka shreyas-goenka Dec 1, 2025

Choose a reason for hiding this comment

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

We might also want to include map keys here. For example tags can have a server side default. See warehouse tags for example:

  "tags": {
    "custom_tags": [
      {
        "key":"Owner",
        "value":"eng-dev-ecosystem-team_at_databricks.com"
      }
    ]

Eventually we might need to push for proto level annotations for these.

Copy link
Contributor Author

@denik denik Dec 1, 2025

Choose a reason for hiding this comment

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

For now focus is on matching Terraform which AFAIK does not have server side default concept for tags.

m[ch.Path.String()] = deployplan.Trigger{
Action: deployplan.ActionTypeSkipString,
Reason: "server_side_default",
Expand Down
4 changes: 4 additions & 0 deletions libs/structs/structpath/path.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ func (p *PathNode) KeyValue() (key, value string, ok bool) {
return "", "", false
}

func (p *PathNode) IsStringKey() bool {
return p != nil && p.index == tagStringKey
}

// StringKey returns either Field() or MapKey() if either is available
func (p *PathNode) StringKey() (string, bool) {
if p == nil {
Expand Down