Strip /login path from dashboard base URL in describe JSON output#15495
Open
JamesNK wants to merge 1 commit intorelease/13.2from
Open
Strip /login path from dashboard base URL in describe JSON output#15495JamesNK wants to merge 1 commit intorelease/13.2from
JamesNK wants to merge 1 commit intorelease/13.2from
Conversation
The describe command was passing BaseUrlWithLoginToken (e.g., http://localhost:18888/login?t=token) directly to the resource snapshot mapper, producing broken dashboard URLs like http://localhost:18888/login?t=token/?resource=redis. Reuse TelemetryCommandHelpers.ExtractDashboardBaseUrl to strip the /login?t=... path before combining with resource URLs.
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15495Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15495" |
JamesNK
added
Servicing-consider
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes aspire describe --format json producing broken per-resource dashboardUrl values by stripping the /login?t=... portion from the dashboard URL before combining it with resource-specific paths.
Changes:
- Reuse
TelemetryCommandHelpers.ExtractDashboardBaseUrl(nowinternal) to derive a clean dashboard base URL inDescribeCommand. - Add integration tests covering both snapshot JSON output and
--followNDJSON output to verify correctdashboardUrlcomposition.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Commands/DescribeCommandTests.cs | Adds tests ensuring dashboard URLs in JSON/NDJSON output don’t include /login?t=... before resource URL composition. |
| src/Aspire.Cli/Commands/TelemetryCommandHelpers.cs | Makes ExtractDashboardBaseUrl internal so it can be reused by other commands. |
| src/Aspire.Cli/Commands/DescribeCommand.cs | Uses ExtractDashboardBaseUrl to avoid combining resource URLs onto /login?t=... dashboard URLs. |
16 tasks
mitchdenny
approved these changes
Mar 23, 2026
Contributor
|
🎬 CLI E2E Test Recordings — 54 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23423898514 |
Member
|
Likley a dumb question, but won't you also need the token for deep linking to work on browsers where the main dashboard hasn't authenticated yet? In that case, should the fix be just to make it send the two query params (with the right |
Member
Correct, you would need the token - the operating assumption is that the user would already be logged in when navigating to these pages |
adamint
approved these changes
Mar 23, 2026
Member
Author
|
Or you login after going to that page using the login screen. |
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.
Description
The
aspire describe --format jsoncommand was usingBaseUrlWithLoginToken(e.g.,http://localhost:18888/login?t=token) directly as the dashboard base URL when building per-resource dashboard URLs.DashboardUrls.CombineUrlsimply concatenates strings, producing broken URLs likehttp://localhost:18888/login?t=token/?resource=redis.This change reuses
TelemetryCommandHelpers.ExtractDashboardBaseUrl(changed fromprivatetointernal) to strip the/login?t=...path before combining with resource URLs, producing correct URLs likehttp://localhost:18888/?resource=redis.Two new integration tests verify the fix for both snapshot and follow/NDJSON JSON output modes.
Fixes #15171
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: