feat: add support for HashiCorp Vault integration#10
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds an opt-in in-topology Vault capability (config, host prep, compose wiring, seeding, validation, tests, docs) and fixes an HTTP generator buffer-aliasing bug with validating tests. ChangesVault Dev-Server Topology
HTTP Generator Buffer Aliasing Fix
Sequence Diagram(s)sequenceDiagram
participant Runner
participant PrepareVault
participant NewComposeRunner
participant writeCompose
participant DockerCompose
Runner->>PrepareVault: create TLS/secrets dirs
PrepareVault->>PrepareVault: serialize secrets to JSON
PrepareVault->>NewComposeRunner: return VaultPaths
NewComposeRunner->>writeCompose: pass RunConfig with paths
writeCompose->>writeCompose: build VaultInitCmd
writeCompose->>DockerCompose: render vault + vault-init
DockerCompose->>DockerCompose: subject depends_on vault-init
DockerCompose->>DockerCompose: mount /vault-tls:ro
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🧹 Nitpick comments (1)
FUTURE-CAPABILITIES.md (1)
270-270: Verify Vault minimum version for-dev-tlsThe
-dev-tlsserver flag was added in Vault v1.12.0-rc1, so “needs >= 1.12” is accurate; consider clarifying it as “>= 1.12.0-rc1”.🤖 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 `@FUTURE-CAPABILITIES.md` at line 270, Update the Vault version note to explicitly reference the pre-release that introduced the -dev-tls flag by changing the “needs >= 1.12” text to “needs >= 1.12.0-rc1” (or add “(introduced in 1.12.0-rc1)”) near the image line `image: "hashicorp/vault:1.20"` so readers know the -dev-tls flag first appeared in v1.12.0-rc1.
🤖 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 `@FUTURE-CAPABILITIES.md`:
- Line 287: The vmetric CA path example is inconsistent: the text shows an
absolute path `/vault-tls/vault-ca.pem` while the vmetric config uses a relative
value `ca_name: "vault-tls/vault-ca.pem"`. Update FUTURE-CAPABILITIES.md so both
the explanatory text and the vmetric example use the same path format (either
both absolute with leading slash or both relative without), and add a short note
explaining whether `ca_name` expects an absolute filesystem path or a
vault-style key (e.g., "use absolute filesystem path for ca_name" or "ca_name is
a vault key and should be specified relative"). Ensure you edit the lines
referencing `/vault-tls/vault-ca.pem` and the `ca_name:
"vault-tls/vault-ca.pem"` example to match.
- Around line 303-306: The documentation is inconsistent about whether the
compose key is kafka: or redpanda; clarify that by updating the
FUTURE-CAPABILITIES.md text to explicitly state the mapping between the two
names (e.g., that kafka: declarations in the schema are implemented/realized as
an internal redpanda service and therefore redpanda and redpanda-init are
reserved), and mention the corresponding reserved endpoint names (redpanda,
redpanda-init vs kafka-init) so readers know which identifiers to use with
endpoints:. Update the lines referencing kafka:, redpanda, redpanda-init, and
kafka-init to include a short parenthetical note explaining the relationship.
---
Nitpick comments:
In `@FUTURE-CAPABILITIES.md`:
- Line 270: Update the Vault version note to explicitly reference the
pre-release that introduced the -dev-tls flag by changing the “needs >= 1.12”
text to “needs >= 1.12.0-rc1” (or add “(introduced in 1.12.0-rc1)”) near the
image line `image: "hashicorp/vault:1.20"` so readers know the -dev-tls flag
first appeared in v1.12.0-rc1.
🪄 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: 6d122f58-2ba5-4f3f-a84c-a395080d145b
📒 Files selected for processing (8)
FUTURE-CAPABILITIES.mdcontainers/generator/http_test.gocontainers/generator/main.gointernal/config/case.gointernal/orchestrator/docker.gointernal/orchestrator/vault.gointernal/orchestrator/vault_test.gointernal/runner/runner.go
Deploying pipebench with
|
| Latest commit: |
0562ca9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fa43e158.pipebench.pages.dev |
| Branch Preview URL: | https://feature-vault-support.pipebench.pages.dev |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/orchestrator/docker.go (1)
53-69:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winClose the Vault block before the cloud-emulator sections.
Lines 53-69 and 457-555 currently scope the AWS/Azure blocks inside
{{ if .VaultEnabled }}. That makes AWS/Azure-only cases render the wrong YAML (subject.depends_on:without the emulator entries, and nolocalstack/azuriteservices), and it leaves the template control flow unbalanced sotemplate.Parsecan fail.🔧 Minimal fix
{{- if .VaultEnabled }} vault-init: condition: service_completed_successfully +{{- end }} {{- if .AWSEnabled }} localstack: condition: service_healthy {{- end }} @@ vault-init: image: "{{ .VaultImage }}" @@ command: - "{{ .VaultInitCmd }}" +{{- end }} {{- if .AWSEnabled }} localstack:Also applies to: 457-555
🤖 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/orchestrator/docker.go` around lines 53 - 69, The Vault conditional block is left open so the AWS/Azure emulator blocks get nested inside {{ if .VaultEnabled }} causing malformed YAML and unbalanced template flow; close the Vault block (add the missing {{ end }} immediately after the vault-init section) so that the subsequent {{ if .AWSEnabled }} (localstack) and {{ if .AzureEnabled }} (azure-init/azurite) blocks are siblings under depends_on, and ensure every opened {{ if ... }} (e.g., {{ if .VaultEnabled }}, {{ if .AWSEnabled }}, {{ if .AzureEnabled }}) has a matching {{ end }} so template.Parse succeeds.
🤖 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.
Outside diff comments:
In `@internal/orchestrator/docker.go`:
- Around line 53-69: The Vault conditional block is left open so the AWS/Azure
emulator blocks get nested inside {{ if .VaultEnabled }} causing malformed YAML
and unbalanced template flow; close the Vault block (add the missing {{ end }}
immediately after the vault-init section) so that the subsequent {{ if
.AWSEnabled }} (localstack) and {{ if .AzureEnabled }} (azure-init/azurite)
blocks are siblings under depends_on, and ensure every opened {{ if ... }}
(e.g., {{ if .VaultEnabled }}, {{ if .AWSEnabled }}, {{ if .AzureEnabled }}) has
a matching {{ end }} so template.Parse succeeds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f32e062d-cbab-4707-aba2-eb141ee457cd
📒 Files selected for processing (4)
containers/generator/main.gointernal/config/case.gointernal/orchestrator/docker.gointernal/runner/runner.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/runner/runner.go
- containers/generator/main.go
- internal/config/case.go
Resolve conflicts where Vault and Minio support landed independently:
- internal/orchestrator/docker.go: combine both conditions into the
subject depends_on guard ({{if or .Kafka .AWS .Azure .Vault .Minio}}).
- internal/config/case.go: keep main's regrouped `reserved` service-name
map (cloud emulators incl. minio, kafka brokers) and add the vault/
vault-init entries as their own group.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Summary by CodeRabbit