feat: add kafka integration support#12
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughAdds Kafka broker authentication support (SASL plain/SCRAM, mTLS, and Kerberos/GSSAPI) across the config model, a new KDC container with Kerberos provisioning, Docker Compose generation for all auth modes, generator container SASL/TLS wiring, and a new ChangesKafka Auth, Kerberos KDC, Cert Rotation, and Orchestration
Sequence Diagram(s)sequenceDiagram
participant Runner
participant PrepareKerberos
participant writeCompose
participant KDC as KDC Container
participant Kafka as Kafka KRaft Broker
participant Generator
Runner->>PrepareKerberos: tmpDir, KafkaConfig{realm, service}
PrepareKerberos-->>Runner: KerberosPaths{Dir, InitCmd}
Runner->>writeCompose: RunConfig{KrbHostDir, KerberosInitCmd, ...}
writeCompose-->>Runner: docker-compose.yml (kdc + kafka + kafka-init services)
rect rgba(100, 149, 237, 0.5)
note over KDC,Kafka: Container startup
KDC->>KDC: kdb5_util create, kadmin add principals, export keytabs
KDC->>KDC: touch /krb5/.ready, start krb5kdc
Kafka->>KDC: health-check /krb5/.ready
Kafka->>Kafka: start with GSSAPI listener + JAAS config
end
Generator->>Kafka: produce via SASL/GSSAPI
sequenceDiagram
participant Runner
participant runKafkaMidDeliveryAction
participant ReceiverMetrics
participant Action as rotateAndReload / restartBroker
Runner->>runKafkaMidDeliveryAction: midDeliveryFlow{action, prepare, verdictLabel}
runKafkaMidDeliveryAction->>runKafkaMidDeliveryAction: validate total_lines
runKafkaMidDeliveryAction->>runKafkaMidDeliveryAction: optional prepare (GenerateTLSCerts, set RunConfig)
loop poll until mid-point
runKafkaMidDeliveryAction->>ReceiverMetrics: check received count >= total/2
end
runKafkaMidDeliveryAction->>Action: fire disruptive action
Action-->>runKafkaMidDeliveryAction: done
loop drain until stable
runKafkaMidDeliveryAction->>ReceiverMetrics: check counts stabilized
end
runKafkaMidDeliveryAction-->>Runner: verdict (PASSED/FAILED) with bounded over-delivery
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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)
internal/config/case.go (1)
474-483:⚠️ Potential issue | 🟠 MajorAdd GSSAPI service names to reserved endpoint validation.
The reserved service-name map (lines 474-483) is missing
kdc,kafka, andkafka-init. These services are unconditionally emitted by the compose template whenKafkaGSSAPIEnabledis true, and users could inadvertently create endpoint name collisions. Add these three service names to the reserved map alongside the existing redpanda/vault services.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/config/case.go` around lines 474 - 483, The reserved map in the case.go file is missing three service names that are unconditionally emitted when KafkaGSSAPIEnabled is true. Add kdc, kafka, and kafka-init as entries to the reserved map alongside the existing redpanda and vault service names to prevent users from inadvertently creating endpoint name collisions with these GSSAPI-related services.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/config/case.go`:
- Around line 570-572: The AuthEnabled() method is incorrectly validating Kafka
authentication configuration by allowing a scenario where the SASL mechanism is
empty but TLS is set to "server". This yields encryption without client
authentication, violating the KafkaAuth contract which specifies that an empty
mechanism is only valid for mTLS scenarios. Modify the AuthEnabled() logic to
reject configurations where there is no SASL mechanism configured (empty
mechanism string) but TLS is set to "server", ensuring that either both a
mechanism and TLS are properly configured for mTLS, or the configuration is
rejected as invalid. This validation should occur in the AuthEnabled() method
itself to catch the error early in configuration validation.
In `@internal/orchestrator/docker.go`:
- Around line 1655-1662: In the if tc.Kafka.UsesGSSAPI() block starting at line
1655, add validation checks before proceeding with GSSAPI configuration to fail
fast when required Kerberos inputs are missing. Check that both cfg.KrbHostDir
and cfg.KerberosInitCmd are non-empty, and if either is empty, return an error
immediately rather than allowing the code to proceed with invalid configuration
that would defer the failure to runtime.
In `@Makefile`:
- Line 97: The Makefile adds build-kdc to the build-containers target but lacks
a corresponding push-kdc target and it is not wired into push-containers,
causing the KDC image to not be pushed to the registry when users run
push-containers. Add a new push-kdc target that pushes the KDC container image
(following the same pattern as the existing push-generator, push-receiver, and
push-collector targets), and then add push-kdc as a dependency to the
push-containers target on line 97 alongside the other push targets.
---
Outside diff comments:
In `@internal/config/case.go`:
- Around line 474-483: The reserved map in the case.go file is missing three
service names that are unconditionally emitted when KafkaGSSAPIEnabled is true.
Add kdc, kafka, and kafka-init as entries to the reserved map alongside the
existing redpanda and vault service names to prevent users from inadvertently
creating endpoint name collisions with these GSSAPI-related services.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9cfa121-fee5-4ff4-9738-44801d16eb21
📒 Files selected for processing (17)
Makefilecmd/harness/main.gocontainers/generator/go.modcontainers/generator/kafka.gocontainers/generator/main.gocontainers/kdc/Dockerfileinternal/config/case.gointernal/config/cloud.gointernal/config/kafka_auth_test.gointernal/config/subject.gointernal/orchestrator/docker.gointernal/orchestrator/kafka_auth_render_test.gointernal/orchestrator/kerberos.gointernal/orchestrator/tls.gointernal/results/report.gointernal/runner/multi.gointernal/runner/runner.go
Deploying pipebench with
|
| Latest commit: |
af57475
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bfb8b4c6.pipebench.pages.dev |
| Branch Preview URL: | https://dt-796.pipebench.pages.dev |
Summary by CodeRabbit
Release Notes