Uh oh!
There was an error while loading. Please reload this page.
Fix secret scope permissions migration from Terraform to Direct engine - #4866
Merged
Conversation
Collaborator
Commit: 2a198db
19 interesting tests: 10 SKIP, 7 KNOWN, 2 flaky
Top 20 slowest tests (at least 2 minutes):
|
denikforce-pushed
the
denik/fix-secrets-migration
branch
from
March 31, 2026 11:04
1f437ff to
9434908Comparedenikforce-pushed
the
denik/fix-secrets-migration
branch
from
March 31, 2026 15:51
9434908 to
2a198dbComparedenik
marked this pull request as ready for review
April 1, 2026 06:57
denik
requested review from
andrewnester, anton-107, pietern, shreyas-goenka and simonfaltum
as code ownersApril 1, 2026 06:57
denik
enabled auto-merge
April 1, 2026 06:58
Contributor
Suggested reviewersBased on git history of the changed files, these people are best suited to review:
Confidence: high Eligible reviewersBased on CODEOWNERS, these people or teams could also review: @andrewnester, @anton-107, @shreyas-goenka, @simonfaltum Suggestions based on git history of 20 changed files (9 scored). See CODEOWNERS for path-specific ownership rules. |
During migration, Terraform's databricks_secret_acl resources are not tracked in the migration state. The direct engine manages secret scope permissions as a sub-resource (secret_scopes.*.permissions), so without a state entry, the post-migration plan shows a "create" action. Add state entries for secret_scopes.*.permissions after migration Apply to prevent phantom drift. Co-authored-by: Isaac
Add databricks_secret_acl to TerraformToGroupName mapping and handle it in parseResourcesState, similar to how databricks_permissions and databricks_grants are handled. Multiple ACL resources per scope map to a single .permissions entry with the scope name as ID. Co-authored-by: Isaac
…rcesState ParseResourcesState now creates .permissions entries for ALL secret scopes (not just those with databricks_secret_acl), so the post-Apply fixup in migrate.go is no longer needed. Co-authored-by: Isaac
Add a test config with a secret scope that has two explicit ACL permissions (READ for users, WRITE for account users). This exercises the multi-ACL migration path in parseResourcesState. Co-authored-by: Isaac
During migration, SecretScopeFixups wasn't running (it's part of PreDeployChecks which migration skips). For scopes without explicit ACLs, parseResourcesState synthesizes .permissions entries in the state, but the config didn't have them, causing a Delete→forced Update→no StateCache entry error chain. Fix: apply SecretScopeFixups(EngineDirect) before CalculatePlan so the config and state agree on .permissions entries. Also simplify the identical if/else branches in the safety net. Co-authored-by: Isaac
The 'account users' group doesn't exist on all test workspaces, causing cloud test failures. Co-authored-by: Isaac
The convertSecretAclResourceNameToKey function and its dedicated databricks_secret_acl branch in parseResourcesState are no longer needed — all secret scope permissions are now handled uniformly through the post-processing loop. Co-authored-by: Isaac
Add SecretScopeFixups(EngineDirect) before reading b.Config.Value() so the config includes .permissions entries for secret scopes. Without this, CalculatePlan sees .permissions in state but not in config and produces incorrect plan entries during YAML sync conversion. Co-authored-by: Isaac
…sState Co-authored-by: Isaac
Check silentlyUpdatedResources before logging unknown resource type warning, matching the pattern used in showplanfile.go. This prevents misleading "Unknown Terraform resource type: databricks_secret_acl" warnings during normal secret scope migrations. Co-authored-by: Isaac
Replace the single-entry map with a direct type check for databricks_secret_acl in both showplanfile.go and util.go. Co-authored-by: Isaac
Add "secret_acls" mapping for databricks_secret_acl instead of special-casing it in the unknown-type check. In parseResourcesState, secret_acls are skipped via the switch (post-processing creates .permissions entries). In populatePlan, secret ACL changes are mapped to resources.secret_scopes.<key>.permissions with GetHigherAction for merging multiple ACL changes per scope. Task: 017.md Co-authored-by: Isaac
Co-authored-by: Isaac
Secret ACL changes now appear as .permissions entries in the Terraform plan output, reflecting the new TerraformToGroupName mapping. Task: 017.md Co-authored-by: Isaac
Instead of a post-processing loop that adds .permissions for every secret scope, create the entry directly in the "secret_scopes" case. This keeps all resource types handled inside the switch. Task: 018.md
When multiple secret ACL changes for the same scope include both creates and deletes, report the net action as "update" instead of "delete". GetHigherAction picks the highest severity (delete > create), but mixed ACL changes represent a permissions update, not a deletion. Task: 019.md Co-authored-by: Isaac
The previous isCreateDeleteMix helper only handled the narrow case of separate Create and Delete actions. In practice, Terraform produces Recreate (delete+create pair) for ACLs with changed principals, mixed with Delete for removed principals. GetHigherAction(Recreate, Delete) returned Delete (severity 7>6), incorrectly reporting permissions as deleted rather than updated. Simplify the logic: for secret ACLs, any mix of different action types means permissions are being updated. Only same-action merges (e.g., all Recreate when scope is recreated) keep the original action. Task-review: /Users/denis.bilenko/work/prompts/features/fix-secrets-migration/021.SUMMARY.md Co-authored-by: Isaac
denikforce-pushed
the
denik/fix-secrets-migration
branch
from
April 1, 2026 07:00
a3675b6 to
b7abfb4Compareandrewnester
approved these changes
Apr 1, 2026
denikforce-pushed
the
denik/fix-secrets-migration
branch
from
April 1, 2026 14:23
6a02590 to
c8f3b0eCompareUh oh!
There was an error while loading. Please reload this page.
deco-sdk-taggingBot
added a commit
that referenced
this pull request
Apr 8, 2026
## Release v0.296.0 ### Notable Changes * Direct deployment engine for DABs is now in Public Preview. Documentation at [docs/direct.md](docs/direct.md). ### CLI * Auth commands now error when --profile and --host conflict ([#4841](#4841)) * Add `--force-refresh` flag to `databricks auth token` to force a token refresh even when the cached token is still valid ([#4767](#4767)) ### Bundles * Deduplicate grant entries with duplicate principals or privileges during initialization ([#4801](#4801)) * Fix `bundle deployment bind` to always pull remote state before modifying ([#4892](#4892)) * engine/direct: Fix drift in grants resource due to privilege reordering ([#4794](#4794)) * engine/direct: Fix 400 error when deploying grants with ALL_PRIVILEGES ([#4801](#4801)) * engine/direct: Fix unwanted recreation of secret scopes when scope_backend_type is not set ([#4834](#4834)) * engine/direct: Fix bind and unbind for non-Terraform resources ([#4850](#4850)) * engine/direct: Fix deploying removed principals ([#4824](#4824)) * engine/direct: Fix secret scope permissions migration from Terraform to Direct engine ([#4866](#4866))
denik added a commit
that referenced
this pull request
May 20, 2026
#4866) ## Summary - Fix `bundle deployment migrate` for bundles with secret scopes to prevent phantom drift on `secret_scopes.*.permissions` after migration from Terraform to Direct engine. - Handle `databricks_secret_acl` in `ParseResourcesState`: multiple ACL resources per scope are mapped to a single `.permissions` state entry with the scope name as ID, similar to how `databricks_permissions` and `databricks_grants` are handled. - Expose resources.secret_scopes.foo.permissions as a separate entry in terraform JSON plan as well to match direct engine. ## Test plan - Re-enable the previously excluded secret scope migration acceptance tests. - New invariant test config for secret scope with ACLs.
denik pushed a commit
that referenced
this pull request
May 20, 2026
## Release v0.296.0 ### Notable Changes * Direct deployment engine for DABs is now in Public Preview. Documentation at [docs/direct.md](docs/direct.md). ### CLI * Auth commands now error when --profile and --host conflict ([#4841](#4841)) * Add `--force-refresh` flag to `databricks auth token` to force a token refresh even when the cached token is still valid ([#4767](#4767)) ### Bundles * Deduplicate grant entries with duplicate principals or privileges during initialization ([#4801](#4801)) * Fix `bundle deployment bind` to always pull remote state before modifying ([#4892](#4892)) * engine/direct: Fix drift in grants resource due to privilege reordering ([#4794](#4794)) * engine/direct: Fix 400 error when deploying grants with ALL_PRIVILEGES ([#4801](#4801)) * engine/direct: Fix unwanted recreation of secret scopes when scope_backend_type is not set ([#4834](#4834)) * engine/direct: Fix bind and unbind for non-Terraform resources ([#4850](#4850)) * engine/direct: Fix deploying removed principals ([#4824](#4824)) * engine/direct: Fix secret scope permissions migration from Terraform to Direct engine ([#4866](#4866))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
bundle deployment migratefor bundles with secret scopes to prevent phantom drift onsecret_scopes.*.permissionsafter migration from Terraform to Direct engine.databricks_secret_aclinParseResourcesState: multiple ACL resources per scope are mapped to a single.permissionsstate entry with the scope name as ID, similar to howdatabricks_permissionsanddatabricks_grantsare handled.Test plan