From 27ccfa7163352f0bd01a492a8bc71175aed0fcaf 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..dbaf7043 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) { + 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 1b4fef54..91eac9c7 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" ) @@ -187,40 +185,23 @@ func parseReflect(input []string, value reflect.Value) error { notFound := make(map[string]struct{}) for _, input := range input { - process: for item := range strings.SplitSeq(input, ",") { 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{}{} } } @@ -233,3 +214,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 4c14092e..e358a0e3 100644 --- a/internal/resource/value/parse_test.go +++ b/internal/resource/value/parse_test.go @@ -25,6 +25,153 @@ type testStructCollections struct { Env map[string]string `name:"env"` } +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 daff3b6c9ab0719fc09183c22610a7d258af279d 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 | 19 ++- internal/cmd/output_test.go | 24 +++- internal/cmd/testdata/TestOutput/instances | 160 ++++++++++++++++++++- 5 files changed, 224 insertions(+), 19 deletions(-) diff --git a/cmd/unikraft/testdata/TestHelp/instances b/cmd/unikraft/testdata/TestHelp/instances index ee580e2d..a123d94b 100644 --- a/cmd/unikraft/testdata/TestHelp/instances +++ b/cmd/unikraft/testdata/TestHelp/instances @@ -62,7 +62,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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- @@ -134,7 +136,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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- @@ -214,7 +218,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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- @@ -296,7 +302,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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- @@ -388,7 +396,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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- @@ -562,7 +572,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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- @@ -755,7 +767,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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- @@ -940,7 +954,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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- @@ -1069,7 +1085,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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 3ab2bbca..b066bbbe 100644 --- a/cmd/unikraft/testdata/TestHelp/run +++ b/cmd/unikraft/testdata/TestHelp/run @@ -58,7 +58,9 @@ Fields: volumes, volumes.*, volumes.*.name, volumes.*.uuid, volumes.*.at, volumes.*.readonly, volumes.*.size roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at - 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 3033990b..3b54de72 100644 --- a/internal/cmd/instances.go +++ b/internal/cmd/instances.go @@ -244,9 +244,22 @@ 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"` + 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 e67a4277..3ba5f3c2 100644 --- a/internal/cmd/output_test.go +++ b/internal/cmd/output_test.go @@ -136,10 +136,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 c4e810df..ebcb1ef5 100644 --- a/internal/cmd/testdata/TestOutput/instances +++ b/internal/cmd/testdata/TestOutput/instances @@ -32,9 +32,19 @@ roms: image: myuser/my-rom:latest at: /rom 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 @@ -88,9 +98,26 @@ roms: image: myuser/my-rom:latest at: /rom 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 @@ -637,6 +664,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", @@ -651,6 +683,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" @@ -659,6 +779,11 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni "elem": { "name": "", "subfields": [ + { + "name": "name", + "value": "", + "verbosity": "long" + }, { "name": "uuid", "value": "", @@ -673,6 +798,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 48a4b7a9d22d93ae1511c92789d4c066b51a4134 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 | 83 +++++++++-- internal/cmd/marshal_test.go | 162 +++++++++++++++++++++ internal/cmd/output_test.go | 2 +- internal/cmd/testdata/TestOutput/instances | 39 ++++- 6 files changed, 336 insertions(+), 24 deletions(-) diff --git a/cmd/unikraft/testdata/TestHelp/instances b/cmd/unikraft/testdata/TestHelp/instances index a123d94b..f08441ea 100644 --- a/cmd/unikraft/testdata/TestHelp/instances +++ b/cmd/unikraft/testdata/TestHelp/instances @@ -64,7 +64,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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- @@ -138,7 +139,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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- @@ -220,7 +222,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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- @@ -304,7 +307,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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- @@ -398,7 +402,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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- @@ -490,6 +495,17 @@ Create flags: --rom=image=,at= ... Attach ROM. [examples: image=myuser/my-rom:latest,at=/rom0,name=my-rom, dir=./mydata,at=/rom] + --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] --service= Service group name or key. -p, --publish=:[/] ... @@ -574,7 +590,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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- @@ -666,6 +683,17 @@ Create flags: --rom=image=,at= ... Attach ROM. [examples: image=myuser/my-rom:latest,at=/rom0,name=my-rom, dir=./mydata,at=/rom] + --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] --service= Service group name or key. -p, --publish=:[/] ... @@ -769,7 +797,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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- @@ -863,6 +892,17 @@ Create flags: --rom=image=,at= ... Attach ROM. [examples: image=myuser/my-rom:latest,at=/rom0,name=my-rom, dir=./mydata,at=/rom] + --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] --service= Service group name or key. -p, --publish=:[/] ... @@ -956,7 +996,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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- @@ -1087,7 +1128,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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 b066bbbe..41332a0f 100644 --- a/cmd/unikraft/testdata/TestHelp/run +++ b/cmd/unikraft/testdata/TestHelp/run @@ -60,7 +60,8 @@ Fields: roms, roms.*, roms.*.name, roms.*.image, roms.*.dir, roms.*.at 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- @@ -154,6 +155,17 @@ Create flags: --rom=image=,at= ... Attach ROM. [examples: image=myuser/my-rom:latest,at=/rom0,name=my-rom, dir=./mydata,at=/rom] + --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] --service= Service group name or key. -p, --publish=:[/] ... diff --git a/internal/cmd/instances.go b/internal/cmd/instances.go index 3b54de72..c23c69cd 100644 --- a/internal/cmd/instances.go +++ b/internal/cmd/instances.go @@ -93,6 +93,8 @@ type InstanceCreateCmd struct { Volume []InstanceVolume `group:"flag-create" shortcut:"volumes" short:"v" sep:"none" help:"Attach volume." placeholder:":[:]" example:"my-vol:/data,cache:/tmp:ro,data:/mnt:size=10GiB"` Rom []InstanceRom `group:"flag-create" shortcut:"roms" sep:"none" help:"Attach ROM." placeholder:"image=,at=" example:"image=myuser/my-rom:latest\\,at=/rom0\\,name=my-rom,dir=./mydata\\,at=/rom"` + Network []InstanceNetwork `group:"flag-create" shortcut:"networks" 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"` + Service InstanceService `group:"flag-create" shortcut:"service" help:"Service group name or key." placeholder:"name"` Publish []Service `group:"flag-create" shortcut:"service.services" short:"p" sep:"none" help:"Publish port." placeholder:":[/]" example:"443:8080/http+tls"` Domain []Domain `group:"flag-create" shortcut:"service.domains" sep:"none" help:"Service domain." placeholder:"fqdn" example:"example.com"` @@ -194,8 +196,8 @@ type Instance struct { Volumes []*InstanceVolume `mirror:"instance.volumes" field:",embed" create:"set" edit:"add,del=strings"` Roms []*InstanceRom `mirror:"instance.roms" field:",embed" create:"set" edit:"set,add,del=strings"` - 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"` + Gpus []InstanceGpu `mirror:"instance.gpus" field:"gpus,embed"` Timestamps struct { Created types.RelativeTime `mirror:"instance.created_at" field:",short"` @@ -244,22 +246,50 @@ 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:"uuid,omitempty" field:",long"` + PrivateIP string `name:"-" mirror:"private_ip" json:"private-ip,omitempty" 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"` - DNS *bool `mirror:"relay_dns" field:",long"` + Name string `name:"name" mirror:"name" json:"name,omitempty" field:",long"` + UUID string `name:"uuid" mirror:"uuid" json:"uuid,omitempty" 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 { @@ -1175,6 +1205,35 @@ func (Instance) Create(ctx context.Context, fields []resource.Field) ([]resource } req.Roms = append(req.Roms, reqRom) } + 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 7e7d239b..85234c5d 100644 --- a/internal/cmd/marshal_test.go +++ b/internal/cmd/marshal_test.go @@ -91,6 +91,36 @@ func TestJSONRoundTrip(t *testing.T) { wantObject: &cmd.InstanceRom{Name: "my-rom", Image: "myuser/my-rom:latest", At: "/rom"}, wantText: &cmd.InstanceRom{Name: "my-rom", Image: "myuser/my-rom:latest", At: "/rom"}, }, + { + 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}`, @@ -365,6 +395,138 @@ 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]", + }, + } + + 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(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 3ba5f3c2..f8ccf131 100644 --- a/internal/cmd/output_test.go +++ b/internal/cmd/output_test.go @@ -136,7 +136,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 ebcb1ef5..46cc21ec 100644 --- a/internal/cmd/testdata/TestOutput/instances +++ b/internal/cmd/testdata/TestOutput/instances @@ -710,6 +710,11 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni ], "verbosity": "long" }, + { + "name": "ip", + "value": "", + "verbosity": "invisible" + }, { "name": "autoconfig", "value": null, @@ -767,6 +772,11 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni ], "verbosity": "long" }, + { + "name": "ip", + "value": "", + "verbosity": "invisible" + }, { "name": "autoconfig", "value": null, @@ -825,6 +835,11 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni ], "verbosity": "long" }, + { + "name": "ip", + "value": "", + "verbosity": "invisible" + }, { "name": "autoconfig", "value": null, @@ -833,7 +848,29 @@ fra my-instance running nginx ["arg1", "arg2"] 256MiB 2 example.uni ], "verbosity": "long" }, - "verbosity": "long" + "verbosity": "long", + "create": { + "set": [ + { + "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" + } + ] + } }, { "name": "gpus", From 3ee5027801a5dfc5805920417cfb669648c9aa4e 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 | 56 +++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/cmd/unikraft/integration/instance_test.go b/cmd/unikraft/integration/instance_test.go index d8d7a1f0..00fdfe03 100644 --- a/cmd/unikraft/integration/instance_test.go +++ b/cmd/unikraft/integration/instance_test.go @@ -234,6 +234,62 @@ cmd: ["cat", "/sys/class/uio/uio0/device/startdata"] 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 := uniq(), uniq(), uniq() + + // A relay points at an interface, and the generated interface name is + // not predictable - it falls back to eth- whenever + // network_interfaces is given explicitly - so the router needs one + // named up front to aim at. + 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) + + 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}) + + // 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) { r := runner(t, true, []string{staging, stable}) instName := uniq()