chore(redpanda)!: use Run function #3430
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BREAKING CHANGE: Option type now returns error - Changed Option type to return error for proper error handling - Migrated from GenericContainer to testcontainers.Run() - Process custom options before building moduleOpts - Listener network validation happens after user options (as final step) - WithListener now returns errors for invalid host:port format - All option functions return error (nil for success) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
mdelapenya
added
chore
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughOption builders now return errors ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant RP as Redpanda.Run
participant Opts as moduleOpts (With*)
participant TC as testcontainers.Run
participant C as Container
User->>RP: Run(ctx, image, opts...)
RP->>RP: Build defaults, parse options (Option now returns error)
alt Option returns error
RP-->>User: return error
end
RP->>Opts: Add WithConfigModifier / WithExposedPorts / WithEntrypoint / WithCmd / WithWaitStrategy
RP->>RP: Aggregate env vars (auth) and files (TLS, bootstrap)
RP->>Opts: WithEnv (conditional)
RP->>Opts: WithFiles (conditional)
RP->>Opts: Append user opts
RP->>Opts: withListeners(...) (conditional, validates networks/ports)
RP->>TC: Run(ctx, image, moduleOpts...)
alt Run error
TC-->>RP: error
RP-->>User: return error
else Success
TC-->>RP: Container
RP->>C: Post-run waits (TLS/readiness as needed)
RP-->>User: return module with container handle
end
note right of RP: Wasm enabled/disabled by image version
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)modules/redpanda/redpanda.go (6)
⏰ 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)
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 |
mdelapenya
removed
the
chore
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/redpanda/redpanda.go (1)
217-223: wait.ForHTTP is passed a port, not a path — will misbehaveForHTTP expects a path; set the port via WithPort. Current code may never probe the Admin API correctly.
- waitHTTP := wait.ForHTTP(defaultAdminAPIPort). - WithStatusCodeMatcher(func(status int) bool { + waitHTTP := wait.ForHTTP("/"). + WithPort(nat.Port(defaultAdminAPIPort)). + WithStatusCodeMatcher(func(status int) bool { // Redpanda's admin API returns 404 for requests to "/". return status == http.StatusNotFound })
🧹 Nitpick comments (3)
modules/redpanda/options.go (1)
152-168: Consider returning an error for invalid HTTP proxy auth methodNew Option signature enables input validation. Instead of silently defaulting to "none", return an error to surface misconfiguration early. If preserving old behavior is desired, consider adding a strict variant.
Example:
func WithHTTPProxyAuthMethod(method HTTPProxyAuthMethod) Option { switch method { case HTTPProxyAuthMethodNone, HTTPProxyAuthMethodHTTPBasic, HTTPProxyAuthMethodOIDC: return func(o *options) error { o.HTTPProxyAuthenticationMethod = method return nil } default: - return func(o *options) error { - // Invalid method, default to "none" - o.HTTPProxyAuthenticationMethod = HTTPProxyAuthMethodNone - return nil - } + return func(o *options) error { + return fmt.Errorf("invalid HTTP proxy auth method: %q", method) + } } }modules/redpanda/redpanda.go (2)
433-448: Minor: clarify comment and param name in isAtLeastVersionComment mentions v8.x; function compares against provided min version (e.g., 23.3). Consider updating for clarity.
-// isAtLeastVersion returns true if the base image (without tag) is in a version or above -func isAtLeastVersion(image, major string) bool { +// isAtLeastVersion returns true if the image tag is >= the given semantic version (e.g., "23.3") +func isAtLeastVersion(image, minVersion string) bool { parts := strings.Split(image, ":") version := parts[len(parts)-1] ... - if semver.IsValid(version) { - return semver.Compare(version, "v"+major) >= 0 // version >= v8.x + if semver.IsValid(version) { + return semver.Compare(version, "v"+minVersion) >= 0 }
193-215: Comment numbering is inconsistent (duplicate 9, out-of-order 6/7)Clean up step numbering for readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
modules/redpanda/options.go(7 hunks)modules/redpanda/redpanda.go(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
modules/redpanda/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/couchbase/options.go (1)
Option(7-7)modules/rabbitmq/options.go (1)
Option(44-44)
modules/redpanda/redpanda.go (4)
modules/redpanda/options.go (1)
Option(88-88)options.go (9)
ContainerCustomizer(22-24)WithConfigModifier(56-62)WithExposedPorts(454-459)WithEntrypoint(438-443)WithCmd(462-467)WithWaitStrategy(366-368)WithEnv(75-85)WithFiles(524-529)CustomizeRequestOption(28-28)container.go (2)
ContainerFile(110-115)Container(41-73)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.25.x, modules/redpanda) / test: modules/redpanda/1.25.x
- GitHub Check: test (1.24.x, modules/redpanda) / test: modules/redpanda/1.24.x
- GitHub Check: Analyze (go)
🔇 Additional comments (2)
modules/redpanda/options.go (1)
88-94: Option now returns error and satisfies ContainerCustomizer — good alignmentType change and NOOP Customize keep options composable while enabling early validation via errors. Looks good.
modules/redpanda/redpanda.go (1)
112-131: Ordering and use of user-provided opts look correctDefaults first, then user opts to allow overrides, then listeners. Good.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Use the Run function in redpanda
Why is it important?
Migrate modules to the new API, improving consistency and leveraging the latest testcontainers functionality.
Breaking Changes
BREAKING CHANGE: The
Optiontype signature has changed to return an error:type Option func(*options)type Option func(*options) errorAll option functions now return
error(returningnilfor success). TheWithListeneroption now properly validates and returns errors for invalid host:port formats instead of silently ignoring them.Related issues
🤖 Generated with Claude Code