feat: add -config-env and -config-env-base64 flags#7
feat: add -config-env and -config-env-base64 flags#7Mickaël Canévet (mcanevet) wants to merge 1 commit into
Conversation
|
Warning Review limit reached
More reviews will be available in 53 minutes and 29 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe CLI gains ChangesEnvironment Variable Configuration Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request introduces support for loading configurations from environment variables via the -config-env flag, with optional base64 decoding support using -config-env-base64. Feedback suggests improving error handling by ensuring CLI validation failures exit with a non-zero status code and print to standard error instead of silently succeeding. Additionally, it is recommended to use base64.MIMEEncoding instead of base64.StdEncoding to robustly handle whitespace and newlines in base64-encoded inputs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@README.md`:
- Around line 28-32: Add a language identifier to the fenced code block
containing the docker run snippet so markdown linters recognize it; edit the
opening backticks for the block that starts with the docker run command (the
block showing the MY_CONFIG env and image
harbor.protontech.ch/systems/talos-meta-tool:latest) and change the fence to
include the language (e.g., bash) immediately after the three backticks.
- Line 29: Update the README entry that shows docker run with MY_CONFIG="$(cat
config.yaml | base64 -w0)" to include cross-platform guidance: note that the -w0
flag is GNU-specific and will fail on macOS/BSD, add a macOS-friendly option and
a portable alternative that ensures the base64 output has no newlines (e.g., by
using the macOS base64 mode or by piping base64 output through a
newline-stripping step), and show an example command for each approach so users
on different platforms can produce a single-line base64 config.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
1c82c3a to
508fdfb
Compare
Andrei Kvapil (kvaps)
left a comment
There was a problem hiding this comment.
Thanks for the env-var feature — closes your original Slack ask, and the mutual-exclusion logic is clean. Two concrete fixes inline below.
One thing that can't be inline: PR #6 just merged, replacing the loose validateYAML with strict schema validation (validateConfig) gated by -skip-validation. After rebasing, please call validateConfig (respecting -skip-validation) instead of validateYAML in both the file and env-var paths, so base64-decoded configs go through the same validation. The gopkg.in/yaml.v3 import can be dropped — it's no longer used on main.
Non-blocking: the new flag-parsing and env-var loading isn't covered by tests — would be worth extracting the load step into a helper (loadConfig(path, envVar string, b64 bool) ([]byte, error)) so precedence and decode errors get table-driven tests.
2f15efc to
e3fd19a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@main_test.go`:
- Line 287: The call to f.Close() is ignoring its error; update the test to
check the returned error from f.Close() and fail the test on error. Replace the
bare f.Close() with an error check such as: if err := f.Close(); err != nil {
t.Fatalf("failed to close file: %v", err) } (or use require.NoError(t, err) if
you prefer testify) so the test records any Close() failure.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85310f05-64f0-45ea-8214-37cc54f52fdc
📒 Files selected for processing (2)
main.gomain_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- main.go
Allow the configuration to be read from an environment variable with -config-env, removing the need for a config file on disk. Add -config-env-base64 to decode the variable's value as base64 before parsing, which avoids YAML line-folding issues when the config is injected into a double-quoted scalar by a template engine (e.g. Tinkerbell). -config and -config-env are mutually exclusive; -config-env-base64 requires -config-env. Config loading is implemented in a loadConfig helper for testability; all whitespace is stripped before base64 decode to handle multi-line output from base64 command variants. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Mickaël Canévet <mickael.canevet@proton.ch>
952fa56 to
6cb7b09
Compare
|
Andrei Kvapil (@kvaps) ready for review |
Summary
-config-envto read the network configuration from an environment variable instead of a file, removing the need to write the config to disk.-config-env-base64to decode the variable's value as base64 before parsing, avoiding YAML line-folding issues when the config is injected into a double-quoted scalar by a template engine (e.g. Tinkerbell).-configand-config-envare mutually exclusive;-config-env-base64requires-config-env.Test plan
talos-meta-tool -device /dev/sda -config-env MY_CONFIGwith a valid YAML config inMY_CONFIGtalos-meta-tool -device /dev/sda -config-env MY_CONFIG -config-env-base64with a base64-encoded config-configand-config-envreturns an error-config-env-base64without-config-envreturns an error🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests