direct: Fix perpetual update on grants with ALL_PRIVILEGES#6064
Open
denik wants to merge 3 commits into
Open
Conversation
buildGrantChanges now computes a per-principal add/remove diff against remote state (Add = desired - remote, Remove = remote - desired) like the terraform provider's diffPermissions, instead of sending a blanket "remove ALL_PRIVILEGES" to wipe each principal. This lets a principal granted ALL_PRIVILEGES converge when the backend also reports concrete privileges for it, instead of reporting update forever (#6030). Also normalize privileges (uppercase, spaces to underscores) and sort them on both the config and read sides so grants do not drift on case or order, and update the grants testserver to mirror that backend behaviour.
Revert the per-principal grant diff and keep the original buildGrantChanges (ALL_PRIVILEGES still wipes other privileges). Instead fix the false drift at comparison time: normalizeAssignments collapses a principal holding ALL_PRIVILEGES down to just ALL_PRIVILEGES on both the config and read sides, so config granting only ALL_PRIVILEGES matches a backend that also reports the concrete privileges it implies (#6030).
MergeGrants deduplicates the raw config strings before normalization, so distinct spellings of the same privilege (e.g. "use schema" and "USE_SCHEMA") only collide once normalizeAssignments canonicalizes them. Compact after sorting so the config side does not keep a duplicate the backend read side never returns, which would otherwise cause perpetual drift.
Contributor
Approval status: pending
|
Collaborator
Integration test reportCommit: 4ff0948
15 interesting tests: 8 flaky, 4 RECOVERED, 3 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
2 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
When a principal is granted ALL_PRIVILEGES,
normalizeAssignmentsnow collapses its privileges down to just ALL_PRIVILEGES on both the config and read sides. Privileges are also uppercased and sorted so grants don't drift on case or ordering.Why
Fixes #6030. On the direct engine
bundle planreportedupdateon every catalog/schema/volume.grantssub-resource that granted ALL_PRIVILEGES: the backend reports ALL_PRIVILEGES plus the concrete privileges it implies, which never matched the config's lone ALL_PRIVILEGES.Tests
New acceptance test
grants/schemas/all_privileges_coexistand invariant configgrants_privilege_normalization; verified on aws-prod-ucws.