direct: grants: Fix "Duplicate privileges" error and drift on duplicates #4801
Merged
direct: grants: Fix "Duplicate privileges" error and drift on duplicates #4801
Conversation
…LEGES The direct engine's applyGrants() puts ALL_PRIVILEGES in both Add and Remove lists, causing the backend to reject with "Duplicate privileges to add and delete for principal". Co-authored-by: Isaac
Match the real backend behavior that returns INVALID_PARAMETER_VALUE when the same privilege appears in both Add and Remove lists for a principal. Co-authored-by: Isaac
- duplicate_principals: same principal listed twice with same privilege causes drift on direct engine after deploy - all_privileges: remove unnecessary sed, add Badness to both tests Co-authored-by: Isaac
… tests - duplicate_privileges: same privilege listed twice for same principal causes drift on direct engine - Use cleanup() function with trap EXIT for destroy in all three tests Co-authored-by: Isaac
Show the actual PATCH requests to the permissions endpoint for each engine, making the differences visible (e.g. direct sends ALL_PRIVILEGES in both Add and Remove, terraform sends clean requests). Co-authored-by: Isaac
denik
requested review from
andrewnester,
anton-107,
pietern,
shreyas-goenka and
simonfaltum
as code owners
Eligible reviewersCould not determine reviewers from git history. Based on CODEOWNERS, these people or teams could review: @andrewnester, @anton-107, @pietern, @shreyas-goenka, @simonfaltum Suggestions based on git history of 28 changed files (0 scored). See CODEOWNERS for path-specific ownership rules. |
Collaborator
|
Commit: 0f8d5e6
20 interesting tests: 9 SKIP, 7 KNOWN, 4 flaky
Top 20 slowest tests (at least 2 minutes):
|
denik
changed the title
denik
changed the title
Merge privileges by principal and deduplicate before sending to the API. Skip adding ALL_PRIVILEGES to the Remove list when it is being granted, preventing the "Duplicate privileges to add and delete" backend error. Co-authored-by: Isaac
Grants with duplicate principals or duplicate privileges were only deduplicated at deploy time in applyGrants(), causing the plan to report spurious changes. Move deduplication to the initialize phase as a normalize mutator, consistent with MergeJobClusters et al. Co-authored-by: Isaac
Deduplication is now handled by the MergeGrants mutator during initialization, so applyGrants can iterate directly. Co-authored-by: Isaac
Use AsString instead of MustString to avoid panics on unexpected config values. Co-authored-by: Isaac
acceptance/bundle/resources/grants/schemas/duplicate_principals/out.requests.direct.txt
Show resolved
Hide resolved
Co-authored-by: Isaac
andrewnester
approved these changes
Mar 23, 2026
acceptance/bundle/resources/grants/schemas/duplicate_principals/out.requests.direct.txt
Show resolved
Hide resolved
| grants: | ||
| - principal: deco-test-user@databricks.com | ||
| privileges: | ||
| - CREATE_TABLE |
Contributor
There was a problem hiding this comment.
Might worth showing a warning to users about such configs
Collaborator
|
Commit: e936087
71 interesting tests: 20 MISS, 15 RECOVERED, 13 flaky, 12 KNOWN, 8 FAIL, 2 PANIC, 1 SKIP
Top 50 slowest tests (at least 2 minutes):
|
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 free
to 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.
Changes
Tests
New acceptance (cloud+local) tests for the above cases. Updated testservers to error like the cloud.