Conversation
Keyed slices are matched by key value alone (the key field is only used to render the path). A diff on a matched element's own key field therefore means the two sides carry the same identity under a different field — e.g. a permission declared under user_name that the Permissions API stores and returns as service_principal_name — which is not a real change. Drop those field diffs; non-key fields still diff normally. This fixes a perpetual no-op "update" of dashboard/job/etc permissions when a service principal is declared under user_name, generically for any keyed slice, without per-resource logic. Alternative to the resource-level fix in #6710. The testserver models the backend's user_name(UUID) -> service_principal_name readback so the case reproduces locally. Co-authored-by: Isaac <no-reply@databricks.com>
Keyed-slice elements are matched by value, so encoding the key field in the path is redundant — and for a permission it is actively wrong, because the backend may return a principal under a different field (a user_name holding a service principal's application ID comes back as service_principal_name), producing a perpetual no-op "update". Introduce libs/structs/registry: a declarative, type-keyed record of which fields identify a slice element (registered from init() next to each type). structdiff keys off it instead of per-call KeyFunc maps and addresses elements as [='value'] (key field omitted); a diff on a matched element's own key field is dropped, so the field difference is no longer a change. structaccess and configsync resolve [='value'] back to an element through the registry (the type for structaccess, the element's members for configsync's dynamic values). This removes the KeyFunc machinery and the adapter's KeyedSlices method, and collapses the job path patterns into nine type registrations. Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
Collaborator
Integration test reportCommit: f8c7ac1
Top 3 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.
Problem
Keyed-slice elements (job tasks, permissions, grants, secret ACLs, …) are matched by value, so encoding the key field in the change path is redundant. For permissions it is also wrong: the Permissions API may return a principal under a different field than configured (a
user_nameholding a service principal's application ID comes back asservice_principal_name), which surfaced as a perpetual no-opupdateof the ACL on every plan/deploy.Approach
Introduce
libs/structs/registry— a declarative, type-keyed record of which fields identify a keyed-slice element, registered frominit()next to each type:structdiffkeys off the registry by element type (no more per-callKeyFuncmaps) and addresses an element as[='value']— the key field is omitted. A diff on a matched element's own key field is dropped, so a principal carried under a different field is no longer reported as a change.structaccessresolves[='value']back to an element via the registry (it has the Go type).configsyncresolves it for dynamic values by recognising the element's key field withregistry.IsKeyField, so remote-drift write-back stays correct.KeyFuncmachinery and the adapter'sKeyedSlicesmethod; the job path-pattern map collapses to nine type registrations.This is the generic-diff line of the permissions false-positive fix (cf. #6710 hook / #6732 side-key merge): here the key field simply leaves the path, and the resolvers recover it from type/registry.
Validation
New
registryunit tests;structpathround-trips[='value']. All keyed-slice acceptance goldens regenerated ([task_key='x']→[='x']). Unit tests acrosslibs/structs,bundle/direct,configsync,terraform_dabs_map; acceptanceinvariantno_drift and fullconfig-remote-syncpass — the config-remote-sync write-back edits the correct element. gofmt + lint clean.This pull request and its description were written by Isaac.