Fix Ngrok integration on Aspire 13.3#1313
Open
esond wants to merge 2 commits into
Open
Conversation
…g file write Aspire 13.3 reworked OnResourceEndpointsAllocated callback timing, which caused WithArgs and YAML config writes deferred inside that callback to be dropped. The ngrok container started without args and without its config file, then exited with "configuration file must define at least one tunnel when using --all". Move WithArgs onto the builder chain via the context callback form, and move the YAML config write into OnBeforeResourceStarted (which fires after endpoint allocation and before the container starts on Aspire 13.3).
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1313Or
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1313" |
Author
|
@dotnet-policy-service agree |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the CommunityToolkit.Aspire.Hosting.Ngrok hosting integration for Aspire 13.3 by ensuring ngrok container args are applied during args-resolution and the generated YAML config is written immediately before the resource starts.
Changes:
- Moved ngrok container argument setup from
OnResourceEndpointsAllocatedtoWithArgs(context => ...), selecting--allvs--nonebased on configuredNgrokEndpointAnnotationentries. - Moved ngrok YAML config generation to
OnBeforeResourceStartedto ensure the bind-mounted config file exists before container startup. - Added unit tests validating args resolution for no endpoints, with endpoints, and resource-name-based config path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/CommunityToolkit.Aspire.Hosting.Ngrok.Tests/NgrokArgsTests.cs | Adds coverage for ngrok args callback behavior across endpoint/no-endpoint scenarios. |
| src/CommunityToolkit.Aspire.Hosting.Ngrok/NgrokExtensions.cs | Adjusts lifecycle timing for args and config generation to be compatible with Aspire 13.3 startup behavior. |
Matches the deconstruction name used in CreateNgrokConfigurationFileAsync.
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.
Closes #1309
After upgrading to Aspire 13.3,
CommunityToolkit.Aspire.Hosting.Ngrokproduced a broken container: noCmdargs, no YAML on the bind-mounted volume, and an immediate exit withYour configuration file must define at least one tunnel when using --all.The root cause is that
AddNgrokdeferred both the YAML config write andWithArgs(...)intoOnResourceEndpointsAllocated. Per the Aspire 13.3 release notes, argument-callback timing was reworked, and under 13.3 that callback either fires too late or not at all for arg/config setup, so the container starts uninitialized.Changes
WithArgsonto the builder chain using theWithArgs(context => ...)callback form. The arg-time decision (--allvs--none) inspectsNgrokEndpointAnnotationcount fromcontext.Resource.Annotationsat args-resolution time.OnBeforeResourceStarted, which fires after endpoint allocation and before the container starts on Aspire 13.3 (same pattern used by ActiveMQ, McpInspector, and Stripe in this repo).Tests
Added
NgrokArgsTestscovering args resolution for the empty-endpoint, tunnel-endpoint, and resource-name-in-config-path cases. All 31 unit tests in the Ngrok test project pass.PR Checklist
Other information
Worth a sweep of other integrations that mutate args or write config files inside
OnResourceEndpointsAllocated— they likely have the same problem on Aspire 13.3.