chore(compose): update to compose-v5#3568
Conversation
Update to compose-v5 and add support for environment files.
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughAdds Go-version gating to CI (lint/test), dynamically selects golangci-lint based on module Go version, migrates Docker Compose usage from v2 to v5 in the compose module with separate compose and testcontainers clients, and bumps the compose module Go requirement to 1.25.0 with many dependency updates. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/compose/compose_local.go (1)
332-344:⚠️ Potential issue | 🟡 MinorAddress golangci-lint prealloc warning.
Pipeline failure indicates
execCmdshould be preallocated with capacity3 + len(args).🔧 Proposed fix for prealloc
- execCmd := []string{"Starting command", dirContext, binary} - execCmd = append(execCmd, args...) + execCmd := make([]string, 0, 3+len(args)) + execCmd = append(execCmd, "Starting command", dirContext, binary) + execCmd = append(execCmd, args...)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/compose/compose_local.go` around lines 332 - 344, The execCmd slice is being appended without preallocating causing a golangci-lint prealloc warning; change the initialization of execCmd in the block that builds the command (the variable execCmd used to populate the returned ExecError) to preallocate capacity with make([]string, 0, 3+len(args)) (so it holds the initial three entries and all args) then append the dirContext, binary and args as before; this resolves the prealloc warning while leaving the ExecError construction (Command, StdoutOutput, StderrOutput, Error, Stderr, Stdout) unchanged.
🧹 Nitpick comments (1)
modules/compose/compose_local.go (1)
121-125: Builder usage is slightly awkward but functionally correct.The
strings.Builderis created but the path separator is still concatenated via+before writing. A cleaner approach would write path and separator separately. However, this is a minor nit in deprecated code.♻️ Optional: cleaner Builder usage
var composeFileEnvVariableValueSb121 strings.Builder for _, abs := range dc.absComposeFilePaths { - composeFileEnvVariableValueSb121.WriteString(abs + string(os.PathListSeparator)) + composeFileEnvVariableValueSb121.WriteString(abs) + composeFileEnvVariableValueSb121.WriteByte(os.PathListSeparator) } composeFileEnvVariableValue += composeFileEnvVariableValueSb121.String()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/compose/compose_local.go` around lines 121 - 125, The Builder usage in compose_local.go is awkward: instead of concatenating the path separator with the path string using + before calling WriteString on composeFileEnvVariableValueSb121, iterate dc.absComposeFilePaths and call WriteString(abs) followed by WriteByte(os.PathListSeparator) (or WriteString(string(os.PathListSeparator))) to append the separator directly to the strings.Builder; finally append composeFileEnvVariableValueSb121.String() to composeFileEnvVariableValue as before. This change affects the loop that builds composeFileEnvVariableValueSb121 and leaves dc.absComposeFilePaths and composeFileEnvVariableValue unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci-lint-go.yml:
- Around line 38-47: The version selection logic around MODULE_GO_VERSION /
MODULE_MAJOR_MINOR is too coarse and may pick an incorrect golangci-lint release
for newer Go minors; replace the current single >= "1.25" branch with an
explicit mapping of supported MODULE_MAJOR_MINOR values (e.g., "1.20" -> v2.0.2,
"1.25" -> v2.5.0, "1.26" -> v2.10.0, etc.) implemented as a case/if-elif chain
that echoes the correct "version=..." to $GITHUB_OUTPUT for each known Go minor,
and add a fail-fast default that prints an error and exits nonzero when
MODULE_MAJOR_MINOR is not recognized so the workflow cannot silently select an
incompatible linter release.
In `@modules/compose/compose.go`:
- Around line 163-176: The NewDockerCompose function creates a testcontainers
provider and dockerClient but never closes them, causing resource leaks; add
resource cleanup by implementing a Close method on the DockerCompose type (e.g.,
func (d *DockerCompose) Close() error) that calls provider.Close() and
dockerClient.Close() (or the provider's Close which closes the underlying
client), and ensure Down() either calls this Close() or that callers invoke
Close() after Down(); update NewDockerCompose to store the created provider and
dockerClient on the DockerCompose struct (if not already) so Close() can access
them and return any close errors wrapped appropriately.
In `@modules/compose/go.mod`:
- Around line 122-131: Update the OpenTelemetry SDK dependency from v1.38.0 to
v1.40.0 (or later) to remediate GHSA-9h8m-3fm2-qjrq; specifically change the
module line for go.opentelemetry.io/otel/sdk in modules/compose/go.mod to
v1.40.0+ and run go mod tidy to refresh indirect dependencies (also verify
related otel packages like go.opentelemetry.io/otel,
go.opentelemetry.io/otel/trace, go.opentelemetry.io/otel/metric, and the otlp
exporters are compatible and updated as needed).
---
Outside diff comments:
In `@modules/compose/compose_local.go`:
- Around line 332-344: The execCmd slice is being appended without preallocating
causing a golangci-lint prealloc warning; change the initialization of execCmd
in the block that builds the command (the variable execCmd used to populate the
returned ExecError) to preallocate capacity with make([]string, 0, 3+len(args))
(so it holds the initial three entries and all args) then append the dirContext,
binary and args as before; this resolves the prealloc warning while leaving the
ExecError construction (Command, StdoutOutput, StderrOutput, Error, Stderr,
Stdout) unchanged.
---
Nitpick comments:
In `@modules/compose/compose_local.go`:
- Around line 121-125: The Builder usage in compose_local.go is awkward: instead
of concatenating the path separator with the path string using + before calling
WriteString on composeFileEnvVariableValueSb121, iterate dc.absComposeFilePaths
and call WriteString(abs) followed by WriteByte(os.PathListSeparator) (or
WriteString(string(os.PathListSeparator))) to append the separator directly to
the strings.Builder; finally append composeFileEnvVariableValueSb121.String() to
composeFileEnvVariableValue as before. This change affects the loop that builds
composeFileEnvVariableValueSb121 and leaves dc.absComposeFilePaths and
composeFileEnvVariableValue unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3953cf13-9c7c-4cb2-b701-ba0d65a01531
⛔ Files ignored due to path filters (1)
modules/compose/go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
.github/workflows/ci-lint-go.yml.github/workflows/ci-test-go.ymlmodules/compose/compose.gomodules/compose/compose_api.gomodules/compose/compose_api_test.gomodules/compose/compose_local.gomodules/compose/go.mod
There was a problem hiding this comment.
🧹 Nitpick comments (1)
modules/compose/compose_local.go (1)
121-125: Unusual variable name with numeric suffix.The variable name
composeFileEnvVariableValueSb121contains what appears to be a line number suffix (121). This looks like a debugging artifact or typo. Consider renaming to something cleaner likecomposeFileEnvVariableValueBuilderor simplysb.✨ Suggested rename
- var composeFileEnvVariableValueSb121 strings.Builder + var sb strings.Builder for _, abs := range dc.absComposeFilePaths { - composeFileEnvVariableValueSb121.WriteString(abs + string(os.PathListSeparator)) + sb.WriteString(abs + string(os.PathListSeparator)) } - composeFileEnvVariableValue += composeFileEnvVariableValueSb121.String() + composeFileEnvVariableValue = sb.String()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/compose/compose_local.go` around lines 121 - 125, The variable name composeFileEnvVariableValueSb121 is a stray/verbose identifier (likely from a debug or line-number artifact) — rename it to a clear name like composeFileEnvVariableValueBuilder (or sb) inside the function in modules/compose/compose_local.go, update its declaration and the call sites where WriteString and String() are used (the loop over dc.absComposeFilePaths and the subsequent append to composeFileEnvVariableValue) so all references match the new name; ensure imports remain unchanged and run tests/build to verify no other references exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@modules/compose/compose_local.go`:
- Around line 121-125: The variable name composeFileEnvVariableValueSb121 is a
stray/verbose identifier (likely from a debug or line-number artifact) — rename
it to a clear name like composeFileEnvVariableValueBuilder (or sb) inside the
function in modules/compose/compose_local.go, update its declaration and the
call sites where WriteString and String() are used (the loop over
dc.absComposeFilePaths and the subsequent append to composeFileEnvVariableValue)
so all references match the new name; ensure imports remain unchanged and run
tests/build to verify no other references exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da90a01a-89ca-4cd1-b268-36cacd91571d
📒 Files selected for processing (1)
modules/compose/compose_local.go
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docker_client.go (1)
73-77: Variable nameinfoLabelsSb72appears to be an automated refactoring artifact.The
72suffix looks like a line number remnant. Consider a cleaner name likelabelBuilderorsb.The change from in-loop string concatenation to
strings.Builderis a valid performance improvement.✨ Suggested variable rename
- var infoLabelsSb72 strings.Builder + var labelBuilder strings.Builder for _, lb := range dockerInfo.Labels { - infoLabelsSb72.WriteString("\n " + lb) + labelBuilder.WriteString("\n " + lb) } - infoLabels += infoLabelsSb72.String() + infoLabels += labelBuilder.String()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker_client.go` around lines 73 - 77, The variable name infoLabelsSb72 is an artifact; rename it to a clear name such as labelBuilder (or sb) in the block where dockerInfo.Labels are iterated and appended to infoLabels, i.e., replace infoLabelsSb72 with labelBuilder (create strings.Builder labelBuilder) and update the WriteString and final infoLabels += labelBuilder.String() calls so all references to infoLabelsSb72 are removed and use the new identifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docker_client.go`:
- Around line 73-77: The variable name infoLabelsSb72 is an artifact; rename it
to a clear name such as labelBuilder (or sb) in the block where
dockerInfo.Labels are iterated and appended to infoLabels, i.e., replace
infoLabelsSb72 with labelBuilder (create strings.Builder labelBuilder) and
update the WriteString and final infoLabels += labelBuilder.String() calls so
all references to infoLabelsSb72 are removed and use the new identifier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e4f5858-abeb-4612-bad3-e4325069fee5
⛔ Files ignored due to path filters (1)
modules/azurite/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
.github/workflows/ci-lint-go.ymldocker_client.gomodules/azurite/go.modwait/errors.go
💤 Files with no reviewable changes (1)
- wait/errors.go
|
Errors are not related to this PR, merging |
…m-v2 * upstream/main: (269 commits) chore(deps): bump actions/checkout from 6.0.1 to 6.0.2 (#3560) chore(deps): bump go.opentelemetry.io/otel/sdk to v1.41.0 (#3589) feat: add TiDB module (#3575) feat: add Forgejo module (#3556) feat: improve container conflict detection (#3574) chore(deps): bump go to 1.25 everywhere (#3572) chore(pulsar): bump base image to 4.x, replacing the wait for log strategy with wait for listening port (deterministic) (#3573) chore(deps): bump github.com/sigstore/sigstore in /modules/compose (#3571) chore(compose): update to compose-v5 (#3568) chore(deps): bump github.com/modelcontextprotocol/go-sdk (#3557) chore(deps): bump mkdocs-codeinclude-plugin from 0.2.1 to 0.3.1 (#3561) chore: update usage metrics (2026-03-02) (#3565) chore(deps): bump mkdocs-include-markdown-plugin from 7.2.0 to 7.2.1 (#3562) chore(deps): bump go.opentelemetry.io/otel/sdk in /modules/grafana-lgtm (#3563) chore(deps): bump go.opentelemetry.io/otel/sdk in /modules/toxiproxy (#3564) feat(azure): add lowkey vault container (#3542) feat(chroma): update to chroma 1.x (#3552) chore(deps): bump mkdocs-include-markdown-plugin from 7.2.0 to 7.2.1 (#3547) chore(deps): bump tj-actions/changed-files from 47.0.0 to 47.0.1 (#3546) chore(deps): bump actions/upload-artifact from 4.6.2 to 6.0.0 (#3545) ...
…archive-temp * upstream/main: chore(deps): bump actions/checkout from 6.0.1 to 6.0.2 (#3560) chore(deps): bump go.opentelemetry.io/otel/sdk to v1.41.0 (#3589) feat: add TiDB module (#3575) feat: add Forgejo module (#3556) feat: improve container conflict detection (#3574) chore(deps): bump go to 1.25 everywhere (#3572) chore(pulsar): bump base image to 4.x, replacing the wait for log strategy with wait for listening port (deterministic) (#3573) chore(deps): bump github.com/sigstore/sigstore in /modules/compose (#3571) chore(compose): update to compose-v5 (#3568) chore(deps): bump github.com/modelcontextprotocol/go-sdk (#3557) chore(deps): bump mkdocs-codeinclude-plugin from 0.2.1 to 0.3.1 (#3561) chore: update usage metrics (2026-03-02) (#3565) chore(deps): bump mkdocs-include-markdown-plugin from 7.2.0 to 7.2.1 (#3562) chore(deps): bump go.opentelemetry.io/otel/sdk in /modules/grafana-lgtm (#3563) chore(deps): bump go.opentelemetry.io/otel/sdk in /modules/toxiproxy (#3564)
…-action * upstream/main: (22 commits) chore(deps): bump golang.org/x/mod in /modules/localstack (#3587) chore(deps): bump golang.org/x/mod in /modules/elasticsearch (#3585) chore(deps): bump golang.org/x/mod in /modules/redpanda (#3588) chore(deps): bump golang.org/x/mod in /modules/kafka (#3586) chore(deps): bump github.com/shirou/gopsutil/v4 from 4.25.12 to 4.26.2 (#3576) chore(deps): bump github.com/moby/go-archive from 0.1.0 to 0.2.0 (#3548) chore(deps): bump github.com/moby/term from 0.5.0 to 0.5.2 (#3081) chore(deps): bump actions/checkout from 6.0.1 to 6.0.2 (#3560) chore(deps): bump go.opentelemetry.io/otel/sdk to v1.41.0 (#3589) feat: add TiDB module (#3575) feat: add Forgejo module (#3556) feat: improve container conflict detection (#3574) chore(deps): bump go to 1.25 everywhere (#3572) chore(pulsar): bump base image to 4.x, replacing the wait for log strategy with wait for listening port (deterministic) (#3573) chore(deps): bump github.com/sigstore/sigstore in /modules/compose (#3571) chore(compose): update to compose-v5 (#3568) chore(deps): bump github.com/modelcontextprotocol/go-sdk (#3557) chore(deps): bump mkdocs-codeinclude-plugin from 0.2.1 to 0.3.1 (#3561) chore: update usage metrics (2026-03-02) (#3565) chore(deps): bump mkdocs-include-markdown-plugin from 7.2.0 to 7.2.1 (#3562) ...
The compose module was upgraded to v5 in testcontainers#3568 but the Makefile's compose-replace target still referenced docker/compose/v2. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The compose module was upgraded to v5 in #3568 but the Makefile's compose-replace target still referenced docker/compose/v2. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
What does this PR do?
Upgrades the
modules/composemodule from Docker Compose v2 (github.com/docker/compose/v2) to v5 (github.com/docker/compose/v5), using the official SDK introduced in v5.Key changes:
docker/compose/v2 v2.40.2→docker/compose/v5 v5.1.0,compose-spec/compose-go/v2bumped fromv2.9.0tov2.10.1LoadProjectAPI: Replaced manual project compilation (cli.NewProjectOptions+compiledOptions.LoadProject) withcomposeService.LoadProject(ctx, api.ProjectLoadOptions{...})from the v5 SDKdocker/docker/clienttomoby/moby/client, making the types incompatible. The compose service now uses its own Docker CLI client, while testcontainers provider and direct API calls use a separatetestcontainers.NewDockerClientWithOpts()clients.EnvFiles(from the compose file'senv_filedeclarations) instead of the oldcli.ProjectOptions.EnvFilesgo 1.25.0(required by compose v5 transitive dependencies). CI workflow updated to skip modules on incompatible Go versionscompose_local.go:determineVersion()now handles compose v5+ (treatsmajorVersion >= 2as v2-style dash separator)Files changed:
modules/compose/compose.go— v5 imports, decoupled client initialization, removedmakeClientmodules/compose/compose_api.go— v5 imports,LoadProjectAPI, env file labels from service configmodules/compose/compose_api_test.go— v5 importmodules/compose/compose_local.go— version detection for v5+modules/compose/go.mod/go.sum— dependency updates.github/workflows/ci-test-go.yml— Go version compatibility check to skip modules requiring newer GoWhy is it important?
Related issues
How to test this PR
All 42 existing tests pass (2 skipped:
TestDockerComposeAPIWithBuildandTestDockerComposeAPI_WithoutReaper).Follow-ups
WithOsEnv()method is now effectively a no-op: compose v5'sLoadProjectalways includescli.WithOsEnvinternally. Consider deprecating itLocalDockerCompose(shell-based) was kept but could be removed in a future major release