chore(openldap): use Run function #3422
Conversation
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughRefactors the OpenLDAP module to use testcontainers.Run with ContainerCustomizer options, converts env option helpers to return ContainerCustomizer, handles initial LDIF via files and lifecycle hooks, and adds post-start container inspection to read environment values and update internal fields, with adjusted error messages. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant OpenLDAP as OpenLDAP Module
participant TC as testcontainers.Run
participant C as OpenLDAP Container
Caller->>OpenLDAP: Start(opts: ContainerCustomizer...)
OpenLDAP->>OpenLDAP: Build moduleOpts (WithImage, WithEnv, ...)
alt Initial LDIF provided
OpenLDAP->>OpenLDAP: Create ContainerFile + lifecycle hook
OpenLDAP->>TC: WithFiles + WithAdditionalLifecycleHooks
end
OpenLDAP->>TC: Run(moduleOpts...)
TC->>C: Create & Start container
TC-->>OpenLDAP: Container handle or error
alt Start ok
OpenLDAP->>C: Inspect()
C-->>OpenLDAP: Env (LDAP_ADMIN_USERNAME/PASSWORD/ROOT)
OpenLDAP->>OpenLDAP: Update internal fields (fallback to defaults)
else Start/Inspect error
OpenLDAP-->>Caller: Return error ("run openldap"/"inspect openldap")
end
OpenLDAP-->>Caller: OpenLDAPContainer handle
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
modules/openldap/openldap.go (2)
70-104: Harden WithInitialLdif: read env from the container (not req.Env) and consider returning ContainerCustomizerAccessing req.Env inside a PostReadies hook can be stale when used outside this module. Inspect the container to obtain final env values, and optionally return ContainerCustomizer for API consistency.
Apply:
-func WithInitialLdif(ldif string) testcontainers.CustomizeRequestOption { - return func(req *testcontainers.GenericContainerRequest) error { +func WithInitialLdif(ldif string) testcontainers.ContainerCustomizer { + return testcontainers.CustomizeRequestOption(func(req *testcontainers.GenericContainerRequest) error { @@ - hook := testcontainers.ContainerLifecycleHooks{ + hook := testcontainers.ContainerLifecycleHooks{ PostReadies: []testcontainers.ContainerHook{ func(ctx context.Context, container testcontainers.Container) error { - username := req.Env["LDAP_ADMIN_USERNAME"] - rootDn := req.Env["LDAP_ROOT"] - password := req.Env["LDAP_ADMIN_PASSWORD"] + inspect, err := container.Inspect(ctx) + if err != nil { + return err + } + + username, password, rootDn := defaultUser, defaultPassword, defaultRoot + for _, e := range inspect.Config.Env { + if v, ok := strings.CutPrefix(e, "LDAP_ADMIN_USERNAME="); ok { + username = v + } + if v, ok := strings.CutPrefix(e, "LDAP_ADMIN_PASSWORD="); ok { + password = v + } + if v, ok := strings.CutPrefix(e, "LDAP_ROOT="); ok { + rootDn = v + } + } + code, output, err := container.Exec(ctx, []string{"ldapadd", "-H", "ldap://localhost:1389", "-x", "-D", fmt.Sprintf("cn=%s,%s", username, rootDn), "-w", password, "-f", "/initial_ldif.ldif"}) if err != nil { return err } if code != 0 { data, _ := io.ReadAll(output) return errors.New(string(data)) } return nil }, }, } @@ - return testcontainers.WithAdditionalLifecycleHooks(hook)(req) - } + return testcontainers.WithAdditionalLifecycleHooks(hook)(req) + }) }
150-165: Optional: extract env parsing into a small helperEnv extraction via CutPrefix is fine; consider a shared helper (e.g., getEnv(inspect.Config.Env, "KEY", fallback)) to reduce duplication across modules.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/openldap/openldap.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/openldap/openldap.go (5)
options.go (7)
ContainerCustomizer(22-24)WithEnv(75-85)CustomizeRequestOption(28-28)WithFiles(524-529)WithAdditionalLifecycleHooks(497-502)WithExposedPorts(454-459)WithWaitStrategy(366-368)generic.go (1)
GenericContainerRequest(21-27)container.go (1)
ContainerFile(110-115)lifecycle.go (1)
ContainerLifecycleHooks(43-55)modules/mysql/mysql.go (1)
Run(54-121)
⏰ 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/openldap) / test: modules/openldap/1.24.x
- GitHub Check: test (1.25.x, modules/openldap) / test: modules/openldap/1.25.x
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
modules/openldap/openldap.go (4)
54-56: Env option helpers migration looks goodReturning ContainerCustomizer via WithEnv aligns with the new Run API and other modules.
Also applies to: 61-63, 66-68
114-125: Defaults and wait strategy are sensibleGood use of WithEnv defaults, exposing 1389/tcp, and ForAll(wait.ForLog + ForListeningPort). Matches patterns in other modules.
129-137: Post-run container wiring LGTMCreating OpenLDAPContainer with defaults, to be overridden after inspect, is clear and safe.
141-149: Error wrapping and inspect behavior is consistentReturning partial container on error mirrors other modules (e.g., mysql). Message prefixes are clear.
1 participant
What does this PR do?
Use the Run function in openldap
Why is it important?
Migrate modules to the new API, improving consistency and leveraging the latest testcontainers functionality.
Related issues