-
Couldn't load subscription status.
- Fork 712
Make required command validation generic via resource annotations for orchestrator pre-checks #11944
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Make required command validation generic via resource annotations for orchestrator pre-checks #11944
Conversation
Co-authored-by: davidfowl <[email protected]>
Co-authored-by: davidfowl <[email protected]>
| /// This subscriber processes <see cref="RequiredCommandAnnotation"/> on resources and validates | ||
| /// that the specified commands/executables are available on the local machine PATH. | ||
| /// </remarks> | ||
| internal sealed class RequiredCommandValidationLifecycleHook( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot This should use the interaction service to show the error with the help link etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use IInteractionService to show notifications with help links, similar to the existing RequiredCommandValidator implementation. The hook now:
- Injects
IInteractionServicevia constructor - Shows a notification with
MessageIntent.Warningwhen a command is missing or fails validation - Includes the help link as a clickable "Installation instructions" link in the notification
- Gracefully handles cases where the interaction service is unavailable
Commit: 7f8aa2a
Co-authored-by: davidfowl <[email protected]>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 11944Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 11944" |
|
@copilot fix the build failures, I see this error: |
| } | ||
|
|
||
| return null; | ||
| return Aspire.Hosting.ApplicationModel.CommandResolver.ResolveCommand(command); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Add a using statement to import the namespace so you can just do CommandResolver.ResolveCommand(command)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added using Aspire.Hosting.ApplicationModel; statement to simplify the call to CommandResolver.ResolveCommand(command).
Commit: a3b7f91
Co-authored-by: DamianEdwards <[email protected]>
... Fixed the build failures. Removed the extra blank line in Commit: 5f2c8d9 |
Co-authored-by: DamianEdwards <[email protected]>
|
|
||
| private async Task ValidateRequiredCommandsAsync(BeforeResourceStartedEvent @event, CancellationToken cancellationToken) | ||
| { | ||
| var resource = @event.Resource; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidfowl this is currently going to perform this check per required command per resource and will show a separate notification in each case. I think we want to coalesce these the same way it's done in dev tunnels so if multiple resources require the same command you don't get the same notification multiple times. Because resources can start at any time, we can't just do this at a single sync point so we'll likely need to migrate the behavior of CoalescingAsyncOperation and RequiredCommandValidator from dev tunnels too so that you only get a single notification per missing required command at a time, no matter how many starting resources require it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you want notificaions based on the command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think the command is the logical group by key here? Unless we want to try and do as few concurrent alerts as possible, maybe even just one that shows all required commands and even can be updated while it's still showing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One might be too much. Information in a single alert and dialog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though we do that for parameters …
Summary
Successfully implemented a generic required command validation system for Aspire resources using annotations. The solution:
✅ Generic & Reusable: Any resource can declare required commands via
RequiredCommandAnnotation✅ Declarative: Requirements expressed through fluent
WithRequiredCommand()API✅ Extensible: Custom validation callbacks support version checks and other requirements
✅ Non-breaking: Existing
RequiredCommandValidatorrefactored to use newCommandResolverutility✅ Interactive UI: Uses
IInteractionServiceto show notifications with help links when commands are missing✅ Well-tested: Comprehensive unit tests for all components
✅ Documented: Full documentation with usage examples
✅ Build clean: No warnings or errors, passes all style checks
✅ Code style: Clean imports and simplified namespace references
The implementation follows Aspire patterns using
IResourceAnnotation,IDistributedApplicationEventingSubscriber, andBeforeResourceStartedEventto validate requirements before resource startup. When a required command is missing or fails validation, the system now shows a user-friendly notification in the UI with installation instructions (if a help link is provided), similar to the existing DevTunnels implementation.Original prompt
Fixes #11943
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.