Conversation
d802a4a to
9e62f01
Compare
nderjung
reviewed
Apr 22, 2026
6a8d7fe to
7043fff
Compare
nderjung
reviewed
Apr 24, 2026
We don't need all this super weird stuff where we have to manually iterate to create the fields. We can just do this automatically in FieldsFromStruct. Signed-off-by: Justin Chadwell <justin@unikraft.com>
7043fff to
19b5b11
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors resource linking so that links can be detected automatically during FieldsFromStruct processing, removing a lot of per-resource manual link wiring.
Changes:
- Replace
resource.Link(struct) with aresource.Linkinterface and update consumers (TUI, sandbox cleanup, JSON marshaling). - Enhance
FieldsFromStructto (a) flatten anonymous embedded structs and (b) detectLinkimplementations and bubble links appropriately. - Introduce generic link helper types in
internal/cmd/link.goand migrate several resources (instances, services, volumes, templates, certificates) to use them; update/extend tests and golden outputs.
Reviewed changes
Copilot reviewed 19 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/types/image.go | Adds Link() to ImageRef so image fields can auto-produce links. |
| internal/resource/value/format.go | Avoids panics by handling nil / nil-pointer values before interface checks. |
| internal/resource/tui/panel_detail.go | Switches TUI link tracking to resource.Link interface and resolves link targets via Link(). |
| internal/resource/struct_test.go | Adds coverage for anonymous embedded structs and link detection/bubbling behavior. |
| internal/resource/struct.go | Refactors struct-to-fields conversion, adds centralized link detection, and flattens anonymous embeddings. |
| internal/resource/sandbox.go | Updates sandbox link traversal to use Link() and derived key strings. |
| internal/resource/resource.go | Replaces Link struct with Link interface and adds Field.MarshalJSON() to serialize links. |
| internal/cmd/volumes.go | Migrates volume fields/refs to LinkName and embedded Link[Instance], removing manual link construction. |
| internal/cmd/volume_templates.go | Migrates metro reference to LinkName and removes manual metro linking. |
| internal/cmd/services.go | Migrates metro and nested refs (instances/certificates) to embedded Link[...], removing manual link construction. |
| internal/cmd/link.go | Introduces generic Link[T] and LinkName[T] types implementing resource.Link. |
| internal/cmd/instances.go | Migrates metro to LinkName and replaces manual link construction with auto-linking; retains hyperlink behavior. |
| internal/cmd/instance_templates.go | Migrates metro to LinkName and removes manual link construction. |
| internal/cmd/certificates.go | Migrates metro to LinkName and removes manual link construction. |
| cmd/unikraft/testdata/TestGolden/volumes/import/serve | Golden output update reflecting field ordering changes. |
| cmd/unikraft/testdata/TestGolden/instances/volume-inline | Adds golden case for inline volume create/inspect/delete. |
| cmd/unikraft/testdata/TestGolden/instances/volume | Adds golden case for named volume attach flow. |
| cmd/unikraft/testdata/TestGolden/instances/help | Golden output update reflecting field ordering changes. |
| cmd/unikraft/testdata/TestGolden/instances/connect | Golden output update reflecting field ordering changes. |
| cmd/unikraft/testdata/TestGolden/instances/autostart | Adds golden case for autostart create/inspect/delete. |
| cmd/unikraft/testdata/TestGolden/instances/add-domain | Golden output update reflecting field ordering changes. |
| cmd/unikraft/instances_test.go | Renames subtests for new/updated instance golden scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
craciunoiuc
approved these changes
Apr 27, 2026
Member
craciunoiuc
left a comment
There was a problem hiding this comment.
Good to go after the typo is fixed.
Reviewed-by: Cezar Craciunoiu cezar.craciunoiu@unikraft.com
Approved-by: Cezar Craciunoiu cezar.craciunoiu@unikraft.com
Signed-off-by: Justin Chadwell <justin@unikraft.com>
19b5b11 to
820865b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We don't need all this super weird stuff where we have to manually iterate to create the fields. We can just do this automatically in
FieldsFromStruct.