chore(rabbitmq)!: use Run function #3428
Conversation
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary by CodeRabbit
WalkthroughReworks RabbitMQ module Run to assemble container customizers (moduleOpts) and call testcontainers.Run; moves env, ports, files, wait strategies, and SSL handling into testcontainers.With* customizers; changes Option builders to return errors; adds a test for custom credentials. No exported symbols removed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant RabbitMQ as modules/rabbitmq.Run
participant Config as withConfig
participant SSL as applySSLSettings
participant TC as testcontainers.Run
Caller->>RabbitMQ: Run(ctx, req)
RabbitMQ->>RabbitMQ: build moduleOpts (WithEnv, WithExposedPorts, WithWaitStrategy)
alt req.Config present
RabbitMQ->>Config: withConfig(req.Config)
Config-->>RabbitMQ: append WithEnv/WithFiles or return error
end
alt req.SSLSettings present
RabbitMQ->>SSL: applySSLSettings(req.SSLSettings)
SSL-->>RabbitMQ: append WithFiles + WithAdditionalWaitStrategy
end
RabbitMQ->>TC: Run(ctx, image, moduleOpts...)
TC-->>RabbitMQ: container handle or error ("run rabbitmq")
RabbitMQ-->>Caller: return container or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 |
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/rabbitmq/rabbitmq.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/rabbitmq/rabbitmq.go (3)
options.go (7)
ContainerCustomizer(22-24)WithEnv(75-85)WithExposedPorts(454-459)WithWaitStrategy(366-368)CustomizeRequestOption(28-28)WithFiles(524-529)WithAdditionalWaitStrategy(371-373)container.go (2)
Container(41-73)ContainerFile(110-115)generic.go (1)
GenericContainerRequest(21-27)
⏰ 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.24.x, modules/rabbitmq) / test: modules/rabbitmq/1.24.x
- GitHub Check: test (1.25.x, modules/rabbitmq) / test: modules/rabbitmq/1.25.x
- GitHub Check: Analyze (go)
- Use settings.AdminUsername and settings.AdminPassword instead of hardcoded defaults - Change Option type to return error for proper error handling - Update for-loop to check and return errors when applying options - Fixes credential handling bug identified in code review 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
mdelapenya
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
modules/rabbitmq/rabbitmq.go (1)
104-117: Credential fix confirmed and implementation looks solid.Lines 106-107 correctly use
settings.AdminUsernameandsettings.AdminPasswordinstead of hardcoded defaults, resolving the bug flagged in previous reviews. The moduleOpts construction properly configures environment variables, exposed ports, wait strategy, and config.
🧹 Nitpick comments (1)
modules/rabbitmq/options.go (1)
44-44: Option signature change improves flexibility.Changing
Optionto return an error aligns with similar modules (opensearch, gcloud, elasticsearch) and enables future input validation. The current implementations correctly returnnilafter applying settings.Consider adding validation in future iterations. For example:
func WithAdminUsername(username string) Option { return func(o *options) error { + if username == "" { + return errors.New("admin username cannot be empty") + } o.AdminUsername = username return nil } }Also applies to: 54-56, 62-64, 70-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
modules/rabbitmq/options.go(2 hunks)modules/rabbitmq/rabbitmq.go(3 hunks)modules/rabbitmq/rabbitmq_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
modules/rabbitmq/rabbitmq_test.go (3)
modules/rabbitmq/rabbitmq.go (1)
Run(82-140)modules/rabbitmq/options.go (2)
WithAdminUsername(61-66)WithAdminPassword(53-58)testing.go (1)
CleanupContainer(91-97)
modules/rabbitmq/options.go (5)
modules/opensearch/options.go (1)
Option(22-22)modules/gcloud/gcloud.go (1)
Option(65-65)modules/elasticsearch/options.go (1)
Option(27-27)modules/redpanda/options.go (1)
Option(87-87)modules/couchbase/options.go (1)
Option(7-7)
modules/rabbitmq/rabbitmq.go (3)
options.go (7)
ContainerCustomizer(22-24)WithEnv(75-85)WithExposedPorts(454-459)WithWaitStrategy(366-368)CustomizeRequestOption(28-28)WithFiles(524-529)WithAdditionalWaitStrategy(371-373)container.go (2)
Container(41-73)ContainerFile(110-115)generic.go (1)
GenericContainerRequest(21-27)
⏰ 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.24.x, modules/rabbitmq) / test: modules/rabbitmq/1.24.x
- GitHub Check: test (1.25.x, modules/rabbitmq) / test: modules/rabbitmq/1.25.x
- GitHub Check: Analyze (go)
🔇 Additional comments (6)
modules/rabbitmq/rabbitmq_test.go (1)
262-290: LGTM! Test validates the custom credentials fix.The test properly verifies that custom admin credentials work end-to-end:
- Container fields reflect the custom credentials
- AMQP URL includes the credentials
- Actual connection succeeds using those credentials
This addresses the issue flagged in previous reviews where credentials were hardcoded.
modules/rabbitmq/rabbitmq.go (5)
87-89: LGTM! Proper error handling for option application.The error check correctly propagates failures from option functions that now return errors.
119-123: LGTM! Clean conditional SSL handling and option merging.SSL settings are conditionally appended when present, and user-provided options are merged last, allowing proper override behavior.
125-139: LGTM! Proper container lifecycle and error handling.The code correctly:
- Calls
testcontainers.Runwith the constructed options- Creates the
RabbitMQContainerwrapper only whenctris non-nil- Returns the container even on error (enabling cleanup)
- Wraps errors with meaningful context
142-154: LGTM! Proper error propagation in config setup.The function correctly chains
WithEnvandWithFiles, returning early on error and propagating the final result.
157-189: LGTM! Batched file mounts and proper wait strategy.The SSL settings function efficiently batches all three certificate files in a single
WithFilescall and adds the TLS listener wait strategy. Error handling is proper throughout.
mdelapenya
added
breaking change
1 participant
What does this PR do?
Use the Run function in rabbitmq
Why is it important?
Migrate modules to the new API, improving consistency and leveraging the latest testcontainers functionality.
Related issues
🤖 Generated with Claude Code