Conversation
…r user_name A service principal listed in bundle permissions under `user_name` (its application ID) is stored and returned by the Permissions API as `service_principal_name`, so the direct engine's plan reported a no-op "update" of the ACL on every run that never converged. Permission entries are already matched by principal value, so treat the same value under `user_name` vs `service_principal_name` as one principal and drop the field-only change; a genuine level change still surfaces. The relaxation lives in ResourcePermissions.OverrideChangeDesc, which now also receives the desired state so the removal side of the swap can be recognized. The testserver models the backend's user_name(UUID) -> service_principal_name readback, and a new acceptance test asserts the plan converges and that a real level change is still reported. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
… change Use each resource's concrete state type for the new OverrideChangeDesc newState parameter instead of `any`. A concrete pointer parameter already accepts an untyped nil through calladapt's existing pointer path, so the interface-nil handling added earlier is no longer needed and is reverted. Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: 3cd1c4f
Top 7 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac <no-reply@databricks.com>
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
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.
Problem
When bundle permissions declare a service principal under
user_name(using its application-ID UUID), the Permissions API stores and returns it asservice_principal_name. The direct engine's plan then compared desired (user_name) against remote (service_principal_name) and reported a no-opupdateof the ACL on every run — a false positive that never converged. Reported by a customer after migrating to the direct engine.Fix
Permission entries are already matched by principal value, so the only difference is the field name.
ResourcePermissions.OverrideChangeDescnow treats the same value underuser_namevsservice_principal_nameas the same principal and drops the field-only change; a genuine level change on that principal still reports an update. This is value-based (no UUID sniffing), so GCP name-based service principals are handled too.OverrideChangeDescnow also receives the desired state (the removal side of the swap needs it); the ~10 sibling implementations gain one unused parameter, andlibs/calladaptnow acceptsnilfor interface-kind parameters.The testserver models the backend's
user_name(UUID) →service_principal_namereadback, and a new acceptance test asserts the plan converges after deploy and that a real level change is still reported.This pull request and its description were written by Isaac.