Make staging self-update persistence E2E hermetic - #19731
Make staging self-update persistence E2E hermetic#19731Mitch Denny (mitchdenny) wants to merge 7 commits into
Conversation
Keep the staging route persisted while resolving stable-stamped replacement builds from their matching nuget.org package graph. Make the self-update E2E deterministic against the current source build. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19731Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19731" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Separates persisted CLI update routing from physical build provenance so staging self-updates can correctly use stable packages.
Changes:
- Tracks effective and assembly build channels separately.
- Pins stable-stamped staging replacements to NuGet.org packages.
- Adds unit and self-update E2E coverage.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Cli/Acquisition/IIdentityResolver.cs |
Adds build-channel resolution. |
src/Aspire.Cli/Acquisition/IdentityResolver.cs |
Resolves physical build identity separately. |
src/Aspire.Cli/CliExecutionContext.cs |
Stores route source and build channel. |
src/Aspire.Cli/Program.cs |
Propagates resolved channel identities. |
src/Aspire.Cli/Packaging/PackageChannel.cs |
Separates channel persistence from NuGet configuration. |
src/Aspire.Cli/Packaging/PackagingService.cs |
Selects staging package provenance and download URL. |
tests/Aspire.Cli.Tests/Utils/TestExecutionContextHelper.cs |
Supports distinct channel identities in tests. |
tests/Aspire.Cli.Tests/Acquisition/IdentityResolverTests.cs |
Verifies sidecar and build identities. |
tests/Aspire.Cli.Tests/Packaging/PackagingServiceTests.cs |
Tests staging provenance decisions. |
tests/Aspire.Cli.Tests/Packaging/PackageChannelTests.cs |
Tests NuGet.org-only staging behavior. |
tests/Aspire.Cli.EndToEnd.Tests/SelfUpdateChannelPersistenceTests.cs |
Exercises local self-update and relaunch. |
docs/cli-staging-validation.md |
Documents provenance and test overrides. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Balanced
| "cp \"$(command -v aspire)\" \"$install_root/bin/aspire\"; " + | ||
| "chmod +x \"$install_root/bin/aspire\"; " + | ||
| "printf '%s\\n' '{\"source\":\"script\",\"channel\":\"stable\"}' > \"$install_root/bin/.aspire-install.json\"; " + | ||
| "printf '{\"source\":\"script\",\"channel\":\"stable\",\"packages\":\"%s\"}\\n' \"$packages_dir\" > \"$install_root/bin/.aspire-install.json\"; " + |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Remove the physical build-channel package routing workaround and keep staging provenance fully driven by the externalized CLI identity. Make the self-update E2E hermetic with a loopback-only archive source and explicit local package identity. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2f02689-8514-4cf8-b38d-f2b62f506754
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
The hermetic self-update feed used python3 -m http.server without --bind, which listens on 0.0.0.0. Bind it to 127.0.0.1 so the listener matches the loopback-only constraint enforced by ASPIRE_CLI_STAGING_DOWNLOAD_BASE_URL, and follow the existing convention in DocsCommandE2ETests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2f02689-8514-4cf8-b38d-f2b62f506754
This comment has been minimized.
This comment has been minimized.
Replace the hand-rolled TcpListener and raw HTTP response bytes with a WebApplication, matching the existing pattern in KubernetesServiceTests. Rewriting the test surfaced that it was passing for the wrong reason. It redirected to https://example.com, and the follow-up request to that host also fails with HttpRequestException, so the assertion held whether or not AllowAutoRedirect was disabled. The redirect now targets a working endpoint on the same loopback server, so following it would produce a successful download. Verified the test fails when AllowAutoRedirect is set back to true. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2f02689-8514-4cf8-b38d-f2b62f506754
There was a problem hiding this comment.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
tests/Aspire.Cli.Tests/Utils/CliDownloaderTests.cs:100
- This test can still pass if redirects are accidentally enabled: following this URL will usually fail or return a non-success status, producing the same
HttpRequestExceptionexpected below. It also lets a regression make an external network request. Point the redirect at a second loopback listener that returns 200 and records requests, then assert that listener was never contacted; that makes the redirect-protection regression test hermetic and conclusive.
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/Aspire.Cli/Utils/CliDownloader.cs:57
- The redirect test does not exercise this wiring: it calls
DownloadFileAsync(..., requireLoopback: true)directly, while the E2E server returns only 200 responses. If this condition were removed or accidentally evaluated to false, every new test would still pass and staging overrides would again use the proxy/redirect-enabled client. Add coverage throughDownloadLatestCliAsync(or refactor URL resolution to return the restriction flag together with the URL) and have the configured staging endpoint return a redirect, so the security guarantee is tested at its entry point.
var requireLoopback = string.Equals(channel.Name, PackageChannelNames.Staging, StringComparisons.ChannelName) &&
environment.GetEnvironmentVariable(StagingDownloadBaseUrlEnvVar) is { Length: > 0 };
tests/Aspire.Cli.EndToEnd.Tests/SelfUpdateChannelPersistenceTests.cs:64
- Using a fixed port makes this E2E fail whenever 38417 is already bound in the test container;
nohupcan hide the bind failure until the readiness curl times out or contacts an unrelated listener. Have the Python server bind port 0 and communicate the selected port back to the shell before exporting the URL, as the Kestrel test below already does.
"feed_root=/tmp/aspire-self-update-feed; port=38417; " +
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
| <!-- CliDownloaderTests hosts a real Kestrel endpoint to prove the loopback-only staging download | ||
| hook rejects an off-loopback redirect instead of following it. --> |
This comment has been minimized.
This comment has been minimized.
…t#19733 PR microsoft#19733 quarantines this test with [QuarantinedTest] linked to microsoft#19708, which preserves quarantine-workflow coverage until the root cause is fixed in microsoft#19731. Our earlier [ActiveIssue] change skipped the test everywhere (including the quarantine workflow) and touched the same method, causing a merge conflict. Remove it so microsoft#19733 owns disabling this test and this branch leaves the file unchanged from base. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…annel-persistence
There was a problem hiding this comment.
Review details
Suppressed comments (1)
tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj:21
- This says the test uses an off-loopback redirect, but the redirect target is
/redirected.tar.gzon the same loopback Kestrel server. The test proves that redirects are disabled; describe that actual coverage rather than a target it does not create.
<!-- CliDownloaderTests hosts a real Kestrel endpoint to prove the loopback-only staging download
hook rejects an off-loopback redirect instead of following it. -->
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
| // Serve the current-source binary in the same archive shape as the acquisition endpoint. | ||
| // Using the live staging endpoint here would relaunch an already-published CLI, so changes | ||
| // under test in this branch could not affect the implicit project update after replacement. | ||
| // The checksum keeps the real self-update validation path in the scenario. |
| if (!Uri.TryCreate(overrideUrl, UriKind.Absolute, out var uri) || | ||
| !uri.IsLoopback || | ||
| (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps)) |
The test quarantined by #19733 is now hermetic: self-update installs an archive of the current-source CLI served from a loopback-only feed, and package resolution uses a local hive supplied through the install sidecar. It no longer depends on a live darc feed containing a matching Aspire.AppHost.Sdk, which was the source of the original flakiness. Removes the QuarantinedTest attribute and the now-unused Aspire.TestUtilities using (IDE0005 is an error in this repo). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2f02689-8514-4cf8-b38d-f2b62f506754
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review details
Suppressed comments (1)
tests/Aspire.Cli.EndToEnd.Tests/SelfUpdateChannelPersistenceTests.cs:15
- Keep this test quarantined after landing the fix. The repository policy explicitly says a fix PR must retain
[QuarantinedTest]until the test has zero failures across all operating systems for 21 consecutive days (docs/unquarantine-policy.md:5-22). Three successful runs do not meet that gate, so removing the attribute immediately bypasses the reliability observation period.
public async Task SelfUpdateToStaging_RelaunchedCliUsesStagingForImplicitProjectUpdate()
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
| // under test in this branch could not affect the implicit project update after replacement. | ||
| // The checksum keeps the real self-update validation path in the scenario. | ||
| await auto.RunCommandAsync( | ||
| "feed_root=/tmp/aspire-self-update-feed; port=38417; " + |
* Isolate source AppHost deployment state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden deployment state loading and replacement Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify deployment state clearing messages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Preserve scalar state migration semantics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix claimed parent migration authority Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Preserve authoritative migrated section state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix deployment state migration edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Preserve deployment state manager compatibility Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Isolate deployment state identity from AppHost path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Canonicalize deployment paths by filesystem semantics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Merge concurrent nested state additions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden deployment state locking and clearing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden deployment state fallback merging Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden deployment state load against edge cases from review LoadDeploymentState now computes state paths inside the try/catch so an environment name containing characters outside [a-zA-Z0-9_-] degrades to skipping the best-effort load instead of throwing ArgumentException from builder construction in publish mode. FileDeploymentStateManager only locks and reads legacy state when the legacy file exists. This avoids manufacturing a phantom legacy directory and lock file for identities that never had legacy state (e.g. brand-new source AppHosts on a clean machine) and avoids re-coupling sibling AppHosts that share the legacy identity on that shared lock. Adds regression tests that fail without each fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Disable SelfUpdateChannelPersistenceTests pending fix The test fails in PR CI and is tracked by #19708, which has a separate fix in progress. Mark it [ActiveIssue] so it does not block unrelated PRs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Harden authoritative state recovery Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert SelfUpdateChannelPersistenceTests disable in favor of #19733 PR #19733 quarantines this test with [QuarantinedTest] linked to #19708, which preserves quarantine-workflow coverage until the root cause is fixed in #19731. Our earlier [ActiveIssue] change skipped the test everywhere (including the quarantine workflow) and touched the same method, causing a merge conflict. Remove it so #19733 owns disabling this test and this branch leaves the file unchanged from base. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Mitch Denny <midenn@microsoft.com>
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
…annel-persistence
Tests selector (audit mode)The full test matrix and all jobs still run in audit mode. The tests and jobs below are what selective CI would run under enforcement. 2 / 102 test projects · 4 jobs, from 4 changed files. Selected test projects (2 / 102)
Selected jobs (4)
How these were chosen — grouped by what changed📦 affected project 🧪 🧪 🧪 Job reasons
Selection computed for commit |
There was a problem hiding this comment.
Review details
Suppressed comments (1)
tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj:21
- This comment says the test uses an off-loopback redirect, but the redirect target is the relative
/redirected.tar.gzroute on the same loopback Kestrel server. The test proves that all redirects are rejected, so describe that behavior rather than an off-loopback redirect.
<!-- CliDownloaderTests hosts a real Kestrel endpoint to prove the loopback-only staging download
hook rejects an off-loopback redirect instead of following it. -->
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
| if (!Uri.TryCreate(overrideUrl, UriKind.Absolute, out var uri) || | ||
| !uri.IsLoopback || | ||
| (uri.Scheme != Uri.UriSchemeHttp && uri.Scheme != Uri.UriSchemeHttps)) |
| "nohup python3 -m http.server \"$port\" --bind 127.0.0.1 --directory \"$feed_root\" >/tmp/aspire-self-update-feed.log 2>&1 & " + | ||
| "for attempt in $(seq 1 20); do curl --fail --silent \"http://127.0.0.1:$port/$(basename \"$archive\").sha512\" >/dev/null && break; sleep 0.1; done; " + | ||
| "curl --fail --silent \"http://127.0.0.1:$port/$(basename \"$archive\").sha512\" >/dev/null; " + | ||
| "export ASPIRE_CLI_STAGING_DOWNLOAD_BASE_URL=\"http://127.0.0.1:$port\"", |
* feat: add Azure Container Apps Sandboxes target Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * fix: address Azure Sandboxes review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * fix: scope Azure sandbox cleanup ownership Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * test: make container output assertion portable Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * test: avoid cmd echo trailing whitespace Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * fix: configure sandbox build options on target creation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Address Azure Sandboxes review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Harden Azure Sandboxes APIs and retries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Secure sandbox diagnostics and image inspection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Strengthen image argument boundary tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Address Azure Sandboxes deployment review Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Address sandbox scope and cleanup review Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Fix sandbox group resource naming Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * fix(ats): support TimeSpan values in DTOs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * docs(sandboxes): cite data owner role Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Fix polyglot sandbox ownership isolation Use source AppHost file identity for deployment-state isolation while migrating legacy state paths, and derive sandbox ownership from the canonical AppHost identity. Keep publish-only sandbox resources out of run mode and document the TypeScript API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Emit required sandbox group properties Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 * Fix deployment state migration path resolution Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a759f61f-23ce-4579-935b-c0e48290edae * Address Azure sandbox review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a759f61f-23ce-4579-935b-c0e48290edae * Fix sandbox state and principal migration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Azure sandbox follow-up review Make publish helpers inert during run mode, prune previous generations when immutable images change, and reconcile create response-loss by deployment labels.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Isolate source AppHost deployment state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden Azure sandbox deployment lifecycle Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add typed container image inspection results Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden deployment state loading and replacement Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify deployment state clearing messages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Handle ADC role assignment propagation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Preserve scalar state migration semantics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix claimed parent migration authority Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Preserve authoritative migrated section state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden sandbox security replacement cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Classify role-less app-only Azure tokens Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix deployment state migration edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Preserve deployment state manager compatibility Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address sandbox runtime review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Handle optional Azure user name claims Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Prune retained sandboxes with runtime configuration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Isolate deployment state identity from AppHost path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Use deployment identity for sandbox ownership Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Canonicalize deployment paths by filesystem semantics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Merge concurrent nested state additions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Allow composite connection endpoints through sandbox egress Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden deployment state locking and clearing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Prune retained sandboxes for modeled commands Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Use existing sandbox scope for cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden deployment state fallback merging Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify existing sandbox identity support Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden deployment state load against edge cases from review LoadDeploymentState now computes state paths inside the try/catch so an environment name containing characters outside [a-zA-Z0-9_-] degrades to skipping the best-effort load instead of throwing ArgumentException from builder construction in publish mode. FileDeploymentStateManager only locks and reads legacy state when the legacy file exists. This avoids manufacturing a phantom legacy directory and lock file for identities that never had legacy state (e.g. brand-new source AppHosts on a clean machine) and avoids re-coupling sibling AppHosts that share the legacy identity on that shared lock. Adds regression tests that fail without each fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Disable SelfUpdateChannelPersistenceTests pending fix The test fails in PR CI and is tracked by #19708, which has a separate fix in progress. Mark it [ActiveIssue] so it does not block unrelated PRs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Harden authoritative state recovery Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert SelfUpdateChannelPersistenceTests disable in favor of #19733 PR #19733 quarantines this test with [QuarantinedTest] linked to #19708, which preserves quarantine-workflow coverage until the root cause is fixed in #19731. Our earlier [ActiveIssue] change skipped the test everywhere (including the quarantine workflow) and touched the same method, causing a merge conflict. Remove it so #19733 owns disabling this test and this branch leaves the file unchanged from base. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Fix response-loss cleanup cancellation assertion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address sandbox analyzer and endpoint feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address sandbox deployment review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden sandbox create reconciliation and egress Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address sandbox cleanup and digest feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Use managed identity for sandbox image pulls Move ADC disk image creation to the V2 managed identity contract and keep sandbox-specific provisioning out of the shared Azure hosting infrastructure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Stabilize response-loss visibility test Allow enough reconciliation time for Windows timer granularity while retaining the one-millisecond poll interval. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify sandbox workload identities Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix sandbox image pull identity handling Avoid sending the ACR pull identity for public registry images, reject pull/workload identity reuse, and require existing pull identities for existing sandbox groups. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ccb1419b-3f5f-4cb3-8933-b0bb1762ed48 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Mitch Denny <midenn@microsoft.com> Copilot-Session: 2fbe254b-3480-468c-99fa-428b6d207542 Copilot-Session: a759f61f-23ce-4579-935b-c0e48290edae Copilot-Session: ccb1419b-3f5f-4cb3-8933-b0bb1762ed48
Important
Superseded — not intended to merge. #19727 fixes #19708 with a tests-only change and is the preferred approach.
Follow-up experiment showed the production hook in this PR is not required to fix the bug. The restore failure is fully addressed by the existing
packagesinstall-sidecar field, which is pure test configuration. TheASPIRE_CLI_STAGING_DOWNLOAD_BASE_URLhook only additionally buys PR-gating of the identity reader path — not worth a permanent, security-sensitive redirect in shippingCliDownloader.cs.Recommendation left on #19727: take that PR, but keep the project-update leg and make its restore hermetic via
packages, so the original test intent (persisted channel landing in a project'saspire.config.json) isn't lost. See #19727 (comment).Retained here as the record of the root-cause investigation and as the heavier hermetic alternative. Will close once #19727 lands.
Description
The failure in #19708 exposed two separate facts:
13.5.3+b5f14331, while the identity-deriveddarc-pub-microsoft-aspire-b5f14331feed did not containAspire.AppHost.Sdk 13.5.3.The CLI's existing behavior is correct for its externalized identity:
channel=staging,version=13.5.3, andcommit=b5f14331must resolve as that staging identity. This PR deliberately does not add physical-binary branching or change package provenance.Instead, the test now serves a checksum-validated archive of the current-source CLI from a local loopback server. Its install sidecar supplies the complete test identity inputs: self-update persists
channel=staging, preserves the explicit local package directory, and the relaunched current-source CLI performs the implicit project update. This keeps the scenario hermetic while continuing to exercise executable replacement, sidecar persistence, relaunch, package restore, and project channel persistence.A dedicated
ASPIRE_CLI_STAGING_DOWNLOAD_BASE_URLtest hook enables the local archive source. It is staging-only, accepts only absolute loopback HTTP(S) URLs, bypasses proxies, rejects redirects, and revalidates the response URI. Production staging package routing is unchanged.Validation:
mainon the first Docker E2E run.SelfUpdateToStaging_RelaunchedCliUsesStagingForImplicitProjectUpdate: 2 consecutive runs passed, followed by a final passing run, with quarantined and outerloop tests excluded.Security considerations
The test hook controls executable acquisition, so it is intentionally constrained to direct loopback HTTP(S) connections. Proxy use and redirects are disabled, the final response URI is checked again, and the existing SHA-512 archive validation remains in place.
Fixes #19708
Checklist
<remarks />and<code />elements on your triple slash comments?This fix was generated using the fix-flaky-test skill.