-
Notifications
You must be signed in to change notification settings - Fork 752
Add multiple container registry validation to Docker Compose and Azure environments #13513
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?
Conversation
…astructure Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13513Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13513" |
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.
Pull request overview
This PR adds validation to DockerComposeInfrastructure.GetContainerRegistry to detect and reject scenarios where multiple container registries exist in the app model without explicit selection. This aligns the behavior with the existing PushPrereq validation in DistributedApplicationPipeline.
Key changes:
- Added validation logic that throws
InvalidOperationExceptionwhen multiple registries are detected without explicit.WithContainerRegistry()call - Included descriptive error message listing all available registries and suggesting the resolution
- Added comprehensive test coverage for the new validation scenario
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Aspire.Hosting.Docker/DockerComposeInfrastructure.cs |
Added multiple registry validation logic in GetContainerRegistry method (lines 89-96) that matches the pattern used in PushPrereq step |
tests/Aspire.Hosting.Docker.Tests/DockerComposeTests.cs |
Added test MultipleRegistries_WithoutExplicitRegistryOnEnvironment_ThrowsException to verify the validation throws with appropriate error message |
The implementation looks good overall. The validation logic is consistent with the existing pattern in DistributedApplicationPipeline.PushPrereq, the error message is clear and actionable, and the test coverage validates the expected behavior. The changes are well-aligned with the repository's coding standards and patterns.
|
@copilot Update the |
…ce environments Per @captainsafia's feedback, added validation in CreateDefaultAzureContainerRegistry to check for multiple IAzureContainerRegistry instances in the app model and throw an error suggesting to call WithContainerRegistry when no explicit selection is made. Co-authored-by: captainsafia <1857993+captainsafia@users.noreply.github.com>
Done in commit 1b18c18. Added validation to both |
Description
Adds validation for multiple container registries in Docker Compose, Azure Container App, and Azure App Service environments to prevent ambiguous registry selection. When multiple container registries exist in the app model without explicit selection, the system now throws a clear error message instead of silently falling back to defaults.
Changes:
DockerComposeInfrastructure.GetContainerRegistrythat throwsInvalidOperationExceptionwhen multiple registries exist without explicit.WithContainerRegistry()callAzureContainerAppEnvironmentinfrastructure configuration to check for multipleAzureContainerRegistryResourceinstancesAzureAppServiceEnvironmentinfrastructure configuration to check for multipleAzureContainerRegistryResourceinstances.WithContainerRegistry(registryBuilder)to resolveBefore:
After:
Implementation Notes:
GetContainerRegistrymethod call.WithContainerRegistry()PushPrereqstep inDistributedApplicationPipelineChecklist
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.