-
-
Couldn't load subscription status.
- Fork 584
chore(postgres): use Run function #3425
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
chore(postgres): use Run function #3425
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughRefactors Postgres module to use testcontainers.ContainerCustomizer-based options and testcontainers.Run. Updates WithDatabase/WithUsername/WithPassword signatures, reworks Run to compose customizers, adds container inspection to sync env-derived fields, and adapts SSL and config-file helpers to WithFiles/WithEntrypoint/WithCmdArgs flows. Error messages updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant PG as Postgres.Run
participant TC as testcontainers.Run
participant D as Docker Daemon
participant I as Inspect
C->>PG: Run(ctx, img, opts...)
PG->>PG: Build moduleOpts (WithEnv, WithExposedPorts, WithCmd, opts...)
PG->>TC: Run(ctx, img, moduleOpts...)
TC->>D: Create/Start container
D-->>TC: Container handle
TC-->>PG: Container
PG->>I: Inspect container env
I-->>PG: Final env (DB, USER, PASSWORD)
PG->>PG: Update PostgresContainer fields
PG-->>C: PostgresContainer
rect rgba(200,255,200,0.2)
note right of PG: Changed: uses ContainerCustomizer<br/>and post-start inspect
end
alt Error starting
TC-->>PG: error
PG-->>C: wrap "run postgres" error
else Error inspecting
I-->>PG: error
PG-->>C: wrap "inspect postgres" error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
3c322b3 to
e966d61
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
modules/postgres/postgres.go (4)
73-78: Guard against missing base CMD when appending config_file argsWithCmdArgs appends to req.Cmd; if no base cmd is set (used outside Run), the container may start with only flags. Add a safeguard to ensure "postgres" is present.
- if err := testcontainers.WithFiles(cfgFile)(req); err != nil { + if err := testcontainers.WithFiles(cfgFile)(req); err != nil { return err } - - return testcontainers.WithCmdArgs("-c", "config_file=/etc/postgresql.conf")(req) + // Ensure a base command exists before appending args + if len(req.Cmd) == 0 { + if err := testcontainers.WithCmd("postgres")(req); err != nil { + return err + } + } + return testcontainers.WithCmdArgs("-c", "config_file=/etc/postgresql.conf")(req)
84-86: Avoid setting POSTGRES_DB when empty to preserve image defaultsSetting POSTGRES_DB to an empty string overrides the image’s “default to POSTGRES_USER” behavior. Make empty a no‑op.
-func WithDatabase(dbName string) testcontainers.ContainerCustomizer { - return testcontainers.WithEnv(map[string]string{"POSTGRES_DB": dbName}) -} +func WithDatabase(dbName string) testcontainers.ContainerCustomizer { + return testcontainers.CustomizeRequestOption(func(req *testcontainers.GenericContainerRequest) error { + if dbName == "" { + return nil + } + if req.Env == nil { + req.Env = map[string]string{} + } + req.Env["POSTGRES_DB"] = dbName + return nil + }) +}
132-137: Trim username to avoid accidental whitespaceSmall hardening; keep default fallback.
-func WithUsername(user string) testcontainers.ContainerCustomizer { - if user == "" { +func WithUsername(user string) testcontainers.ContainerCustomizer { + user = strings.TrimSpace(user) + if user == "" { user = defaultUser } return testcontainers.WithEnv(map[string]string{"POSTGRES_USER": user}) }
235-261: Tighten TLS file permissions and script mode
- CA & server cert: set FileMode to 0o644 instead of defaultPermission (0o600)
- Server key: keep defaultPermission (0o600)
- Entrypoint script: set FileMode to 0o755
- Entrypoint already delegates to
/usr/local/bin/docker-entrypoint.sh "$@"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/postgres/postgres.go(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/postgres/postgres.go (3)
options.go (7)
WithFiles(524-529)WithCmdArgs(470-475)ContainerCustomizer(22-24)WithEnv(75-85)WithExposedPorts(454-459)WithCmd(462-467)WithEntrypoint(438-443)modules/mysql/mysql.go (1)
Run(54-121)generic.go (1)
Run(122-149)
⏰ 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/postgres) / test: modules/postgres/1.25.x
- GitHub Check: test (1.24.x, modules/postgres) / test: modules/postgres/1.24.x
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
modules/postgres/postgres.go (4)
124-126: LGTM; combine with central validation in RunReturn type change is correct. We should validate empty passwords in Run (see comment at Lines 155-167).
169-178: LGTM: container struct initializationDefaults are fine and will be reconciled by the inspect step.
181-183: LGTM: clearer error context“run postgres” improves diagnostics.
184-205: LGTM: env sync via InspectAccurately derives POSTGRES_DB/USER/PASSWORD from container env.
What does this PR do?
Use the Run function in postgres
Why is it important?
Migrate modules to the new API, improving consistency and leveraging the latest testcontainers functionality.
Related issues