Route direct Testcontainers usage through the custom Docker Hub proxy - #1605
Merged
Merged
Conversation
Test fixtures that create containers directly via DotNet.Testcontainers (bypassing Aspire's container resource model) were still pulling images straight from their configured registry, so they weren't protected by the existing CUSTOM_CONTAINER_REGISTRY rate-limit mitigation used for Aspire-hosted resources. A generic '.WithContainerRegistryMirror()' builder extension isn't possible here: Testcontainers' fluent builder interface only exposes 'WithImage(string)', with no public getter for the image/registry already configured on a builder, so an extension method can't rewrite just the registry portion of an already-set image. Add a TestContainerRegistry.Resolve(string registry) helper to CommunityToolkit.Aspire.Testing instead, and use it when constructing the image string in every fixture that builds a Testcontainers image directly (Ollama, Meilisearch, SurrealDb, GoFeatureFlag, KurrentDB, SqlServer). The helper only swaps the registry when it's the default docker.io Docker Hub, so it's a no-op for the fixtures already using a different registry (SqlServer, KurrentDB) while still future-proofing them if that ever changes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.sh | bash -s -- 1605Or
iex "& { $(irm https://raw.githubusercontent.com/CommunityToolkit/Aspire/main/eng/scripts/dogfood-pr.ps1) } 1605" |
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Configure the Testcontainers Ryuk image for the mirror and add resolver test coverage.
Review effort: Lite
Findings: None
What changed in this PR
Routes direct Testcontainers image pulls through CUSTOM_CONTAINER_REGISTRY for Docker Hub images while preserving other registries.
Changes:
- Added
TestContainerRegistry.Resolve. - Applied it across six direct Testcontainers fixtures.
| File | Summary |
|---|---|
tests/CommunityToolkit.Aspire.Testing/TestContainerRegistry.cs |
Adds shared registry resolution. |
tests/CommunityToolkit.Aspire.SurrealDb.Tests/SurrealDbContainerFixture.cs |
Resolves the container image registry. |
tests/CommunityToolkit.Aspire.OllamaSharp.Tests/OllamaContainerFeature.cs |
Resolves the container image registry. |
tests/CommunityToolkit.Aspire.Meilisearch.Tests/MeilisearchContainerFixture.cs |
Resolves the container image registry. |
tests/CommunityToolkit.Aspire.KurrentDB.Tests/KurrentDBContainerFixture.cs |
Applies registry resolution to image construction. |
tests/CommunityToolkit.Aspire.Hosting.SqlDatabaseProjects.Tests/SqlServerContainerFixture.cs |
Applies registry resolution to image construction. |
tests/CommunityToolkit.Aspire.GoFeatureFlag.Tests/GoFeatureFlagContainerFixture.cs |
Resolves the container image registry. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
|
re Copilot comment Ryuk image is used by test containers to cleanup images. The agent will be blown away in CI, so not a problem. Test failure due to microsoft/aspire#20316 |
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.
Summary
Some test fixtures create containers directly via
DotNet.Testcontainers(bypassing Aspire's container resource model), so they weren't covered by the existingCUSTOM_CONTAINER_REGISTRYrate-limit mitigation thatAspireIntegrationTestFixtureapplies to Aspire-hosted resources.Changes
TestContainerRegistry.Resolve(string registry)toCommunityToolkit.Aspire.Testing. It swaps the registry to the value of theCUSTOM_CONTAINER_REGISTRYenvironment variable, but only when the input registry is the default Docker Hub (docker.io) registry — otherwise it returns the registry unchanged.TestContainerRegistry.Resolve(...)when constructing the image reference in every fixture that builds a Testcontainers container directly:CommunityToolkit.Aspire.OllamaSharp.TestsCommunityToolkit.Aspire.Meilisearch.TestsCommunityToolkit.Aspire.SurrealDb.TestsCommunityToolkit.Aspire.GoFeatureFlag.TestsCommunityToolkit.Aspire.KurrentDB.TestsCommunityToolkit.Aspire.Hosting.SqlDatabaseProjects.TestsThe last two (KurrentDB, SqlServer) already pull from non-Docker-Hub registries, so this is a no-op for them today but future-proofs them if that ever changes.
Why not a generic
.WithContainerRegistryMirror()builder extension?I considered a fluent extension method on Testcontainers'
IContainerBuilder<>(mirroring Aspire's ownWithImageRegistry), but it isn't feasible generically: the builder interface only exposesWithImage(string)/WithImage(IImage)with no public getter for the image/registry already configured on a builder. Without being able to read back the existing image/repository/tag, an extension can't isolate and rewrite just the registry portion. A static helper used when building the image string is the simplest approach that still works uniformly across all fixtures.Testing
Built each modified test project (
dotnet build) — all succeed with no new warnings/errors.