fix(unifi): address CodeRabbit review feedback

- Reject conflicting --wired and --wireless flags in clients command
- Complete --type flag help text with bgp and ospf route types
- URL-escape site name in routes API path
- Wrap all command errors with log.E for contextual diagnostics
- Set TLS MinVersion to 1.2 on UniFi client
- Simplify redundant fmt.Sprintf in Print calls

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Snider 2026-02-05 01:34:13 +00:00
parent e1923095e1
commit 57057e64d1
7 changed files with 31 additions and 20 deletions

View file

@ -1,9 +1,11 @@
package unifi package unifi
import ( import (
"errors"
"fmt" "fmt"
"github.com/host-uk/core/pkg/cli" "github.com/host-uk/core/pkg/cli"
"github.com/host-uk/core/pkg/log"
uf "github.com/host-uk/core/pkg/unifi" uf "github.com/host-uk/core/pkg/unifi"
) )
@ -33,9 +35,13 @@ func addClientsCommand(parent *cli.Command) {
} }
func runClients() error { func runClients() error {
if clientsWired && clientsWireless {
return log.E("unifi.clients", "conflicting flags", errors.New("--wired and --wireless cannot both be set"))
}
client, err := uf.NewFromConfig("", "", "", "") client, err := uf.NewFromConfig("", "", "", "")
if err != nil { if err != nil {
return err return log.E("unifi.clients", "failed to initialise client", err)
} }
clients, err := client.GetClients(uf.ClientFilter{ clients, err := client.GetClients(uf.ClientFilter{
@ -44,7 +50,7 @@ func runClients() error {
Wireless: clientsWireless, Wireless: clientsWireless,
}) })
if err != nil { if err != nil {
return err return log.E("unifi.clients", "failed to fetch clients", err)
} }
if len(clients) == 0 { if len(clients) == 0 {
@ -79,7 +85,7 @@ func runClients() error {
} }
cli.Blank() cli.Blank()
cli.Print(" %s\n\n", fmt.Sprintf("%d clients", len(clients))) cli.Print(" %d clients\n\n", len(clients))
table.Render() table.Render()
return nil return nil

View file

@ -1,10 +1,10 @@
package unifi package unifi
import ( import (
"fmt"
"strings" "strings"
"github.com/host-uk/core/pkg/cli" "github.com/host-uk/core/pkg/cli"
"github.com/host-uk/core/pkg/log"
uf "github.com/host-uk/core/pkg/unifi" uf "github.com/host-uk/core/pkg/unifi"
) )
@ -34,12 +34,12 @@ func addDevicesCommand(parent *cli.Command) {
func runDevices() error { func runDevices() error {
client, err := uf.NewFromConfig("", "", "", "") client, err := uf.NewFromConfig("", "", "", "")
if err != nil { if err != nil {
return err return log.E("unifi.devices", "failed to initialise client", err)
} }
devices, err := client.GetDeviceList(devicesSite, strings.ToLower(devicesType)) devices, err := client.GetDeviceList(devicesSite, strings.ToLower(devicesType))
if err != nil { if err != nil {
return err return log.E("unifi.devices", "failed to fetch devices", err)
} }
if len(devices) == 0 { if len(devices) == 0 {
@ -67,7 +67,7 @@ func runDevices() error {
} }
cli.Blank() cli.Blank()
cli.Print(" %s\n\n", fmt.Sprintf("%d devices", len(devices))) cli.Print(" %d devices\n\n", len(devices))
table.Render() table.Render()
return nil return nil

View file

@ -4,6 +4,7 @@ import (
"fmt" "fmt"
"github.com/host-uk/core/pkg/cli" "github.com/host-uk/core/pkg/cli"
"github.com/host-uk/core/pkg/log"
uf "github.com/host-uk/core/pkg/unifi" uf "github.com/host-uk/core/pkg/unifi"
) )
@ -31,12 +32,12 @@ func addNetworksCommand(parent *cli.Command) {
func runNetworks() error { func runNetworks() error {
client, err := uf.NewFromConfig("", "", "", "") client, err := uf.NewFromConfig("", "", "", "")
if err != nil { if err != nil {
return err return log.E("unifi.networks", "failed to initialise client", err)
} }
networks, err := client.GetNetworks(networksSite) networks, err := client.GetNetworks(networksSite)
if err != nil { if err != nil {
return err return log.E("unifi.networks", "failed to fetch networks", err)
} }
if len(networks) == 0 { if len(networks) == 0 {

View file

@ -4,6 +4,7 @@ import (
"fmt" "fmt"
"github.com/host-uk/core/pkg/cli" "github.com/host-uk/core/pkg/cli"
"github.com/host-uk/core/pkg/log"
uf "github.com/host-uk/core/pkg/unifi" uf "github.com/host-uk/core/pkg/unifi"
) )
@ -25,7 +26,7 @@ func addRoutesCommand(parent *cli.Command) {
} }
cmd.Flags().StringVar(&routesSite, "site", "", "Site name (default: \"default\")") cmd.Flags().StringVar(&routesSite, "site", "", "Site name (default: \"default\")")
cmd.Flags().StringVar(&routesType, "type", "", "Filter by route type (static, connected, kernel)") cmd.Flags().StringVar(&routesType, "type", "", "Filter by route type (static, connected, kernel, bgp, ospf)")
parent.AddCommand(cmd) parent.AddCommand(cmd)
} }
@ -33,12 +34,12 @@ func addRoutesCommand(parent *cli.Command) {
func runRoutes() error { func runRoutes() error {
client, err := uf.NewFromConfig("", "", "", "") client, err := uf.NewFromConfig("", "", "", "")
if err != nil { if err != nil {
return err return log.E("unifi.routes", "failed to initialise client", err)
} }
routes, err := client.GetRoutes(routesSite) routes, err := client.GetRoutes(routesSite)
if err != nil { if err != nil {
return err return log.E("unifi.routes", "failed to fetch routes", err)
} }
// Filter by type if requested // Filter by type if requested
@ -78,7 +79,7 @@ func runRoutes() error {
} }
cli.Blank() cli.Blank()
cli.Print(" %s\n\n", fmt.Sprintf("%d routes", len(routes))) cli.Print(" %d routes\n\n", len(routes))
table.Render() table.Render()
return nil return nil

View file

@ -1,9 +1,8 @@
package unifi package unifi
import ( import (
"fmt"
"github.com/host-uk/core/pkg/cli" "github.com/host-uk/core/pkg/cli"
"github.com/host-uk/core/pkg/log"
uf "github.com/host-uk/core/pkg/unifi" uf "github.com/host-uk/core/pkg/unifi"
) )
@ -24,12 +23,12 @@ func addSitesCommand(parent *cli.Command) {
func runSites() error { func runSites() error {
client, err := uf.NewFromConfig("", "", "", "") client, err := uf.NewFromConfig("", "", "", "")
if err != nil { if err != nil {
return err return log.E("unifi.sites", "failed to initialise client", err)
} }
sites, err := client.GetSites() sites, err := client.GetSites()
if err != nil { if err != nil {
return err return log.E("unifi.sites", "failed to fetch sites", err)
} }
if len(sites) == 0 { if len(sites) == 0 {
@ -47,7 +46,7 @@ func runSites() error {
} }
cli.Blank() cli.Blank()
cli.Print(" %s\n\n", fmt.Sprintf("%d sites", len(sites))) cli.Print(" %d sites\n\n", len(sites))
table.Render() table.Render()
return nil return nil

View file

@ -28,7 +28,10 @@ func New(url, user, pass, apikey string) (*Client, error) {
// Skip TLS verification for self-signed certs // Skip TLS verification for self-signed certs
httpClient := &http.Client{ httpClient := &http.Client{
Transport: &http.Transport{ Transport: &http.Transport{
TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec TLSClientConfig: &tls.Config{
InsecureSkipVerify: true, //nolint:gosec
MinVersion: tls.VersionTLS12,
},
}, },
} }

View file

@ -3,6 +3,7 @@ package unifi
import ( import (
"encoding/json" "encoding/json"
"fmt" "fmt"
"net/url"
"github.com/host-uk/core/pkg/log" "github.com/host-uk/core/pkg/log"
) )
@ -31,7 +32,7 @@ func (c *Client) GetRoutes(siteName string) ([]Route, error) {
siteName = "default" siteName = "default"
} }
path := fmt.Sprintf("/api/s/%s/stat/routing", siteName) path := fmt.Sprintf("/api/s/%s/stat/routing", url.PathEscape(siteName))
raw, err := c.api.GetJSON(path) raw, err := c.api.GetJSON(path)
if err != nil { if err != nil {