detached 35b3ba9

fix: address code review findings across CLI and docs

- Add nil guard in runConfigShow and empty-state for external routes
- Normalize nil tags to empty slice in images JSON output
- Distinguish NotFound from operational errors in routes show
- Use json:"-" for network DTO ID/Labels to prevent serialization
- Emit snake_case keys in networks JSON output for consistency
- Vary permission bullet lead text in images docs
commit
35b3ba99099cbbbcdee7d116bb7d78394e7333e0
author
brice · · 2026-03-14 04:02 UTC
parent
658b8a7
signature
unsigned
7 files · +59 -36
Unified
UnifiedSplit
M docs/cli/images.md +3 -3
@@ -137,9 +137,9 @@ gordon images prune --no-confirm
137137
138138 ## Required Permissions
139139
140-- `list` requires `admin:status:read`.
141-- `prune` requires `admin:config:write`.
142-- `tags` requires `admin:status:read`.
140+- The `list` subcommand requires `admin:status:read`.
141+- The `prune` subcommand requires `admin:config:write`.
142+- The `tags` subcommand requires `admin:status:read`.
143143
144144 ## Related
145145
@@ -137,9 +137,9 @@ gordon images prune --no-confirm
137 137
138 ## Required Permissions138 ## Required Permissions
139 139
140-- `list` requires `admin:status:read`.140+- The `list` subcommand requires `admin:status:read`.
141-- `prune` requires `admin:config:write`.141+- The `prune` subcommand requires `admin:config:write`.
142-- `tags` requires `admin:status:read`.142+- The `tags` subcommand requires `admin:status:read`.
143 143
144 ## Related144 ## Related
145 145
M docs/cli/networks.md +3 -3
@@ -37,9 +37,9 @@ gordon networks list --remote https://gordon.mydomain.com --token $TOKEN
3737 ```json
3838 [
3939 {
40- "Name": "gordon_myapp",
41- "Driver": "bridge",
42- "Containers": ["container1", "container2"]
40+ "name": "gordon_myapp",
41+ "driver": "bridge",
42+ "containers": ["container1", "container2"]
4343 }
4444 ]
4545 ```
@@ -37,9 +37,9 @@ gordon networks list --remote https://gordon.mydomain.com --token $TOKEN
37 ```json37 ```json
38 [38 [
39 {39 {
40- "Name": "gordon_myapp",40+ "name": "gordon_myapp",
41- "Driver": "bridge",41+ "driver": "bridge",
42- "Containers": ["container1", "container2"]42+ "containers": ["container1", "container2"]
43 }43 }
44 ]44 ]
45 ```45 ```
M internal/adapters/dto/admin_networks.go +2 -2
@@ -2,11 +2,11 @@ package dto
22
33 // Network represents a network in API responses.
44 type Network struct {
5- ID string `json:"id,omitempty"`
5+ ID string `json:"-"`
66 Name string `json:"name"`
77 Driver string `json:"driver"`
88 Containers []string `json:"containers"`
9- Labels map[string]string `json:"labels,omitempty"`
9+ Labels map[string]string `json:"-"`
1010 }
1111
1212 // NetworksResponse represents a list of networks.
@@ -2,11 +2,11 @@ package dto
2 2
3 // Network represents a network in API responses.3 // Network represents a network in API responses.
4 type Network struct {4 type Network struct {
5- ID string `json:"id,omitempty"`5+ ID string `json:"-"`
6 Name string `json:"name"`6 Name string `json:"name"`
7 Driver string `json:"driver"`7 Driver string `json:"driver"`
8 Containers []string `json:"containers"`8 Containers []string `json:"containers"`
9- Labels map[string]string `json:"labels,omitempty"`9+ Labels map[string]string `json:"-"`
10 }10 }
11 11
12 // NetworksResponse represents a list of networks.12 // NetworksResponse represents a list of networks.
M internal/adapters/in/cli/config.go +29 -24
@@ -57,6 +57,10 @@ func runConfigShow(ctx context.Context, cp ControlPlane, out io.Writer, jsonOut
5757 return fmt.Errorf("failed to get config: %w", err)
5858 }
5959
60+ if config == nil {
61+ return fmt.Errorf("no config available")
62+ }
63+
6064 if jsonOut {
6165 return writeJSON(out, config)
6266 }
@@ -138,34 +142,35 @@ func renderConfigRoutes(out io.Writer, config *remote.Config) error {
138142 }
139143
140144 func renderConfigExternalRoutes(out io.Writer, config *remote.Config) error {
141- if len(config.ExternalRoutes) > 0 {
142- if err := cliWriteLine(out, ""); err != nil {
143- return err
144- }
145+ if err := cliWriteLine(out, ""); err != nil {
146+ return err
147+ }
148+ if len(config.ExternalRoutes) == 0 {
145149 if err := cliWriteLine(out, cliRenderTitle("External Routes")); err != nil {
146150 return err
147151 }
148- domains := make([]string, 0, len(config.ExternalRoutes))
149- for d := range config.ExternalRoutes {
150- domains = append(domains, d)
151- }
152- sort.Strings(domains)
152+ return cliWriteLine(out, cliRenderMuted("No external routes configured"))
153+ }
153154
154- rows := make([][]string, 0, len(domains))
155- for _, d := range domains {
156- rows = append(rows, []string{d, config.ExternalRoutes[d]})
157- }
158- table := components.NewTable(
159- components.WithColumns([]components.TableColumn{
160- {Title: "Domain", Width: 30},
161- {Title: "Target", Width: 45},
162- }),
163- components.WithRows(rows),
164- )
165- if err := cliWriteLine(out, table.View()); err != nil {
166- return err
167- }
155+ if err := cliWriteLine(out, cliRenderTitle("External Routes")); err != nil {
156+ return err
157+ }
158+ domains := make([]string, 0, len(config.ExternalRoutes))
159+ for d := range config.ExternalRoutes {
160+ domains = append(domains, d)
168161 }
162+ sort.Strings(domains)
169163
170- return nil
164+ rows := make([][]string, 0, len(domains))
165+ for _, d := range domains {
166+ rows = append(rows, []string{d, config.ExternalRoutes[d]})
167+ }
168+ table := components.NewTable(
169+ components.WithColumns([]components.TableColumn{
170+ {Title: "Domain", Width: 30},
171+ {Title: "Target", Width: 45},
172+ }),
173+ components.WithRows(rows),
174+ )
175+ return cliWriteLine(out, table.View())
171176 }
@@ -57,6 +57,10 @@ func runConfigShow(ctx context.Context, cp ControlPlane, out io.Writer, jsonOut
57 return fmt.Errorf("failed to get config: %w", err)57 return fmt.Errorf("failed to get config: %w", err)
58 }58 }
59 59
60+ if config == nil {
61+ return fmt.Errorf("no config available")
62+ }
63+
60 if jsonOut {64 if jsonOut {
61 return writeJSON(out, config)65 return writeJSON(out, config)
62 }66 }
@@ -138,34 +142,35 @@ func renderConfigRoutes(out io.Writer, config *remote.Config) error {
138 }142 }
139 143
140 func renderConfigExternalRoutes(out io.Writer, config *remote.Config) error {144 func renderConfigExternalRoutes(out io.Writer, config *remote.Config) error {
141- if len(config.ExternalRoutes) > 0 {145+ if err := cliWriteLine(out, ""); err != nil {
142- if err := cliWriteLine(out, ""); err != nil {146+ return err
143- return err147+ }
144- }148+ if len(config.ExternalRoutes) == 0 {
145 if err := cliWriteLine(out, cliRenderTitle("External Routes")); err != nil {149 if err := cliWriteLine(out, cliRenderTitle("External Routes")); err != nil {
146 return err150 return err
147 }151 }
148- domains := make([]string, 0, len(config.ExternalRoutes))152+ return cliWriteLine(out, cliRenderMuted("No external routes configured"))
149- for d := range config.ExternalRoutes {153+ }
150- domains = append(domains, d)
151- }
152- sort.Strings(domains)
153 154
154- rows := make([][]string, 0, len(domains))155+ if err := cliWriteLine(out, cliRenderTitle("External Routes")); err != nil {
155- for _, d := range domains {156+ return err
156- rows = append(rows, []string{d, config.ExternalRoutes[d]})157+ }
157- }158+ domains := make([]string, 0, len(config.ExternalRoutes))
158- table := components.NewTable(159+ for d := range config.ExternalRoutes {
159- components.WithColumns([]components.TableColumn{160+ domains = append(domains, d)
160- {Title: "Domain", Width: 30},
161- {Title: "Target", Width: 45},
162- }),
163- components.WithRows(rows),
164- )
165- if err := cliWriteLine(out, table.View()); err != nil {
166- return err
167- }
168 }161 }
162+ sort.Strings(domains)
169 163
170- return nil164+ rows := make([][]string, 0, len(domains))
165+ for _, d := range domains {
166+ rows = append(rows, []string{d, config.ExternalRoutes[d]})
167+ }
168+ table := components.NewTable(
169+ components.WithColumns([]components.TableColumn{
170+ {Title: "Domain", Width: 30},
171+ {Title: "Target", Width: 45},
172+ }),
173+ components.WithRows(rows),
174+ )
175+ return cliWriteLine(out, table.View())
171 }176 }
M internal/adapters/in/cli/images.go +3 -0
@@ -168,6 +168,9 @@ func runImagesTags(ctx context.Context, cp ControlPlane, out io.Writer, reposito
168168 }
169169
170170 if jsonOut {
171+ if tags == nil {
172+ tags = []string{}
173+ }
171174 payload := map[string]any{
172175 "repository": repository,
173176 "tags": tags,
@@ -168,6 +168,9 @@ func runImagesTags(ctx context.Context, cp ControlPlane, out io.Writer, reposito
168 }168 }
169 169
170 if jsonOut {170 if jsonOut {
171+ if tags == nil {
172+ tags = []string{}
173+ }
171 payload := map[string]any{174 payload := map[string]any{
172 "repository": repository,175 "repository": repository,
173 "tags": tags,176 "tags": tags,
M internal/adapters/in/cli/networks.go +12 -1
@@ -58,7 +58,18 @@ func runNetworksList(ctx context.Context, cp ControlPlane, out io.Writer, jsonOu
5858 }
5959
6060 if jsonOut {
61- return writeJSON(out, networks)
61+ result := make([]map[string]any, 0, len(networks))
62+ for _, net := range networks {
63+ if net == nil {
64+ continue
65+ }
66+ result = append(result, map[string]any{
67+ "name": net.Name,
68+ "driver": net.Driver,
69+ "containers": net.Containers,
70+ })
71+ }
72+ return writeJSON(out, result)
6273 }
6374
6475 if len(networks) == 0 {
@@ -58,7 +58,18 @@ func runNetworksList(ctx context.Context, cp ControlPlane, out io.Writer, jsonOu
58 }58 }
59 59
60 if jsonOut {60 if jsonOut {
61- return writeJSON(out, networks)61+ result := make([]map[string]any, 0, len(networks))
62+ for _, net := range networks {
63+ if net == nil {
64+ continue
65+ }
66+ result = append(result, map[string]any{
67+ "name": net.Name,
68+ "driver": net.Driver,
69+ "containers": net.Containers,
70+ })
71+ }
72+ return writeJSON(out, result)
62 }73 }
63 74
64 if len(networks) == 0 {75 if len(networks) == 0 {
M internal/adapters/in/cli/routes.go +7 -3
@@ -2,16 +2,17 @@ package cli
22
33 import (
44 "context"
5+ "errors"
56 "fmt"
67 "io"
78 "strings"
89
10+ "github.com/spf13/cobra"
11+
912 "github.com/bnema/gordon/internal/adapters/in/cli/remote"
1013 "github.com/bnema/gordon/internal/adapters/in/cli/ui/components"
1114 "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles"
1215 "github.com/bnema/gordon/internal/domain"
13-
14- "github.com/spf13/cobra"
1516 )
1617
1718 // configPath for local operations. If empty, config is auto-discovered
@@ -167,7 +168,10 @@ Examples:
167168 func runRoutesShow(ctx context.Context, cp ControlPlane, out io.Writer, routeDomain string, jsonOut bool) error {
168169 route, err := cp.GetRoute(ctx, routeDomain)
169170 if err != nil {
170- return fmt.Errorf("route not found: %w", err)
171+ if errors.Is(err, domain.ErrRouteNotFound) {
172+ return fmt.Errorf("route %q not found", routeDomain)
173+ }
174+ return fmt.Errorf("failed to get route: %w", err)
171175 }
172176
173177 health, _ := cp.GetHealth(ctx)
@@ -2,16 +2,17 @@ package cli
2 2
3 import (3 import (
4 "context"4 "context"
5+ "errors"
5 "fmt"6 "fmt"
6 "io"7 "io"
7 "strings"8 "strings"
8 9
10+ "github.com/spf13/cobra"
11+
9 "github.com/bnema/gordon/internal/adapters/in/cli/remote"12 "github.com/bnema/gordon/internal/adapters/in/cli/remote"
10 "github.com/bnema/gordon/internal/adapters/in/cli/ui/components"13 "github.com/bnema/gordon/internal/adapters/in/cli/ui/components"
11 "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles"14 "github.com/bnema/gordon/internal/adapters/in/cli/ui/styles"
12 "github.com/bnema/gordon/internal/domain"15 "github.com/bnema/gordon/internal/domain"
13-
14- "github.com/spf13/cobra"
15 )16 )
16 17
17 // configPath for local operations. If empty, config is auto-discovered18 // configPath for local operations. If empty, config is auto-discovered
@@ -167,7 +168,10 @@ Examples:
167 func runRoutesShow(ctx context.Context, cp ControlPlane, out io.Writer, routeDomain string, jsonOut bool) error {168 func runRoutesShow(ctx context.Context, cp ControlPlane, out io.Writer, routeDomain string, jsonOut bool) error {
168 route, err := cp.GetRoute(ctx, routeDomain)169 route, err := cp.GetRoute(ctx, routeDomain)
169 if err != nil {170 if err != nil {
170- return fmt.Errorf("route not found: %w", err)171+ if errors.Is(err, domain.ErrRouteNotFound) {
172+ return fmt.Errorf("route %q not found", routeDomain)
173+ }
174+ return fmt.Errorf("failed to get route: %w", err)
171 }175 }
172 176
173 health, _ := cp.GetHealth(ctx)177 health, _ := cp.GetHealth(ctx)

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 7

Tags 106

↑↓ moveenter openesc close