Uh oh!
There was an error while loading. Please reload this page.
acc: reproduce permanent drift on permissions after out-of-band parent recreate - #5587
Merged
Merged
Conversation
… same name After deleting and recreating a model serving endpoint remotely with the same name but a different endpoint_id, V1 permissions API behavior results in permanent drift on permissions: bundle plan keeps showing an update for permissions even after a successful deploy. The V1 endpoints do not delete ACLs immediately when the parent is gone, so DoRead from the old object_id keeps returning ACL data, while plan computes the new object_id from the recreated endpoint. This is the V1 counterpart to vector_search_endpoints/drift/recreated_same_name, which exercises V2 behavior (404 → create plan, no drift after deploy). Co-authored-by: Isaac
When the parent resource is recreated remotely with a different identifier (e.g. a model serving endpoint deleted and recreated under the same name), PermissionsState.ObjectID changes between deploys. Previously the framework called DoUpdate, which kept the deployment state ID pointing at the old, gone object_id. On V1 permissions APIs (jobs, pipelines, model serving), DoRead with the old object_id keeps returning ACL data for the deleted parent due to eventual consistency, so plan saw the same ObjectID drift on every subsequent run — a permanent update on permissions. Add DoUpdateWithID that returns newState.ObjectID as the new resource ID so the framework persists the new ID in deployment state, and wire up update_id_on_changes for object_id on every *.permissions resource that uses ResourcePermissions. Subsequent plans then compare against the new ObjectID and see no drift. Update the model_serving_endpoints drift acceptance test to assert no permanent drift after deploy (same shape as the vector_search V2 test). Co-authored-by: Isaac
DoRead previously took only the deployment-state id, which is stale after an out-of-band recreate of the parent resource: the id points at the gone object_id while the new plan has already resolved newState.ObjectID to the freshly-created identifier. The permissions resource needs to read against the new identifier, otherwise on V1 permissions APIs (jobs, pipelines, model serving) DoRead keeps returning ACL data for the deleted parent and produces a permanent drift; on V2 (vector search) it 404s when it should be reading the new endpoint's empty ACLs. Add a newState parameter to DoRead across all resources. Most resources ignore it and continue to read by id. ResourcePermissions uses newState.ObjectID when available so subsequent plans see no drift after a parent recreate, and the test-server's new-endpoint ACL state is correctly observed. For the delete path in plan, where there is no planned new state, pass nil so the adapter falls back to id-based read. For refreshRemoteState post-deploy, thread newState (sv.Value) through from bundle_apply. Update vector_search_endpoints/drift/recreated_same_name acceptance to match the new behaviour: the plan now reads the freshly-created endpoint and shows an "update" on permissions instead of "create" — both end with no permanent drift after deploy. Co-authored-by: Isaac
Co-authored-by: Isaac
janniklasrosetemporarily deployed
to
test-trigger-is
June 12, 2026 20:12 — with 
GitHub Actions
Inactive
janniklasrosetemporarily deployed
to
test-trigger-is
June 12, 2026 20:12 — with GitHub Actions
Inactive
janniklasrosetemporarily deployed
to
test-trigger-is
June 12, 2026 20:13 — with GitHub Actions
Inactive
janniklasrosetemporarily deployed
to
test-trigger-is
June 12, 2026 20:13 — with GitHub Actions
Inactive
Collaborator
Integration test reportCommit: db21512
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 6 slowest tests (at least 2 minutes):
|
| RecordRequests = false | ||
| EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] | ||
| Badness = "Recreating a parent resource (schema) should propagate Recreate to dependents (registered_model) that reference it, but the planner does not. The dependent plans Skip while the parent is deleted underneath it. On a real workspace this either fails the parent delete (children still attached) or orphans the child. Tracked as a framework-level fix in bundle/direct/bundle_plan.go." |
ContributorAuthor
There was a problem hiding this comment.
not permissions, but also recreate cascade issue
Back out the two fix commits (DoUpdateWithID on ResourcePermissions plus the update_id_on_changes entries, and the newState parameter threaded through DoRead) so this PR only documents the broken behaviour as Badness. The fix touched every resource's DoRead signature, which made it hard to review alongside the bug it was addressing; reproducing first keeps the two separable. The model_serving_endpoints drift test goes back to asserting the permanent update on permissions after deploy, and its Badness text describes the V1 permissions API leaving an ACL on the deleted endpoint id. The vector search drift output and permission_level_migration goldens return to their pre-fix values, and the changelog entry is dropped since no behaviour changes. Co-authored-by: Isaac
This reverts commit ee3d0b4. That test documents an unrelated bug: recreating a parent schema does not propagate Recreate to a dependent registered_model. Keeping it here mixes two independent badness reports in one PR. It is preserved on the branch janniklasrose/registered-models-recreate-cascade for a follow-up. Co-authored-by: Isaac
janniklasrose
marked this pull request as ready for review
August 5, 2026 08:26
janniklasrose
enabled auto-merge
August 5, 2026 21:20
shreyas-goenka
approved these changes
Aug 6, 2026
andrewnester
approved these changes
Aug 6, 2026
janniklasrose
disabled auto-merge
August 7, 2026 11:49
Uh oh!
There was an error while loading. Please reload this page.
Sankalp-Mittal added a commit
that referenced
this pull request
Aug 7, 2026
#6196 removed the Local config field and rewrote the other test.toml files to drop it, but #5587 added this test.toml with Local = true around the same time, so it was not swept. The acceptance harness rejects undecoded keys, failing the test on config parse before any assertion runs. Remove the key. Co-authored-by: Isaac
eng-dev-ecosystem-bot
commented
Aug 7, 2026
Collaborator
Integration test reportCommit: 5e3ea1c
15 interesting tests: 7 FAIL, 5 RECOVERED, 2 SKIP, 1 KNOWN
Top 50 slowest tests (at least 2 minutes):
|
eng-dev-ecosystem-bot
commented
Aug 9, 2026
Collaborator
Integration test reportCommit: 5e3ea1c
19 interesting tests: 7 FAIL, 6 RECOVERED, 4 flaky, 1 KNOWN, 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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Reproduce permanent drift for permissions of resources using ACL v1 APIs that have eventual consistency on resource deletion. This uses
model_serving_endpointsas an example but applies to most resources.