Repository navigation
Revert "Preserve authenticated scope in cached template lookups (#217)" - #734
Merged
Merged
Conversation
This reverts commit e03f027.
nickdujay-pplx
marked this pull request as ready for review
October 20, 2026 23:00
nickdujay-pplx
enabled auto-merge
October 20, 2026 23:00
mabdelraouf-pplx
approved these changes
Oct 20, 2026
This was referenced Oct 21, 2026
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.
Reverts #217 (
e03f0278).Why
#217 has no mixed-version compatibility step. During a rolling upgrade,
sandbox-managerandtemplate-managerare separate DaemonSets, so a node can briefly run a newsandbox-managernext to an oldtemplate-manager. The new client requires a scope acknowledgement that the old server never sends. Template resolution on create and on restore then fails withFAILED_PRECONDITION "template lookup scope was not acknowledged"until both pods on that node are upgraded. The reverse pairing (old client, new server) mostly works, but every lookup goes to the catalog instead of the local cache.We can't make every DaemonSet on a node reach the new version at the same moment, so this break would happen on every deploy that ships #217. We're reverting so the next release can go out safely. The change will come back in two phases:
Conflict resolution
The revert conflicted with later changes, and those later changes are kept:
PullSpecandresolve_elapsedmaterialize signature and therequested_hashmetric tagging stay. Only the scope plumbing and thelocal_executioncache gate are removed.snapshot_restoretests stay. Only the Preserve authenticated scope in cached template lookups #217restore_template_lookup_uses_validated_ownertest is removed.claim_create_targetandrelease_create_targetin the direct substrate are removed together with their Preserve authenticated scope in cached template lookups #217 call sites. The test for them that Bind the last substrate literals and forbid new ones with archcheck #611 added insubstrate_scope.rsis removed as well.Cargo.lock: therstestdev-dependency that Preserve authenticated scope in cached template lookups #217 added topplx-space-sbx-mgris removed.Testing
I didn't build this locally. CI is the compile and test gate.
AI assistance: Perplexity Computer prepared this revert and resolved the conflicts. A maintainer reviews it before merge.