diff --git a/README.md b/README.md index 795b91a..b21abc5 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,10 @@ anexia network vlan list --location --status Active anexia network vlan create --location --description "lab" --vm-provisioning anexia network vlan update --description "lab (retired)" --vm-provisioning=false anexia network address list --prefix --version 4 +anexia network address create --prefix --address 192.0.2.10 --description "web" --role Default --rdns web.example.com +anexia network address update --description "web (retired)" --role Default --rdns old-web.example.com +anexia network address delete --yes +anexia network address reserve --location --vlan --count 2 --prefix --reservation-period 1h ``` Boolean payload flags such as `--vm-provisioning` are switched off on `update` with an explicit @@ -209,8 +213,8 @@ implemented one. The distinction matters to whoever picks the work up, so the ta when the library says which, but a `-` is never evidence about the Engine on its own. The `core`, `network`, `dns` and `kubernetes` groups below are implemented. Within `network`, `vlan` has every -verb because go-anxcloud models it generically; `prefix` has every verb hand-written against the -older client, and `address` is read only for now because its write verbs are still to be declared. +verb because go-anxcloud models it generically; `prefix` and `address` have every verb hand-written against the +older client. The remaining groups, starting with `vsphere`, are roadmap items read off go-anxcloud v0.14.5 and not verified against the Engine. @@ -229,7 +233,7 @@ not verified against the Engine. | --- | :-: | :-: | :-: | :-: | :-: | --- | | `network vlan` | [x] | [x] | [x] | [x] | [x] | `--status` and `--location` filters [x]; a VLAN's location is fixed at creation, so `update` has no `--location` | | `network prefix` | [x] | [x] | [x] | [x] | [x] | `--search` [x]; a prefix's name is its Engine-assigned CIDR, so `update` offers `--description` only | -| `network address` | [x] | [x] | [ ] | [ ] | [ ] | `--search` [x]; field filters [x]; `reserve` [ ] | +| `network address` | [x] | [x] | [x] | [x] | [x] | `--search` [x]; field filters [x]; `reserve` [x] | ### vsphere @@ -326,8 +330,8 @@ handle updates differently from the other four, so their write verbs may not all The write verbs landed with the `dns` group, the first resources the Engine lets the CLI write through the registry, and `network vlan` followed the same way. Most other resources reachable today -are read-only in the Engine anyway. `core tag` and `network prefix` drive the legacy client directly; -addresses are writable in the library and their write verbs are still to be declared. +are read-only in the Engine anyway. `core tag`, `network prefix` and `network address` drive the legacy +client directly. ## Development diff --git a/docs/cli-design.md b/docs/cli-design.md index 32d44ab..7c3b5b8 100644 --- a/docs/cli-design.md +++ b/docs/cli-design.md @@ -47,8 +47,8 @@ verb. | `list` | none | `List` | Paged. Always available if the Engine can enumerate the resource. | | `get` | `` | `Get` | One object by identifier. | | `create` | none, flags carry the payload | `Create` | | -| `update` | ``, flags carry the changes | Read then `Update` | The read is `Get`, or `List` where the Engine has no single-object read (`dns record`). `network prefix` sends a sparse `Update` without the read, see below. | -| `delete` | `` | `Destroy` | Confirms first. Aliased to `destroy`, which is never a command name. The legacy clients (`core tag`, `network prefix`) call it `Delete`. | +| `update` | ``, flags carry the changes | Read then `Update` | The read is `Get`, or `List` where the Engine has no single-object read (`dns record`). `network prefix` sends a sparse `Update` without the read; `network address` reads only to retain rDNS, see below. | +| `delete` | `` | `Destroy` | Confirms first. Aliased to `destroy`, which is never a command name. The legacy clients (`core tag`, `network prefix`, `network address`) call it `Delete`. | A resource only gets the verbs the Engine actually supports. `core location` is read-only in the Engine, so it exposes `list` and `get` and nothing else. This is deliberate: a `create` that @@ -64,8 +64,8 @@ A resource that is only reachable inside another, such as a DNS record inside it `Scope` hook. Scope flags are required, and unlike list filters they apply to every verb, because without one there is no collection to address at all. -Some resources the CLI already reaches are writable in the Engine and do not offer the verbs yet, -`network address` among them. `network prefix` shows how they get declared: hand-written against +Some resources the CLI will reach are writable in the Engine before the CLI offers the verbs. +`network prefix` and `network address` show how they get declared: hand-written against the legacy client, but with the same verbs, flags-as-payload, confirmation and error shape as the registry, so a user cannot tell which half of the CLI served the command. @@ -84,7 +84,11 @@ create a tag object, and `anexia core resource tag add` does not. Four operations have no honest CRUD spelling and are allowed as leaf verbs: `network address reserve`, `dns zone import`, `dns zone apply`, and -`storage bucket empty-and-delete`. The two DNS ones ship; the others are planned. +`storage bucket empty-and-delete`. The DNS operations and `network address reserve` ship; +`storage bucket empty-and-delete` is planned. +`network address reserve` takes `--reservation-period` as a duration, sends it to the Engine as +whole seconds, and requires at least 1s when passed. Without it, the Engine applies its 30m default. +An address reservation that returns no addresses is an error, not an empty table. `import` and `apply` take a document via `--file` rather than payload flags, because the Engine accepts a BIND zone file and a JSON changeset respectively, and spelling either out as repeated flags would mean inventing a small language @@ -149,6 +153,9 @@ is not past anything, so it stays a plain not-found and exits 4. Flag names are lowercase and use dashes, never underscores. Every flag has a usage string. A command never registers a local flag whose name collides with a global one. +`network address create` calls the Engine's `name` field `--address`, because the value is an IP +address and `--name` would read as a label. `list` and `get` still show it in the NAME column. + Filter flags on `list` are named after the field they filter, in the singular: `--tag`, `--name`, `--location`, `--status`, `--service`. Not after the Engine's query parameter, which is why `core tag list` takes `--name` even though the Engine calls it `query`. Repeatable filters would @@ -167,8 +174,9 @@ that accepts sorting. Write verbs on resources that report a provisioning state will get `--wait` and `--wait-timeout`. Resources without a state must not get the flags at all, so `--wait` is never accepted only to -fail later. `network vlan` (`Pending`, `Active`, `Marked for deletion`) and `network prefix` (`status`) -are the implemented resources that report one; their `--wait` is still to come. +fail later. `network vlan` (`Pending`, `Active`, `Marked for deletion`), `network prefix` (`status`) +and `network address` (`status`) are the implemented resources that report one; their `--wait` is +still to come. An `update` that names no field is refused before the write. The Engine would accept it, and on a resource that versions its contents that means a revision nobody asked for, reported as success. @@ -186,6 +194,12 @@ field and the Engine keeps the rest. It offers `--description` only: the legacy `name`, but a prefix's name is its CIDR, assigned by the Engine, so it is not offered for the same reason `dns zone update` has no `--name`. `--description ""` is refused for the same reason `vlan` does. +`network address update` reads the address only to retain `rdns_name`, the one field in +go-anxcloud's `address.Update` without `omitempty`. `description_customer` and `role` are sparse, +so `--description ""` and `--role ""` are refused with the same "cannot be emptied" wording as +prefix; `--rdns ""` reaches the Engine and clears the reverse DNS name. The legacy update also +carries `name`, but an address's name is the IP itself, so it is not offered, as with prefix. + A field the Engine cannot change safely does not get a flag. `dns zone update` has no `--name`, because the Engine's zone update carries the name only in the request body with no old name anywhere in the request, so what a changed name does is undefined. Renaming is not offered rather @@ -218,9 +232,10 @@ Four formats, one flag. `table` is the default and is meant for humans: aligned columns, uppercase headers, no borders. Column sets are short on purpose, up to five fields, because a table wider than a terminal is -useless. Fewer when the Engine returns less: a prefix write is answered with the list summary, -so `network prefix create` and `update` show its three fields, and the full object is a -`network prefix get -o json` away. Everywhere else the full object is one `-o json` away. +useless. Fewer when the Engine returns less: prefix and address writes are answered with their list +summaries, so `network prefix create`, `network address create` and their `update` verbs show the +list columns only, and the full object is a `get -o json` away. Everywhere else the full +object is one `-o json` away. `tsv` is `table` without the alignment: raw values, lowercase headers, tab-separated. This is the one to pipe into `cut` and `awk`. diff --git a/internal/cli/conformance_test.go b/internal/cli/conformance_test.go index 29e3cb1..341afda 100644 --- a/internal/cli/conformance_test.go +++ b/internal/cli/conformance_test.go @@ -120,6 +120,10 @@ var engineActions = map[string]string{ "anexia network prefix delete": `deleting prefix "placeholder"`, "anexia network address list": "listing addresses", "anexia network address get": `reading address "placeholder"`, + "anexia network address create": "creating address", + "anexia network address update": `reading address "placeholder"`, + "anexia network address delete": `deleting address "placeholder"`, + "anexia network address reserve": "reserving addresses", "anexia dns zone list": "listing zones", "anexia dns zone get": `reading zone "placeholder"`, "anexia dns zone create": `creating zone "placeholder"`, @@ -257,6 +261,7 @@ func TestConformanceLeafAliasesUseKnownVerbs(t *testing.T) { "anexia core tag delete": {"destroy": true}, "anexia network vlan delete": {"destroy": true}, "anexia network prefix delete": {"destroy": true}, + "anexia network address delete": {"destroy": true}, "anexia dns zone delete": {"destroy": true}, "anexia dns record delete": {"destroy": true}, "anexia kubernetes cluster delete": {"destroy": true}, @@ -426,6 +431,8 @@ var invocationFlags = map[string]string{ "admin-email": "admin@example.com", "location": "placeholder", "description": "placeholder", + "prefix": "placeholder", + "address": "10.0.0.1", "version": "4", "netmask": "24", "vlan": "placeholder", diff --git a/internal/cli/network.go b/internal/cli/network.go index e4d0737..f1299fa 100644 --- a/internal/cli/network.go +++ b/internal/cli/network.go @@ -23,8 +23,8 @@ func newNetworkCommand(opts *globalOptions) *cobra.Command { // newNetworkVlanCommand builds "network vlan". VLANs are the one object in // this group go-anxcloud models generically, so this is a Spec with every verb. -// prefix is hand-written against the legacy client with the same verbs, and -// address is read-only until its write verbs are declared. +// prefix and address are hand-written against legacy clients with their +// supported verbs, including address reservation. func newNetworkVlanCommand(opts *globalOptions) *cobra.Command { return resource.Command(opts, resource.Spec[vlanv1.VLAN, *vlanv1.VLAN]{ Noun: "vlan", diff --git a/internal/cli/network_address.go b/internal/cli/network_address.go index e2a9e46..a2ecead 100644 --- a/internal/cli/network_address.go +++ b/internal/cli/network_address.go @@ -1,15 +1,19 @@ package cli import ( + "errors" "fmt" "strconv" + "time" "github.com/spf13/cobra" "github.com/spf13/pflag" "go.anx.io/go-anxcloud/pkg/ipam/address" "go.anx.io/go-anxcloud/pkg/utils/param" + "github.com/ProbstenHias/anexia-cli/internal/confirm" "github.com/ProbstenHias/anexia-cli/internal/errmap" + "github.com/ProbstenHias/anexia-cli/internal/output" "github.com/ProbstenHias/anexia-cli/internal/resource" ) @@ -17,16 +21,36 @@ import ( // in go-anxcloud's generic pkg/apis tree, so these commands drive the legacy // ipam/address client directly, sharing the paging and rendering helpers. // -// The library implements create, update, delete and ReserveRandom here as -// well. They are left for the change that gives the resource registry its -// write verbs, so the two halves of the CLI keep offering the same verbs. +// The write verbs drive the same legacy client, using its Create, Update, and +// ReserveRandom types as the Engine payload contract. func newNetworkAddressCommand(opts *globalOptions) *cobra.Command { return resource.Noun("address", "addresses", "Work with Anexia IP addresses", newNetworkAddressListCommand(opts), newNetworkAddressGetCommand(opts), + newNetworkAddressCreateCommand(opts), + newNetworkAddressUpdateCommand(opts), + newNetworkAddressDeleteCommand(opts), + newNetworkAddressReserveCommand(opts), ) } +// addressColumns is the projection list, create and update share: the Engine +// answers both writes with the list summary rather than the full object. +var addressColumns = []string{"identifier", "name", "role", "description"} + +func addressRow(s *address.Summary) []string { + return []string{s.ID, s.Name, s.Role, s.DescriptionCustomer} +} + +// renderAddressSummary prints a write's summary answer as a table or object. +func renderAddressSummary(w *output.Writer, s *address.Summary) error { + if w.Format().Structured() { + return w.Object(s) + } + + return w.Table(addressColumns, [][]string{addressRow(s)}) +} + // addressFilters registers the fields the Engine's filtered address endpoint // accepts and returns the ones the user set. Names follow the field rather // than the query parameter, so --role sets role_text and --organization sets @@ -163,12 +187,7 @@ func newNetworkAddressListCommand(opts *globalOptions) *cobra.Command { return opts.Fail(fmt.Errorf("listing addresses: %w", err)) } - return resource.RenderList(cmd, w, "addresses", found, - []string{"identifier", "name", "role", "description"}, - func(s *address.Summary) []string { - return []string{s.ID, s.Name, s.Role, s.DescriptionCustomer} - }, - ) + return resource.RenderList(cmd, w, "addresses", found, addressColumns, addressRow) } return cmd @@ -221,3 +240,321 @@ func newNetworkAddressGetCommand(opts *globalOptions) *cobra.Command { }, } } + +// addressCreateFlags holds the create payload before it becomes an address.Create. +type addressCreateFlags struct { + prefix string + address string + description string + role string + organization string + rdns string +} + +func (f *addressCreateFlags) register(flags *pflag.FlagSet) { + flags.StringVar(&f.prefix, "prefix", "", "prefix identifier the address belongs to") + flags.StringVar(&f.address, "address", "", "IP address to create") + flags.StringVar(&f.description, "description", "", "customer description") + flags.StringVar(&f.role, "role", "Default", "address role") + flags.StringVar(&f.organization, "organization", "", "organization identifier the address belongs to") + flags.StringVar(&f.rdns, "rdns", "", "reverse DNS name") +} + +func (f *addressCreateFlags) payload() (address.Create, error) { + if f.prefix == "" { + return address.Create{}, errmap.Usagef("--prefix is required") + } + + if err := resource.ValidateIdentifier("prefix", f.prefix); err != nil { + return address.Create{}, err + } + + if f.address == "" { + return address.Create{}, errmap.Usagef("--address is required") + } + + if f.organization != "" { + if err := resource.ValidateIdentifier("organization", f.organization); err != nil { + return address.Create{}, err + } + } + + return address.Create{ + PrefixID: f.prefix, + Address: f.address, + DescriptionCustomer: f.description, + Role: f.role, + Organization: f.organization, + RDNSName: f.rdns, + }, nil +} + +func newNetworkAddressCreateCommand(opts *globalOptions) *cobra.Command { + var f addressCreateFlags + + cmd := &cobra.Command{ + Use: "create", + Short: "Create an address", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + body, err := f.payload() + if err != nil { + return err + } + + w, err := opts.Writer(cmd.OutOrStdout()) + if err != nil { + return err + } + + c, err := opts.Client(cmd.Flags()) + if err != nil { + return err + } + + ctx, cancel := opts.Context(cmd.Context()) + defer cancel() + + created, err := address.NewAPI(c).Create(ctx, body) + if err != nil { + return opts.Fail(fmt.Errorf("creating address: %w", err)) + } + + return renderAddressSummary(w, &created) + }, + } + + f.register(cmd.Flags()) + + return cmd +} + +// newNetworkAddressUpdateCommand reads the address first only to carry +// rdns_name, the one address.Update field without omitempty, so an unset +// --rdns does not wipe reverse DNS. Description and role are sparse fields, so +// emptying them is refused while --rdns "" clears the reverse DNS name. The +// update body also carries name, but an address's name is the IP itself, so no +// flag is offered for it. +func newNetworkAddressUpdateCommand(opts *globalOptions) *cobra.Command { + var description, role, rdns string + + cmd := &cobra.Command{ + Use: "update ", + Short: "Update an address", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := resource.ValidateIdentifier("address", args[0]); err != nil { + return err + } + + flags := cmd.Flags() + if !flags.Changed("description") && !flags.Changed("role") && !flags.Changed("rdns") { + return errmap.Usagef("nothing to update: pass at least one field to change") + } + + if flags.Changed("description") && description == "" { + return errmap.Usagef("--description cannot be emptied: go-anxcloud drops an empty description from the request") + } + + if flags.Changed("role") && role == "" { + return errmap.Usagef("--role cannot be emptied: go-anxcloud drops an empty role from the request") + } + + w, err := opts.Writer(cmd.OutOrStdout()) + if err != nil { + return err + } + + c, err := opts.Client(flags) + if err != nil { + return err + } + + ctx, cancel := opts.Context(cmd.Context()) + defer cancel() + + a := address.NewAPI(c) + current, err := a.Get(ctx, pathValue(args[0])) + if err != nil { + return opts.Fail(fmt.Errorf("reading address %q: %w", args[0], err)) + } + + body := address.Update{RDNSName: current.RDNSName} + if flags.Changed("description") { + body.DescriptionCustomer = description + } + if flags.Changed("role") { + body.Role = role + } + if flags.Changed("rdns") { + body.RDNSName = rdns + } + + updated, err := a.Update(ctx, pathValue(args[0]), body) + if err != nil { + return opts.Fail(fmt.Errorf("updating address %q: %w", args[0], err)) + } + + return renderAddressSummary(w, &updated) + }, + } + + cmd.Flags().StringVar(&description, "description", "", "customer description") + cmd.Flags().StringVar(&role, "role", "", "address role") + cmd.Flags().StringVar(&rdns, "rdns", "", "reverse DNS name, empty clears it") + + return cmd +} + +func newNetworkAddressDeleteCommand(opts *globalOptions) *cobra.Command { + return &cobra.Command{ + Use: "delete ", + Aliases: []string{"destroy"}, + Short: "Delete an address", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := resource.ValidateIdentifier("address", args[0]); err != nil { + return err + } + + if err := confirm.Prompt(cmd.InOrStdin(), cmd.ErrOrStderr(), fmt.Sprintf("delete address %q", args[0]), opts.AssumeYes()); err != nil { + return err + } + + c, err := opts.Client(cmd.Flags()) + if err != nil { + return err + } + + ctx, cancel := opts.Context(cmd.Context()) + defer cancel() + + if err := address.NewAPI(c).Delete(ctx, pathValue(args[0])); err != nil { + return opts.Fail(fmt.Errorf("deleting address %q: %w", args[0], err)) + } + + _, err = fmt.Fprintf(cmd.ErrOrStderr(), "deleted address %s\n", args[0]) + + return err + }, + } +} + +type addressReserveFlags struct { + location string + vlan string + count int + prefix string + version int + reservationPeriod time.Duration +} + +func (f *addressReserveFlags) register(flags *pflag.FlagSet) { + flags.StringVar(&f.location, "location", "", "location identifier to reserve addresses in") + flags.StringVar(&f.vlan, "vlan", "", "VLAN identifier to reserve addresses in") + flags.IntVar(&f.count, "count", 1, "number of addresses to reserve") + flags.StringVar(&f.prefix, "prefix", "", "prefix identifier to reserve addresses from") + flags.IntVar(&f.version, "version", 0, "IP version to reserve, 4 or 6") + flags.DurationVar(&f.reservationPeriod, "reservation-period", 0, "how long to reserve the addresses (the Engine defaults to 30m)") +} + +func (f *addressReserveFlags) payload(flags *pflag.FlagSet) (address.ReserveRandom, error) { + if f.location == "" { + return address.ReserveRandom{}, errmap.Usagef("--location is required") + } + + if err := resource.ValidateIdentifier("location", f.location); err != nil { + return address.ReserveRandom{}, err + } + + if f.vlan == "" { + return address.ReserveRandom{}, errmap.Usagef("--vlan is required") + } + + if err := resource.ValidateIdentifier("vlan", f.vlan); err != nil { + return address.ReserveRandom{}, err + } + + if f.count < 1 { + return address.ReserveRandom{}, errmap.Usagef("--count must be at least 1") + } + + if flags.Changed("version") && f.version != 4 && f.version != 6 { + return address.ReserveRandom{}, errmap.Usagef("--version %d must be 4 or 6", f.version) + } + + if f.prefix != "" { + if err := resource.ValidateIdentifier("prefix", f.prefix); err != nil { + return address.ReserveRandom{}, err + } + } + + if flags.Changed("reservation-period") && f.reservationPeriod < time.Second { + return address.ReserveRandom{}, errmap.Usagef("--reservation-period must be at least 1s") + } + + seconds := f.reservationPeriod / time.Second + // #nosec G115 -- the value is 0 when unset and otherwise at least 1, so it is never negative. + period := uint(seconds) + + return address.ReserveRandom{ + LocationID: f.location, + VlanID: f.vlan, + Count: f.count, + PrefixID: f.prefix, + IPVersion: address.IPReserveVersionLimit(f.version), + ReservationPeriod: period, + }, nil +} + +func newNetworkAddressReserveCommand(opts *globalOptions) *cobra.Command { + var f addressReserveFlags + + cmd := &cobra.Command{ + Use: "reserve", + Short: "Reserve random addresses", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + body, err := f.payload(cmd.Flags()) + if err != nil { + return err + } + + w, err := opts.Writer(cmd.OutOrStdout()) + if err != nil { + return err + } + + c, err := opts.Client(cmd.Flags()) + if err != nil { + return err + } + + ctx, cancel := opts.Context(cmd.Context()) + defer cancel() + + reserved, err := address.NewAPI(c).ReserveRandom(ctx, body) + if err != nil { + return opts.Fail(fmt.Errorf("reserving addresses: %w", err)) + } + if len(reserved.Data) == 0 { + return opts.Fail(errors.New("reserving addresses: the Engine returned no addresses")) + } + + if w.Format().Structured() { + return w.Object(reserved) + } + + return resource.RenderList(cmd, w, "reserved addresses", reserved.Data, + []string{"identifier", "address", "prefix"}, + func(ip *address.ReservedIP) []string { + return []string{ip.ID, ip.Address, ip.Prefix} + }, + ) + }, + } + + f.register(cmd.Flags()) + + return cmd +} diff --git a/internal/cli/network_test.go b/internal/cli/network_test.go index 7ef7ef1..ef8e9cd 100644 --- a/internal/cli/network_test.go +++ b/internal/cli/network_test.go @@ -51,16 +51,13 @@ func TestNetworkWithoutSubcommandPrintsHelp(t *testing.T) { require.Contains(t, stdout, "address") } -func TestNetworkAddressOnlyHasReadVerbs(t *testing.T) { +func TestNetworkAddressHasEveryVerb(t *testing.T) { isolate(t) stdout, _, err := run(t, "network", "address") require.NoError(t, err) - require.Contains(t, stdout, "list") - require.Contains(t, stdout, "get") - for _, absent := range []string{"create", "update", "delete", "destroy", "reserve"} { - require.NotContains(t, stdout, absent, - "address write verbs are still to be declared") + for _, verb := range []string{"list", "get", "create", "update", "delete", "reserve"} { + require.Contains(t, stdout, verb) } } @@ -1232,6 +1229,523 @@ func TestNetworkAddressesPluralAlias(t *testing.T) { require.Contains(t, stdout, "10.0.0.1") } +const ( + oneAddress = `{"identifier":"a-1","name":"10.0.0.1","role_text":"Default","description_customer":"gateway","rdns_name":"old.example.com"}` + createdAddress = `{"identifier":"a-1","name":"10.0.0.9","role_text":"Default","description_customer":"gateway","rdns_name":"host.example.com"}` +) + +// TestNetworkAddressCreateSendsTheLegacyCreateBody pins the address creation payload and Engine response rendering. +func TestNetworkAddressCreateSendsTheLegacyCreateBody(t *testing.T) { + isolate(t) + + var sent []request + srv := recordingServer(t, &sent, createdAddress) + + stdout, _, err := run(t, "network", "address", "create", + "--prefix", "p-1", "--address", "10.0.0.1", "--description", "gateway", + "--organization", "o-1", "--rdns", "host.example.com", + "--token", "tok", "--api-base-url", srv) + require.NoError(t, err) + require.Len(t, sent, 1) + require.Equal(t, http.MethodPost, sent[0].method) + require.Equal(t, "/api/ipam/v1/address.json", sent[0].path) + require.Empty(t, sent[0].query) + + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(sent[0].body), &body)) + require.Equal(t, map[string]any{ + "prefix": "p-1", + "name": "10.0.0.1", + "description_customer": "gateway", + "role": "Default", + "organization": "o-1", + "rdns_name": "host.example.com", + }, body) + require.Equal(t, + "IDENTIFIER NAME ROLE DESCRIPTION\n"+ + "a-1 10.0.0.9 Default gateway\n", + stdout) +} + +// TestNetworkAddressCreateCarriesTheSelectedAndDefaultFields pins create payload values without omitempty. +func TestNetworkAddressCreateCarriesTheSelectedAndDefaultFields(t *testing.T) { + tests := []struct { + name string + args []string + want map[string]any + }{ + { + name: "selected role", + args: []string{"--prefix", "p-1", "--address", "10.0.0.1", "--role", "Reserved"}, + want: map[string]any{ + "prefix": "p-1", "name": "10.0.0.1", "description_customer": "", "role": "Reserved", "organization": "", "rdns_name": "", + }, + }, + { + name: "defaults", + args: []string{"--prefix", "p-1", "--address", "10.0.0.1"}, + want: map[string]any{ + "prefix": "p-1", "name": "10.0.0.1", "description_customer": "", "role": "Default", "organization": "", "rdns_name": "", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, createdAddress) + + args := append([]string{"network", "address", "create"}, tt.args...) + args = append(args, "--token", "tok", "--api-base-url", srv) + _, _, err := run(t, args...) + require.NoError(t, err) + require.Len(t, sent, 1) + + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(sent[0].body), &body)) + require.Equal(t, tt.want, body) + }) + } +} + +// TestNetworkAddressCreateRejectsBadFlags pins that invalid create input never reaches the Engine. +func TestNetworkAddressCreateRejectsBadFlags(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + {name: "missing prefix", args: []string{"--address", "10.0.0.1"}, want: "--prefix is required"}, + {name: "missing address", args: []string{"--prefix", "p-1"}, want: "--address is required"}, + {name: "invalid prefix", args: []string{"--prefix", "p/1", "--address", "10.0.0.1"}, want: `prefix "p/1" does not name a prefix`}, + {name: "invalid organization", args: []string{"--prefix", "p-1", "--address", "10.0.0.1", "--organization", "o/1"}, want: `organization "o/1" does not name a organization`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, oneAddress) + + args := append([]string{"network", "address", "create"}, tt.args...) + args = append(args, "--token", "tok", "--api-base-url", srv) + _, _, err := run(t, args...) + require.Error(t, err) + require.Equal(t, errmap.ExitUsage, errmap.ExitCode(err)) + require.Contains(t, errmap.Message(err), tt.want) + require.Empty(t, sent) + }) + } +} + +// TestNetworkAddressUpdateKeepsRDNSAndRendersTheWrite pins rDNS preservation and rendering the PUT response. +func TestNetworkAddressUpdateKeepsRDNSAndRendersTheWrite(t *testing.T) { + isolate(t) + + var sent []request + updated := strings.Replace(oneAddress, `"gateway"`, `"lab"`, 1) + srv := recordingServer(t, &sent, oneAddress, updated) + + stdout, _, err := run(t, "network", "address", "update", "a-1", "--description", "lab", "-o", "json", + "--token", "tok", "--api-base-url", srv) + require.NoError(t, err) + require.Len(t, sent, 2) + require.Equal(t, http.MethodGet, sent[0].method) + require.Equal(t, "/api/ipam/v1/address.json/a-1", sent[0].path) + require.Empty(t, sent[0].query) + require.Equal(t, http.MethodPut, sent[1].method) + require.Equal(t, "/api/ipam/v1/address.json/a-1", sent[1].path) + require.Empty(t, sent[1].query) + + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(sent[1].body), &body)) + require.Equal(t, map[string]any{ + "description_customer": "lab", + "rdns_name": "old.example.com", + }, body) + + // The PUT answer is rendered whole: every summary field, including the + // ones the table drops (rdns_name, role_text), must survive into -o json. + require.JSONEq(t, updated, stdout) +} + +// TestNetworkAddressUpdateHandlesRDNSAndRoleChanges pins sparse updates and explicit rDNS overrides. +func TestNetworkAddressUpdateHandlesRDNSAndRoleChanges(t *testing.T) { + tests := []struct { + name string + args []string + want map[string]any + }{ + {name: "clears rdns", args: []string{"--rdns", ""}, want: map[string]any{"rdns_name": ""}}, + {name: "changes role", args: []string{"--role", "Reserved"}, want: map[string]any{"role": "Reserved", "rdns_name": "old.example.com"}}, + {name: "overrides rdns", args: []string{"--rdns", "new.example.com"}, want: map[string]any{"rdns_name": "new.example.com"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, oneAddress, oneAddress) + + args := append([]string{"network", "address", "update", "a-1"}, tt.args...) + args = append(args, "--token", "tok", "--api-base-url", srv) + _, _, err := run(t, args...) + require.NoError(t, err) + require.Len(t, sent, 2) + + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(sent[1].body), &body)) + require.Equal(t, tt.want, body) + }) + } +} + +// TestNetworkAddressUpdateReportsAFailedWrite pins that a failed PUT is named as an update. +func TestNetworkAddressUpdateReportsAFailedWrite(t *testing.T) { + isolate(t) + + var methods []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + methods = append(methods, r.Method) + w.Header().Set("Content-Type", "application/json") + if r.Method == http.MethodPut { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":{"code":500}}`)) + + return + } + _, _ = w.Write([]byte(oneAddress)) + })) + t.Cleanup(srv.Close) + + _, _, err := run(t, "network", "address", "update", "a-1", "--description", "lab", + "--token", "tok", "--api-base-url", srv.URL) + require.Error(t, err) + require.Equal(t, []string{http.MethodGet, http.MethodPut}, methods) + require.Contains(t, errmap.Message(err), `updating address "a-1"`) +} + +// TestNetworkAddressUpdateStopsWhenTheReadFails pins rDNS preservation by refusing to write after a failed read. +func TestNetworkAddressUpdateStopsWhenTheReadFails(t *testing.T) { + isolate(t) + + srv, last := server(t, http.StatusNotFound, `{"error":{"code":404}}`) + + _, _, err := run(t, "network", "address", "update", "a-1", "--description", "lab", + "--token", "tok", "--api-base-url", srv.URL) + require.Error(t, err) + require.Equal(t, http.MethodGet, last.method, "a PUT afterwards would overwrite the recorded GET") + require.Equal(t, errmap.ExitNotFound, errmap.ExitCode(err)) + require.Contains(t, errmap.Message(err), `reading address "a-1"`) +} + +// TestNetworkAddressUpdateRejectsInvalidChanges pins refused sparse update values before a read or write. +func TestNetworkAddressUpdateRejectsInvalidChanges(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + {name: "nothing changed", want: "nothing to update"}, + {name: "empty description", args: []string{"--description", ""}, want: "--description cannot be emptied"}, + {name: "empty role", args: []string{"--role", ""}, want: "--role cannot be emptied"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, oneAddress) + + args := append([]string{"network", "address", "update", "a-1"}, tt.args...) + args = append(args, "--token", "tok", "--api-base-url", srv) + _, _, err := run(t, args...) + require.Error(t, err) + require.Equal(t, errmap.ExitUsage, errmap.ExitCode(err)) + require.Contains(t, errmap.Message(err), tt.want) + require.Empty(t, sent) + }) + } +} + +// TestNetworkAddressUpdateOffersTheWritableFields pins the complete update flag surface. +func TestNetworkAddressUpdateOffersTheWritableFields(t *testing.T) { + isolate(t) + + stdout, _, err := run(t, "network", "address", "update", "--help") + require.NoError(t, err) + + _, after, found := strings.Cut(stdout, "Flags:\n") + require.True(t, found) + local, _, _ := strings.Cut(after, "Global Flags:") + var names []string + for _, m := range regexp.MustCompile(`(?m)^\s+(?:-\w, )?--([\w-]+)`).FindAllStringSubmatch(local, -1) { + names = append(names, m[1]) + } + require.Equal(t, []string{"description", "help", "rdns", "role"}, names) +} + +// TestNetworkAddressWriteVerbsGuardTheIdentifier pins URL path safety for address writes. +func TestNetworkAddressWriteVerbsGuardTheIdentifier(t *testing.T) { + verbs := []struct { + name string + args func(string) []string + method string + requests int + }{ + {name: "update", args: func(id string) []string { return []string{"network", "address", "update", id, "--description", "lab"} }, method: http.MethodPut, requests: 2}, + {name: "delete", args: func(id string) []string { return []string{"network", "address", "delete", id, "--yes"} }, method: http.MethodDelete, requests: 1}, + } + badIDs := map[string]string{ + "a slash": "a/1", "nothing": "", "whitespace": " ", "a dot": ".", "two dots": "..", "padded dots": " .. ", + } + + for _, verb := range verbs { + for label, id := range badIDs { + t.Run(verb.name+" refuses "+label, func(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, oneAddress) + + args := append(verb.args(id), "--token", "tok", "--api-base-url", srv) + _, _, err := run(t, args...) + require.Error(t, err) + require.Equal(t, errmap.ExitUsage, errmap.ExitCode(err)) + require.Contains(t, errmap.Message(err), fmt.Sprintf("address %q does not name a address", id)) + require.Empty(t, sent) + }) + } + + t.Run(verb.name+" escapes the identifier", func(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, oneAddress) + + args := append(verb.args("a 1?x=y"), "--token", "tok", "--api-base-url", srv) + _, _, err := run(t, args...) + require.NoError(t, err) + require.Len(t, sent, verb.requests) + require.Equal(t, verb.method, sent[len(sent)-1].method) + for _, req := range sent { + require.Equal(t, "/api/ipam/v1/address.json/a 1?x=y", req.path) + require.Empty(t, req.query) + } + }) + } +} + +// TestNetworkAddressDeleteConfirms pins the confirmation prompt and deletion note. +func TestNetworkAddressDeleteConfirms(t *testing.T) { + isolate(t) + + var sent []request + srv := recordingServer(t, &sent, `{}`) + + stdout, stderr, err := runWithInput(t, "y\n", "network", "address", "delete", "a-1", + "--token", "tok", "--api-base-url", srv) + require.NoError(t, err) + require.Empty(t, stdout) + require.Len(t, sent, 1) + require.Equal(t, http.MethodDelete, sent[0].method) + require.Equal(t, "/api/ipam/v1/address.json/a-1", sent[0].path) + require.Empty(t, sent[0].body) + require.Contains(t, stderr, `delete address "a-1"`) + require.Contains(t, stderr, "deleted address a-1") +} + +// TestNetworkAddressDeleteStopsOnARefusal pins that a declined confirmation makes no request. +func TestNetworkAddressDeleteStopsOnARefusal(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, `{}`) + + _, _, err := runWithInput(t, "n\n", "network", "address", "delete", "a-1", + "--token", "tok", "--api-base-url", srv) + require.Error(t, err) + require.Equal(t, errmap.ExitCanceled, errmap.ExitCode(err)) + require.Empty(t, sent) +} + +// TestNetworkAddressDeleteWithYesSkipsThePrompt pins the destroy alias and noninteractive delete. +func TestNetworkAddressDeleteWithYesSkipsThePrompt(t *testing.T) { + isolate(t) + srv, last := server(t, http.StatusOK, `{}`) + + _, stderr, err := run(t, "network", "address", "destroy", "a-1", "--yes", + "--token", "tok", "--api-base-url", srv.URL) + require.NoError(t, err) + require.Equal(t, http.MethodDelete, last.method) + require.NotContains(t, stderr, "[y/N]") + require.Contains(t, stderr, "deleted address a-1") +} + +// TestNetworkAddressDeleteReportsTheFailure pins the action wording and not-found exit code. +func TestNetworkAddressDeleteReportsTheFailure(t *testing.T) { + isolate(t) + srv, _ := server(t, http.StatusNotFound, `{"error":{"code":404}}`) + + _, _, err := run(t, "network", "address", "delete", "a-1", "--yes", + "--token", "tok", "--api-base-url", srv.URL) + require.Error(t, err) + require.Equal(t, errmap.ExitNotFound, errmap.ExitCode(err)) + require.Contains(t, errmap.Message(err), `deleting address "a-1"`) +} + +// TestNetworkAddressReserveSendsThePayloadAndRendersTheData pins an explicit +// reservation body and list rendering. ReserveRandom sleeps up to one second +// before each request, so reserve tests are slow by design. +func TestNetworkAddressReserveSendsThePayloadAndRendersTheData(t *testing.T) { + isolate(t) + + var sent []request + srv := recordingServer(t, &sent, `{"limit":1,"page":1,"total_items":1,"total_pages":1,"data":[{"identifier":"a-9","text":"10.0.0.9","prefix":"10.0.0.0/24"}]}`) + + stdout, _, err := run(t, "network", "address", "reserve", "--location", "l-1", "--vlan", "v-1", + "--count", "2", "--prefix", "p-1", "--version", "6", "--reservation-period", "30m", + "--token", "tok", "--api-base-url", srv) + require.NoError(t, err) + require.Len(t, sent, 1) + require.Equal(t, http.MethodPost, sent[0].method) + require.Equal(t, "/api/ipam/v1/address/reserve/ip/count.json", sent[0].path) + require.Empty(t, sent[0].query) + + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(sent[0].body), &body)) + require.Equal(t, map[string]any{ + "location_identifier": "l-1", + "vlan_identifier": "v-1", + "count": float64(2), + "prefix_identifier": "p-1", + "ip_version": float64(6), + "reservation_period": float64(1800), + }, body) + require.Equal(t, + "IDENTIFIER ADDRESS PREFIX\n"+ + "a-9 10.0.0.9 10.0.0.0/24\n", + stdout) +} + +// TestNetworkAddressReserveSendsOnlyTheRequiredFieldsByDefault pins the Engine default reservation payload. +func TestNetworkAddressReserveSendsOnlyTheRequiredFieldsByDefault(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, `{"data":[{"identifier":"a-9","text":"10.0.0.9","prefix":"10.0.0.0/24"}]}`) + + _, _, err := run(t, "network", "address", "reserve", "--location", "l-1", "--vlan", "v-1", + "--token", "tok", "--api-base-url", srv) + require.NoError(t, err) + require.Len(t, sent, 1) + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(sent[0].body), &body)) + require.Equal(t, map[string]any{ + "location_identifier": "l-1", + "vlan_identifier": "v-1", + "count": float64(1), + }, body) +} + +// TestNetworkAddressReserveTruncatesAcceptedPeriods pins the one-second +// boundary and the whole-seconds payload conversion. +func TestNetworkAddressReserveTruncatesAcceptedPeriods(t *testing.T) { + tests := []struct { + period string + want float64 + }{ + {period: "1s", want: 1}, + {period: "1500ms", want: 1}, + {period: "1h", want: 3600}, + } + + for _, tt := range tests { + t.Run(tt.period, func(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, `{"data":[{"identifier":"a-9","text":"10.0.0.9","prefix":"10.0.0.0/24"}]}`) + + _, _, err := run(t, "network", "address", "reserve", "--location", "l-1", "--vlan", "v-1", + "--reservation-period", tt.period, "--token", "tok", "--api-base-url", srv) + require.NoError(t, err) + require.Len(t, sent, 1) + + var body map[string]any + require.NoError(t, json.Unmarshal([]byte(sent[0].body), &body)) + require.Equal(t, tt.want, body["reservation_period"]) + }) + } +} + +// TestNetworkAddressReserveKeepsTheSummaryInJSON pins structured output of the full reservation summary. +func TestNetworkAddressReserveKeepsTheSummaryInJSON(t *testing.T) { + isolate(t) + srv, _ := server(t, http.StatusOK, `{"total_items":1,"data":[{"identifier":"a-9","text":"10.0.0.9","prefix":"10.0.0.0/24"}]}`) + + stdout, _, err := run(t, "network", "address", "reserve", "--location", "l-1", "--vlan", "v-1", "-o", "json", + "--token", "tok", "--api-base-url", srv.URL) + require.NoError(t, err) + var summary struct { + TotalItems int `json:"total_items"` + Data []struct { + ID string `json:"identifier"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &summary)) + require.Equal(t, 1, summary.TotalItems) + require.Len(t, summary.Data, 1) + require.Equal(t, "a-9", summary.Data[0].ID) +} + +// TestNetworkAddressReserveRejectsAnEmptyEngineResponse pins that an accepted reservation always contains addresses. +func TestNetworkAddressReserveRejectsAnEmptyEngineResponse(t *testing.T) { + isolate(t) + srv, _ := server(t, http.StatusOK, `{"data":[]}`) + + stdout, stderr, err := run(t, "network", "address", "reserve", "--location", "l-1", "--vlan", "v-1", + "--token", "tok", "--api-base-url", srv.URL) + require.Error(t, err) + require.Equal(t, errmap.ExitError, errmap.ExitCode(err)) + require.Empty(t, stdout) + require.Empty(t, stderr) + require.Contains(t, errmap.Message(err), "reserving addresses: the Engine returned no addresses") +} + +// TestNetworkAddressReserveRejectsBadFlags pins reserve validation before the Engine is contacted. +func TestNetworkAddressReserveRejectsBadFlags(t *testing.T) { + tests := []struct { + name string + args []string + want string + }{ + {name: "missing location", args: []string{"--vlan", "v-1"}, want: "--location is required"}, + {name: "missing vlan", args: []string{"--location", "l-1"}, want: "--vlan is required"}, + {name: "zero count", args: []string{"--location", "l-1", "--vlan", "v-1", "--count", "0"}, want: "--count must be at least 1"}, + {name: "invalid version", args: []string{"--location", "l-1", "--vlan", "v-1", "--version", "5"}, want: "--version 5 must be 4 or 6"}, + {name: "zero version", args: []string{"--location", "l-1", "--vlan", "v-1", "--version", "0"}, want: "--version 0 must be 4 or 6"}, + {name: "invalid location", args: []string{"--location", "l/1", "--vlan", "v-1"}, want: `location "l/1" does not name a location`}, + {name: "invalid vlan", args: []string{"--location", "l-1", "--vlan", "v/1"}, want: `vlan "v/1" does not name a vlan`}, + {name: "invalid prefix", args: []string{"--location", "l-1", "--vlan", "v-1", "--prefix", "p/1"}, want: `prefix "p/1" does not name a prefix`}, + {name: "sub-second reservation period", args: []string{"--location", "l-1", "--vlan", "v-1", "--reservation-period", "500ms"}, want: "--reservation-period must be at least 1s"}, + {name: "negative reservation period", args: []string{"--location", "l-1", "--vlan", "v-1", "--reservation-period", "-1s"}, want: "--reservation-period must be at least 1s"}, + {name: "zero reservation period", args: []string{"--location", "l-1", "--vlan", "v-1", "--reservation-period", "0s"}, want: "--reservation-period must be at least 1s"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isolate(t) + var sent []request + srv := recordingServer(t, &sent, `{}`) + + args := append([]string{"network", "address", "reserve"}, tt.args...) + args = append(args, "--token", "tok", "--api-base-url", srv) + _, _, err := run(t, args...) + require.Error(t, err) + require.Equal(t, errmap.ExitUsage, errmap.ExitCode(err)) + require.Contains(t, errmap.Message(err), tt.want) + require.Empty(t, sent) + }) + } +} + // TestNetworkListAllCarriesFiltersAcrossPages pins that the parameters that // pick what is listed survive a multi-page walk. Both nouns build their // request per page, so a filter left out of the second request would silently