feat: add the ability to configure the provider type in the properties file or as an env var #3494
Conversation
… or as an env var Using WithProvider may not be portable when some developers running tests may be using Podman and others using Docker. Allowing setting the Provider type in the global configuration or environment ensures that the provider is a per-developer configurable setting.
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a new Config.Provider field (bound to Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant PT as ProviderType.UnderlyingProviderType()
participant CFG as Config (properties / file)
participant ENV as Environment (TESTCONTAINERS_PROVIDER)
participant AUTO as Auto-detection (host/docker info)
App->>PT: Request effective provider
rect rgb(220,235,255)
PT->>PT: Check explicit ProviderType argument
alt explicit provided
PT-->>App: Return explicit ProviderType
end
end
rect rgb(235,255,220)
PT->>CFG: Check Config.Provider
PT->>ENV: Check TESTCONTAINERS_PROVIDER
alt env set
PT-->>App: Return provider from ENV
else cfg set
PT-->>App: Return provider from CFG
end
end
rect rgb(255,245,220)
PT->>AUTO: Perform auto-detect (e.g., inspect Docker host for Podman)
alt auto-detect Podman
PT-->>App: Return Podman
else
PT-->>App: Return Docker (default)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mkeeler
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
provider.go (1)
104-109: Consider case-insensitive provider matching.The provider string matching is case-sensitive, which may lead to configuration errors if users specify "Docker" or "Podman" with capital letters.
Consider making the comparison case-insensitive:
conf := config.Read() - switch conf.Provider { - case "docker": + switch strings.ToLower(conf.Provider) { + case "docker": return ProviderDocker - case "podman": + case "podman": return ProviderPodman }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/config/config.go(2 hunks)internal/config/config_test.go(5 hunks)provider.go(2 hunks)provider_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
provider.go (2)
internal/config/config.go (1)
Read(99-105)internal/core/docker_host.go (1)
MustExtractDockerHost(87-98)
provider_test.go (2)
provider.go (4)
ProviderType(23-23)ProviderDocker(17-17)ProviderPodman(18-18)ProviderDefault(16-16)internal/config/config.go (1)
Reset(111-113)
internal/config/config_test.go (1)
internal/config/config.go (2)
Read(99-105)Config(32-93)
🔇 Additional comments (6)
provider.go (1)
96-118: LGTM! Well-structured precedence logic.The
UnderlyingProviderType()method correctly implements the precedence rules: explicit code setting → environment variable → properties file → auto-detection → default. The implementation cleanly centralizes provider resolution logic.internal/config/config.go (2)
89-92: LGTM! Clear documentation and proper field definition.The
Providerfield is well-documented with the environment variable name and example values, following the same pattern as other configuration fields in the struct.
149-152: LGTM! Consistent with other environment variable handling.The environment variable reading for
TESTCONTAINERS_PROVIDERfollows the same pattern as other configuration fields and correctly applies the precedence (environment variables override properties file values).internal/config/config_test.go (2)
29-29: LGTM! Proper test environment cleanup.Adding
TESTCONTAINERS_PROVIDERto the environment reset ensures clean test state between test runs.
524-557: LGTM! Comprehensive test coverage for Provider configuration.The three new test cases properly validate:
- Provider set via properties file alone
- Provider set via environment variable alone
- Environment variable precedence over properties file
This aligns with the expected configuration precedence behavior.
provider_test.go (1)
15-120: LGTM! Thorough test coverage for UnderlyingProviderType.The test function comprehensively validates the provider resolution logic across all precedence levels:
- Explicit provider type always wins (lines 23-31, 71-87)
- Environment variable overrides properties file (lines 62-68)
- Properties file is respected when no env var is set (lines 34-43)
The test setup properly isolates each case with
config.Reset()and temporary directories, ensuring clean state between runs.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/system_requirements/using_podman.md(1 hunks)provider.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
provider.go (2)
internal/config/config.go (1)
Read(99-105)internal/core/docker_host.go (1)
MustExtractDockerHost(87-98)
🔇 Additional comments (3)
docs/system_requirements/using_podman.md (1)
85-86: LGTM! Clear documentation of the new configuration option.The documentation accurately describes the new feature, providing both the environment variable and properties file approaches as alternatives to per-test provider configuration.
provider.go (2)
96-118: Well-structured provider resolution with clear precedence.The implementation correctly follows the documented precedence: explicit code provider → configuration → auto-detection → default. The logic is clean and the comments clearly explain each step.
130-130: LGTM! Proper delegation to the new resolution logic.The change correctly delegates to
UnderlyingProviderType()to apply the full precedence chain before selecting the appropriate provider implementation.
| switch conf.Provider { | ||
| case "docker": | ||
| return ProviderDocker | ||
| case "podman": | ||
| return ProviderPodman | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify case handling for provider configuration values.
The string comparison for conf.Provider is case-sensitive. Ensure that the configuration parsing in internal/config/config.go normalizes the provider value to lowercase before storing it, otherwise users who set TESTCONTAINERS_PROVIDER=PODMAN (uppercase) might experience unexpected behavior.
🏁 Script executed:
#!/bin/bash
# Description: Check how config.Read() processes the Provider field and whether it normalizes case.
# Search for Provider field assignment and any case normalization logic
ast-grep --pattern $'Provider$$$=$$$'
# Also check for any strings.ToLower or similar normalization
rg -n -C3 'Provider.*=' internal/config/config.goLength of output: 371
Now I need to identify which library provides the properties tag parsing and whether it handles case normalization.
Based on my verification, the original review comment correctly identifies a real case-sensitivity issue. The evidence shows:
-
From the script output, the environment variable
TESTCONTAINERS_PROVIDERis read directly viaos.Getenv()and assigned toconfig.Providerwithout anystrings.ToLower()call. -
The switch statement in provider.go (lines 104-109) performs case-sensitive string matching for "docker" and "podman".
-
The magiconair/properties library used for the
propertiestag does not automatically normalize string values to lowercase—it preserves case as-is.
This means if a user sets TESTCONTAINERS_PROVIDER=PODMAN (uppercase), it won't match the case-sensitive "podman" check and will fall through to the default, causing unexpected behavior.
Add case normalization to environment variable assignment in internal/config/config.go.
At line 151, normalize the environment variable to lowercase before assigning it:
providerEnv := os.Getenv("TESTCONTAINERS_PROVIDER")
if providerEnv != "" {
config.Provider = strings.ToLower(providerEnv)
}This ensures the switch statement in provider.go (lines 104-109) will correctly match provider values regardless of the case used when setting the environment variable.
🏁 Script executed:
#!/bin/bash
# Find the library that handles properties tag parsing
cd /tmp && git clone https://github.com/testcontainers/testcontainers-go.git 2>/dev/null || true
cd testcontainers-go
# Search for imports in config.go that might handle properties tag
rg -n "import" internal/config/config.go
# Search for config.Read implementation
rg -n "func.*Read" internal/config/config.go -A 20
# Check go.mod for dependencies that might handle properties tags
rg -n "properties" go.modLength of output: 723
🏁 Script executed:
#!/bin/bash
# Search for github packages that provide properties tag parsing
curl -s "https://api.github.com/search/repositories?q=go+properties+tag+parser" | head -50Length of output: 163056
Based on the web search results, the magiconair/properties library's string field handling simply returns the string value as-is: case isString(t): v, err = s, nil. This means the library does NOT normalize string values to lowercase.
Verification confirms the original review comment is correct. The case-sensitivity bug exists:
- Only GetBool does case-insensitive comparison, not string values
- From the earlier script results, the environment variable is assigned directly without normalization:
config.Provider = providerEnv - The switch statement in provider.go performs case-sensitive matching for "docker" and "podman"
Add case normalization to environment variable assignment in internal/config/config.go.
At line 151, normalize the environment variable to lowercase before assigning it:
providerEnv := os.Getenv("TESTCONTAINERS_PROVIDER")
if providerEnv != "" {
config.Provider = strings.ToLower(providerEnv)
}This ensures the switch statement in provider.go (lines 104-109) will correctly match provider values regardless of the case used when setting the environment variable. Without this fix, users setting TESTCONTAINERS_PROVIDER=PODMAN (uppercase, a common practice) will silently have their configuration ignored, with the provider defaulting to Docker instead.
🤖 Prompt for AI Agents
In internal/config/config.go around line 151, the environment variable
TESTCONTAINERS_PROVIDER is assigned verbatim which yields case-sensitive values
and causes provider matching in provider.go (lines 104-109) to fail; update the
assignment to normalize the env var to lowercase before setting config.Provider
(i.e., set config.Provider = strings.ToLower(providerEnv) when providerEnv is
non-empty) and ensure the strings package is imported in the file.
|
Hi @mkeeler thanks for opening this PR. We are working on moving the core of this library to use https://github.com/docker/go-sdk so that the selection of the container runtime (the provider) will be managed by the Docker config file and its current docker context. So selecting Podman, Orbstack or Colima would be just a matter of changing the current context. We have examples here: https://github.com/mdelapenya/go-sdk-examples In any case, for an initial shallow review, I think this PR is harmless, although I need to start a more in-depth one, just wanted to give you feedback the soonest. |
|
@mdelapenya Thanks for the update. Using docker contexts would definitely be better. If moving to the docker/go-sdk is something that would land relatively soon then I see no reason to review and merge this PR. If it will take a while longer, then this does solve the test portability issue for now. |
2 participants
What does this PR do?
This PR allows for setting the Provider type (Podman or Docker) in either the properties file (e.g.
provider=podman) or as an environment variable (e.g.TESTCONTAINERS_PROVIDER=docker)Why is it important?
Using
WithProviderin tests results in tests that are not portable to other developer environments when not all contributors to a codebase all use the same container engine. This PR allows for user/environment settings to control the provider type ensuring that tests remain portable.Related issues
How to test this PR
WithProvideroption.TESTCONTAINERS_PROVIDER=podmanenv var.provider=podmanto the properties file.