Skip to content

Fixed bundle deploy to not update permissions for unbound resources#3642

Merged
andrewnester merged 10 commits intomainfrom
fix/unbind-permissions
Sep 24, 2025
Merged

Fixed bundle deploy to not update permissions for unbound resources#3642
andrewnester merged 10 commits intomainfrom
fix/unbind-permissions

Conversation

@andrewnester
Copy link
Copy Markdown
Contributor

@andrewnester andrewnester commented Sep 22, 2025 •
edited
Loading

Changes

Fixed bundle deploy to not update permissions for unbound resources

Why

The original issue occurred because we hadn't removed the permissions section for the corresponding resources from the TF state. and therefore, permissions were continued to be managed by TF and, as a result, cleared out.

Tests

Added an acceptance test

andrewnester
andrewnester commented Sep 22, 2025
View reviewed changes
libs/testserver/jobs.go
}

s.Jobs[jobId] = jobs.Job{JobId: jobId, Settings: &jobSettings}
s.Jobs[jobId] = jobs.Job{JobId: jobId, Settings: &jobSettings, CreatorUserName: TestUser.UserName}
Copy link
Copy Markdown
Contributor Author

@andrewnester andrewnester Sep 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

CreatorUserName field is used by TF to check if the resource exists or not. CreatorUserName should be non-empty for the resource to be considered as "exists"

https://github.com/databricks/terraform-provider-databricks/blob/main/permissions/permission_definitions.go#L108

Copy link
Copy Markdown
Contributor

@pietern pietern Sep 24, 2025

Choose a reason for hiding this comment

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

Useful context, please include as a comment to persist it.

@eng-dev-ecosystem-bot
Copy link
Copy Markdown
Collaborator

eng-dev-ecosystem-bot commented Sep 22, 2025 •
edited
Loading

Run: 17972827773

Env ✅​pass 🙈​skip
✅​ aws linux 312 530
✅​ aws windows 313 529
✅​ aws-ucws linux 425 427
✅​ aws-ucws windows 426 426
✅​ azure linux 312 529
✅​ azure windows 313 528
✅​ azure-ucws linux 425 426
✅​ azure-ucws windows 426 425
✅​ gcp linux 311 531
✅​ gcp windows 312 530

shreyas-goenka
shreyas-goenka reviewed Sep 23, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@shreyas-goenka shreyas-goenka left a comment

Choose a reason for hiding this comment

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

This PR fixes the issue for all resource types right? Can we fix that in the PR title?

@andrewnester andrewnester changed the title Fixed bundle deploy to not update permissions for unbound jobs Fixed bundle deploy to not update permissions for unbound resources Sep 23, 2025
pietern
pietern reviewed Sep 24, 2025
View reviewed changes
libs/testserver/jobs.go
}

s.Jobs[jobId] = jobs.Job{JobId: jobId, Settings: &jobSettings}
s.Jobs[jobId] = jobs.Job{JobId: jobId, Settings: &jobSettings, CreatorUserName: TestUser.UserName}
Copy link
Copy Markdown
Contributor

@pietern pietern Sep 24, 2025

Choose a reason for hiding this comment

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

Useful context, please include as a comment to persist it.

pietern
pietern approved these changes Sep 24, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

Can you confirm if the acceptance test fails if unbind doesn't remove the permissions from the state? The permissions for the first and second resources are identical, and the resource key in the state is identical as well.

libs/testserver/fake_workspace.go
Monitors map[string]catalog.MonitorInfo
Apps map[string]apps.App
Schemas map[string]catalog.SchemaInfo
SchemasGrants map[string][]catalog.PrivilegeAssignment
Copy link
Copy Markdown
Contributor

@pietern pietern Sep 24, 2025

Choose a reason for hiding this comment

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

Nit: should also be singular, SchemaGrants, as in "grants for a schema".

@andrewnester
Copy link
Copy Markdown
Contributor Author

andrewnester commented Sep 24, 2025

@pietern yes, I wrote the acceptance test with this failure first and confirmed that removing the fix makes the test fail

=== NAME  TestAccept/bundle/deployment/unbind/permissions
    acceptance_test.go:803: Diff:
        --- bundle/deployment/unbind/permissions/output.txt
        +++ /var/folders/nt/xjv68qzs45319w4k36dhpylc0000gp/T/TestAcceptbundledeploymentunbindpermissions1723835858/001/output.txt
        @@ -29,15 +29,6 @@
         
         === Permissions should be the same as before unbind
         >>> [CLI] jobs get-permissions [NUMID] --output json
        -{
        -  "all_permissions": [
        -    {
        -      "inherited": false,
        -      "permission_level": "CAN_MANAGE"
        -    }
        -  ],
        -  "group_name": "users"
        -}

         === Grants should be the same as before unbind
         >>> [CLI] grants get schema main.test-schema-[UNIQUE_NAME] --output json
        -{
        -  "principal": "account users",
        -  "privileges": [
        -    "CREATE_VOLUME",
        -    "SELECT"
        -  ]
        -}
        +jq: error (at <stdin>:1): Cannot iterate over null (null)

@andrewnester andrewnester added this pull request to the merge queue Sep 24, 2025
Merged via the queue into main with commit 4291bcc Sep 24, 2025
13 checks passed
@andrewnester andrewnester deleted the fix/unbind-permissions branch September 24, 2025 11:37
deco-sdk-tagging bot added a commit that referenced this pull request Sep 24, 2025
@deco-sdk-tagging
[Release] Release v0.270.0
a8ac8e0 
## Release v0.270.0

### Notable Changes
* Add 'databricks bundle plan' command. This command shows the deployment plan for the current bundle configuration without making any changes. ([#3530](#3530))

### Bundles
* Add 'databricks bundle plan' command ([#3530](#3530))
* Add new Lakeflow Pipelines support for bundle generate ([#3568](#3568))
* Fix bundle deploy to not update permissions or grants for unbound resources ([#3642](#3642))
* Introduce new bundle variable: `${workspace.current_user.domain_friendly_name}` ([#3623](#3623))
* Improve the output of bundle run when bundle is not deployed ([#3652](#3652))
@github-actions github-actions bot mentioned this pull request Jan 3, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pietern pietern pietern approved these changes

@denik denik Awaiting requested review from denik denik is a code owner

@anton-107 anton-107 Awaiting requested review from anton-107 anton-107 is a code owner

@shreyas-goenka shreyas-goenka Awaiting requested review from shreyas-goenka shreyas-goenka is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@andrewnester @eng-dev-ecosystem-bot @pietern @shreyas-goenka