Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an inspect command to os-image-composer for analyzing RAW OS image files. The command supports multiple output formats (text, JSON, YAML) with optional JSON pretty-printing.
Changes:
- Added new
inspectsubcommand with support for text, JSON, and YAML output formats - Enhanced OS release information parsing to return sorted key-value pairs for deterministic output
- Updated text renderer to display parsed OS release data in a structured format with fallback to raw output
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/os-image-composer/main.go | Registered the new inspect command |
| cmd/os-image-composer/inspect_cmd.go | Implements the inspect command with format validation and output rendering |
| cmd/os-image-composer/inspect_cmd_test.go | Comprehensive unit tests for the inspect command using fake inspector |
| internal/image/imageinspect/imageinspect.go | Added KeyValue type and OSReleaseSorted field; updated DisplaySummary signature |
| internal/image/imageinspect/bootloader_pe.go | Modified parseOSRelease to return both map and sorted key-value pairs |
| internal/image/imageinspect/imageinspect_core_test.go | Updated test to handle new parseOSRelease return signature |
| internal/image/imageinspect/renderer_text.go | Added printOSReleaseKV function and updated PrintSummary to use sorted OS release data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| if err != nil { | ||
| return fmt.Errorf("marshal json: %w", err) | ||
| } | ||
| _, _ = fmt.Fprintln(out, string(b)) |
| imageinspect.PrintSummary(out, summary) | ||
| return nil | ||
|
|
||
| case "json": |
There was a problem hiding this comment.
| case "json": | |
| case "json": | |
| enc := json.NewEncoder(out) | |
| if pretty { | |
| enc.SetIndent("", " ") | |
| } | |
| if err := enc.Encode(summary); err != nil { | |
| return fmt.Errorf("encode json: %w", err) | |
| } | |
| return nil |
|
In ADR we mentnioed |
|
|
||
| // createInspectCommand creates the inspect subcommand | ||
| func createInspectCommand() *cobra.Command { | ||
| validateCmd := &cobra.Command{ |
There was a problem hiding this comment.
variable name is confusing:
| validateCmd := &cobra.Command{ | |
| inspectCmd := &cobra.Command{ |
There was a problem hiding this comment.
Will address, thank you!
| newInspector = func() inspector { | ||
| return &fakeInspector{err: errors.New("boom")} | ||
| } | ||
| defer resetInspectFlags() |
There was a problem hiding this comment.
This defer is redundant since t.Cleanup(resetInspectFlags above already handles cleanup after the test completes.
| } | ||
| } | ||
|
|
||
| defer resetInspectFlags() |
There was a problem hiding this comment.
This defer is redundant since t.Cleanup(resetInspectFlags above already handles cleanup after the test completes.
arodage
left a comment
arodage
left a comment
There was a problem hiding this comment.
Added minor comments can be addressed later in next PR
Merge Checklist
All boxes should be checked before merging the PR
Description
This PR adds a command in os-image-composer to inspect a RAW
OS image file and displays output in the desired format. Supported formats are currently
The flag
prettyuses JSON indentation properly and omitting the flag will print raw JSON without tabs.Unit tests was added to cover the command with fake interface to the imageinspect in places required to better cover the code base thoroughly.
Any Newly Introduced Dependencies
How Has This Been Tested?
Unit-tests passes