feat(init): add --image flag to override the preset values.yaml image - #150
Conversation
talm init writes the preset chart unchanged, including a hard-coded
installer image (e.g. ghcr.io/cozystack/cozystack/talos:v1.12.6 in
cozystack). Operators using a custom or factory-built Talos image
have to edit values.yaml after every fresh init. The new --image flag
makes the override declarative at init time:
talm init --preset cozystack --name cluster --image \
factory.talos.dev/installer/<sha>:<version>
Implementation is a minimal regex substitution on the preset values
content before write. The helper applyImageOverride is line-anchored,
returns the input unchanged for an empty override OR for a values
file that does not declare image (so a preset without the field is
not silently fabricated), and %q-quotes the override value so a
reference with characters YAML would otherwise re-interpret stays
parsed as a string.
Tests in pkg/commands/init_test.go cover the four contract corners:
empty override, present-image substitution with surrounding content
preserved, missing-image short-circuit, and shell-meta safe quoting.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new --image flag to the talm init command, allowing users to override the Talos installer image in the preset's values.yaml during initial setup. The implementation includes validation to ensure the flag is only used on initial initialization and that the selected preset supports an image field. Feedback suggests optimizing regex usage by using MatchString to avoid unnecessary allocations and notes a redundant validation check in the image application logic.
| if !imageLineRe.Match(values) { | ||
| return nil, fmt.Errorf("--image was set but the preset values.yaml does not declare a top-level image: field; remove --image, choose a different preset, or add the image field manually") | ||
| } |
There was a problem hiding this comment.
This check is redundant because validateImageOverride is already called in RunE (line 425) to ensure the image: field exists before any files are written. While it makes the function safer in isolation, it results in a second regex match for the same content during the write loop.
There was a problem hiding this comment.
The duplication is intentional. Documented in ab4313d — the inner check is unreachable under the talm init flow because validateImageOverride runs first in RunE against the same bytes, but the guard makes applyImageOverride safe for direct callers (the unit tests in init_test.go exercise it that way, and a future code path that skips the validator should not silently corrupt values.yaml). The cost is one regex match on a values.yaml that fits in a few KB; happy to take that for the looser coupling.
| if parts[1] != "values.yaml" { | ||
| continue | ||
| } | ||
| if !imageLineRe.Match([]byte(content)) { |
There was a problem hiding this comment.
Use imageLineRe.MatchString(content) instead of converting the string to a byte slice. This avoids an unnecessary allocation and is more idiomatic when the input is already a string.
| if !imageLineRe.Match([]byte(content)) { | |
| if !imageLineRe.MatchString(content) { |
There was a problem hiding this comment.
Applied in ea09b7c — switched to imageLineRe.MatchString(content) since the input is already a string.
Address review feedback from gemini-code-assist on pkg/commands/init.go:548: the inner imageLineRe.Match is redundant under the talm init flow because validateImageOverride runs first against the same bytes from presetFiles. Document that the guard is intentional defense in depth so the helper stays safe for direct callers (unit tests, future code paths that might skip the validator) instead of dropping it and creating a hidden coupling between the validator and the helper. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
Address review feedback from gemini-code-assist on pkg/commands/init.go:574: the input is already a string, so call imageLineRe.MatchString(content) directly instead of forcing a []byte conversion. Tiny allocation saving and the idiomatic Go form when the regex input arrives as a string. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
2 participants
What changed
talm initwrites the chosen preset chart unchanged, including the hard-coded installer image (thecozystackpreset pinsghcr.io/cozystack/cozystack/talos:v1.12.6). Operators using a custom or factory-built Talos image have to editvalues.yamlafter every fresh init. The new--imageflag makes the override declarative at init time:How
image:line in the preset'svalues.yamlandReplaceAllFuncrewrites it.ReplaceAllFuncis used overReplaceAllbecause the latter expands$0/$1/$name/${name}in the replacement, which would silently corrupt image refs containing$.--imageis rejected up front inPreRunEwhen combined with--encrypt,--decrypt, or--update— the flag rewrites preset content at write time and silently no-ops on those paths otherwise.validateImageOverrideruns inRunEbefore any file is written, so a flag/preset mismatch (e.g.--image --preset generic, sincegenerichas noimage:field) errors out before the project is half-initialized on disk.Tests
applyImageOverridecovers: empty-override no-op, replacement preserving surrounding content, missing-field error, four quoting styles (double/single/unquoted/trailing-comment), four$-expansion forms ($0,$1,$tenant,${name}) round-tripping verbatim throughyaml.Unmarshal.validateImageOverridecovers happy + sad paths.initCmd.PreRunErejects--imagewith each of--encrypt,--decrypt,--update.updateTalmLibraryChartdefensively rejects--imagefor direct callers.Closes #24.