Skip to content

Generate k8s manifests for recipes#404

Merged
canercidam merged 2 commits intomainfrom
caner/k8s-support
Mar 24, 2026
Merged

Generate k8s manifests for recipes#404
canercidam merged 2 commits intomainfrom
caner/k8s-support

Conversation

@canercidam
Copy link
Copy Markdown
Member

  • New command builder-playground dir which can be used like cd $(builder-playground dir)/k8s to navigate to the k8s files of the latest run
  • New --k8s flag for start. Usage:
    • Checkout to this branch, do go install . or do go build . and use local binary
    • Install kompose CLI
    • To try with L1 recipe, builder-playground start l1 --dry-run --k8s
    • cd $(builder-playground dir)/k8s
    • In one terminal, do ./minikube-mount.sh
    • On the second terminal, do minikube start if you haven't started yet and do kubectl apply -f .
    • See from minikube dashboard
    • On another terminal, kubectl port-forward service/el 8545:8545
    • And builder-playground test to verify that the deployed stack can build a new block with a new tx

I tested this with the opstack recipe. It doesn't work yet but we're close.

The outputted files are useful for kickstarting a k8s setup and iterating from there. Playground doesn't deploy to k8s yet.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 20, 2026

Claude finished @canercidam's task in 1m 49s —— View job


Review of PR #404: Generate k8s manifests for recipes

  • Read all changed files
  • Review code quality, bugs, security, and performance
  • Post inline comments and summary

Summary

This PR adds k8s manifest generation by shelling out to kompose and then patching the output. The approach is reasonable for kickstarting a k8s setup, but there are several issues to address:

Bugs:

  • Command injection in minikube-mount.sh — host paths are interpolated into the shell script without escaping. Paths with spaces or metacharacters will break or execute arbitrary commands.
  • :ro/:rw volume suffixes not handledSplitN(vol, ":", 2) leaves options like :ro attached to the container path, producing incorrect mount paths.
  • os.Stat for volume classification is host-dependent — whether a volume is treated as a file or directory depends on the filesystem state of the machine running generation, making output non-deterministic across environments (local dev vs CI).

Design issues:

  • --k8s silently implies --dry-run — the flag description doesn't mention this. Users may expect the Docker stack to start alongside manifest generation.
  • Compose file parsed 3 timesparseNamedDockerVolumes and parseComposeVolumesPerService each independently read and unmarshal the same file. Parse once and share.
  • No kompose availability check — a missing kompose binary produces a confusing error. exec.LookPath check with a helpful message would improve UX.

Suggestion:

  • Use gopkg.in/yaml.v3 instead of v2 — since this is a new package, v3's map[string]interface{} default would eliminate the verbose map[interface{}]interface{} casting throughout.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 24, 2026

Claude finished @canercidam's task in 1m 54s —— View job


Review of PR #404 (updated — commit 35e36b0)

  • Read all changed files
  • Analyze code quality, bugs, and security
  • Post inline comments and summary

Summary

The new commit (35e36b0) adds session-dir mount path normalization to /data and filters temp-dir volumes from the minikube mount script. These are good fixes.

Remaining issues:

Severity Issue Status
Bug :ro/:rw volume suffixes not stripped — SplitN(vol, ":", 2) leaves options attached to containerPath, breaking mount path matching and producing invalid k8s manifests Inline comment
Design Session-dir /data normalization is done in two independent places (line 53-60 and line 211-222) that must stay in sync manually Inline comment
Design Compose file read+parsed twice independently Inline comment
Noted kompose availability check — author acknowledged, will fix before merge

The :ro/:rw bug is the most impactful — it will silently produce wrong manifests for any volume with access mode options.

@canercidam canercidam merged commit 6752b4d into main Mar 24, 2026
16 checks passed
@canercidam canercidam deleted the caner/k8s-support branch March 24, 2026 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant