From 4366377296a08dfbd8243697b5623a84aa7de2fb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 17:00:48 +0000 Subject: [PATCH 1/3] feat(network): add router/security group list filters and router show interfaces_info `router list` accepted only --name, so `--project` exited 2 with a usage error; it now takes upstream's --project/--project-domain, --enable/--disable and the tag filters. `security group list` took no filters at all; it now takes --project/--project-domain, the tag filters and a koc-native exact --name, and shows upstream's Tags column. Both also accept --all-projects (defaulting from ALL_PROJECTS, exclusive with --project). Neutron has no cross-project switch and both tables already show the Project column, so it changes nothing; it exists so a script carrying it gets a listing instead of a usage error. `router show` now emits upstream's interfaces_info: one entry per fixed IP of each non-gateway port with device_id=, the same extra port list upstream does, so callers no longer need their own `port list --router`. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RCegCom2HEfpHM4H58tBfa --- README.md | 4 + docs/coverage.md | 19 +++- internal/cli/network/network_verbs_test.go | 111 +++++++++++++++++++- internal/cli/network/router.go | 112 +++++++++++++++++++-- internal/cli/network/securitygroup.go | 54 ++++++++-- 5 files changed, 283 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 7c4c501..2ad62fc 100644 --- a/README.md +++ b/README.md @@ -1006,6 +1006,10 @@ items are deferred and worth noting: `Project ID` column, without which the rows of a multi-project result cannot be told apart. It also honours `ALL_PROJECTS` and is refused together with `--project`, which narrows to one project. +- **`router list` / `security group list --all-projects` is a no-op.** Same + neutron reasoning as `port list`, but both tables already show the Project + column, so the flag is accepted only so a script carrying it gets a listing + rather than a usage error. Use `--project` to narrow to one project. ## KeyStack documentation caveat diff --git a/docs/coverage.md b/docs/coverage.md index b962d84..e1a0f4a 100644 --- a/docs/coverage.md +++ b/docs/coverage.md @@ -67,7 +67,9 @@ Counts are **leaf commands, not flags**. A command can be present and still lag upstream's option surface, so a flag-parity pass changes no number here: `port list` was counted from the start, but only carried 4 of upstream's 17 filters until `055dc95`, and `subnet list` was counted while accepting no filters at all -until the history-parity pass. `image list` was counted while rejecting `--all`, +until the history-parity pass. `router list` accepted only `--name`, and +`security group list` no filter at all, until the network filter-parity pass; +and `router show` lacked upstream's `interfaces_info` until the same pass. `image list` was counted while rejecting `--all`, and every designate verb was counted while only some of them accepted `--all-projects`, until the cross-project/name pass. When auditing a noun, diff its flags against the upstream parser, not just its presence in these tables. @@ -80,8 +82,9 @@ command that omits one is a regression rather than a known gap: / `--all-stores` / `--target-all-projects`. Every command in this tree that upstream gives one of these now has it; `ALL_PROJECTS` in the environment defaults `--all-projects` on the compute and block-storage verbs upstream reads - it for (`internal/cli/allprojects`). `network port list` carries one upstream - does not define at all — see "koc-native commands". + it for (`internal/cli/allprojects`). `network port list`, `router list` and + `security group list` carry one upstream does not define at all — see + "koc-native commands". - **`--name`** — both as a list filter and as the *create* spelling. Upstream python-designateclient and python-octaviaclient name a new resource with `--name` and take no positional for it, where koc grew the positional first; @@ -557,6 +560,16 @@ indistinguishable (designate's `zone list` inserts the same column for the same reason). It defaults from `ALL_PROJECTS` like the compute and block-storage verbs, and is mutually exclusive with `--project`. +`router list --all-projects` and `security group list --all-projects` are the +same flag with nothing left to do: both tables already carry the Project +column, so it changes neither the request nor the output. It exists because a +script that reached for it got a usage error (exit 2) instead of a listing, and +a caller that falls back on failure then silently acts on nothing. Same +`ALL_PROJECTS` default, same exclusion with `--project`. `security group list +--name` is koc-native too — upstream's parser has `--project` and the tag +filters but no name filter — and is neutron's exact-match `name` query, so it +filters server-side instead of leaving every caller to do it client-side. + ## Updating this document The tables are derived, not hand-maintained. To re-derive after a version bump diff --git a/internal/cli/network/network_verbs_test.go b/internal/cli/network/network_verbs_test.go index c520902..a01ad75 100644 --- a/internal/cli/network/network_verbs_test.go +++ b/internal/cli/network/network_verbs_test.go @@ -547,7 +547,7 @@ func TestRunRouterList_RequestAndOutput(t *testing.T) { client := networkClient(fakeServer) o := &output.Options{Format: output.FormatTable} var buf bytes.Buffer - if err := runRouterList(context.Background(), client, o, "", &buf); err != nil { + if err := runRouterList(context.Background(), client, o, &routerListFlags{}, "", &buf); err != nil { t.Fatalf("runRouterList: %v", err) } for _, want := range []string{"rtr-1", "r", "ACTIVE", "UP"} { @@ -566,6 +566,11 @@ func TestRunRouterShow_RequestAndOutput(t *testing.T) { th.TestMethod(t, r, http.MethodGet) writeJSON(t, w, http.StatusOK, `{"router":{"id":"rtr-1","name":"r","status":"ACTIVE"}}`) }) + fakeServer.Mux.HandleFunc("/ports", func(w http.ResponseWriter, r *http.Request) { + th.TestMethod(t, r, http.MethodGet) + th.TestFormValues(t, r, map[string]string{"device_id": "rtr-1"}) + writeJSON(t, w, http.StatusOK, `{"ports":[]}`) + }) client := networkClient(fakeServer) o := &output.Options{Format: output.FormatValue} @@ -578,6 +583,76 @@ func TestRunRouterShow_RequestAndOutput(t *testing.T) { } } +// Neutron's router body has no interface list, so upstream's `router show` +// builds interfaces_info from the router's ports: one entry per fixed IP of +// every port except the external gateway. Scripts read it to find what is still +// attached before `router delete`, so the shape and key order have to match. +func TestRunRouterShow_InterfacesInfo(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + + emptyLookup(t, fakeServer, "/routers", "routers") + fakeServer.Mux.HandleFunc("/routers/rtr-1", func(w http.ResponseWriter, _ *http.Request) { + writeJSON(t, w, http.StatusOK, `{"router":{"id":"rtr-1","name":"r","status":"ACTIVE"}}`) + }) + fakeServer.Mux.HandleFunc("/ports", func(w http.ResponseWriter, r *http.Request) { + th.TestFormValues(t, r, map[string]string{"device_id": "rtr-1"}) + writeJSON(t, w, http.StatusOK, `{"ports":[ + {"id":"gw-port","device_owner":"network:router_gateway", + "fixed_ips":[{"subnet_id":"ext-sub","ip_address":"203.0.113.10"}]}, + {"id":"if-port","device_owner":"network:router_interface", + "fixed_ips":[{"subnet_id":"sub-a","ip_address":"192.0.2.1"}, + {"subnet_id":"sub-b","ip_address":"2001:db8::1"}]} + ]}`) + }) + + o := &output.Options{Format: output.FormatValue, Columns: []string{"interfaces_info"}} + var buf bytes.Buffer + if err := runRouterShow(context.Background(), networkClient(fakeServer), o, "rtr-1", &buf); err != nil { + t.Fatalf("runRouterShow: %v", err) + } + want := `[{"port_id":"if-port","ip_address":"192.0.2.1","subnet_id":"sub-a"},` + + `{"port_id":"if-port","ip_address":"2001:db8::1","subnet_id":"sub-b"}]` + "\n" + if got := buf.String(); got != want { + t.Errorf("interfaces_info =\n%s\nwant\n%s", got, want) + } +} + +// "router list" accepted only --name, so an `openstack router list --project X` +// invocation died on a usage error. Every filter must reach the query string. +func TestRunRouterList_SendsEveryFilter(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + + fakeServer.Mux.HandleFunc("/routers", func(w http.ResponseWriter, r *http.Request) { + th.TestMethod(t, r, http.MethodGet) + th.TestFormValues(t, r, map[string]string{ + "name": "r", + "project_id": "p1", + "admin_state_up": "false", + "tags": "a,b", + "tags-any": "c", + "not-tags": "d", + "not-tags-any": "e", + }) + writeJSON(t, w, http.StatusOK, `{"routers":[{"id":"rtr-1","name":"r","project_id":"p1"}]}`) + }) + + down := false + f := &routerListFlags{ + name: "r", adminStateUp: &down, + tags: []string{"a", "b"}, anyTags: []string{"c"}, notTags: []string{"d"}, notAnyTags: []string{"e"}, + } + o := &output.Options{Format: output.FormatValue} + var buf bytes.Buffer + if err := runRouterList(context.Background(), networkClient(fakeServer), o, f, "p1", &buf); err != nil { + t.Fatalf("runRouterList: %v", err) + } + if !strings.Contains(buf.String(), "rtr-1") { + t.Errorf("router list output missing rtr-1:\n%s", buf.String()) + } +} + func TestRunRouterCreate_Disabled(t *testing.T) { fakeServer := th.SetupHTTP() defer fakeServer.Teardown() @@ -927,7 +1002,7 @@ func TestRunSecurityGroupList_RequestAndOutput(t *testing.T) { client := networkClient(fakeServer) o := &output.Options{Format: output.FormatTable} var buf bytes.Buffer - if err := runSecurityGroupList(context.Background(), client, o, &buf); err != nil { + if err := runSecurityGroupList(context.Background(), client, o, &secGroupListFlags{}, "", &buf); err != nil { t.Fatalf("runSecurityGroupList: %v", err) } for _, want := range []string{"sg-1", "default", "desc"} { @@ -937,6 +1012,38 @@ func TestRunSecurityGroupList_RequestAndOutput(t *testing.T) { } } +// "security group list" took no filters at all, so callers listed everything +// and filtered client-side. Every filter must reach the query string. +func TestRunSecurityGroupList_SendsEveryFilter(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + + fakeServer.Mux.HandleFunc("/security-groups", func(w http.ResponseWriter, r *http.Request) { + th.TestMethod(t, r, http.MethodGet) + th.TestFormValues(t, r, map[string]string{ + "name": "web", + "project_id": "p1", + "tags": "a", + "tags-any": "b", + "not-tags": "c", + "not-tags-any": "d", + }) + writeJSON(t, w, http.StatusOK, `{"security_groups":[{"id":"sg-1","name":"web","project_id":"p1","tags":["a"]}]}`) + }) + + f := &secGroupListFlags{ + name: "web", tags: []string{"a"}, anyTags: []string{"b"}, notTags: []string{"c"}, notAnyTags: []string{"d"}, + } + o := &output.Options{Format: output.FormatValue} + var buf bytes.Buffer + if err := runSecurityGroupList(context.Background(), networkClient(fakeServer), o, f, "p1", &buf); err != nil { + t.Fatalf("runSecurityGroupList: %v", err) + } + if got, want := buf.String(), "sg-1\tweb\t\tp1\ta\n"; got != want { + t.Errorf("security group list = %q, want %q", got, want) + } +} + func TestRunSecurityGroupShow_RequestAndOutput(t *testing.T) { fakeServer := th.SetupHTTP() defer fakeServer.Teardown() diff --git a/internal/cli/network/router.go b/internal/cli/network/router.go index f33c58b..cc1a6a7 100644 --- a/internal/cli/network/router.go +++ b/internal/cli/network/router.go @@ -9,9 +9,11 @@ import ( "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/layer3/routers" + "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/ports" "github.com/spf13/cobra" "github.com/ftarasenko/go-openstackclient/internal/auth" + "github.com/ftarasenko/go-openstackclient/internal/cli/allprojects" "github.com/ftarasenko/go-openstackclient/internal/cli/batchdelete" "github.com/ftarasenko/go-openstackclient/internal/output" ) @@ -46,8 +48,26 @@ func routerShowFields(r *routers.Router) ([]string, []any) { return fields, values } +// routerListFlags holds the filters accepted by "router list". Upstream OSC's +// parser (network/v2/router.py ListRouter) is the reference; --all-projects is +// the one koc-native addition — see allProjectsNetworkList. +type routerListFlags struct { + name string + project string + projectDomain string + enable bool + disable bool + tags []string + anyTags []string + notTags []string + notAnyTags []string + allProjects bool + + adminStateUp *bool +} + func newRouterListCommand(a *auth.Options, o *output.Options) *cobra.Command { - var name string + f := &routerListFlags{} cmd := &cobra.Command{ Use: "list", Short: "List routers", @@ -56,20 +76,60 @@ func newRouterListCommand(a *auth.Options, o *output.Options) *cobra.Command { if err := o.Validate(); err != nil { return err } + fl := cmd.Flags() + if err := mutuallyExclusive(fl, "enable", "disable"); err != nil { + return err + } + f.adminStateUp = enableDisable(fl, f.enable, f.disable) ctx := cmd.Context() - client, err := newNetworkClient(ctx, a) + client, session, err := newNetworkSession(ctx, a) if err != nil { return err } - return runRouterList(ctx, client, o, name, cmd.OutOrStdout()) + projectID, err := resolveProjectRef(ctx, session, f.project, f.projectDomain) + if err != nil { + return err + } + return runRouterList(ctx, client, o, f, projectID, cmd.OutOrStdout()) }, } - cmd.Flags().StringVar(&name, "name", "", "filter routers by name") + fl := cmd.Flags() + fl.StringVar(&f.name, "name", "", "list only routers with this name") + fl.StringVar(&f.project, "project", "", "list only routers owned by this project (name or ID)") + fl.StringVar(&f.projectDomain, "project-domain", "", "domain owning --project, to disambiguate the name (name or ID)") + fl.BoolVar(&f.enable, "enable", false, "list only enabled routers (admin state up)") + fl.BoolVar(&f.disable, "disable", false, "list only disabled routers (admin state down)") + fl.StringSliceVar(&f.tags, "tags", nil, "list only routers with all of these tags (comma-separated)") + fl.StringSliceVar(&f.anyTags, "any-tags", nil, "list only routers with any of these tags (comma-separated)") + fl.StringSliceVar(&f.notTags, "not-tags", nil, "exclude routers with all of these tags (comma-separated)") + fl.StringSliceVar(&f.notAnyTags, "not-any-tags", nil, "exclude routers with any of these tags (comma-separated)") + allprojects.Bind(cmd, &f.allProjects, allProjectsNetworkList) + cmd.MarkFlagsMutuallyExclusive("project", "all-projects") return cmd } -func runRouterList(ctx context.Context, client *gophercloud.ServiceClient, o *output.Options, name string, w io.Writer) error { - pages, err := routers.List(client, routers.ListOpts{Name: name}).AllPages(ctx) +// allProjectsNetworkList is the --all-projects help text for the neutron list +// verbs whose table already carries the Project column. As with port list +// (allProjectsPortList), neutron has no cross-project switch — an admin token +// already lists every project's resources — so the flag changes nothing on the +// wire. It is accepted so a script that reaches for it is not rejected with a +// usage error, which is exactly how a missing filter used to go unnoticed. +const allProjectsNetworkList = "list across all projects (admin); an admin token already sees them all, " + + "so this is accepted for compatibility and changes nothing" + +func runRouterList(ctx context.Context, client *gophercloud.ServiceClient, o *output.Options, + f *routerListFlags, projectID string, w io.Writer, +) error { + opts := routers.ListOpts{ + Name: f.name, + ProjectID: projectID, + AdminStateUp: f.adminStateUp, + Tags: strings.Join(f.tags, ","), + TagsAny: strings.Join(f.anyTags, ","), + NotTags: strings.Join(f.notTags, ","), + NotTagsAny: strings.Join(f.notAnyTags, ","), + } + pages, err := routers.List(client, opts).AllPages(ctx) if err != nil { return fmt.Errorf("listing routers: %w", err) } @@ -113,10 +173,50 @@ func runRouterShow(ctx context.Context, client *gophercloud.ServiceClient, o *ou if err != nil { return fmt.Errorf("getting router %s: %w", nameOrID, err) } + interfaces, err := routerInterfaces(ctx, client, r.ID) + if err != nil { + return err + } fields, values := routerShowFields(r) + fields = append(fields, "interfaces_info") + values = append(values, interfaces) return o.WriteSingle(w, fields, values) } +// routerInterface is one entry of router show's interfaces_info: a single fixed +// IP of one of the router's internal ports. The field order matches upstream's +// dict, so the rendered JSON is byte-identical to `openstack router show`. +type routerInterface struct { + PortID string `json:"port_id"` + IPAddress string `json:"ip_address"` + SubnetID string `json:"subnet_id"` +} + +// routerInterfaces derives interfaces_info the way upstream OSC's ShowRouter +// does: neutron's router body carries no interface list, so the router's ports +// are listed by device_id and every fixed IP of each non-gateway port becomes an +// entry. It is one extra GET, which is what `router show` costs upstream too. +func routerInterfaces(ctx context.Context, client *gophercloud.ServiceClient, routerID string) ([]routerInterface, error) { + pages, err := ports.List(client, ports.ListOpts{DeviceID: routerID}).AllPages(ctx) + if err != nil { + return nil, fmt.Errorf("listing ports of router %s: %w", routerID, err) + } + all, err := ports.ExtractPorts(pages) + if err != nil { + return nil, fmt.Errorf("parsing ports of router %s: %w", routerID, err) + } + out := make([]routerInterface, 0, len(all)) + for _, p := range all { + if p.DeviceOwner == "network:router_gateway" { + continue + } + for _, ip := range p.FixedIPs { + out = append(out, routerInterface{PortID: p.ID, IPAddress: ip.IPAddress, SubnetID: ip.SubnetID}) + } + } + return out, nil +} + type routerCreateFlags struct { enable bool disable bool diff --git a/internal/cli/network/securitygroup.go b/internal/cli/network/securitygroup.go index d29f00c..deb7a47 100644 --- a/internal/cli/network/securitygroup.go +++ b/internal/cli/network/securitygroup.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/cobra" "github.com/ftarasenko/go-openstackclient/internal/auth" + "github.com/ftarasenko/go-openstackclient/internal/cli/allprojects" "github.com/ftarasenko/go-openstackclient/internal/cli/batchdelete" "github.com/ftarasenko/go-openstackclient/internal/output" ) @@ -39,7 +40,24 @@ func secGroupShowFields(g *groups.SecGroup) ([]string, []any) { return fields, values } +// secGroupListFlags holds the filters accepted by "security group list". +// Upstream OSC (network/v2/security_group.py ListSecurityGroup) takes --project, +// --project-domain and the tag filters; --name and --all-projects are +// koc-native additions (see allProjectsNetworkList for the latter). --name is a +// plain neutron query filter, so it is exact-match and server-side. +type secGroupListFlags struct { + name string + project string + projectDomain string + tags []string + anyTags []string + notTags []string + notAnyTags []string + allProjects bool +} + func newSecurityGroupListCommand(a *auth.Options, o *output.Options) *cobra.Command { + f := &secGroupListFlags{} cmd := &cobra.Command{ Use: "list", Short: "List security groups", @@ -49,18 +67,42 @@ func newSecurityGroupListCommand(a *auth.Options, o *output.Options) *cobra.Comm return err } ctx := cmd.Context() - client, err := newNetworkClient(ctx, a) + client, session, err := newNetworkSession(ctx, a) if err != nil { return err } - return runSecurityGroupList(ctx, client, o, cmd.OutOrStdout()) + projectID, err := resolveProjectRef(ctx, session, f.project, f.projectDomain) + if err != nil { + return err + } + return runSecurityGroupList(ctx, client, o, f, projectID, cmd.OutOrStdout()) }, } + fl := cmd.Flags() + fl.StringVar(&f.name, "name", "", "list only security groups with this name") + fl.StringVar(&f.project, "project", "", "list only security groups owned by this project (name or ID)") + fl.StringVar(&f.projectDomain, "project-domain", "", "domain owning --project, to disambiguate the name (name or ID)") + fl.StringSliceVar(&f.tags, "tags", nil, "list only security groups with all of these tags (comma-separated)") + fl.StringSliceVar(&f.anyTags, "any-tags", nil, "list only security groups with any of these tags (comma-separated)") + fl.StringSliceVar(&f.notTags, "not-tags", nil, "exclude security groups with all of these tags (comma-separated)") + fl.StringSliceVar(&f.notAnyTags, "not-any-tags", nil, "exclude security groups with any of these tags (comma-separated)") + allprojects.Bind(cmd, &f.allProjects, allProjectsNetworkList) + cmd.MarkFlagsMutuallyExclusive("project", "all-projects") return cmd } -func runSecurityGroupList(ctx context.Context, client *gophercloud.ServiceClient, o *output.Options, w io.Writer) error { - pages, err := groups.List(client, groups.ListOpts{}).AllPages(ctx) +func runSecurityGroupList(ctx context.Context, client *gophercloud.ServiceClient, o *output.Options, + f *secGroupListFlags, projectID string, w io.Writer, +) error { + opts := groups.ListOpts{ + Name: f.name, + ProjectID: projectID, + Tags: strings.Join(f.tags, ","), + TagsAny: strings.Join(f.anyTags, ","), + NotTags: strings.Join(f.notTags, ","), + NotTagsAny: strings.Join(f.notAnyTags, ","), + } + pages, err := groups.List(client, opts).AllPages(ctx) if err != nil { return fmt.Errorf("listing security groups: %w", err) } @@ -68,9 +110,9 @@ func runSecurityGroupList(ctx context.Context, client *gophercloud.ServiceClient if err != nil { return fmt.Errorf("parsing security group list: %w", err) } - t := output.Table{Columns: []string{"ID", "Name", "Description", "Project"}, Rows: make([][]any, 0, len(all))} + t := output.Table{Columns: []string{"ID", "Name", "Description", "Project", "Tags"}, Rows: make([][]any, 0, len(all))} for _, g := range all { - t.Rows = append(t.Rows, []any{g.ID, g.Name, g.Description, g.ProjectID}) + t.Rows = append(t.Rows, []any{g.ID, g.Name, g.Description, g.ProjectID, g.Tags}) } return o.WriteList(w, t) } From 179740143ac61743e2f4241345de96b9001698f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 17:00:48 +0000 Subject: [PATCH 2/3] feat(server): add --wait and --force to server delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `server delete --wait` issues every DELETE, then polls each server until nova answers 404 — the point its ports are released — and prints the confirmation only then, so cleanup scripts no longer need their own poll loop. --wait-timeout bounds it per server. A SOFT_DELETED server fails the wait immediately (nova's reclaim window would outlast any timeout) and points at --force, which sends nova's forceDelete action as upstream's does. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RCegCom2HEfpHM4H58tBfa --- README.md | 5 ++ docs/coverage.md | 3 +- internal/cli/server/server.go | 61 ++++++++++++-- internal/cli/server/server_more_test.go | 107 +++++++++++++++++++++++- internal/cli/server/state.go | 49 +++++++++++ 5 files changed, 214 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 2ad62fc..9028c42 100644 --- a/README.md +++ b/README.md @@ -1010,6 +1010,11 @@ items are deferred and worth noting: neutron reasoning as `port list`, but both tables already show the Project column, so the flag is accepted only so a script carrying it gets a listing rather than a usage error. Use `--project` to narrow to one project. +- **`server delete --wait` fails fast on `SOFT_DELETED`.** With nova's + `reclaim_instance_interval` set, a plain delete parks the server until the + window lapses — past any sensible `--wait-timeout` — and it keeps its ports + meanwhile. koc reports that at once instead of timing out; `--force` deletes + it immediately, as upstream's does. ## KeyStack documentation caveat diff --git a/docs/coverage.md b/docs/coverage.md index e1a0f4a..7031f36 100644 --- a/docs/coverage.md +++ b/docs/coverage.md @@ -69,7 +69,8 @@ list` was counted from the start, but only carried 4 of upstream's 17 filters until `055dc95`, and `subnet list` was counted while accepting no filters at all until the history-parity pass. `router list` accepted only `--name`, and `security group list` no filter at all, until the network filter-parity pass; -and `router show` lacked upstream's `interfaces_info` until the same pass. `image list` was counted while rejecting `--all`, +`router show` lacked upstream's `interfaces_info` and `server delete` lacked +`--wait`/`--force` until the same pass. `image list` was counted while rejecting `--all`, and every designate verb was counted while only some of them accepted `--all-projects`, until the cross-project/name pass. When auditing a noun, diff its flags against the upstream parser, not just its presence in these tables. diff --git a/internal/cli/server/server.go b/internal/cli/server/server.go index 4cea3b0..c35d195 100644 --- a/internal/cli/server/server.go +++ b/internal/cli/server/server.go @@ -912,8 +912,17 @@ func runServerCreate(ctx context.Context, client *gophercloud.ServiceClient, o * return o.WriteSingle(w, fields, values) } +// serverDeleteFlags holds the options of "server delete". Names follow +// upstream OSC's DeleteServer parser (compute/v2/server.py). +type serverDeleteFlags struct { + allProjects bool + force bool + wait bool + waitTimeout time.Duration +} + func newServerDeleteCommand(a *auth.Options, o *output.Options) *cobra.Command { - var allProjects bool + f := &serverDeleteFlags{} cmd := &cobra.Command{ Use: "delete [ ...]", Short: "Delete one or more servers", @@ -927,30 +936,64 @@ func newServerDeleteCommand(a *auth.Options, o *output.Options) *cobra.Command { if err != nil { return err } - return runServerDelete(ctx, client, args, cmd.OutOrStdout()) + return runServerDelete(ctx, client, args, f, cmd.OutOrStdout()) }, } - allprojects.Bind(cmd, &allProjects, allProjectsAlwaysOn) + fl := cmd.Flags() + fl.BoolVar(&f.force, "force", false, "force-delete the server(s), bypassing nova's soft-delete reclaim window") + fl.BoolVar(&f.wait, "wait", false, "wait until each server is gone before returning") + fl.DurationVar(&f.waitTimeout, flagWaitTimeout, statusPollTimeout, helpWaitTimeout+" (per server)") + allprojects.Bind(cmd, &f.allProjects, allProjectsAlwaysOn) return cmd } -func runServerDelete(ctx context.Context, client *gophercloud.ServiceClient, refs []string, w io.Writer) error { +// runServerDelete deletes every ref and, with --wait, then follows each one +// until nova stops returning it. The two phases are separate on purpose, as +// upstream's are: nova tears the servers down concurrently, so issuing every +// DELETE before polling any of them makes the wait roughly as long as the +// slowest server instead of the sum of all of them. +func runServerDelete(ctx context.Context, client *gophercloud.ServiceClient, refs []string, f *serverDeleteFlags, w io.Writer) error { + type accepted struct{ ref, id string } + var pending []accepted // Attempt every ref; batchdelete.Each collects failures so one bad server // does not prevent the rest from being deleted, then reports all of them // together. - return batchdelete.Each(refs, func(ref string) error { + err := batchdelete.Each(refs, func(ref string) error { id, err := resolveServerID(ctx, client, ref) if err != nil { return err } - if err := servers.Delete(ctx, client, id).ExtractErr(); err != nil { + if err := deleteServer(ctx, client, id, f.force); err != nil { return fmt.Errorf("deleting server %q: %w", ref, err) } - if _, err := fmt.Fprintf(w, "Deleted server %s\n", ref); err != nil { - return err + if f.wait { + pending = append(pending, accepted{ref: ref, id: id}) + return nil } - return nil + _, err = fmt.Fprintf(w, "Deleted server %s\n", ref) + return err }) + errs := []error{err} + for _, p := range pending { + if err := waitForServerDeleted(ctx, client, p.id, f.waitTimeout); err != nil { + errs = append(errs, fmt.Errorf("waiting for server %q to delete: %w", p.ref, err)) + continue + } + if _, err := fmt.Fprintf(w, "Deleted server %s\n", p.ref); err != nil { + return err + } + } + return errors.Join(errs...) +} + +// deleteServer issues nova's delete, or its forceDelete action for --force. The +// action is what upstream sends too: it is the only route past a +// reclaim_instance_interval soft delete. +func deleteServer(ctx context.Context, client *gophercloud.ServiceClient, id string, force bool) error { + if force { + return servers.ForceDelete(ctx, client, id).ExtractErr() + } + return servers.Delete(ctx, client, id).ExtractErr() } // serverSetFlags holds the mutable attributes accepted by "server set". diff --git a/internal/cli/server/server_more_test.go b/internal/cli/server/server_more_test.go index 746aee8..f56dbb9 100644 --- a/internal/cli/server/server_more_test.go +++ b/internal/cli/server/server_more_test.go @@ -5,10 +5,12 @@ import ( "context" "encoding/base64" "encoding/json" + "fmt" "io" "net/http" "strings" "testing" + "time" "github.com/gophercloud/gophercloud/v2/openstack/compute/v2/servers" th "github.com/gophercloud/gophercloud/v2/testhelper" @@ -436,7 +438,7 @@ func TestRunServerDelete_MultipleServers(t *testing.T) { client := computeClient(fakeServer, "2.79") var buf bytes.Buffer - if err := runServerDelete(context.Background(), client, []string{serverUUID, other}, &buf); err != nil { + if err := runServerDelete(context.Background(), client, []string{serverUUID, other}, &serverDeleteFlags{}, &buf); err != nil { t.Fatalf("runServerDelete: %v", err) } if deleted[serverUUID] != http.MethodDelete || deleted[other] != http.MethodDelete { @@ -448,6 +450,109 @@ func TestRunServerDelete_MultipleServers(t *testing.T) { } } +// --wait follows each server until nova answers 404, so a cleanup script can +// delete the server's ports and networks next without its own poll loop. Every +// DELETE is sent before any polling starts, and the confirmation is printed only +// once the server is actually gone. +func TestRunServerDelete_WaitPollsUntilGone(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + defer func(prev time.Duration) { statusPollInterval = prev }(statusPollInterval) + statusPollInterval = time.Millisecond + + const other = "22222222-2222-2222-2222-222222222222" + var events []string + gets := map[string]int{} + for _, id := range []string{serverUUID, other} { + fakeServer.Mux.HandleFunc("/servers/"+id, func(w http.ResponseWriter, r *http.Request) { + events = append(events, r.Method+" "+id) + if r.Method == http.MethodDelete { + w.WriteHeader(http.StatusNoContent) + return + } + gets[id]++ + if gets[id] < 3 { + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"server":{"id":%q,"status":"ACTIVE"}}`, id) + return + } + w.WriteHeader(http.StatusNotFound) + }) + } + + var buf bytes.Buffer + f := &serverDeleteFlags{wait: true, waitTimeout: time.Second} + if err := runServerDelete(context.Background(), computeClient(fakeServer, "2.79"), []string{serverUUID, other}, f, &buf); err != nil { + t.Fatalf("runServerDelete --wait: %v", err) + } + if len(events) < 2 || events[0] != "DELETE "+serverUUID || events[1] != "DELETE "+other { + t.Errorf("both DELETEs must precede polling; requests = %v", events) + } + if gets[serverUUID] != 3 || gets[other] != 3 { + t.Errorf("GET counts = %v, want 3 each (two ACTIVE, then 404)", gets) + } + want := "Deleted server " + serverUUID + "\nDeleted server " + other + "\n" + if got := buf.String(); got != want { + t.Errorf("output = %q, want %q", got, want) + } +} + +// A soft delete never reaches 404 inside any sensible timeout, so --wait says +// so immediately instead of spinning out --wait-timeout. +func TestRunServerDelete_WaitFailsFastOnSoftDelete(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + defer func(prev time.Duration) { statusPollInterval = prev }(statusPollInterval) + statusPollInterval = time.Millisecond + + fakeServer.Mux.HandleFunc("/servers/"+serverUUID, func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + w.WriteHeader(http.StatusNoContent) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"server":{"id":%q,"status":"SOFT_DELETED"}}`, serverUUID) + }) + + var buf bytes.Buffer + f := &serverDeleteFlags{wait: true, waitTimeout: time.Minute} + err := runServerDelete(context.Background(), computeClient(fakeServer, "2.79"), []string{serverUUID}, f, &buf) + if err == nil || !strings.Contains(err.Error(), "SOFT_DELETED") || !strings.Contains(err.Error(), "--force") { + t.Fatalf("error = %v, want a SOFT_DELETED error pointing at --force", err) + } + if buf.Len() != 0 { + t.Errorf("a server that is not gone must not be reported deleted:\n%s", buf.String()) + } +} + +// --force sends nova's forceDelete action instead of the plain DELETE, and only +// that — it must not issue both. +func TestRunServerDelete_ForceSendsForceDeleteAction(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + + var methods []string + fakeServer.Mux.HandleFunc("/servers/"+serverUUID, func(_ http.ResponseWriter, r *http.Request) { + methods = append(methods, r.Method) + }) + fakeServer.Mux.HandleFunc("/servers/"+serverUUID+"/action", func(w http.ResponseWriter, r *http.Request) { + th.TestMethod(t, r, http.MethodPost) + th.TestJSONRequest(t, r, `{"forceDelete": ""}`) + w.WriteHeader(http.StatusAccepted) + }) + + var buf bytes.Buffer + if err := runServerDelete(context.Background(), computeClient(fakeServer, "2.79"), []string{serverUUID}, &serverDeleteFlags{force: true}, &buf); err != nil { + t.Fatalf("runServerDelete --force: %v", err) + } + if len(methods) != 0 { + t.Errorf("--force must not also send a plain DELETE; saw %v", methods) + } + if !strings.Contains(buf.String(), "Deleted server "+serverUUID) { + t.Errorf("output missing confirmation:\n%s", buf.String()) + } +} + func TestRunServerSet_NameAndProperties(t *testing.T) { fakeServer := th.SetupHTTP() defer fakeServer.Teardown() diff --git a/internal/cli/server/state.go b/internal/cli/server/state.go index 1bd00f1..b45b3aa 100644 --- a/internal/cli/server/state.go +++ b/internal/cli/server/state.go @@ -2,8 +2,10 @@ package server import ( "context" + "errors" "fmt" "io" + "net/http" "strings" "time" @@ -419,6 +421,53 @@ func waitForServerStatuses(ctx context.Context, client *gophercloud.ServiceClien } } +// waitForServerDeleted polls until nova answers 404 for the server, which is +// the only signal that its ports and volumes have been released — the point a +// cleanup script deleting the server's network next is waiting for. +// +// SOFT_DELETED fails fast rather than running out the timeout: with nova's +// reclaim_instance_interval set, a plain delete parks the server there and the +// real delete happens only when the reclaim window lapses, well past any +// sensible --wait-timeout. ERROR is deliberately not a failure here, unlike the +// status waits above: deleting a server that is already in ERROR is the common +// case, and it stays ERROR until it disappears. +func waitForServerDeleted(ctx context.Context, client *gophercloud.ServiceClient, id string, timeout time.Duration) error { + if timeout <= 0 { + timeout = statusPollTimeout + } + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + ticker := time.NewTicker(statusPollInterval) + defer ticker.Stop() + + var last string + for { + s, err := servers.Get(ctx, client, id).Extract() + switch { + case gophercloud.ResponseCodeIs(err, http.StatusNotFound): + return nil + case err != nil && ctx.Err() != nil: + return fmt.Errorf("waiting for deletion%s: %w", lastStatus(last), ctx.Err()) + case err != nil: + return err + } + last = s.Status + switch s.Status { + case "DELETED": + return nil + case "SOFT_DELETED": + return errors.New("server is SOFT_DELETED: nova defers the real delete until its reclaim " + + "interval lapses; use --force to delete it now") + } + select { + case <-ctx.Done(): + return fmt.Errorf("waiting for deletion%s: %w", lastStatus(last), ctx.Err()) + case <-ticker.C: + } + } +} + // waitForImageActive polls glance rather than nova: nova returns as soon as the // snapshot is queued, and the image is not usable until glance reports active. func waitForImageActive(ctx context.Context, ac *auth.Client, imageID string, timeout time.Duration) error { From c29f1d29c68a9247fa2610194483aac9d7ecd510 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 22 Sep 2026 18:05:18 +0000 Subject: [PATCH 3/3] feat(network): add router list --long/--agent and security group list --share/--no-share `router list --long` adds upstream's Routes, External gateway info, Availability zones (when the router_availability_zone extension reports them) and Tags columns; Distributed and HA now appear whenever neutron returned them, as upstream shows them. `--agent ` lists the L3 agent's hosted routers and re-applies the other filters client-side, since that subresource takes no query parameters. `security group list --share/--no-share` sends neutron's `shared` filter, and the list gains upstream's Shared column. gophercloud's groups.List has no builder or shared field, so the pager is built by a small isolated helper. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RCegCom2HEfpHM4H58tBfa --- internal/cli/network/network_verbs_test.go | 126 ++++++++++++++++- internal/cli/network/router.go | 153 ++++++++++++++++++++- internal/cli/network/securitygroup.go | 78 ++++++++++- 3 files changed, 345 insertions(+), 12 deletions(-) diff --git a/internal/cli/network/network_verbs_test.go b/internal/cli/network/network_verbs_test.go index a01ad75..90ea2ec 100644 --- a/internal/cli/network/network_verbs_test.go +++ b/internal/cli/network/network_verbs_test.go @@ -3,6 +3,7 @@ package network import ( "bytes" "context" + "fmt" "net/http" "net/url" "strings" @@ -653,6 +654,94 @@ func TestRunRouterList_SendsEveryFilter(t *testing.T) { } } +// Upstream adds Distributed and HA only when neutron returned them (they are +// admin-only by policy), and --long adds Routes, External gateway info, +// Availability zones (only when the extension reports them) and Tags. +func TestRunRouterList_LongAndOptionalColumns(t *testing.T) { + for _, tc := range []struct { + name string + body string + long bool + columns string + }{ + { + name: "non-admin, short", + body: `{"id":"rtr-1","name":"r","status":"ACTIVE","admin_state_up":true,"project_id":"p1"}`, + columns: "ID Name Status State Project", + }, + { + name: "admin, short", + body: `{"id":"rtr-1","name":"r","status":"ACTIVE","admin_state_up":true,"project_id":"p1", + "distributed":false,"ha":true}`, + columns: "ID Name Status State Project Distributed HA", + }, + { + name: "non-admin, long, no AZ extension", + body: `{"id":"rtr-1","name":"r","status":"ACTIVE","admin_state_up":true,"project_id":"p1","tags":["x"]}`, + long: true, + columns: "ID Name Status State Project Routes External gateway info Tags", + }, + { + name: "admin, long, AZ extension", + body: `{"id":"rtr-1","name":"r","status":"ACTIVE","admin_state_up":true,"project_id":"p1", + "distributed":true,"ha":false,"availability_zones":["nova"],"tags":[]}`, + long: true, + columns: "ID Name Status State Project Distributed HA Routes External gateway info Availability zones Tags", + }, + } { + t.Run(tc.name, func(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + fakeServer.Mux.HandleFunc("/routers", func(w http.ResponseWriter, _ *http.Request) { + writeJSON(t, w, http.StatusOK, `{"routers":[`+tc.body+`]}`) + }) + + o := &output.Options{Format: output.FormatCSV} + var buf bytes.Buffer + if err := runRouterList(context.Background(), networkClient(fakeServer), o, &routerListFlags{long: tc.long}, "", &buf); err != nil { + t.Fatalf("runRouterList: %v", err) + } + header, _, _ := strings.Cut(buf.String(), "\n") + if got := strings.ReplaceAll(strings.ReplaceAll(header, `"`, ""), ",", " "); got != tc.columns { + t.Errorf("columns = %q, want %q", got, tc.columns) + } + }) + } +} + +// --agent lists the agent's l3-routers subresource, which takes no query +// filters, so the list filters are applied client-side. +func TestRunRouterList_AgentFiltersClientSide(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + + fakeServer.Mux.HandleFunc("/agents/agent-1/l3-routers", func(w http.ResponseWriter, r *http.Request) { + th.TestMethod(t, r, http.MethodGet) + if r.URL.RawQuery != "" { + t.Errorf("l3-routers takes no filters; got query %q", r.URL.RawQuery) + } + writeJSON(t, w, http.StatusOK, `{"routers":[ + {"id":"keep","name":"r","admin_state_up":true,"project_id":"p1","tags":["a","b"]}, + {"id":"wrong-project","name":"r","admin_state_up":true,"project_id":"p2","tags":["a","b"]}, + {"id":"disabled","name":"r","admin_state_up":false,"project_id":"p1","tags":["a","b"]}, + {"id":"missing-tag","name":"r","admin_state_up":true,"project_id":"p1","tags":["a"]}, + {"id":"excluded-tag","name":"r","admin_state_up":true,"project_id":"p1","tags":["a","b","z"]}, + {"id":"other-name","name":"s","admin_state_up":true,"project_id":"p1","tags":["a","b"]} + ]}`) + }) + + up := true + f := &routerListFlags{agent: "agent-1", name: "r", adminStateUp: &up, tags: []string{"a", "b"}, notAnyTags: []string{"z"}} + o := &output.Options{Format: output.FormatValue, Columns: []string{"ID"}} + var buf bytes.Buffer + if err := runRouterList(context.Background(), networkClient(fakeServer), o, f, "p1", &buf); err != nil { + t.Fatalf("runRouterList --agent: %v", err) + } + if got := buf.String(); got != "keep\n" { + t.Errorf("--agent output = %q, want only \"keep\"", got) + } +} + func TestRunRouterCreate_Disabled(t *testing.T) { fakeServer := th.SetupHTTP() defer fakeServer.Teardown() @@ -1039,11 +1128,46 @@ func TestRunSecurityGroupList_SendsEveryFilter(t *testing.T) { if err := runSecurityGroupList(context.Background(), networkClient(fakeServer), o, f, "p1", &buf); err != nil { t.Fatalf("runSecurityGroupList: %v", err) } - if got, want := buf.String(), "sg-1\tweb\t\tp1\ta\n"; got != want { + if got, want := buf.String(), "sg-1\tweb\t\tp1\ta\t\n"; got != want { t.Errorf("security group list = %q, want %q", got, want) } } +// --share/--no-share become neutron's `shared` query filter, and the Shared +// column shows the attribute when the security-groups-shared-filtering +// extension returns it. +func TestRunSecurityGroupList_Share(t *testing.T) { + for _, tc := range []struct { + name string + shared bool + query string + }{ + {name: "share", shared: true, query: "true"}, + {name: "no-share", shared: false, query: "false"}, + } { + t.Run(tc.name, func(t *testing.T) { + fakeServer := th.SetupHTTP() + defer fakeServer.Teardown() + + fakeServer.Mux.HandleFunc("/security-groups", func(w http.ResponseWriter, r *http.Request) { + th.TestFormValues(t, r, map[string]string{"shared": tc.query, "name": "web"}) + writeJSON(t, w, http.StatusOK, fmt.Sprintf( + `{"security_groups":[{"id":"sg-1","name":"web","project_id":"p1","shared":%t}]}`, tc.shared)) + }) + + f := &secGroupListFlags{name: "web", shared: &tc.shared} + o := &output.Options{Format: output.FormatValue, Columns: []string{"ID", "Shared"}} + var buf bytes.Buffer + if err := runSecurityGroupList(context.Background(), networkClient(fakeServer), o, f, "", &buf); err != nil { + t.Fatalf("runSecurityGroupList: %v", err) + } + if got, want := buf.String(), fmt.Sprintf("sg-1\t%t\n", tc.shared); got != want { + t.Errorf("output = %q, want %q", got, want) + } + }) + } +} + func TestRunSecurityGroupShow_RequestAndOutput(t *testing.T) { fakeServer := th.SetupHTTP() defer fakeServer.Teardown() diff --git a/internal/cli/network/router.go b/internal/cli/network/router.go index cc1a6a7..f9f5316 100644 --- a/internal/cli/network/router.go +++ b/internal/cli/network/router.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/gophercloud/gophercloud/v2" + "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/agents" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/layer3/routers" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/ports" "github.com/spf13/cobra" @@ -61,6 +62,8 @@ type routerListFlags struct { anyTags []string notTags []string notAnyTags []string + agent string + long bool allProjects bool adminStateUp *bool @@ -103,6 +106,8 @@ func newRouterListCommand(a *auth.Options, o *output.Options) *cobra.Command { fl.StringSliceVar(&f.anyTags, "any-tags", nil, "list only routers with any of these tags (comma-separated)") fl.StringSliceVar(&f.notTags, "not-tags", nil, "exclude routers with all of these tags (comma-separated)") fl.StringSliceVar(&f.notAnyTags, "not-any-tags", nil, "exclude routers with any of these tags (comma-separated)") + fl.StringVar(&f.agent, "agent", "", "list only routers hosted by this L3 agent (ID only)") + fl.BoolVar(&f.long, "long", false, "list additional fields in output") allprojects.Bind(cmd, &f.allProjects, allProjectsNetworkList) cmd.MarkFlagsMutuallyExclusive("project", "all-projects") return cmd @@ -117,6 +122,48 @@ func newRouterListCommand(a *auth.Options, o *output.Options) *cobra.Command { const allProjectsNetworkList = "list across all projects (admin); an admin token already sees them all, " + "so this is accepted for compatibility and changes nothing" +// routerExtAttrs carries the router attributes whose *presence* decides a list +// column. gophercloud's Router models distributed as a plain bool and omits ha +// and availability_zones, but neutron sends distributed/ha only to an admin +// (policy) and availability_zones only with the router_availability_zone +// extension — upstream adds the Distributed/HA/Availability zones columns only +// when the attribute came back, so absence has to stay distinguishable. +type routerExtAttrs struct { + Distributed *bool `json:"distributed"` + HA *bool `json:"ha"` + AvailabilityZones *[]string `json:"availability_zones"` +} + +// routerListRow pairs a router with its routerExtAttrs. The page is decoded +// twice rather than into one struct embedding both, because the two would +// share the `distributed` key and Router's own UnmarshalJSON would swallow the +// rest. +type routerListRow struct { + routers.Router + ext routerExtAttrs +} + +// extractRouterRows decodes one routers body into rows. extract is the +// page/result's slice extractor, called once per target. +func extractRouterRows(extract func(v any) error) ([]routerListRow, error) { + var base []routers.Router + var ext []routerExtAttrs + if err := extract(&base); err != nil { + return nil, err + } + if err := extract(&ext); err != nil { + return nil, err + } + rows := make([]routerListRow, len(base)) + for i := range base { + rows[i].Router = base[i] + if i < len(ext) { + rows[i].ext = ext[i] + } + } + return rows, nil +} + func runRouterList(ctx context.Context, client *gophercloud.ServiceClient, o *output.Options, f *routerListFlags, projectID string, w io.Writer, ) error { @@ -129,19 +176,117 @@ func runRouterList(ctx context.Context, client *gophercloud.ServiceClient, o *ou NotTags: strings.Join(f.notTags, ","), NotTagsAny: strings.Join(f.notAnyTags, ","), } + if f.agent != "" { + // The agent's l3-routers subresource takes no query filters (upstream + // notes the same), so every filter is re-applied client-side. + res := agents.ListL3Routers(ctx, client, f.agent) + hosted, err := extractRouterRows(func(v any) error { return res.ExtractIntoSlicePtr(v, "routers") }) + if err != nil { + return fmt.Errorf("listing routers hosted by agent %s: %w", f.agent, err) + } + all := make([]routerListRow, 0, len(hosted)) + for _, r := range hosted { + if routerMatches(r.Router, opts) { + all = append(all, r) + } + } + return o.WriteList(w, routerListTable(all, f.long)) + } pages, err := routers.List(client, opts).AllPages(ctx) if err != nil { return fmt.Errorf("listing routers: %w", err) } - all, err := routers.ExtractRouters(pages) + all, err := extractRouterRows(func(v any) error { return routers.ExtractRoutersInto(pages, v) }) if err != nil { return fmt.Errorf("parsing router list: %w", err) } - t := output.Table{Columns: []string{"ID", "Name", "Status", "State", "Project"}, Rows: make([][]any, 0, len(all))} + return o.WriteList(w, routerListTable(all, f.long)) +} + +// routerListTable lays out upstream's columns: the five fixed ones, then +// Distributed and HA when any router carried them, then --long's Routes, +// External gateway info, Availability zones (when the extension reported them) +// and Tags. +func routerListTable(all []routerListRow, long bool) output.Table { + var distributed, ha, azs bool + for _, r := range all { + distributed = distributed || r.ext.Distributed != nil + ha = ha || r.ext.HA != nil + azs = azs || r.ext.AvailabilityZones != nil + } + cols := []string{"ID", "Name", "Status", "State", "Project"} + if distributed { + cols = append(cols, "Distributed") + } + if ha { + cols = append(cols, "HA") + } + if long { + cols = append(cols, "Routes", "External gateway info") + if azs { + cols = append(cols, "Availability zones") + } + cols = append(cols, "Tags") + } + t := output.Table{Columns: cols, Rows: make([][]any, 0, len(all))} for _, r := range all { - t.Rows = append(t.Rows, []any{r.ID, r.Name, r.Status, adminState(r.AdminStateUp), r.ProjectID}) + row := []any{r.ID, r.Name, r.Status, adminState(r.AdminStateUp), r.ProjectID} + if distributed { + row = append(row, derefOrNil(r.ext.Distributed)) + } + if ha { + row = append(row, derefOrNil(r.ext.HA)) + } + if long { + row = append(row, r.Routes, r.GatewayInfo) + if azs { + row = append(row, derefOrNil(r.ext.AvailabilityZones)) + } + row = append(row, r.Tags) + } + t.Rows = append(t.Rows, row) + } + return t +} + +// derefOrNil renders an absent optional attribute as an empty cell rather than +// as the type's zero value, which would read as a real false/empty list. +func derefOrNil[T any](p *T) any { + if p == nil { + return nil + } + return *p +} + +// routerMatches applies the router list filters to one router, for the +// --agent path where neutron cannot. Tag semantics follow neutron's: tags = +// all of, tags-any = any of, not-tags = not all of, not-tags-any = none of. +func routerMatches(r routers.Router, opts routers.ListOpts) bool { + switch { + case opts.Name != "" && r.Name != opts.Name, + opts.ProjectID != "" && r.ProjectID != opts.ProjectID, + opts.AdminStateUp != nil && r.AdminStateUp != *opts.AdminStateUp: + return false + } + hasAll := func(csv string) bool { + for _, t := range strings.Split(csv, ",") { + if !slices.Contains(r.Tags, t) { + return false + } + } + return true + } + hasAny := func(csv string) bool { + return slices.ContainsFunc(strings.Split(csv, ","), func(t string) bool { return slices.Contains(r.Tags, t) }) + } + switch { + case opts.Tags != "" && !hasAll(opts.Tags), + opts.TagsAny != "" && !hasAny(opts.TagsAny), + opts.NotTags != "" && hasAll(opts.NotTags), + opts.NotTagsAny != "" && hasAny(opts.NotTagsAny): + return false } - return o.WriteList(w, t) + return true } func newRouterShowCommand(a *auth.Options, o *output.Options) *cobra.Command { diff --git a/internal/cli/network/securitygroup.go b/internal/cli/network/securitygroup.go index deb7a47..5c3b20b 100644 --- a/internal/cli/network/securitygroup.go +++ b/internal/cli/network/securitygroup.go @@ -4,11 +4,13 @@ import ( "context" "fmt" "io" + "strconv" "strings" "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/security/groups" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/security/rules" + "github.com/gophercloud/gophercloud/v2/pagination" "github.com/spf13/cobra" "github.com/ftarasenko/go-openstackclient/internal/auth" @@ -53,7 +55,11 @@ type secGroupListFlags struct { anyTags []string notTags []string notAnyTags []string + share bool + noShare bool allProjects bool + + shared *bool } func newSecurityGroupListCommand(a *auth.Options, o *output.Options) *cobra.Command { @@ -66,6 +72,11 @@ func newSecurityGroupListCommand(a *auth.Options, o *output.Options) *cobra.Comm if err := o.Validate(); err != nil { return err } + fl := cmd.Flags() + if err := mutuallyExclusive(fl, flagShare, flagNoShare); err != nil { + return err + } + f.shared = enableDisable(fl, f.share, f.noShare, flagShare, flagNoShare) ctx := cmd.Context() client, session, err := newNetworkSession(ctx, a) if err != nil { @@ -86,11 +97,31 @@ func newSecurityGroupListCommand(a *auth.Options, o *output.Options) *cobra.Comm fl.StringSliceVar(&f.anyTags, "any-tags", nil, "list only security groups with any of these tags (comma-separated)") fl.StringSliceVar(&f.notTags, "not-tags", nil, "exclude security groups with all of these tags (comma-separated)") fl.StringSliceVar(&f.notAnyTags, "not-any-tags", nil, "exclude security groups with any of these tags (comma-separated)") + fl.BoolVar(&f.share, flagShare, false, "list only security groups shared between projects") + fl.BoolVar(&f.noShare, flagNoShare, false, "list only security groups not shared between projects") allprojects.Bind(cmd, &f.allProjects, allProjectsNetworkList) cmd.MarkFlagsMutuallyExclusive("project", "all-projects") return cmd } +// SecGroupSharedAttr is neutron's `shared` attribute of a security group (the +// security-groups-shared-filtering extension), which gophercloud's SecGroup +// does not model. A pointer, so a cloud without the extension renders an empty +// cell rather than a misleading False. +type SecGroupSharedAttr struct { + Shared *bool `json:"shared"` +} + +// secGroupListRow is a SecGroup plus its shared attribute. Both are anonymous +// embeds so gophercloud's ExtractIntoSlicePtr decodes each separately — +// SecGroup's own UnmarshalJSON would otherwise swallow the extension field. The embed is +// exported because gophercloud reflects into each embedded struct, and +// reflect cannot read an unexported one (it panics). +type secGroupListRow struct { + groups.SecGroup + SecGroupSharedAttr +} + func runSecurityGroupList(ctx context.Context, client *gophercloud.ServiceClient, o *output.Options, f *secGroupListFlags, projectID string, w io.Writer, ) error { @@ -102,21 +133,54 @@ func runSecurityGroupList(ctx context.Context, client *gophercloud.ServiceClient NotTags: strings.Join(f.notTags, ","), NotTagsAny: strings.Join(f.notAnyTags, ","), } - pages, err := groups.List(client, opts).AllPages(ctx) + all, err := listSecGroups(ctx, client, opts, f.shared) if err != nil { - return fmt.Errorf("listing security groups: %w", err) + return err } - all, err := groups.ExtractGroups(pages) - if err != nil { - return fmt.Errorf("parsing security group list: %w", err) + t := output.Table{ + Columns: []string{"ID", "Name", "Description", "Project", "Tags", "Shared"}, + Rows: make([][]any, 0, len(all)), } - t := output.Table{Columns: []string{"ID", "Name", "Description", "Project", "Tags"}, Rows: make([][]any, 0, len(all))} for _, g := range all { - t.Rows = append(t.Rows, []any{g.ID, g.Name, g.Description, g.ProjectID, g.Tags}) + t.Rows = append(t.Rows, []any{g.ID, g.Name, g.Description, g.ProjectID, g.Tags, derefOrNil(g.Shared)}) } return o.WriteList(w, t) } +// listSecGroups is groups.List with a `shared` filter. gophercloud's List takes +// the concrete ListOpts (no builder interface) and has no shared field, and +// there is no ExtractGroupsInto, so the pager is built here over the same URL +// and page type. Replace it once gophercloud models the attribute. +func listSecGroups(ctx context.Context, client *gophercloud.ServiceClient, opts groups.ListOpts, shared *bool) ([]secGroupListRow, error) { + q, err := gophercloud.BuildQueryString(&opts) + if err != nil { + return nil, fmt.Errorf("building security group query: %w", err) + } + params := q.Query() + if shared != nil { + params.Set("shared", strconv.FormatBool(*shared)) + } + u := client.ServiceURL("security-groups") + if len(params) > 0 { + u += "?" + params.Encode() + } + pages, err := pagination.NewPager(client, u, func(r pagination.PageResult) pagination.Page { + return groups.SecGroupPage{LinkedPageBase: pagination.LinkedPageBase{PageResult: r}} + }).AllPages(ctx) + if err != nil { + return nil, fmt.Errorf("listing security groups: %w", err) + } + page, ok := pages.(groups.SecGroupPage) + if !ok { + return nil, fmt.Errorf("parsing security group list: unexpected page type %T", pages) + } + var all []secGroupListRow + if err := page.ExtractIntoSlicePtr(&all, "security_groups"); err != nil { + return nil, fmt.Errorf("parsing security group list: %w", err) + } + return all, nil +} + func newSecurityGroupShowCommand(a *auth.Options, o *output.Options) *cobra.Command { cmd := &cobra.Command{ Use: "show ",