From 119a7783679a868e778eabc1324ad4dcc9cbffe6 Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 31 Aug 2026 15:00:42 +0100 Subject: [PATCH 1/4] feat(resource/value): Reach nested fields with dotted keys The flat key=value form could only set a nested struct field by giving the outer field a text form of its own, which then had to encode every subfield in one value. Cut the key on the first dot instead and walk into the named field, allocating a nil pointer on the way. Render flattens the same way. A rendered struct is fed straight back to Parse - a shortcut flag becomes a --set string - and a nested struct rendered inline would reparse its commas as fields of the outer struct. Descent stops at a type owning the matching half of the conversion, which takes the whole value, and at anonymous fields, which are already addressed whole under their type name. The two directions check separately: sharing one test would let a marshal-only type block parsing, and vice versa. Parse and Render now share namedFields so a key they disagree on cannot exist, and walkFields is the rendering inverse of assignField. Also report a bare word as a malformed pair rather than a field named after its own value, which is what a reader of "unknown fields: [my-router-eth0]" has to work out for themselves. Signed-off-by: Justin Chadwell --- internal/resource/value/fields.go | 120 ++++++++++++++++++++ internal/resource/value/format.go | 47 ++++---- internal/resource/value/format_test.go | 69 ++++++++++++ internal/resource/value/parse.go | 59 +++++----- internal/resource/value/parse_test.go | 147 +++++++++++++++++++++++++ 5 files changed, 386 insertions(+), 56 deletions(-) create mode 100644 internal/resource/value/fields.go diff --git a/internal/resource/value/fields.go b/internal/resource/value/fields.go new file mode 100644 index 00000000..e9686df6 --- /dev/null +++ b/internal/resource/value/fields.go @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2026, Unikraft GmbH and The Unikraft CLI Authors. +// Licensed under the BSD-3-Clause License (the "License"). +// You may not use this file except in compliance with the License. + +package value + +import ( + "encoding" + "fmt" + "iter" + "reflect" + + "github.com/ettle/strcase" +) + +type namedField struct { + Name string + Value reflect.Value + Anonymous bool + Type reflect.Type +} + +// namedFields yields the fields of struct v keyed by their "name" tag, which +// is a separate namespace from the "field" tag so that a field excluded from +// the field system stays settable. Parse and Render share this so a key they +// disagree on cannot exist. +func namedFields(v reflect.Value) iter.Seq[namedField] { + return func(yield func(namedField) bool) { + t := v.Type() + for i := range t.NumField() { + field := t.Field(i) + if !field.IsExported() { + continue + } + name := field.Tag.Get("name") + if name == "-" { + continue + } + if name == "" { + name = strcase.ToKebab(field.Name) + } + if !yield(namedField{ + Name: name, + Value: v.Field(i), + Anonymous: field.Anonymous, + Type: field.Type, + }) { + return + } + } + } +} + +// structValue returns f as a struct for a dotted path to descend into. An +// embedded field is skipped, having no key of its own to sit under. +// +// alloc fills in nil pointers on the way, for parsing; rendering leaves them +// alone and so finds nothing to descend into. +func (f namedField) structValue(alloc bool) (reflect.Value, bool) { + if f.Anonymous { + return reflect.Value{}, false + } + v := f.Value + for v.Kind() == reflect.Pointer { + if v.IsNil() { + if !alloc { + return reflect.Value{}, false + } + v.Set(reflect.New(v.Type().Elem())) + } + v = v.Elem() + } + if v.Kind() != reflect.Struct { + return reflect.Value{}, false + } + return v, true +} + +// implements reports whether f, or a pointer to it, satisfies any of ifaces. +// Converting to or from a string as a whole makes a field opaque: its own +// fields are not separately addressable. +func (f namedField) implements(ifaces ...reflect.Type) bool { + for _, iface := range ifaces { + if f.Type.Implements(iface) || reflect.PointerTo(f.Type).Implements(iface) { + return true + } + } + return false +} + +var ( + textUnmarshaler = reflect.TypeFor[encoding.TextUnmarshaler]() + textMarshaler = reflect.TypeFor[encoding.TextMarshaler]() + stringer = reflect.TypeFor[fmt.Stringer]() + renderer = reflect.TypeFor[Renderer]() +) + +// walkFields yields every leaf of struct v as the dotted path addressing it. +// Flattening is what lets a rendered struct parse back: rendered inline, a +// nested struct's commas reparse as fields of the outer struct. +func walkFields(v reflect.Value, prefix string) iter.Seq2[string, reflect.Value] { + return func(yield func(string, reflect.Value) bool) { + for field := range namedFields(v) { + path := prefix + field.Name + if sub, ok := field.structValue(false); ok && + !field.implements(renderer, stringer, textMarshaler) { + for path, leaf := range walkFields(sub, path+".") { + if !yield(path, leaf) { + return + } + } + continue + } + if !yield(path, field.Value) { + return + } + } + } +} diff --git a/internal/resource/value/format.go b/internal/resource/value/format.go index 9f9e9d6c..6fe057fd 100644 --- a/internal/resource/value/format.go +++ b/internal/resource/value/format.go @@ -12,8 +12,6 @@ import ( "slices" "strconv" "strings" - - "github.com/ettle/strcase" ) // RenderOpts controls how a value is rendered to a string via Render. @@ -102,34 +100,29 @@ func Render(value any, opt RenderOpts) (string, error) { slices.Sort(result) return strings.Join(result, ", "), nil case reflect.Struct: - var result []string - for i := range v.NumField() { - field := v.Type().Field(i) - if !field.IsExported() { - continue - } - val := v.Field(i) - valStr, err := Render(val.Interface(), opt) - if err != nil { - return "", err - } - if valStr == "" { - continue - } - // Use "name" tag for value formatting, separate from "field" tag - // This allows fields to be excluded from the field system (field:"-") - // while still being formattable for --set values - name := field.Tag.Get("name") - if name == "-" { - continue - } - if name == "" { - name = strcase.ToKebab(field.Name) - } - result = append(result, fmt.Sprintf("%s=%s", name, valStr)) + result, err := renderStructFields(v, opt) + if err != nil { + return "", err } return strings.Join(result, ", "), nil default: return fmt.Sprintf("%v", value), nil } } + +// renderStructFields renders a struct as the flat key=value pairs that +// value.Parse reads back. +func renderStructFields(v reflect.Value, opt RenderOpts) ([]string, error) { + var result []string + for path, leaf := range walkFields(v, "") { + str, err := Render(leaf.Interface(), opt) + if err != nil { + return nil, err + } + if str == "" { + continue + } + result = append(result, fmt.Sprintf("%s=%s", path, str)) + } + return result, nil +} diff --git a/internal/resource/value/format_test.go b/internal/resource/value/format_test.go index 15dd1983..603ff4e3 100644 --- a/internal/resource/value/format_test.go +++ b/internal/resource/value/format_test.go @@ -93,3 +93,72 @@ func TestFormat(t *testing.T) { }) } } + +// TestFormatNestedPaths pairs with TestParseNestedPaths: a rendered struct is +// fed straight back to Parse (a shortcut flag becomes a --set string), so a +// nested struct has to come out under dotted keys. Rendered inline, its commas +// would reparse as fields of the outer struct. +func TestFormatNestedPaths(t *testing.T) { + t.Run("nested struct flattens to dotted keys", func(t *testing.T) { + v := testStructNested{Name: "outer"} + v.Inner.Label = "x" + v.Ptr = &testStructInner{Flag: new(true), Label: "y"} + + got, err := Render(&v, RenderOpts{}) + require.NoError(t, err) + assert.Equal(t, "name=outer, inner.label=x, ptr.flag=true, ptr.label=y", got) + }) + + t.Run("round-trips through Parse", func(t *testing.T) { + want := testStructNested{Name: "outer"} + want.Ptr = &testStructInner{Flag: new(false), Label: "y"} + want.Text.Raw = "whole value" + + rendered, err := Render(&want, RenderOpts{}) + require.NoError(t, err) + + got, err := Parse[testStructNested]([]string{rendered}) + require.NoError(t, err, "rendered as %q", rendered) + assert.Equal(t, want, got, "rendered as %q", rendered) + }) + + // The mirror of the parse-side check: only the marshalling interfaces + // decide this direction. + t.Run("opacity follows MarshalText alone", func(t *testing.T) { + v := testStructNested{} + v.WriteTo.Raw = "opaque" + v.ReadTo.Raw = "flattened" + + got, err := Render(&v, RenderOpts{}) + require.NoError(t, err) + assert.Equal(t, "read-to.raw=flattened, write-to=opaque", got) + }) + + t.Run("embedded fields render whole, under their type name", func(t *testing.T) { + var v testStructNested + v.Label = "x" + + got, err := Render(&v, RenderOpts{}) + require.NoError(t, err) + assert.Equal(t, "test-struct-embedded=label=x", got) + + back, err := Parse[testStructNested]([]string{got}) + require.NoError(t, err) + assert.Equal(t, v, back) + }) + + t.Run("a text form stays whole", func(t *testing.T) { + v := testStructNested{} + v.Text.Raw = "opaque" + + got, err := Render(&v, RenderOpts{}) + require.NoError(t, err) + assert.Equal(t, "text=opaque", got) + }) + + t.Run("nil pointer renders nothing", func(t *testing.T) { + got, err := Render(&testStructNested{Name: "outer"}, RenderOpts{}) + require.NoError(t, err) + assert.Equal(t, "name=outer", got) + }) +} diff --git a/internal/resource/value/parse.go b/internal/resource/value/parse.go index abf5b114..c98d950c 100644 --- a/internal/resource/value/parse.go +++ b/internal/resource/value/parse.go @@ -13,8 +13,6 @@ import ( "strconv" "strings" - "github.com/ettle/strcase" - xmaps "unikraft.com/cli/internal/x/maps" ) @@ -261,40 +259,23 @@ func parseReflect(input []string, value reflect.Value) error { if err != nil { return err } - process: for _, item := range items { item = strings.TrimSpace(item) if item == "" { continue } - k, v, _ := strings.Cut(item, "=") + k, v, hasValue := strings.Cut(item, "=") + if !hasValue { + return fmt.Errorf("invalid value %q: expected =", item) + } - for i := range s.NumField() { - field := s.Type().Field(i) - if !field.IsExported() { - continue - } - // Use "name" tag for value parsing, separate from "field" tag - // This allows fields to be excluded from the field system (field:"-") - // while still being parseable for --set values - name := field.Tag.Get("name") - if name == "-" { - continue - } - if name == "" { - name = field.Name - name = strcase.ToKebab(name) - } - if k == name { - fieldVal := s.Field(i) - err := parseReflect([]string{v}, fieldVal) - if err != nil { - return err - } - continue process - } + ok, err := assignField(s, k, v) + if err != nil { + return err + } + if !ok { + notFound[k] = struct{}{} } - notFound[k] = struct{}{} } } @@ -307,3 +288,23 @@ func parseReflect(input []string, value reflect.Value) error { return fmt.Errorf("unsupported type: %T", value.Interface()) } } + +// assignField sets one key=value pair on struct s, descending into a named +// struct field when the key is dotted. Reports whether the key matched. +func assignField(s reflect.Value, k, v string) (bool, error) { + head, rest, dotted := strings.Cut(k, ".") + + for field := range namedFields(s) { + if k == field.Name { + return true, parseReflect([]string{v}, field.Value) + } + if !dotted || head != field.Name || field.implements(textUnmarshaler) { + continue + } + if sub, ok := field.structValue(true); ok { + return assignField(sub, rest, v) + } + } + + return false, nil +} diff --git a/internal/resource/value/parse_test.go b/internal/resource/value/parse_test.go index 98e7a2a5..71ade816 100644 --- a/internal/resource/value/parse_test.go +++ b/internal/resource/value/parse_test.go @@ -115,6 +115,153 @@ func TestParseStructJSONValue(t *testing.T) { }) } +type testStructNested struct { + Name string `name:"name"` + Inner testStructInner `name:"inner"` + Ptr *testStructInner `name:"ptr"` + Hidden testStructInner `name:"-"` + Text testStructWithText `name:"text"` + ReadTo testStructReadOnly `name:"read-to"` + WriteTo testStructWriteOnly `name:"write-to"` + TestStructEmbedded +} + +// TestStructEmbedded is exported so that embedding it produces an exported +// field, which is the only way the anonymous case is reached at all. +type TestStructEmbedded struct { + Label string `name:"label"` +} + +// testStructReadOnly converts only from a string, so parsing takes its whole +// value while rendering still flattens it. +type testStructReadOnly struct { + Raw string `name:"raw"` +} + +func (t *testStructReadOnly) UnmarshalText(text []byte) error { + t.Raw = string(text) + return nil +} + +// testStructWriteOnly converts only to a string, the mirror of +// testStructReadOnly. +type testStructWriteOnly struct { + Raw string `name:"raw"` +} + +func (t testStructWriteOnly) MarshalText() ([]byte, error) { + return []byte(t.Raw), nil +} + +type testStructInner struct { + Flag *bool `name:"flag"` + Label string `name:"label"` +} + +// testStructWithText owns its whole value, so a dotted key must not reach +// past it into its fields. +type testStructWithText struct { + Raw string `name:"raw"` +} + +func (t *testStructWithText) UnmarshalText(text []byte) error { + t.Raw = string(text) + return nil +} + +func (t testStructWithText) MarshalText() ([]byte, error) { + return []byte(t.Raw), nil +} + +// TestParseNestedPaths covers dotted keys, which are how the flat key=value +// form reaches a nested struct field. Without them a nested field is only +// settable by giving the outer field a text form of its own, which then has +// to encode every subfield in one value. +func TestParseNestedPaths(t *testing.T) { + t.Run("nested field", func(t *testing.T) { + got, err := Parse[testStructNested]([]string{"name=outer,inner.label=x"}) + require.NoError(t, err) + assert.Equal(t, "outer", got.Name) + assert.Equal(t, "x", got.Inner.Label) + }) + + t.Run("allocates a nil pointer", func(t *testing.T) { + got, err := Parse[testStructNested]([]string{"ptr.flag=true"}) + require.NoError(t, err) + require.NotNil(t, got.Ptr) + require.NotNil(t, got.Ptr.Flag) + assert.True(t, *got.Ptr.Flag) + }) + + t.Run("several keys reach the same nested struct", func(t *testing.T) { + got, err := Parse[testStructNested]([]string{"ptr.label=y,ptr.flag=false"}) + require.NoError(t, err) + require.NotNil(t, got.Ptr) + assert.Equal(t, "y", got.Ptr.Label) + require.NotNil(t, got.Ptr.Flag) + assert.False(t, *got.Ptr.Flag) + }) + + t.Run("order does not matter", func(t *testing.T) { + a, err := Parse[testStructNested]([]string{"inner.label=x,name=outer"}) + require.NoError(t, err) + b, err := Parse[testStructNested]([]string{"name=outer,inner.label=x"}) + require.NoError(t, err) + assert.Equal(t, a, b) + }) + + t.Run("unknown nested key reports the whole path", func(t *testing.T) { + _, err := Parse[testStructNested]([]string{"inner.bogus=1"}) + require.ErrorContains(t, err, "unknown fields: [inner.bogus]") + }) + + t.Run("unknown head reports the whole path", func(t *testing.T) { + _, err := Parse[testStructNested]([]string{"nope.label=1"}) + require.ErrorContains(t, err, "unknown fields: [nope.label]") + }) + + t.Run("name:\"-\" is not reachable", func(t *testing.T) { + _, err := Parse[testStructNested]([]string{"hidden.label=x"}) + require.ErrorContains(t, err, "unknown fields: [hidden.label]") + }) + + t.Run("a bare word is not a field name", func(t *testing.T) { + _, err := Parse[testStructNested]([]string{"inner=x"}) + require.ErrorContains(t, err, `invalid value "x": expected =`) + }) + + t.Run("does not descend past a text form", func(t *testing.T) { + _, err := Parse[testStructNested]([]string{"text.raw=x"}) + require.ErrorContains(t, err, "unknown fields: [text.raw]") + + got, err := Parse[testStructNested]([]string{"text=whole value"}) + require.NoError(t, err) + assert.Equal(t, "whole value", got.Text.Raw) + }) + + // Only UnmarshalText decides this direction. Sharing one check with + // Render would let a marshal-only type block parsing, and vice versa. + t.Run("opacity follows UnmarshalText alone", func(t *testing.T) { + _, err := Parse[testStructNested]([]string{"read-to.raw=x"}) + require.ErrorContains(t, err, "unknown fields: [read-to.raw]") + + got, err := Parse[testStructNested]([]string{"write-to.raw=x"}) + require.NoError(t, err) + assert.Equal(t, "x", got.WriteTo.Raw) + }) + + // An embedded field is addressed as a whole under its type name, so a + // dotted key must not also reach into it. + t.Run("embedded fields are not descended into", func(t *testing.T) { + _, err := Parse[testStructNested]([]string{"test-struct-embedded.label=x"}) + require.ErrorContains(t, err, "unknown fields: [test-struct-embedded.label]") + + got, err := Parse[testStructNested]([]string{"test-struct-embedded=label=x"}) + require.NoError(t, err) + assert.Equal(t, "x", got.Label) + }) +} + func TestParseStandalone(t *testing.T) { t.Run("string", func(t *testing.T) { got, err := Parse[string]([]string{"hello"}) From 0e982c74a5290aa4d70d80ccdf64ecc11da996ea Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 31 Aug 2026 15:01:38 +0100 Subject: [PATCH 2/4] feat(instances): Show network interface names, relays and tap devices instances_status_add_network reports name, tap_name, autoconfig and relay, and none of them were mirrored. The interface name matters most: a relay is referenced by interface name, so without it there is no way to tell what an instance could relay through. Relay holds plain identifiers rather than a Link. It targets another instance's interface, and interfaces have no API of their own for a resource type to sit on, so Link()'s TUI drill-down would dead-end. Its DNS toggle is relay.dns, not relay.relay-dns as the API spells it - the prefix is already carried by the field it sits under. Signed-off-by: Justin Chadwell --- cmd/unikraft/testdata/TestHelp/instances | 36 +++-- cmd/unikraft/testdata/TestHelp/run | 4 +- internal/cmd/instances.go | 20 ++- internal/cmd/output_test.go | 24 +++- internal/cmd/testdata/TestOutput/instances | 160 ++++++++++++++++++++- 5 files changed, 225 insertions(+), 19 deletions(-) diff --git a/cmd/unikraft/testdata/TestHelp/instances b/cmd/unikraft/testdata/TestHelp/instances index 1104f54b..92322aa8 100644 --- a/cmd/unikraft/testdata/TestHelp/instances +++ b/cmd/unikraft/testdata/TestHelp/instances @@ -73,7 +73,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -146,7 +148,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -227,7 +231,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -310,7 +316,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -418,7 +426,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -616,7 +626,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -822,7 +834,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -1016,7 +1030,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -1154,7 +1170,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- diff --git a/cmd/unikraft/testdata/TestHelp/run b/cmd/unikraft/testdata/TestHelp/run index 21b61b34..efba195e 100644 --- a/cmd/unikraft/testdata/TestHelp/run +++ b/cmd/unikraft/testdata/TestHelp/run @@ -63,7 +63,9 @@ Fields: volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config - networks, networks.*, networks.*.uuid, networks.*.private-ip, networks.*.mac + networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, + networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, + networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- diff --git a/internal/cmd/instances.go b/internal/cmd/instances.go index 455f44a9..99ba99c2 100644 --- a/internal/cmd/instances.go +++ b/internal/cmd/instances.go @@ -178,9 +178,23 @@ type Instance struct { } type InstanceNetwork struct { - UUID string `mirror:"uuid" field:",long"` - PrivateIP string `mirror:"private_ip" field:",long"` - MAC string `mirror:"mac" field:",long"` + Name string `mirror:"name" field:",long"` + UUID string `mirror:"uuid" field:",long"` + PrivateIP string `mirror:"private_ip" field:",long"` + MAC string `mirror:"mac" field:",long"` + TapName string `mirror:"tap_name" field:"tap-name,long"` + Relay *InstanceNetworkRelay `mirror:"relay" field:",embed"` + Autoconfig *bool `mirror:"autoconfig" field:",long"` +} + +// InstanceNetworkRelay is the interface all of this interface's traffic is +// routed through. The target is another instance's interface, which has no +// API of its own, so this holds plain identifiers rather than a Link. +type InstanceNetworkRelay struct { + Name string `mirror:"name" field:",long"` + UUID string `mirror:"uuid" field:",long"` + // The API names this member relay_dns, inside the relay object itself. + DNS *bool `mirror:"relay_dns" field:",long"` } type InstanceGpu struct { diff --git a/internal/cmd/output_test.go b/internal/cmd/output_test.go index a61da71f..1c80517e 100644 --- a/internal/cmd/output_test.go +++ b/internal/cmd/output_test.go @@ -137,10 +137,26 @@ func instancesOutputTests(t *testing.T) { sample.Runtime.Env = map[string]string{"KEY1": "val1", "KEY2": "val2"} sample.Resources.Memory = 256 sample.Resources.VCPUs = 2 - sample.Networks = append(sample.Networks, cmd.InstanceNetwork{}) - sample.Networks[0].UUID = "net-uuid-1234" - sample.Networks[0].PrivateIP = "192.168.1.10" - sample.Networks[0].MAC = "aa:bb:cc:dd:ee:ff" + sample.Networks = []cmd.InstanceNetwork{ + { + Name: "my-instance-eth0", + UUID: "net-uuid-1234", + PrivateIP: "192.168.1.10", + MAC: "aa:bb:cc:dd:ee:ff", + Relay: &cmd.InstanceNetworkRelay{ + Name: "my-router-eth0", + UUID: "net-uuid-5678", + DNS: new(true), + }, + }, + { + Name: "my-instance-eth1", + UUID: "net-uuid-9abc", + PrivateIP: "192.168.1.11", + MAC: "aa:bb:cc:dd:ee:00", + TapName: "tap0", + }, + } vmType := platform.InstanceTypeFull sample.Type_ = &vmType sample.Gpus = []cmd.InstanceGpu{ diff --git a/internal/cmd/testdata/TestOutput/instances b/internal/cmd/testdata/TestOutput/instances index d6523753..7486f16e 100644 --- a/internal/cmd/testdata/TestOutput/instances +++ b/internal/cmd/testdata/TestOutput/instances @@ -36,9 +36,19 @@ plugins: image: plugins/sandbox:latest config: {"timeout":30} networks: -- uuid: net-uuid-1234 +- name: my-instance-eth0 + uuid: net-uuid-1234 private-ip: 192.168.1.10 mac: aa:bb:cc:dd:ee:ff + relay: + name: my-router-eth0 + uuid: net-uuid-5678 + dns: true +- name: my-instance-eth1 + uuid: net-uuid-9abc + private-ip: 192.168.1.11 + mac: aa:bb:cc:dd:ee:00 + tap-name: tap0 gpus: - uuid: gpu-uuid-1234 model: 10de:1eb8 @@ -96,9 +106,26 @@ plugins: image: plugins/sandbox:latest config: {"timeout":30} networks: -- uuid: net-uuid-1234 +- name: my-instance-eth0 + uuid: net-uuid-1234 private-ip: 192.168.1.10 mac: aa:bb:cc:dd:ee:ff + tap-name: + relay: + name: my-router-eth0 + uuid: net-uuid-5678 + dns: true + autoconfig: +- name: my-instance-eth1 + uuid: net-uuid-9abc + private-ip: 192.168.1.11 + mac: aa:bb:cc:dd:ee:00 + tap-name: tap0 + relay: + name: + uuid: + dns: + autoconfig: gpus: - uuid: gpu-uuid-1234 model: 10de:1eb8 @@ -722,6 +749,11 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni { "name": "0", "subfields": [ + { + "name": "name", + "value": "my-instance-eth0", + "verbosity": "long" + }, { "name": "uuid", "value": "net-uuid-1234", @@ -736,6 +768,94 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni "name": "mac", "value": "aa:bb:cc:dd:ee:ff", "verbosity": "long" + }, + { + "name": "tap-name", + "value": "", + "verbosity": "long" + }, + { + "name": "relay", + "subfields": [ + { + "name": "name", + "value": "my-router-eth0", + "verbosity": "long" + }, + { + "name": "uuid", + "value": "net-uuid-5678", + "verbosity": "long" + }, + { + "name": "dns", + "value": true, + "verbosity": "long" + } + ], + "verbosity": "long" + }, + { + "name": "autoconfig", + "value": null, + "verbosity": "long" + } + ], + "verbosity": "long" + }, + { + "name": "1", + "subfields": [ + { + "name": "name", + "value": "my-instance-eth1", + "verbosity": "long" + }, + { + "name": "uuid", + "value": "net-uuid-9abc", + "verbosity": "long" + }, + { + "name": "private-ip", + "value": "192.168.1.11", + "verbosity": "long" + }, + { + "name": "mac", + "value": "aa:bb:cc:dd:ee:00", + "verbosity": "long" + }, + { + "name": "tap-name", + "value": "tap0", + "verbosity": "long" + }, + { + "name": "relay", + "subfields": [ + { + "name": "name", + "value": "", + "verbosity": "long" + }, + { + "name": "uuid", + "value": "", + "verbosity": "long" + }, + { + "name": "dns", + "value": null, + "verbosity": "long" + } + ], + "verbosity": "long" + }, + { + "name": "autoconfig", + "value": null, + "verbosity": "long" } ], "verbosity": "long" @@ -744,6 +864,11 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni "elem": { "name": "", "subfields": [ + { + "name": "name", + "value": "", + "verbosity": "long" + }, { "name": "uuid", "value": "", @@ -758,6 +883,37 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni "name": "mac", "value": "", "verbosity": "long" + }, + { + "name": "tap-name", + "value": "", + "verbosity": "long" + }, + { + "name": "relay", + "subfields": [ + { + "name": "name", + "value": "", + "verbosity": "long" + }, + { + "name": "uuid", + "value": "", + "verbosity": "long" + }, + { + "name": "dns", + "value": null, + "verbosity": "long" + } + ], + "verbosity": "long" + }, + { + "name": "autoconfig", + "value": null, + "verbosity": "long" } ], "verbosity": "long" From 0c5a0bb51ae59661cc619ce279911fc310de60ec Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 31 Aug 2026 15:02:27 +0100 Subject: [PATCH 3/4] feat(instances): Allow configuring network interfaces on create Exposes the whole interface, not just the relay: name, ip, mac, tap-name, autoconfig and relay. There is no patch counterpart because /v1/instances has no update type for interfaces at all, so this is create-only and immutable afterwards. Each --network occurrence carries one whole interface, since there is no way to address a slice element - networks.0 does not exist on a create, where the slice is still empty. Within one occurrence the relay is reached by dotted key, so relay.name, relay.uuid and relay.dns read the same in --set, --filter and the output. There is deliberately no bare relay= spelling; the one that names the field it sets is enough. uuid and private-ip are reported by the API and never sent, so they are name:"-" and rejected as unknown keys rather than accepted and dropped. mac goes through AdditionalProperties: /v1/instances has accepted it since MAC-only custom interfaces landed, but the spec's network interface still omits it, same as roms at=. Note the whole array sits behind the net-manager permission, which the API reports as an unknown member rather than a permission error. Signed-off-by: Justin Chadwell --- cmd/unikraft/testdata/TestHelp/instances | 60 ++++++-- cmd/unikraft/testdata/TestHelp/run | 14 +- internal/cmd/instances.go | 81 ++++++++-- internal/cmd/marshal_test.go | 167 +++++++++++++++++++++ internal/cmd/output_test.go | 2 +- internal/cmd/testdata/TestOutput/instances | 35 ++++- 6 files changed, 335 insertions(+), 24 deletions(-) diff --git a/cmd/unikraft/testdata/TestHelp/instances b/cmd/unikraft/testdata/TestHelp/instances index 92322aa8..c69bfb92 100644 --- a/cmd/unikraft/testdata/TestHelp/instances +++ b/cmd/unikraft/testdata/TestHelp/instances @@ -75,7 +75,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -150,7 +151,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -233,7 +235,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -318,7 +321,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -428,7 +432,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -534,6 +539,17 @@ Create flags: --plugin=name=,image=[,config=] ... Load plugin into the instance. [examples: name=sandbox,image=plugins/sandbox:latest, name=sandbox,image=plugins/sandbox:latest,config={"persist_path":"/data"}] + --network== ... + Attach network interface. + name: interface name + relay.name: interface to route all traffic through + relay.uuid: same, by uuid + relay.dns: whether the relay forwards DNS (default true) + ip: address in CIDR notation, requires tap-name + mac: address, requires tap-name + tap-name: TAP device to bring your own interface + autoconfig: whether the guest configures the interface itself. + [examples: relay.name=my-router-eth0, relay.name=my-router-eth0,relay.dns=false, name=eth1,tap-name=tap0,ip=10.0.0.5/24] --scale-to-zero== Scale-to-zero options. policy: on | idle | off @@ -628,7 +644,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -734,6 +751,17 @@ Create flags: --plugin=name=,image=[,config=] ... Load plugin into the instance. [examples: name=sandbox,image=plugins/sandbox:latest, name=sandbox,image=plugins/sandbox:latest,config={"persist_path":"/data"}] + --network== ... + Attach network interface. + name: interface name + relay.name: interface to route all traffic through + relay.uuid: same, by uuid + relay.dns: whether the relay forwards DNS (default true) + ip: address in CIDR notation, requires tap-name + mac: address, requires tap-name + tap-name: TAP device to bring your own interface + autoconfig: whether the guest configures the interface itself. + [examples: relay.name=my-router-eth0, relay.name=my-router-eth0,relay.dns=false, name=eth1,tap-name=tap0,ip=10.0.0.5/24] --scale-to-zero== Scale-to-zero options. policy: on | idle | off @@ -836,7 +864,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -944,6 +973,17 @@ Create flags: --plugin=name=,image=[,config=] ... Load plugin into the instance. [examples: name=sandbox,image=plugins/sandbox:latest, name=sandbox,image=plugins/sandbox:latest,config={"persist_path":"/data"}] + --network== ... + Attach network interface. + name: interface name + relay.name: interface to route all traffic through + relay.uuid: same, by uuid + relay.dns: whether the relay forwards DNS (default true) + ip: address in CIDR notation, requires tap-name + mac: address, requires tap-name + tap-name: TAP device to bring your own interface + autoconfig: whether the guest configures the interface itself. + [examples: relay.name=my-router-eth0, relay.name=my-router-eth0,relay.dns=false, name=eth1,tap-name=tap0,ip=10.0.0.5/24] --scale-to-zero== Scale-to-zero options. policy: on | idle | off @@ -1032,7 +1072,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -1172,7 +1213,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- diff --git a/cmd/unikraft/testdata/TestHelp/run b/cmd/unikraft/testdata/TestHelp/run index efba195e..91acaea5 100644 --- a/cmd/unikraft/testdata/TestHelp/run +++ b/cmd/unikraft/testdata/TestHelp/run @@ -65,7 +65,8 @@ Fields: plugins, plugins.*, plugins.*.name, plugins.*.image, plugins.*.config networks, networks.*, networks.*.name, networks.*.uuid, networks.*.private-ip, networks.*.mac, networks.*.tap-name, networks.*.relay, networks.*.relay.name, - networks.*.relay.uuid, networks.*.relay.dns, networks.*.autoconfig + networks.*.relay.uuid, networks.*.relay.dns, networks.*.ip, + networks.*.autoconfig gpus, gpus.*, gpus.*.uuid, gpus.*.model timestamps, timestamps.created, timestamps.started, timestamps.stopped scale-to-zero, scale-to-zero.enabled, scale-to-zero.policy, scale-to- @@ -173,6 +174,17 @@ Create flags: --plugin=name=,image=[,config=] ... Load plugin into the instance. [examples: name=sandbox,image=plugins/sandbox:latest, name=sandbox,image=plugins/sandbox:latest,config={"persist_path":"/data"}] + --network== ... + Attach network interface. + name: interface name + relay.name: interface to route all traffic through + relay.uuid: same, by uuid + relay.dns: whether the relay forwards DNS (default true) + ip: address in CIDR notation, requires tap-name + mac: address, requires tap-name + tap-name: TAP device to bring your own interface + autoconfig: whether the guest configures the interface itself. + [examples: relay.name=my-router-eth0, relay.name=my-router-eth0,relay.dns=false, name=eth1,tap-name=tap0,ip=10.0.0.5/24] --scale-to-zero== Scale-to-zero options. policy: on | idle | off diff --git a/internal/cmd/instances.go b/internal/cmd/instances.go index 99ba99c2..7e092a24 100644 --- a/internal/cmd/instances.go +++ b/internal/cmd/instances.go @@ -128,8 +128,8 @@ type Instance struct { Roms []*InstanceRom `mirror:"instance.roms" field:",embed" create:"set" edit:"set,add,del=strings" flag:"rom" sep:"none" help:"Attach ROM." placeholder:"name=,image=,at=" example:"name=my-rom\\,image=myuser/my-rom:latest\\,at=/rom0,name=mydata\\,dir=./mydata\\,at=/rom"` Plugins []*InstancePlugin `mirror:"instance.plugins" field:",embed" create:"set" edit:"set,add,del=strings" flag:"plugin" sep:"none" help:"Load plugin into the instance." placeholder:"name=,image=[,config=]" example:"name=sandbox\\,image=plugins/sandbox:latest,name=sandbox\\,image=plugins/sandbox:latest\\,config={\"persist_path\":\"/data\"}"` - Networks []InstanceNetwork `mirror:"instance.network_interfaces" field:",embed"` - Gpus []InstanceGpu `mirror:"instance.gpus" field:"gpus,embed"` + Networks []*InstanceNetwork `mirror:"instance.network_interfaces" field:",embed" create:"set" flag:"network" sep:"none" help:"Attach network interface.\n name: interface name\n relay.name: interface to route all traffic through\n relay.uuid: same, by uuid\n relay.dns: whether the relay forwards DNS (default true)\n ip: address in CIDR notation, requires tap-name\n mac: address, requires tap-name\n tap-name: TAP device to bring your own interface\n autoconfig: whether the guest configures the interface itself" placeholder:"=" example:"relay.name=my-router-eth0,relay.name=my-router-eth0\\,relay.dns=false,name=eth1\\,tap-name=tap0\\,ip=10.0.0.5/24"` + Gpus []InstanceGpu `mirror:"instance.gpus" field:"gpus,embed"` Timestamps struct { Created types.RelativeTime `mirror:"instance.created_at" field:",short"` @@ -178,23 +178,51 @@ type Instance struct { } type InstanceNetwork struct { - Name string `mirror:"name" field:",long"` - UUID string `mirror:"uuid" field:",long"` - PrivateIP string `mirror:"private_ip" field:",long"` - MAC string `mirror:"mac" field:",long"` - TapName string `mirror:"tap_name" field:"tap-name,long"` - Relay *InstanceNetworkRelay `mirror:"relay" field:",embed"` - Autoconfig *bool `mirror:"autoconfig" field:",long"` + Name string `name:"name" mirror:"name" json:"name,omitempty" field:",long"` + UUID string `name:"-" mirror:"uuid" json:"-" field:",long"` + PrivateIP string `name:"-" mirror:"private_ip" json:"-" field:",long"` + MAC string `name:"mac" mirror:"mac" json:"mac,omitempty" field:",long"` + TapName string `name:"tap-name" mirror:"tap_name" json:"tap-name,omitempty" field:"tap-name,long"` + + Relay *InstanceNetworkRelay `name:"relay" mirror:"relay" json:"relay,omitempty" field:",embed"` + + IP string `name:"ip" json:"ip,omitempty" field:"ip,invisible"` + Autoconfig *bool `name:"autoconfig" mirror:"autoconfig" json:"autoconfig,omitempty" field:",long"` } // InstanceNetworkRelay is the interface all of this interface's traffic is // routed through. The target is another instance's interface, which has no // API of its own, so this holds plain identifiers rather than a Link. type InstanceNetworkRelay struct { - Name string `mirror:"name" field:",long"` - UUID string `mirror:"uuid" field:",long"` + Name string `name:"name" mirror:"name" json:"name,omitempty" field:",long"` + UUID string `name:"uuid" mirror:"uuid" json:"uuid,omitempty" field:",long"` // The API names this member relay_dns, inside the relay object itself. - DNS *bool `mirror:"relay_dns" field:",long"` + DNS *bool `name:"dns" mirror:"relay_dns" json:"dns,omitempty" field:",long"` +} + +func (n *InstanceNetwork) UnmarshalText(data []byte) error { + type alias InstanceNetwork + parsed, err := value.Parse[alias]([]string{string(data)}) + if err != nil { + return err + } + *n = InstanceNetwork(parsed) + if n.Relay != nil && n.Relay.Name == "" && n.Relay.UUID == "" { + return fmt.Errorf("relay requires relay.name or relay.uuid") + } + return nil +} + +func (n *InstanceNetwork) UnmarshalJSON(data []byte) error { + if len(data) != 0 && data[0] == '"' { + var text string + if err := json.Unmarshal(data, &text); err != nil { + return err + } + return n.UnmarshalText([]byte(text)) + } + type networkJSON InstanceNetwork // alias to avoid recursion + return json.Unmarshal(data, (*networkJSON)(n)) } type InstanceGpu struct { @@ -1363,6 +1391,35 @@ func (Instance) Create(ctx context.Context, fields []resource.Field) ([]resource } req.Plugins = append(req.Plugins, reqPlugin) } + case "networks": + for _, net := range field.Create.Set.([]*InstanceNetwork) { + reqNet := platform.CreateInstanceRequestNetworkInterface{ + Name: ptr.NilIfZero(net.Name), + TapName: ptr.NilIfZero(net.TapName), + Ip: ptr.NilIfZero(net.IP), + Autoconfig: net.Autoconfig, + } + if net.Relay != nil { + if net.Relay.Name == "" && net.Relay.UUID == "" { + return nil, fmt.Errorf("relay requires a name or uuid") + } + reqNet.Relay = &platform.NetworkInterfaceRelay{ + Name: ptr.NilIfZero(net.Relay.Name), + Uuid: ptr.NilIfZero(net.Relay.UUID), + RelayDns: net.Relay.DNS, + } + } + if net.MAC != "" { + // HACK: the spec's network interface omits mac, though + // /v1/instances has accepted it since MAC-only custom + // interfaces landed. + macJSON, _ := json.Marshal(net.MAC) + reqNet.AdditionalProperties = map[string]jsontext.Value{ + "mac": jsontext.Value(macJSON), + } + } + req.NetworkInterfaces = append(req.NetworkInterfaces, reqNet) + } case "service": svc := field.Create.Set.(*InstanceService) if req.ServiceGroup == nil { diff --git a/internal/cmd/marshal_test.go b/internal/cmd/marshal_test.go index 6f34dc50..46cf2243 100644 --- a/internal/cmd/marshal_test.go +++ b/internal/cmd/marshal_test.go @@ -99,6 +99,36 @@ func TestJSONRoundTrip(t *testing.T) { wantObject: &cmd.InstancePlugin{Name: "logger", Image: "plugins/logger:latest", Config: `{"level":"debug"}`}, wantText: &cmd.InstancePlugin{Name: "logger", Image: "plugins/logger:latest", Config: `{"level":"debug"}`}, }, + { + name: "InstanceNetwork", + object: `{"name":"eth1","mac":"aa:bb:cc:dd:ee:ff","tap-name":"tap0","ip":"10.0.0.5/24","autoconfig":false}`, + text: `"name=eth1,tap-name=tap0"`, + into: func() any { return &cmd.InstanceNetwork{} }, + wantObject: &cmd.InstanceNetwork{ + Name: "eth1", + MAC: "aa:bb:cc:dd:ee:ff", + TapName: "tap0", + IP: "10.0.0.5/24", + Autoconfig: new(false), + }, + wantText: &cmd.InstanceNetwork{Name: "eth1", TapName: "tap0"}, + marshalsToObject: true, + }, + { + name: "InstanceNetworkRelay", + object: `{"relay":{"name":"my-router-eth0","uuid":"9f8e-7d6c","dns":false}}`, + text: `"relay.name=my-router-eth0"`, + into: func() any { return &cmd.InstanceNetwork{} }, + wantObject: &cmd.InstanceNetwork{ + Relay: &cmd.InstanceNetworkRelay{ + Name: "my-router-eth0", + UUID: "9f8e-7d6c", + DNS: new(false), + }, + }, + wantText: &cmd.InstanceNetwork{Relay: &cmd.InstanceNetworkRelay{Name: "my-router-eth0"}}, + marshalsToObject: true, + }, { name: "InstanceScaleToZero", object: `{"policy":"on","stateful":true,"cooldown-time":500,"notify-time":100}`, @@ -397,6 +427,143 @@ func TestEditPatches(t *testing.T) { } } +// TestCreatePatches covers the create --set path, where a repeated flag +// carries one whole element each time. Network interfaces only exist here: +// /v1/instances has no patch property for them. +func TestCreatePatches(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + spec map[string][]string + want []*cmd.InstanceNetwork + wantErr string + }{ + { + name: "relay by name", + spec: map[string][]string{"networks": {"relay.name=my-router-eth0"}}, + want: []*cmd.InstanceNetwork{ + {Relay: &cmd.InstanceNetworkRelay{Name: "my-router-eth0"}}, + }, + }, + { + name: "relay with dns opt-out", + spec: map[string][]string{"networks": {"relay.name=my-router-eth0,relay.dns=false"}}, + want: []*cmd.InstanceNetwork{ + {Relay: &cmd.InstanceNetworkRelay{Name: "my-router-eth0", DNS: new(false)}}, + }, + }, + { + name: "dns without a relay target rejected", + spec: map[string][]string{"networks": {"name=eth1,relay.dns=false"}}, + wantErr: "relay requires relay.name or relay.uuid", + }, + { + name: "relay by uuid", + spec: map[string][]string{"networks": {"relay.uuid=c1d2e3f4-5678-90ab-cdef-1234567890ab"}}, + want: []*cmd.InstanceNetwork{ + {Relay: &cmd.InstanceNetworkRelay{UUID: "c1d2e3f4-5678-90ab-cdef-1234567890ab"}}, + }, + }, + { + name: "repeated builds one interface each", + spec: map[string][]string{"networks": { + "relay.name=my-router-eth0", + "name=eth1,tap-name=tap0,ip=10.0.0.5/24,mac=aa:bb:cc:dd:ee:ff,autoconfig=false", + }}, + want: []*cmd.InstanceNetwork{ + {Relay: &cmd.InstanceNetworkRelay{Name: "my-router-eth0"}}, + { + Name: "eth1", + TapName: "tap0", + IP: "10.0.0.5/24", + MAC: "aa:bb:cc:dd:ee:ff", + Autoconfig: new(false), + }, + }, + }, + { + name: "unknown key rejected", + spec: map[string][]string{"networks": {"relay.name=r1,gateway=10.0.0.1"}}, + wantErr: "unknown fields: [gateway]", + }, + { + name: "unknown nested key rejected", + spec: map[string][]string{"networks": {"relay.bogus=1"}}, + wantErr: "unknown fields: [relay.bogus]", + }, + { + // uuid and private-ip are reported by the API, never set. + name: "read-only keys rejected", + spec: map[string][]string{"networks": {"uuid=abc"}}, + wantErr: "unknown fields: [uuid]", + }, + { + name: "the json form cannot set read-only keys either", + spec: map[string][]string{"networks": {`[{"name":"eth1","uuid":"abc","private-ip":"10.0.0.5"}]`}}, + want: []*cmd.InstanceNetwork{{Name: "eth1"}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fields, err := cmd.Instance{}.Fields(t.Context()) + require.NoError(t, err) + + patched, err := patch.PatchedFields(t.Context(), fields, patch.PatchSpec{ + Create: true, + Set: tt.spec, + }) + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + return + } + require.NoError(t, err) + + var got []*cmd.InstanceNetwork + for path, f := range resource.IterFields(patched) { + if path.String() != "networks" || f.Create == nil { + continue + } + got = f.Create.Set.([]*cmd.InstanceNetwork) + } + assert.Equal(t, tt.want, got) + }) + } +} + +// TestNetworkTextRoundTrip guards the compact form against the render/parse +// cycle a shortcut flag goes through: ApplyShortcutFlags renders --network +// back to a --set string, which PatchedFields then parses again. Anything the +// rendered form cannot express is silently dropped there. +func TestNetworkTextRoundTrip(t *testing.T) { + t.Parallel() + + for _, input := range []string{ + "relay.name=my-router-eth0", + "relay.name=my-router-eth0,relay.dns=false", + "relay.name=my-router-eth0,relay.dns=true", + "relay.uuid=c1d2e3f4-5678-90ab-cdef-1234567890ab", + "name=eth1,tap-name=tap0,ip=10.0.0.5/24,mac=aa:bb:cc:dd:ee:ff,autoconfig=false", + } { + t.Run(input, func(t *testing.T) { + t.Parallel() + + want, err := value.Parse[cmd.InstanceNetwork]([]string{input}) + require.NoError(t, err) + + rendered, err := value.Render(&want, value.RenderOpts{}) + require.NoError(t, err) + + got, err := value.Parse[cmd.InstanceNetwork]([]string{rendered}) + require.NoError(t, err, "rendered as %q", rendered) + assert.Equal(t, want, got, "rendered as %q", rendered) + }) + } +} + // TestLinkCollection guards cross-resource links on list fields. // resource.FieldsFromStruct harvests the resource.Link interface only from // ANONYMOUS struct fields, so a list element must stay a struct embedding diff --git a/internal/cmd/output_test.go b/internal/cmd/output_test.go index 1c80517e..4595e09d 100644 --- a/internal/cmd/output_test.go +++ b/internal/cmd/output_test.go @@ -137,7 +137,7 @@ func instancesOutputTests(t *testing.T) { sample.Runtime.Env = map[string]string{"KEY1": "val1", "KEY2": "val2"} sample.Resources.Memory = 256 sample.Resources.VCPUs = 2 - sample.Networks = []cmd.InstanceNetwork{ + sample.Networks = []*cmd.InstanceNetwork{ { Name: "my-instance-eth0", UUID: "net-uuid-1234", diff --git a/internal/cmd/testdata/TestOutput/instances b/internal/cmd/testdata/TestOutput/instances index 7486f16e..6f8cf4a8 100644 --- a/internal/cmd/testdata/TestOutput/instances +++ b/internal/cmd/testdata/TestOutput/instances @@ -795,6 +795,11 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni ], "verbosity": "long" }, + { + "name": "ip", + "value": "", + "verbosity": "invisible" + }, { "name": "autoconfig", "value": null, @@ -852,6 +857,11 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni ], "verbosity": "long" }, + { + "name": "ip", + "value": "", + "verbosity": "invisible" + }, { "name": "autoconfig", "value": null, @@ -910,6 +920,11 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni ], "verbosity": "long" }, + { + "name": "ip", + "value": "", + "verbosity": "invisible" + }, { "name": "autoconfig", "value": null, @@ -918,7 +933,25 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni ], "verbosity": "long" }, - "verbosity": "long" + "verbosity": "long", + "create": { + "set": [ + { + "name": "my-instance-eth0", + "mac": "aa:bb:cc:dd:ee:ff", + "relay": { + "name": "my-router-eth0", + "uuid": "net-uuid-5678", + "dns": true + } + }, + { + "name": "my-instance-eth1", + "mac": "aa:bb:cc:dd:ee:00", + "tap-name": "tap0" + } + ] + } }, { "name": "gpus", From 883fbb4c2478b416da09486e2391b2190bef98bf Mon Sep 17 00:00:00 2001 From: Justin Chadwell Date: Mon, 31 Aug 2026 15:27:15 +0100 Subject: [PATCH 4/4] test(integration): Cover instance relays Drives the whole create surface through the CLI: a router with an explicitly named interface, a client relaying through it, and the relay.dns opt-out. The router needs the explicit name because passing network_interfaces at all switches interface naming to the eth- fallback, so the generated name cannot be predicted. Both rejections are covered too - a relay naming no existing interface, and a chain through an interface that is itself relayed. Deleting the router needs a retry. Its datapath is torn down asynchronously once the last client goes, and until that lands the delete comes back as -EBUSY, which the CLI surfaces as "Unknown error -16". Sending timeout_s=-1 as instance delete does makes it fail rather than wait, so this is not something the caller can ask to block on. Gated to staging and stable like create-env; relay landed after prod. Signed-off-by: Justin Chadwell --- cmd/unikraft/integration/instance_test.go | 60 +++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/cmd/unikraft/integration/instance_test.go b/cmd/unikraft/integration/instance_test.go index 2cc756ab..f9ca608e 100644 --- a/cmd/unikraft/integration/instance_test.go +++ b/cmd/unikraft/integration/instance_test.go @@ -246,6 +246,66 @@ func TestInstances(t *testing.T) { r.Run(t, []string{"unikraft", "instance", "delete", "test-" + instName}) }) + t.Run("create-relay", func(t *testing.T) { + r := runner(t, true, []string{staging, stable}) + routerName, clientName, optOutName, byUUIDName := uniq(), uniq(), uniq(), uniq() + + iface := "test-" + routerName + "-eth0" + create := func(name string, opts ...string) []string { + return append([]string{ + "unikraft", "instance", "create", + "--name", "test-" + name, + "--metro", r.Config.MetroName, + "--image", "nginx:latest", + "--memory", "128", + "--vcpus", "1", + "--set", "autostart=false", + }, opts...) + } + + r.Run(t, create(routerName, "--network", "name="+iface, "--output", "quiet")) + + out := r.Run(t, create(clientName, "--network", "relay.name="+iface)) + assert.Regexp(t, `relay:`, out) + assert.Regexp(t, `name:\s+`+regexp.QuoteMeta(iface), out) + assert.Regexp(t, `dns:\s+true`, out) + + // relay.dns is a dotted key rather than a nested value because relay= + // would take the whole value as the interface name. + out = r.Run(t, create(optOutName, "--network", "relay.name="+iface+",relay.dns=false")) + assert.Regexp(t, `dns:\s+false`, out) + + ifaceUUID := strings.TrimSpace(r.Run(t, []string{ + "unikraft", "instance", "get", "test-" + routerName, + "--output", "template={{ (index .networks 0).uuid }}", + })) + require.NotEmpty(t, ifaceUUID) + out = r.Run(t, create(byUUIDName, "--network", "relay.uuid="+ifaceUUID)) + assert.Regexp(t, `uuid:\s+`+regexp.QuoteMeta(ifaceUUID), out) + + out = r.Run(t, create(uniq(), "--network", "relay.name=test-"+routerName+"-nonexistent"), integ.ExpectFail()) + assert.Regexp(t, `Invalid relay`, out) + + // Relay chains are rejected, so the client's own interface cannot + // itself be relayed through. + clientIface := strings.TrimSpace(r.Run(t, []string{ + "unikraft", "instance", "get", "test-" + clientName, + "--output", "template={{ (index .networks 0).name }}", + })) + require.NotEmpty(t, clientIface) + out = r.Run(t, create(uniq(), "--network", "relay.name="+clientIface), integ.ExpectFail()) + assert.Regexp(t, `Invalid relay`, out) + + r.Run(t, []string{"unikraft", "instance", "delete", "test-" + clientName, "test-" + optOutName, "test-" + byUUIDName}) + + // The relay's datapath is torn down asynchronously after its last + // client goes, and until it is the target instance deletes as -EBUSY. + require.Eventually(t, func() bool { + _, err := r.RunRaw(t, []string{"unikraft", "instance", "delete", "test-" + routerName}, integ.WithoutCancel()) + return err == nil + }, 2*time.Minute, 5*time.Second, "relay target never became deletable") + }) + t.Run("create-oom", func(t *testing.T) { // TODO: Add 'stable' back when it runs platform version 13. Older // versions send a duplicate "status" member that breaks every wait.