feat(atlaslocal): add MongoDB Atlas Local module #3254
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
prestonvasquez
force-pushed
the
feat/atlaslocal-module
branch
from
242d6d9 to
2e72f6f
Compare
prestonvasquez
changed the title
prestonvasquez
changed the title
prestonvasquez
changed the title
prestonvasquez
changed the title
prestonvasquez
force-pushed
the
feat/atlaslocal-module
branch
from
2e72f6f to
b4f178a
Compare
There was a problem hiding this comment.
Hi @prestonvasquez thanks for such a well-structured PR, the code is following our patterns in the majority of the places, and is pretty easy to read, thanks for this.
I left a couple of comments, but let me start with this: do we need a separate module for Atlas, or is it fine to have an specialised container in the MongoDB module? I ask because in tc-java the atlas container lives in the same JAR file. Of course, we are not coupled to that, but as you work for Mongo, you probably have a better opinion on this. Let's discuss this and then we can move forward with the rest of the review 🙏
Cheers!
modules/atlaslocal/atlaslocal.go
Outdated
| // WithMongotLogFile sets the path to the file where you want to store the logs | ||
| // of Atlas Search (mongot) by setting the MONGOT_LOG_FILE environment variable. | ||
| // Note that this can be set to /dev/stdout or /dev/stderr for convenience. | ||
| func WithMongotLogFile(logFile string) testcontainers.CustomizeRequestOption { |
There was a problem hiding this comment.
question: should we simplify the API adding types for stdout/stderr so users don't configure a non existing path? same for the runner log file
There was a problem hiding this comment.
Good idea. Suggest we use the following functions:
WithMongotLogToStdout()
WithMongotLogToStderr()
WithMongotLogToFile() // always logs to "/tmp/mongot.log"And then a convenience to copy (wrapping CopyFileFromContainer) the file from container:
(*Container) CopyMongotLog(ctx context.Context) (io.ReadCloser, error) {} 
mdelapenya
added
the
feature
|
@mdelapenya Thanks for the thoughtful feedback!
I have the following concerns with combining
func WithUsername(username string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) error {
if username != "" {
req.Env["MONGODB_INITDB_ROOT_USERNAME"] = username // atlaslocal
req.Env["MONGO_INITDB_ROOT_USERNAME"] = username // mongodb
}
return nil
}
}
I agree that package main
import (
"github.com/testcontainers/testcontainers-go/modules/mongodb/atlaslocal"
)
// ...If this is acceptable, could we retain the existing documentation for this at |
|
@prestonvasquez thanks for your detailed response. Indeed, I think following the gcloud module pattern would be of interest, having a About the docs, I'm totally fine if the entry is This PR looks promising! |
prestonvasquez
force-pushed
the
feat/atlaslocal-module
branch
from
3e4dfef to
ba459ad
Compare
| github.com/testcontainers/testcontainers-go v0.38.0 | ||
| go.mongodb.org/mongo-driver v1.13.1 | ||
| go.mongodb.org/mongo-driver v1.17.4 | ||
| go.mongodb.org/mongo-driver/v2 v2.3.0 |
There was a problem hiding this comment.
@mdelapenya Combining atlaslocal into modules/mongodb requires that we maintain both v1 and v2 in the go.mod file. I anticipate that the Go toolchain will prune this data from the dependency topology if a user is only using modules/mongodb. Perhaps we should open a ticket to create a v2 for the mongodb testcontainer?
There was a problem hiding this comment.
@stevenh wdyt? I really don't like how Go manages major versions, as it comes with a lot of maintenance work
There was a problem hiding this comment.
@prestonvasquez sorry for the delay, I'm in a conference. Let's make progress by keeping the original approach of a separate atlaslocal module. The fact that the project name is relevant enough and we bypass the dependencies issue you raised, makes me think it deserves to have its own namespace. Sorry for the back and forth 🙏
There was a problem hiding this comment.
Hi, I'm another member of the MongoDB Go Driver team. It's easy to update the MongoDB Go Driver to v2 in the mongodb module (see #3278). I'm in favor of keeping atlaslocal in the mongodb module.
@mdelapenya are there other reasons to separate the atlaslocal module, or just the v1/v2 driver dependency?
There was a problem hiding this comment.
Hi @matthewdale thanks for the bump PR. Given the driver version matches, we can keep this one as a submodule of the parent mongodb module. I just wanted to make progress and not block you with this one.
|
|
||
| // Run creates an instance of the MongoDBAtlasLocal container type | ||
| func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustomizer) (*Container, error) { | ||
| req := testcontainers.ContainerRequest{ |
There was a problem hiding this comment.
suggestion: we are moving away from defining the container request here in the modules, but instead using the more modern testcontainers.Run function, which needs an slice of options. You basically need to convert the current req struct into the options slice.
PS: I can do this for you if you feel it's too much work for the review. I would do it adding commits on top of yours, or in a follow-up
| github.com/testcontainers/testcontainers-go v0.38.0 | ||
| go.mongodb.org/mongo-driver v1.13.1 | ||
| go.mongodb.org/mongo-driver v1.17.4 | ||
| go.mongodb.org/mongo-driver/v2 v2.3.0 |
There was a problem hiding this comment.
@stevenh wdyt? I really don't like how Go manages major versions, as it comes with a lot of maintenance work
prestonvasquez
force-pushed
the
feat/atlaslocal-module
branch
2 times, most recently
from
8167d6c to
d284caa
Compare
|
@mdelapenya Preston is going to be out for a few weeks, is it possible to move this PR along while he's out? I see that merging #3278 created merge conflicts in this PR. I've created a merge commit matthewdale@ce8a9e4 that resolves the conflicts. I made that commit by running: gh pr checkout 3254
git merge main
git checkout main modules/mongodb/go.* modules/mongodb/*.go
git merge --continueAre you able to resolve the merge conflicts? Are there any other changes needed before merging this PR? |
|
@matthewdale sure, I can take over and resolve the conflicts by myself, and push the changes to this branch. Thanks for letting me know 🙏 |
* main: (22 commits) chore(deps): bump golang.org/x/net from 0.28.0 to 0.38.0 (testcontainers#3299) feat: allow saving specific platforms for an image (testcontainers#3218) chore(deps): bump dario.cat/mergo from 1.0.1 to 1.0.2 (testcontainers#3238) chore(deps): bump golang.org/x/sys from 0.32.0 to 0.36.0 (testcontainers#3282) feat(redpanda): add support for http proxy (testcontainers#3258) chore(deps): bump github/codeql-action from 3.29.3 to 3.30.3 (testcontainers#3287) chore(go): bump to Go 1.24 as minimal version (testcontainers#3298) deps(mongodb): update MongoDB Go Driver to v2 (testcontainers#3278) chore(deps): bump github.com/shirou/gopsutil/v4 from 4.25.5 to 4.25.6 (testcontainers#3224) chore(deps): bump mkdocs-include-markdown-plugin from 7.1.6 to 7.1.7 (testcontainers#3284) docs: clarify no client SDKs in production modules/images, in contributing.md (testcontainers#3279) chore(deps): bump github.com/docker/go-connections from 0.5.0 to 0.6.0 (testcontainers#3285) chore(deps): bump tj-actions/changed-files from 46.0.3 to 47.0.0 (testcontainers#3283) chore(modulegen): detect missing project files after new module creation (testcontainers#3281) chore(deps): bump github.com/docker/docker in /modules/nebulagraph (testcontainers#3277) feat(nebulagraph): add NebulaGraph module (testcontainers#3266) fix: preserve unix socket schema in testcontainers host from properties (testcontainers#3213) feat(registry): add helper functions to pull and tag images (testcontainers#3275) fix(reaper): remove termSignal override (testcontainers#3261) chore(deps): bump ryuk to v0.13.0, which uses scratch as base image (testcontainers#3274) ...
It also resolves some Ipv6 issues
Summary by CodeRabbit
WalkthroughAdds a new MongoDB Atlas Local module: implementation (container wrapper and options), docs and mkdocs nav entry, integration tests and examples, plus connection-string and log-reading APIs for Testcontainers-Go. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant Run as atlaslocal.Run
participant Docker as Docker Engine
participant C as Container
participant Mongo as MongoDB (in Container)
Dev->>Run: Run(ctx, image, opts...)
Run->>Run: apply & validate options (env, mounts, logs)
Run->>Docker: create & start container (expose 27017/tcp, wait)
Docker-->>Run: container ready
Run-->>Dev: *Container
Dev->>C: ConnectionString(ctx)
C-->>Dev: mongodb://host:port[/db]/?directConnection=true[&authSource=admin]
Dev->>Mongo: connect & ping
Mongo-->>Dev: OK
alt read mongot logs
Dev->>C: ReadMongotLogs(ctx)
C-->>Dev: io.ReadCloser (file or stdout/stderr)
end
alt read runner logs
Dev->>C: ReadRunnerLogs(ctx)
C-->>Dev: io.ReadCloser (file or stdout/stderr)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (19)
mkdocs.yml (1)
103-103: Consider renaming the documentation entry to "MongoDB Atlas Local"Per the reviewer's comment, the documentation entry should be titled "MongoDB Atlas Local" for clarity and consistency with the module's full name.
Apply this change to improve documentation clarity:
- - modules/mongodb-atlaslocal.md + - MongoDB Atlas Local: modules/mongodb-atlaslocal.mdmodules/mongodb/atlaslocal/options.go (4)
32-33: Typo in struct field nameThe field
localUsernamefileshould belocalUsernameFilefor consistency with Go naming conventions.Apply this fix:
- localUsernamefile string + localUsernameFile string
89-89: Update field references after fixing typoAfter fixing the typo in the struct field name, update all references throughout the file.
Apply these fixes to update references:
- usernameFile := opts.localUsernamefile + usernameFile := opts.localUsernameFile- if opts.username == "" && opts.localUsernamefile == "" { + if opts.username == "" && opts.localUsernameFile == "" {- if opts.username != "" && opts.localUsernamefile != "" { + if opts.username != "" && opts.localUsernameFile != "" {- r, err := os.ReadFile(opts.localUsernamefile) + r, err := os.ReadFile(opts.localUsernameFile)- opts.localUsernamefile = usernameFile + opts.localUsernameFile = usernameFileAlso applies to: 111-111, 115-115, 123-123, 205-205
148-148: Incorrect module reference in commentThe comment incorrectly references "Redpanda container" instead of "MongoDB Atlas Local container".
Apply this fix:
-// Option is an option for the Redpanda container. +// Option is an option for the MongoDB Atlas Local container.
298-337: Decide whether WithInitScripts should be additive or replace previous mountsVerification: most modules implement additive semantics (append init scripts) — e.g. modules/postgres/postgres.go, modules/clickhouse/clickhouse.go, modules/cockroachdb/options.go, modules/cassandra/cassandra.go. modules/mongodb/atlaslocal currently replaces prior mounts (documented in modules/mongodb/atlaslocal/options.go and asserted by modules/mongodb/atlaslocal/atlaslocal_test.go TestWithInitScripts_MultipleScripts).
Action: choose one:
- Make atlaslocal additive to match other modules (remove the filter that clears /docker-entrypoint-initdb.d), or
- Keep replacement behavior but make it explicit in the public API/docs (or add an explicitly additive variant) and update docs/tests accordingly.
docs/modules/mongodb-atlaslocal.md (2)
18-20: Add language specifier to fenced code blockThe fenced code block should specify the language for proper syntax highlighting.
Apply this fix:
-``` +```bash go get github.com/testcontainers/testcontainers-go/modules/mongodb/atlaslocal
74-74: Fix typo: missing space after "at"There's a missing space between "at" and the backtick in the documentation.
Apply this fix:
-This functional option mounts a local file as the MongoDB root username secret at`/run/secrets/mongo-root-username` +This functional option mounts a local file as the MongoDB root username secret at `/run/secrets/mongo-root-username`modules/mongodb/atlaslocal/atlaslocal.go (4)
63-66: Use the declared constant instead of a magic string.Minor consistency/readability nit: reuse
defaultPort.- endpoint, err := ctr.PortEndpoint(ctx, "27017/tcp", "") + endpoint, err := ctr.PortEndpoint(ctx, defaultPort, "")
109-113: Fix comment grammar.“This method returns …” (plural agreement).
-// This method return the os.ErrNotExist sentinel error if it is called with +// This method returns the os.ErrNotExist sentinel error if it is called with
127-132: Fix comment grammar and remove parentheses from method name.Keep comments consistent with Go doc style.
-// ReadRunnerLogs() returns a reader for runner logs in the container. Reads +// ReadRunnerLogs returns a reader for runner logs in the container. Reads @@ -// This method return the os.ErrNotExist sentinel error if it is called with +// This method returns the os.ErrNotExist sentinel error if it is called with
37-42: Guard healthcheck wait or pin image tagConfirmed: mongodb/mongodb-atlas-local includes a built-in HEALTHCHECK (MongoDB docs), but Feb 2025 image pushes caused intermittent healthcheck/unhealthy failures — pinning to a known-good tag/sha was recommended.
File: modules/mongodb/atlaslocal/atlaslocal.go (lines 37–42)
- Make wait.ForHealthCheck() optional or implement a fallback to wait.ForListeningPort(defaultPort) if the healthcheck times out (or expose this as a configurable option).
- Alternatively, pin the image to a stable tag/sha and document the HEALTHCHECK requirement and rationale.
modules/mongodb/atlaslocal/atlaslocal_test.go (8)
28-31: Schedule cleanup only after ensuring container creation succeeded.Calling
CleanupContainer(t, ctr)beforerequire.NoError(t, err)risks a typed-nil panic ifRunfails. Reorder to assert first, then schedule cleanup. Apply this pattern across all tests.Example diffs:
@@ TestMongoDBAtlasLocal - ctr, err := atlaslocal.Run(ctx, latestImage) - testcontainers.CleanupContainer(t, ctr) - require.NoError(t, err) + ctr, err := atlaslocal.Run(ctx, latestImage) + require.NoError(t, err) + testcontainers.CleanupContainer(t, ctr)@@ inside TestSCRAMAuth loop - ctr, err := atlaslocal.Run(context.Background(), latestImage, opts...) - testcontainers.CleanupContainer(t, ctr) + ctr, err := atlaslocal.Run(context.Background(), latestImage, opts...) ... - require.NoError(t, err) + require.NoError(t, err) + testcontainers.CleanupContainer(t, ctr)@@ TestWithNoTelemetry subtests (apply to both) - ctr, err := atlaslocal.Run(context.Background(), latestImage, atlaslocal.WithNoTelemetry()) - testcontainers.CleanupContainer(t, ctr) - require.NoError(t, err) + ctr, err := atlaslocal.Run(context.Background(), latestImage, atlaslocal.WithNoTelemetry()) + require.NoError(t, err) + testcontainers.CleanupContainer(t, ctr)(Repeat the same ordering fix in all places listed in the line ranges.)
Also applies to: 190-203, 229-232, 237-240, 247-249, 258-261, 269-275, 284-289, 301-308, 311-318, 321-327, 334-341, 360-363, 437-440, 482-485, 533-535
573-582: Close the reader to avoid leaking resources.
ReadMongotLogsreturns anio.ReadCloserthat must be closed.func requireMongotLogs(t *testing.T, ctr testcontainers.Container) { @@ - reader, err := ctr.(*atlaslocal.Container).ReadMongotLogs(context.Background()) + reader, err := ctr.(*atlaslocal.Container).ReadMongotLogs(context.Background()) require.NoError(t, err) - - buf := make([]byte, 1) - _, _ = reader.Read(buf) // read at least one byte to ensure non-empty + defer reader.Close() + buf := make([]byte, 1) + _, _ = reader.Read(buf) // read at least one byte to ensure non-empty }
565-571: Generic assertion message for env var helper.Message currently always mentions DO_NOT_TRACK, which is misleading for other vars.
- require.Equal(t, expected, out, "DO_NOT_TRACK env var value mismatch") + require.Equalf(t, expected, out, "%s env var value mismatch", envVarName)
724-733: Don’t assert onlsexit code; directory presence varies by image.
/docker-entrypoint-initdb.dmay exist even when empty, making the “no scripts” case brittle. Drop the exit-code assertion; rely on per-file checks below.- exit, r, err := ctr.Exec(context.Background(), []string{"sh", "-lc", "ls -l " + dstDir}) + exit, r, err := ctr.Exec(context.Background(), []string{"sh", "-lc", "ls -l " + dstDir + " || true"}) require.NoError(t, err) - - // If the map is empty, the command returns exit code 2. - if len(expectedScripts) == 0 { - require.Equal(t, 2, exit) - } else { - require.Equal(t, 0, exit) - } + _ = exit // ignore, we read listing instead
760-768: Make path absolute and avoid relying oncdsuccess.Use an absolute path and list defensively; then assert absence.
- for filename := range expectedScripts { - cmd := []string{"sh", "-c", "cd docker-entrypoint-initdb.d && ls -l"} - - exitCode, reader, err := ctr.Exec(context.Background(), cmd) - require.NoError(t, err) - require.Zero(t, exitCode, "Expected exit code 0 for command: %v", cmd) - - content, _ := io.ReadAll(reader) - require.NotContains(t, string(content), filename) - } + for filename := range expectedScripts { + cmd := []string{"sh", "-lc", "ls -1 /docker-entrypoint-initdb.d 2>/dev/null || true"} + _, reader, err := ctr.Exec(context.Background(), cmd) + require.NoError(t, err) + content, _ := io.ReadAll(reader) + require.NotContains(t, string(content), filename) + }
542-549: Avoid brittle host slicing; parse host:port instead.Providers may return different hosts (e.g., 127.0.0.1). Split and assert both parts are non-empty.
- require.Equal(t, "mongodb", connString.Scheme) - require.Equal(t, "localhost", connString.Hosts[0][:9]) - require.NotEmpty(t, connString.Hosts[0][10:], "Port should be non-empty") + require.Equal(t, "mongodb", connString.Scheme) + host, port, err := net.SplitHostPort(connString.Hosts[0]) + require.NoError(t, err, "Invalid host:port") + require.NotEmpty(t, host) + require.NotEmpty(t, port)Add:
+ "net"to the imports at the top of the test file.
793-804: Use restrictive permissions for secret test files.Prefer
0600for username/password test files to mirror secret semantics and avoid permissive perms.- err := os.WriteFile(usernameFilepath, []byte("file_testuser"), 0o755) + err := os.WriteFile(usernameFilepath, []byte("file_testuser"), 0o600) @@ - err = os.WriteFile(passwordFilepath, []byte("file_testpass"), 0o755) + err = os.WriteFile(passwordFilepath, []byte("file_testpass"), 0o600)
23-24: Pin test image to a known tag (or digest) for stability.Using
:latestcan introduce flakiness when the upstream image changes. Consider pinning to a specific tag or digest, optionally allowing override via env.-const latestImage = "mongodb/mongodb-atlas-local:latest" +const latestImage = "mongodb/mongodb-atlas-local:7.0.14" // example; consider updating to a tested tag or allow override via envWould you like me to check the currently recommended stable tag and open a follow-up PR to pin and document it?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/dependabot.yml(1 hunks)docs/modules/mongodb-atlaslocal.md(1 hunks)mkdocs.yml(1 hunks)modules/mongodb/atlaslocal/atlaslocal.go(1 hunks)modules/mongodb/atlaslocal/atlaslocal_test.go(1 hunks)modules/mongodb/atlaslocal/examples_test.go(1 hunks)modules/mongodb/atlaslocal/options.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
modules/mongodb/atlaslocal/examples_test.go (4)
modules/mongodb/examples_test.go (2)
ExampleRun(16-42)ExampleRun_connect(44-82)modules/mongodb/atlaslocal/atlaslocal.go (1)
Run(23-57)cleanup.go (1)
TerminateContainer(97-108)modules/mongodb/atlaslocal/options.go (2)
WithMongotLogFile(363-369)WithRunnerLogFile(393-399)
modules/mongodb/atlaslocal/atlaslocal.go (5)
options.go (4)
ContainerCustomizer(22-24)WithExposedPorts(464-469)WithWaitStrategy(376-378)WithFiles(534-539)modules/mongodb/atlaslocal/options.go (1)
Option(149-149)wait/all.go (1)
ForAll(44-48)wait/host_port.go (1)
ForListeningPort(67-69)wait/health.go (1)
ForHealthCheck(55-57)
modules/mongodb/atlaslocal/atlaslocal_test.go (4)
modules/mongodb/atlaslocal/atlaslocal.go (2)
Run(23-57)Container(17-20)testing.go (1)
CleanupContainer(91-97)options.go (1)
ContainerCustomizer(22-24)modules/mongodb/atlaslocal/options.go (13)
WithUsername(159-167)WithPassword(171-179)WithUsernameFile(184-215)WithPasswordFile(220-251)WithNoTelemetry(255-261)WithMongotLogFile(363-369)WithMongotLogToStdout(343-349)WithMongotLogToStderr(353-359)WithRunnerLogFile(393-399)WithRunnerLogToStdout(373-379)WithRunnerLogToStderr(383-389)WithInitDatabase(266-272)WithInitScripts(278-339)
modules/mongodb/atlaslocal/options.go (3)
options.go (1)
ContainerCustomizer(22-24)container.go (1)
ContainerFile(110-115)generic.go (1)
GenericContainerRequest(21-27)
🪛 markdownlint-cli2 (0.17.2)
docs/modules/mongodb-atlaslocal.md
18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
modules/mongodb/atlaslocal/examples_test.go (1)
1-174: LGTM! Well-structured examplesThe example tests are comprehensive and follow best practices:
- Proper cleanup with
deferandTerminateContainer- Clear demonstration of each feature (basic run, connection, log reading)
- Appropriate error handling throughout
- Good use of example output comments for testable examples
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/mongodb/mongodb.go (1)
57-66: Bug: reading env from the wrong request object breaks auth/replica set logic.Options mutate genericContainerReq.Env, but the code reads req.Env (the original, unmodified struct). This causes missing validation and replica set misconfiguration when options are provided.
Apply this diff:
- username := req.Env["MONGO_INITDB_ROOT_USERNAME"] - password := req.Env["MONGO_INITDB_ROOT_PASSWORD"] + username := genericContainerReq.Env["MONGO_INITDB_ROOT_USERNAME"] + password := genericContainerReq.Env["MONGO_INITDB_ROOT_PASSWORD"] @@ - replicaSet := req.Env[replicaSetOptEnvKey] + replicaSet := genericContainerReq.Env[replicaSetOptEnvKey]Also applies to: 68-73
🧹 Nitpick comments (10)
modules/mongodb/mongodb.go (1)
196-199: Avoid hard-coding internal port "27017" in replica set initiation.Define a single internal port constant to prevent future drift.
Apply this diff:
@@ -const ( - defaultPort = "27017/tcp" +const ( + defaultPort = "27017/tcp" + internalMongoPort = "27017" @@ - cmd := cli.eval( - "rs.initiate({ _id: '%s', members: [ { _id: 0, host: '%s:27017' } ] })", - replSetName, - ip, - ) + cmd := cli.eval( + "rs.initiate({ _id: '%s', members: [ { _id: 0, host: '%s:%s' } ] })", + replSetName, + ip, + internalMongoPort, + )docs/modules/mongodb-atlaslocal.md (2)
18-20: Fix markdownlint MD040: add language to fenced block.Apply this diff:
-``` -go get github.com/testcontainers/testcontainers-go/modules/mongodb/atlaslocal -``` +```bash +go get github.com/testcontainers/testcontainers-go/modules/mongodb/atlaslocal +```
37-39: Use "go" as the code fence language (not "golang").This matches common tooling/highlighters.
Apply this diff:
-```golang +```go func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustomizer) (*Container, error)</blockquote></details> <details> <summary>modules/mongodb/atlaslocal/atlaslocal_test.go (7)</summary><blockquote> `28-31`: **Schedule container cleanup only after successful start.** Calling CleanupContainer before verifying err may pass a nil container to the terminator in failure cases. Apply this pattern across the file: ```diff -ctr, err := atlaslocal.Run(context.Background(), latestImage, opts...) -testcontainers.CleanupContainer(t, ctr) -require.NoError(t, err) +ctr, err := atlaslocal.Run(context.Background(), latestImage, opts...) +require.NoError(t, err) +testcontainers.CleanupContainer(t, ctr)Also applies to: 191-201, 229-233, 237-241, 246-250, 258-261, 269-276, 284-288, 302-305, 311-315, 321-325, 335-339, 360-363, 438-440, 483-485, 533-536
543-549: Make host/port assertions robust.String slicing assumes "localhost:". Use net.SplitHostPort and don’t assume the host literal.
Apply this diff:
@@ - require.Equal(t, "mongodb", connString.Scheme) - require.Equal(t, "localhost", connString.Hosts[0][:9]) - require.NotEmpty(t, connString.Hosts[0][10:], "Port should be non-empty") + require.Equal(t, "mongodb", connString.Scheme) + host, port, err := net.SplitHostPort(connString.Hosts[0]) + require.NoError(t, err) + require.NotEmpty(t, port, "Port should be non-empty") + require.Contains(t, []string{"localhost", "127.0.0.1"}, host)And add the import:
import ( "context" "io" - "math/rand/v2" + "math/rand" + "net" "os"
23-24: Allow overriding the test image via env var to improve reproducibility.Pinning or overriding avoids flakiness from the moving "latest" tag.
Apply this diff:
-const latestImage = "mongodb/mongodb-atlas-local:latest" +var latestImage = func() string { + if v := os.Getenv("ATLASLOCAL_IMAGE"); v != "" { + return v + } + return "mongodb/mongodb-atlas-local:latest" +}()
566-572: Clarify assertion message in requireEnvVar().Use the actual env var name in failure messages.
Apply this diff:
- out := strings.TrimSpace(string(outBytes[8:])) - require.Equal(t, expected, out, "DO_NOT_TRACK env var value mismatch") + out := strings.TrimSpace(string(outBytes[8:])) + require.Equal(t, expected, out, "%s env var value mismatch", envVarName)
762-766: Use absolute path when listing init scripts.Avoid relying on container working directory.
Apply this diff:
- cmd := []string{"sh", "-c", "cd docker-entrypoint-initdb.d && ls -l"} + cmd := []string{"sh", "-c", "ls -l /docker-entrypoint-initdb.d"}
795-806: Use restrictive permissions for credential files in tests.Secrets should not be executable.
Apply this diff:
- err := os.WriteFile(usernameFilepath, []byte("file_testuser"), 0o755) + err := os.WriteFile(usernameFilepath, []byte("file_testuser"), 0o600) @@ - err = os.WriteFile(passwordFilepath, []byte("file_testpass"), 0o755) + err = os.WriteFile(passwordFilepath, []byte("file_testpass"), 0o600)
574-584: Consider checking Read*Logs actually returns data.Read and assert n > 0 to avoid false positives if EOF.
Apply this diff:
- buf := make([]byte, 1) - _, _ = reader.Read(buf) // read at least one byte to ensure non-empty + buf := make([]byte, 1) + n, err := reader.Read(buf) + require.Greater(t, n, 0)And similarly in requireRunnerLogs.
Also applies to: 601-609
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/modules/mongodb-atlaslocal.md(1 hunks)modules/mongodb/atlaslocal/atlaslocal.go(1 hunks)modules/mongodb/atlaslocal/atlaslocal_test.go(1 hunks)modules/mongodb/atlaslocal/options.go(1 hunks)modules/mongodb/mongodb.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- modules/mongodb/atlaslocal/atlaslocal.go
- modules/mongodb/atlaslocal/options.go
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mdelapenya
PR: testcontainers/testcontainers-go#3254
File: .github/dependabot.yml:21-21
Timestamp: 2025-09-18T08:24:27.458Z
Learning: In the testcontainers-go repository, submodules like atlaslocal that are part of a parent module (e.g., mongodb) share the same go.mod file and should not have separate Dependabot entries. They are already monitored through the parent module's Dependabot configuration entry.
📚 Learning: 2025-09-18T08:24:27.458Z
Learnt from: mdelapenya
PR: testcontainers/testcontainers-go#3254
File: .github/dependabot.yml:21-21
Timestamp: 2025-09-18T08:24:27.458Z
Learning: In the testcontainers-go repository, submodules like atlaslocal that are part of a parent module (e.g., mongodb) share the same go.mod file and should not have separate Dependabot entries. They are already monitored through the parent module's Dependabot configuration entry.
Applied to files:
modules/mongodb/atlaslocal/atlaslocal_test.godocs/modules/mongodb-atlaslocal.md
🧬 Code graph analysis (2)
modules/mongodb/mongodb.go (1)
wait/host_port.go (1)
ForListeningPort(67-69)
modules/mongodb/atlaslocal/atlaslocal_test.go (4)
modules/mongodb/atlaslocal/atlaslocal.go (2)
Run(23-57)Container(17-20)testing.go (1)
CleanupContainer(91-97)options.go (1)
ContainerCustomizer(22-24)modules/mongodb/atlaslocal/options.go (13)
WithUsername(159-167)WithPassword(171-179)WithUsernameFile(184-215)WithPasswordFile(220-251)WithNoTelemetry(255-261)WithMongotLogFile(363-369)WithMongotLogToStdout(343-349)WithMongotLogToStderr(353-359)WithRunnerLogFile(393-399)WithRunnerLogToStdout(373-379)WithRunnerLogToStderr(383-389)WithInitDatabase(266-272)WithInitScripts(278-339)
🪛 markdownlint-cli2 (0.17.2)
docs/modules/mongodb-atlaslocal.md
18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (1.25.x, modules/mongodb) / test: modules/mongodb/1.25.x
- GitHub Check: test (1.24.x, modules/mongodb) / test: modules/mongodb/1.24.x
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
modules/mongodb/mongodb.go (4)
20-20: Good: centralized MongoDB port constant.Replacing string literals with a single constant improves maintainability.
44-48: LGTM: consistent use of defaultPort in request and wait strategy.This removes duplication and reduces drift risk.
122-123: LGTM: ConnectionString now uses defaultPort.Consistent with the other changes.
20-20: No export needed — atlaslocal defines its own defaultPort.
modules/mongodb/atlaslocal/atlaslocal.go declaresconst defaultPort = "27017/tcp"(line 14) and uses it; modules/mongodb/mongodb.go has its own unexporteddefaultPort(line 20). No cross-package export or change required.modules/mongodb/atlaslocal/atlaslocal_test.go (1)
185-189: Incorrect — repo targets Go 1.24; math/rand/v2 is supported.
go.mod declares "go 1.24.0"; modules/mongodb/atlaslocal/atlaslocal_test.go imports "math/rand/v2" and uses rand.Shuffle — no change required.Likely an incorrect or invalid review comment.
|
@prestonvasquez @matthewdale thanks again for your work here. If possible, checking that the mongodb module also follows the best practices recommended by the vendor would be really helpful 🙏 |
|
@prestonvasquez @matthewdale the module is present in our modules catalog here: https://testcontainers.com/modules/mongodb-atlas/?language=go Thanks! |
3 participants
What does this PR do?
This PR introducing a new module for spinning up a MongoDB Atlas Local instance in Docker for integration testing. See documentation for mongodb/mongodb-atlas-local.
Why is it important?
This module enables users to test against Atlas Search and Atlas Vector Search in CI environments without needing to start a live MongoDB Atlas cluster. While users sometimes attempt to use
mongodb/mongodb-atlas-localwith the standardmongodbpackage—which may appear to work because of the port27017wait condition—the underlying options are actually incompatible. For more details, see the related issue.Related issues