feat: add -read flag to print current META config to stdout#8
feat: add -read flag to print current META config to stdout#8Mickaël Canévet (mcanevet) wants to merge 2 commits into
Conversation
Add readConfig which loads the ADV from any io.ReaderAt and returns the raw bytes stored under FixedTag. Wire it up behind a -read flag so the tool can inspect what is currently written without modifying the partition. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Mickaël Canévet <mickael.canevet@proton.ch>
Mickaël Canévet (mcanevet)
force-pushed
the
feat/read-flag
branch
from
f4e7580 to
0787ebb
Compare
![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 adds support for reading and printing the network configuration from the Talos META partition using a new -read flag, along with corresponding unit tests. The review feedback suggests several improvements: validating command-line flags immediately after parsing to avoid opening the device prematurely and to enforce mutual exclusivity; simplifying the parameter type in readConfig to io.ReaderAt; and correcting the error handling in readConfig to check err != nil instead of adv == nil to prevent masking corruption errors.
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.
| if *devicePath == "" { | ||
| fmt.Fprintln(os.Stderr, "Usage: talos-meta-tool -device <disk-device> (-config <file> | -config-env <VAR> [-config-env-base64])") | ||
| os.Exit(1) | ||
| } | ||
| if *configPath == "" && *configEnv == "" { | ||
| fmt.Fprintln(os.Stderr, "Usage: talos-meta-tool -device <disk-device> (-config <file> | -config-env <VAR> [-config-env-base64])") | ||
| os.Exit(1) | ||
| } | ||
| if *configPath != "" && *configEnv != "" { | ||
| fmt.Fprintln(os.Stderr, "Error: -config and -config-env are mutually exclusive") | ||
| os.Exit(1) | ||
| } | ||
| if *configEnvBase64 && *configEnv == "" { | ||
| fmt.Fprintln(os.Stderr, "Error: -config-env-base64 requires -config-env") | ||
| fmt.Fprintln(os.Stderr, "Usage: talos-meta-tool -device <disk-device> (-read | -config <file> | -config-env <VAR> [-config-env-base64])") | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
Validating command-line flags immediately after parsing is a best practice. Currently, if a user provides invalid flags (e.g., missing both -read and -config), the tool will still attempt to open the device with os.O_RDWR and read the GPT partition table before validating the flags and exiting. This can cause unnecessary permission errors or side-effects.
Additionally, this ensures that -read is explicitly validated as mutually exclusive with -config and -config-env rather than silently ignoring them.
if *devicePath == "" {
fmt.Fprintln(os.Stderr, "Usage: talos-meta-tool -device <disk-device> (-read | -config <file> | -config-env <VAR> [-config-env-base64])")
os.Exit(1)
}
if !*read && *configPath == "" && *configEnv == "" {
fmt.Fprintln(os.Stderr, "Usage: talos-meta-tool -device <disk-device> (-read | -config <file> | -config-env <VAR> [-config-env-base64])")
os.Exit(1)
}
if *read && (*configPath != "" || *configEnv != "") {
fmt.Fprintln(os.Stderr, "Error: -read is mutually exclusive with -config and -config-env")
os.Exit(1)
}
if *configPath != "" && *configEnv != "" {
fmt.Fprintln(os.Stderr, "Error: -config and -config-env are mutually exclusive")
os.Exit(1)
}
if *configEnvBase64 && *configEnv == "" {
fmt.Fprintln(os.Stderr, "Error: -config-env-base64 requires -config-env")
os.Exit(1)
}| func readConfig(dev interface{ io.ReaderAt }) ([]byte, error) { | ||
| adv, err := talos.NewADV(io.NewSectionReader(dev, 0, int64(talos.Size))) | ||
| if adv == nil { | ||
| return nil, fmt.Errorf("loading ADV: %w", err) | ||
| } | ||
| data, ok := adv.ReadTagBytes(FixedTag) | ||
| if !ok { | ||
| return nil, fmt.Errorf("tag %#x not found", FixedTag) | ||
| } | ||
| return data, nil | ||
| } |
There was a problem hiding this comment.
Using interface{ io.ReaderAt } as a parameter type is redundant and non-idiomatic in Go; it can be simplified to io.ReaderAt.
Additionally, in readConfig, we should check if err != nil instead of if adv == nil. While writeConfig only checks adv == nil to allow overwriting a corrupt/empty ADV, readConfig is a read-only operation. If the ADV is corrupt or uninitialized (where adv != nil but err != nil), we cannot reliably read tags from it and should report the initialization/corruption error immediately rather than silently falling through and returning a misleading "tag not found" error.
| func readConfig(dev interface{ io.ReaderAt }) ([]byte, error) { | |
| adv, err := talos.NewADV(io.NewSectionReader(dev, 0, int64(talos.Size))) | |
| if adv == nil { | |
| return nil, fmt.Errorf("loading ADV: %w", err) | |
| } | |
| data, ok := adv.ReadTagBytes(FixedTag) | |
| if !ok { | |
| return nil, fmt.Errorf("tag %#x not found", FixedTag) | |
| } | |
| return data, nil | |
| } | |
| func readConfig(dev io.ReaderAt) ([]byte, error) { | |
| adv, err := talos.NewADV(io.NewSectionReader(dev, 0, int64(talos.Size))) | |
| if err != nil { | |
| return nil, fmt.Errorf("loading ADV: %w", err) | |
| } | |
| data, ok := adv.ReadTagBytes(FixedTag) | |
| if !ok { | |
| return nil, fmt.Errorf("tag %#x not found", FixedTag) | |
| } | |
| return data, nil | |
| } |
| if *read { | ||
| data, err := readConfig(meta) | ||
| if err != nil { | ||
| log.Fatalf("Error reading config: %v", err) | ||
| } | ||
| fmt.Print(string(data)) | ||
| return | ||
| } | ||
|
|
||
| if *configPath == "" && *configEnv == "" { | ||
| fmt.Fprintln(os.Stderr, "Usage: talos-meta-tool -device <disk-device> (-read | -config <file> | -config-env <VAR> [-config-env-base64])") | ||
| os.Exit(1) | ||
| } | ||
| if *configPath != "" && *configEnv != "" { | ||
| fmt.Fprintln(os.Stderr, "Error: -config and -config-env are mutually exclusive") | ||
| os.Exit(1) | ||
| } | ||
| if *configEnvBase64 && *configEnv == "" { | ||
| fmt.Fprintln(os.Stderr, "Error: -config-env-base64 requires -config-env") | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
Since all flag validations have been moved to the beginning of main (immediately after flag.Parse()), we can simplify this block and remove the redundant validation checks.
| if *read { | |
| data, err := readConfig(meta) | |
| if err != nil { | |
| log.Fatalf("Error reading config: %v", err) | |
| } | |
| fmt.Print(string(data)) | |
| return | |
| } | |
| if *configPath == "" && *configEnv == "" { | |
| fmt.Fprintln(os.Stderr, "Usage: talos-meta-tool -device <disk-device> (-read | -config <file> | -config-env <VAR> [-config-env-base64])") | |
| os.Exit(1) | |
| } | |
| if *configPath != "" && *configEnv != "" { | |
| fmt.Fprintln(os.Stderr, "Error: -config and -config-env are mutually exclusive") | |
| os.Exit(1) | |
| } | |
| if *configEnvBase64 && *configEnv == "" { | |
| fmt.Fprintln(os.Stderr, "Error: -config-env-base64 requires -config-env") | |
| os.Exit(1) | |
| } | |
| if *read { | |
| data, err := readConfig(meta) | |
| if err != nil { | |
| log.Fatalf("Error reading config: %v", err) | |
| } | |
| fmt.Print(string(data)) | |
| return | |
| } |
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ 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 |
There was a problem hiding this comment.
Pull request overview
This PR adds a non-destructive “read” mode to the Talos META metadata tool so operators can inspect the currently stored network metadata without writing to disk.
Changes:
- Added
readConfigto load an ADV from anio.ReaderAtand extract bytes forFixedTag. - Wired
readConfiginto the CLI behind a new-readflag and documented the usage in the README. - Added unit tests covering read round-trip, empty ADV, and bad device behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| README.md | Updates documentation and adds an example for -read usage. |
| main.go | Introduces readConfig and adds the -read flag flow in the CLI. |
| main_test.go | Adds tests to validate reading behavior and error cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func readConfig(dev interface{ io.ReaderAt }) ([]byte, error) { | ||
| adv, err := talos.NewADV(io.NewSectionReader(dev, 0, int64(talos.Size))) | ||
| if adv == nil { | ||
| return nil, fmt.Errorf("loading ADV: %w", err) | ||
| } | ||
| data, ok := adv.ReadTagBytes(FixedTag) | ||
| if !ok { | ||
| return nil, fmt.Errorf("tag %#x not found", FixedTag) | ||
| } | ||
| return data, nil | ||
| } |
| if *read { | ||
| data, err := readConfig(meta) | ||
| if err != nil { | ||
| log.Fatalf("Error reading config: %v", err) | ||
| } |
| @@ -1,6 +1,6 @@ | |||
| # Talos metadata writer tool | |||
- Move flag validation immediately after flag.Parse(), before opening the device or reading the GPT table, and make -read mutually exclusive with -config/-config-env. - Open the device O_RDONLY for -read instead of O_RDWR. - readConfig: take io.ReaderAt directly instead of an inline interface, and check err != nil rather than adv == nil so ADV corruption is reported instead of masked as "tag not found". - README: fix title, still said "writer tool" after -read was added. Signed-off-by: Mickaël Canévet <mickael.canevet@proton.ch>
Summary
readConfig, which loads the ADV from anyio.ReaderAtand returns the raw bytes stored underFixedTag.-readflag so the tool can inspect what is currently written to the META partition without modifying it.Test plan
go build ./...go vet ./...go test ./...(33 passed, including newTestReadConfigRoundTrip,TestReadConfigEmpty,TestReadConfigBadDevice)