diff --git a/docs/plan-protect-only-support.md b/docs/plan-protect-only-support.md new file mode 100644 index 00000000..22d2b6a7 --- /dev/null +++ b/docs/plan-protect-only-support.md @@ -0,0 +1,124 @@ +# Plan: Protect-only console support (issue #1066) + +v5 added UniFi Protect device metrics, but they cannot be collected from a **Protect-only +console** — a UNVR or UNVR Pro, which runs UniFi Protect with no Network application +installed. The Protect collectors are complete; only controller initialisation blocks them. + +## Design decision: a `disable_network` flag, not a separate input plugin + +UNAS Pro got its own plugin (`pkg/inputunas`, see [plan-unas-support.md](plan-unas-support.md)) +because it shares nothing with a UniFi controller: its own credentials, its own host, its own +JSON API. A Protect-only console is the opposite case. It is reached at the same URL, with the +same `Controller` config, by the same `*unifi.Unifi` client, and `collectProtect` / +`collectProtectLogs` already live in `inputunifi` and are already not site-scoped. A second +plugin would duplicate the controller config block to gate two calls it already makes. + +So: a per-controller `disable_network` flag, defaulting to `false`, as +[@platinummonkey asked for on the issue](https://github.com/unpoller/unpoller/issues/1066). + +Detection is by explicit flag only. The issue documents two unauthenticated probes that +identify a UNVR (`/api/system` reporting `hardware.shortname`, and `/proxy/network/status` +answering HTML rather than a 401). Auto-detection was cut: it adds network calls and cached +state to every startup, and the flag has to exist as an override regardless. + +### The blockers + +1. **`unifi.NewUnifi()` cannot construct a client for the console at all.** It ends in + `GetServerData()` → GET `APIStatusPath` (`/status`), which `path()` rewrites to + `/proxy/network/status`. With no Network application to route to, UniFi OS serves its own + SPA HTML and the call fails on its first byte: + `invalid character '<' looking for beginning of value`. `getUnifi()` treats any non-429 + error as fatal, so the entry never even prints a config summary. +2. **`pollController` aborts on `getFilteredSites`** long before it reaches `collectProtect`. + `collectControllerEvents` aborts in the same place, before `collectProtectLogs`. +3. **`Metrics()` counts a poll successful only if it produced devices or clients.** A + Protect-only console produces neither, so a filtered scrape of one — the Prometheus + per-target path — falls through to the dynamic-controller branch and reports + `ErrDynamicLookupsDisabled` despite a successful collection. + +What already works in our favour: `path()` passes anything starting with `/proxy/` through +untouched, so the Protect Integration paths need no changes; `collectProtect` and +`collectProtectLogs` take no `sites` argument; and every output plugin already handles +`ProtectDevices`. + +## Part 1 — `../unifi` (github.com/unpoller/unifi/v6) + +`NewProtectClient(config *Config) (*Unifi, error)` in `protect.go`, modelled on +`NewUNASClient` in `unas.go`, which exists for the same underlying reason. + +- Rejects a nil config, and a config with neither `ProtectAPIKey` nor `APIKey`: + Integration/v1 is `X-API-Key` only and has no cookie fallback. +- Sets `u.new = true` directly rather than probing with `checkNewStyleAPI()`. A Protect + console is always a UniFi OS console, so `Login()` resolves to `/api/auth/login` without + depending on how the console answers a GET of `/`. +- Logs in **only** when `APIKey == "" && User != ""`. Integration/v1 needs no session, but the + legacy Protect endpoints (`GetProtectLogs`, `GetProtectEventThumbnail`) authenticate with a + session cookie. Skipping login when `APIKey` is set is required for correctness: `Login()` + routes through `/status` in that case, the one endpoint this console cannot serve. +- Ends by probing `/v1/meta/info`, the way `NewUnifi` ends by probing Network, so a caller + that gets no error has a console it can really poll. +- Returns a plain `*Unifi` rather than a wrapper type, unlike `NewUNASClient`: the Protect + getters are already `*Unifi` methods. + +**`ServerStatus` must be populated, and that is not cosmetic.** `Unifi` embeds +`*ServerStatus`, so leaving it nil turns any caller's `u.ServerVersion` into a nil +dereference — including `inputunifi`'s config summary. `ServerVersion` holds the *Protect* +application version, since a Protect-only console has no Network version to report. + +## Part 2 — `unpoller` + +### 2a. Config + +`Controller.DisableNetwork *bool`, tagged for json/toml/xml/yaml, defaulting to `false` in +`setDefaults` and inheritable from `[unifi.defaults]` via `setControllerDefaults`, matching +every neighbouring flag. Added to `formatControllers` so the web UI reflects it. + +### 2b. Skipping the Network application + +| Site | Change | +|---|---| +| `getUnifi` | Calls `unifi.NewProtectClient` instead of `unifi.NewUnifi`, inside the unchanged 429-retry loop. | +| `Initialize`, `DebugInput` | Skip `checkSites`. | +| `pollController` | The Network pass is extracted into a new `pollNetwork(c, sites, m) error` and skipped wholesale, leaving site discovery and `collectProtect` behind. | +| `collectControllerEvents` | Skips site discovery and reduces the collector list to `collectProtectLogs`, the only site-independent one. | +| `Metrics` | Counts `ProtectDevices` toward a successful poll. | +| `RawMetrics` | Answers the raw-path kind and rejects the site-scoped kinds with `ErrNetworkDisabled`, rather than returning a confusing empty result. | +| `logController` | Marks the mode and omits the Network-only lines. | + +`extractDevices` also gains a nil guard on `metrics.Devices`, which it dereferenced +unconditionally — a latent panic in its own right, now reachable whenever the Network pass +does not run. + +### 2c. Warnings, not failures + +`warnProtectOnly` logs an error at startup for the two configurations that can never collect +anything: `disable_network` with neither Protect save flag, and `save_protect_devices` with no +`protect_api_key` or `api_key`. Neither is fatal — silently collecting nothing is the failure +mode hardest to diagnose from a log, so it is called out rather than acted on. + +### 2d. Docs and tests + +`pkg/inputunifi/README.md` gains a "Protect-only consoles (UNVR)" section; the three +`examples/up.*.example` files gain `disable_network`, defaulting off. + +`pkg/inputunifi` had no tests before this change. The new `input_test.go` follows +`pkg/inputunas/input_test.go`: external test package, an `httptest` fake UNVR that serves the +console's SPA HTML for everything but the Protect paths and the login, and a prose comment +above each test. It covers initialisation, metrics, events, the filtered scrape, `RawMetrics`, +both warnings, config binding across toml/json/yaml/env, and that the shipped examples do not +disable the Network application. `TestProtectOnlyControllerFailsWithoutFlag` pins the original +bug against the same fake console. + +## Sequencing across the two repos + +`unpoller` consumes `unpoller/unifi` as a tagged module, so `NewProtectClient` has to exist +and be released before anything can call it. Part 1 lands and is tagged first; Part 2 stays a +draft until then, developed against a local `go.work` workspace so `go.mod` is never +temporarily rewritten. + +## Open items + +- Auto-detection of Protect-only consoles, if operators find the flag a papercut. +- The Protect bootstrap API (`/proxy/protect/api/bootstrap`) carries richer per-camera data — + `isRecording`, `isConnected`, NVR `version` — but it is private and undocumented, so it is + out of scope here as it was in #1015. diff --git a/examples/up.conf.example b/examples/up.conf.example index ea30a176..2c42923b 100644 --- a/examples/up.conf.example +++ b/examples/up.conf.example @@ -233,6 +233,12 @@ # this controller. save_protect_devices = false + # Set this on a Protect-only console -- a UNVR or UNVR Pro, which runs UniFi Protect + # with no Network application installed. UnPoller then skips the Network API entirely: + # no sites, clients, devices, DPI, events, alarms or IDs are collected, only Protect. + # Leave it false for every normal controller, including a UDM or UCG that runs both. + disable_network = false + # Enable collection of Deep Packet Inspection data. This data breaks down traffic # types for each client and site, it powers a dedicated DPI dashboard. # Enabling this adds roughly 150 data points per client. That's 6000 metrics for @@ -284,6 +290,7 @@ # protect_thumbnails = false # save_protect_devices = false # protect_api_key = "unifiprotectapikey" +# disable_network = false # save_dpi = false # save_traffic = false # save_rogue = false diff --git a/examples/up.json.example b/examples/up.json.example index 24c7280d..07f46bd1 100644 --- a/examples/up.json.example +++ b/examples/up.json.example @@ -66,6 +66,7 @@ "protect_thumbnails": false, "save_protect_devices": false, "protect_api_key": "unifiprotectapikey", + "disable_network": false, "save_dpi": false, "save_sites": true, "hash_pii": false, @@ -86,6 +87,7 @@ "protect_thumbnails": false, "save_protect_devices": false, "protect_api_key": "unifiprotectapikey", + "disable_network": false, "save_dpi": false, "save_sites": true, "hash_pii": false, diff --git a/examples/up.yaml.example b/examples/up.yaml.example index a1ae3737..e67636ca 100644 --- a/examples/up.yaml.example +++ b/examples/up.yaml.example @@ -68,6 +68,8 @@ unifi: protect_thumbnails: false save_protect_devices: false # protect_api_key: "unifiprotectapikey" + # Set true only on a Protect-only console (UNVR): skips the Network API entirely. + disable_network: false save_dpi: false save_sites: true hash_pii: false @@ -90,6 +92,8 @@ unifi: protect_thumbnails: false save_protect_devices: false # protect_api_key: "unifiprotectapikey" + # Set true only on a Protect-only console (UNVR): skips the Network API entirely. + disable_network: false save_dpi: false save_sites: true hash_pii: false diff --git a/pkg/inputunifi/README.md b/pkg/inputunifi/README.md index 10a6daae..bf0f3f9d 100644 --- a/pkg/inputunifi/README.md +++ b/pkg/inputunifi/README.md @@ -1,3 +1,61 @@ # inputunifi ## UnPoller Input Plugin + +Polls UniFi controllers and hands their metrics and events to every configured output. +All configuration lives under `[unifi]` — see the commented `[[unifi.controller]]` block in +[`examples/up.conf.example`](../../examples/up.conf.example) for every available option. + +## Protect-only consoles (UNVR) + +A **UNVR** or **UNVR Pro** runs UniFi Protect with *no Network application installed*. +UnPoller's normal startup probes the Network API to read the controller version, which on +these appliances returns the UniFi OS SPA HTML rather than JSON: + +``` +[ERROR] Controller 3 of 3 Auth or Connection Error, retrying: unifi controller: + unable to get server version: invalid character '<' looking for beginning of value +``` + +Set `disable_network = true` on that controller. UnPoller then skips the Network API +entirely and collects only UniFi Protect: + +```toml +[[unifi.controller]] + url = "https://unvr.example.com" + # Optional: a local read-only account. Only needed for save_protect_logs, which uses the + # legacy Protect endpoints and authenticates with a session cookie. + user = "unpoller" + pass = "unpoller" + # Required. Mint this in Protect under Settings -> Control Plane -> Integrations. + protect_api_key = "unifiprotectapikey" + + disable_network = true + save_protect_devices = true + save_protect_logs = false + verify_ssl = false +``` + +As an environment variable this is `UP_UNIFI_CONTROLLER_0_DISABLE_NETWORK=true`. + +### What is and isn't collected + +| | With `disable_network = true` | +| --- | --- | +| Protect devices — cameras, sensors, lights, bridges, link stations, NVR | ✅ `save_protect_devices` | +| Protect event logs | ✅ `save_protect_logs` | +| Sites, clients, devices, DPI, traffic, rogue APs, speed tests | ❌ never polled | +| Events, syslog, alarms, anomalies, IDs | ❌ never polled | + +The Network-only `save_*` options are ignored rather than honoured, so leaving them at their +defaults is fine. UnPoller logs an error at startup if `disable_network` is set with neither +`save_protect_devices` nor `save_protect_logs` — that combination collects nothing at all. + +A console that runs *both* applications (a UDM, UCG, or a UniFi OS Server with Protect +installed) should leave `disable_network` at its default of `false` and simply set +`save_protect_devices = true`. This flag is only for consoles with no Network application. + +Mixing is fine: a Protect-only console is configured as one more `[[unifi.controller]]` +alongside your normal ones. + +See [unpoller/unpoller#1066](https://github.com/unpoller/unpoller/issues/1066). diff --git a/pkg/inputunifi/collectevents.go b/pkg/inputunifi/collectevents.go index fa7f11d3..a62b9a15 100644 --- a/pkg/inputunifi/collectevents.go +++ b/pkg/inputunifi/collectevents.go @@ -24,20 +24,29 @@ func (u *InputUnifi) collectControllerEvents(c *Controller) ([]any, error) { } } + type caller func([]any, []*unifi.Site, *Controller) ([]any, error) + var ( logs = []any{} newLogs []any + sites []*unifi.Site + err error + calls = []caller{u.collectIDs, u.collectAnomalies, u.collectAlarms, u.collectEvents, u.collectSyslog, u.collectProtectLogs} ) - // Get the sites we care about. - sites, err := u.getFilteredSites(c) - if err != nil { - return nil, fmt.Errorf("unifi.GetSites(): %w", err) + // A Protect-only console (UNVR) has no sites and no Network application, so every + // site-scoped collector below would fail. collectProtectLogs is the only one that is not + // site-scoped -- it already ignores the argument. See unpoller/unpoller#1066. + if *c.DisableNetwork { + calls = []caller{u.collectProtectLogs} + } else { + // Get the sites we care about. + if sites, err = u.getFilteredSites(c); err != nil { + return nil, fmt.Errorf("unifi.GetSites(): %w", err) + } } - type caller func([]any, []*unifi.Site, *Controller) ([]any, error) - - for _, call := range []caller{u.collectIDs, u.collectAnomalies, u.collectAlarms, u.collectEvents, u.collectSyslog, u.collectProtectLogs} { + for _, call := range calls { if newLogs, err = call(logs, sites, c); err != nil { if c.Remote && (errors.Is(err, unifi.ErrInvalidStatusCode) || errors.Is(err, unifi.ErrEndpointNotFound)) { // The remote API (api.ui.com) does not support all event endpoints. diff --git a/pkg/inputunifi/collector.go b/pkg/inputunifi/collector.go index 1c14425e..216d2f2b 100644 --- a/pkg/inputunifi/collector.go +++ b/pkg/inputunifi/collector.go @@ -111,13 +111,54 @@ func (u *InputUnifi) pollController(c *Controller) (*poller.Metrics, error) { u.LogDebugf("Polling controller: %s (%s)", c.URL, c.ID) - // Get the sites we care about. - sites, err := u.getFilteredSites(c) - if err != nil { - return nil, fmt.Errorf("unifi.GetSites(): %w", err) + m := &Metrics{TS: time.Now()} + + // A Protect-only console (UNVR) has no Network application: no sites, and nothing behind + // /proxy/network to poll. Skipping the whole Network pass is what lets it reach the + // Protect collection below, which is not site-scoped. See unpoller/unpoller#1066. + if !*c.DisableNetwork { + // Get the sites we care about. + sites, err := u.getFilteredSites(c) + if err != nil { + return nil, fmt.Errorf("unifi.GetSites(): %w", err) + } + + m.Sites = sites + + if err := u.pollNetwork(c, sites, m); err != nil { + return nil, err + } } - m := &Metrics{TS: time.Now(), Sites: sites} + // Protect Integration API — opt-in, requires protect_api_key (or api_key as a fallback). + if c.SaveProtectDevices != nil && *c.SaveProtectDevices && (c.ProtectAPIKey != "" || c.APIKey != "") { + u.collectProtect(c, m) + } + + // Update web UI only on success; call explicitly so we never run with nil c/c.Unifi (no defer). + // Recover so a panic in updateWeb (e.g. old image, race) never kills the poller. + if c != nil && c.Unifi != nil { + func() { + defer func() { + if r := recover(); r != nil { + u.LogErrorf("updateWeb panic recovered (upgrade image if this persists): %v", r) + } + }() + + updateWeb(c, m) + }() + } + + return u.augmentMetrics(c, m), nil +} + +// pollNetwork collects everything served by the UniFi Network application into m. It is +// split out of pollController so a Protect-only console, which has no Network application at +// all, can skip the entire pass rather than failing on its first site-scoped call. +// +//nolint:cyclop +func (u *InputUnifi) pollNetwork(c *Controller, sites []*unifi.Site, m *Metrics) error { + var err error // FIXME needs to be last poll time maybe st := m.TS.Add(-1 * pollDuration) @@ -125,7 +166,7 @@ func (u *InputUnifi) pollController(c *Controller) (*poller.Metrics, error) { if c.SaveRogue != nil && *c.SaveRogue { if m.RogueAPs, err = c.Unifi.GetRogueAPs(sites); err != nil { - return nil, fmt.Errorf("unifi.GetRogueAPs(%s): %w", c.URL, err) + return fmt.Errorf("unifi.GetRogueAPs(%s): %w", c.URL, err) } u.LogDebugf("Found %d RogueAPs entries", len(m.RogueAPs)) @@ -133,13 +174,13 @@ func (u *InputUnifi) pollController(c *Controller) (*poller.Metrics, error) { if c.SaveDPI != nil && *c.SaveDPI { if m.SitesDPI, err = c.Unifi.GetSiteDPI(sites); err != nil { - return nil, fmt.Errorf("unifi.GetSiteDPI(%s): %w", c.URL, err) + return fmt.Errorf("unifi.GetSiteDPI(%s): %w", c.URL, err) } u.LogDebugf("Found %d SitesDPI entries", len(m.SitesDPI)) if m.ClientsDPI, err = c.Unifi.GetClientsDPI(sites); err != nil { - return nil, fmt.Errorf("unifi.GetClientsDPI(%s): %w", c.URL, err) + return fmt.Errorf("unifi.GetClientsDPI(%s): %w", c.URL, err) } u.LogDebugf("Found %d ClientsDPI entries", len(m.ClientsDPI)) @@ -147,7 +188,7 @@ func (u *InputUnifi) pollController(c *Controller) (*poller.Metrics, error) { if c.SaveTraffic != nil && *c.SaveTraffic { if m.CountryTraffic, err = c.Unifi.GetCountryTraffic(sites, &tp); err != nil { - return nil, fmt.Errorf("unifi.GetCountryTraffic(%s): %w", c.URL, err) + return fmt.Errorf("unifi.GetCountryTraffic(%s): %w", c.URL, err) } u.LogDebugf("Found %d CountryTraffic entries", len(m.CountryTraffic)) @@ -174,13 +215,13 @@ func (u *InputUnifi) pollController(c *Controller) (*poller.Metrics, error) { // Get all the points. if m.Clients, err = c.Unifi.GetClients(sites); err != nil { - return nil, fmt.Errorf("unifi.GetClients(%s): %w", c.URL, err) + return fmt.Errorf("unifi.GetClients(%s): %w", c.URL, err) } u.LogDebugf("Found %d Clients entries", len(m.Clients)) if m.Devices, err = c.Unifi.GetDevices(sites); err != nil { - return nil, fmt.Errorf("unifi.GetDevices(%s): %w", c.URL, err) + return fmt.Errorf("unifi.GetDevices(%s): %w", c.URL, err) } u.LogDebugf("Found %d UBB, %d UXG, %d PDU, %d UCI, %d UDB, %d UAP %d USG %d USW %d UDM devices", @@ -273,26 +314,7 @@ func (u *InputUnifi) pollController(c *Controller) (*poller.Metrics, error) { u.collectIntegrationV1(c, sites, m) } - // Protect Integration API — opt-in, requires protect_api_key (or api_key as a fallback). - if *c.SaveProtectDevices && (c.ProtectAPIKey != "" || c.APIKey != "") { - u.collectProtect(c, m) - } - - // Update web UI only on success; call explicitly so we never run with nil c/c.Unifi (no defer). - // Recover so a panic in updateWeb (e.g. old image, race) never kills the poller. - if c != nil && c.Unifi != nil { - func() { - defer func() { - if r := recover(); r != nil { - u.LogErrorf("updateWeb panic recovered (upgrade image if this persists): %v", r) - } - }() - - updateWeb(c, m) - }() - } - - return u.augmentMetrics(c, m), nil + return nil } // FIXME this would be better implemented on FlexInt itself @@ -1286,6 +1308,12 @@ func extractDevices(metrics *Metrics) (*poller.Metrics, map[string]string, map[s devices := make(map[string]string) bssdIDs := make(map[string]string) + // Devices is nil whenever GetDevices never ran -- a Protect-only console skips the whole + // Network pass -- and every loop below would otherwise dereference it. + if metrics.Devices == nil { + metrics.Devices = &unifi.Devices{} + } + for _, r := range metrics.Devices.UAPs { devices[r.Mac] = r.Name m.Devices = append(m.Devices, r) diff --git a/pkg/inputunifi/input.go b/pkg/inputunifi/input.go index 441196e5..a1e3dbe5 100644 --- a/pkg/inputunifi/input.go +++ b/pkg/inputunifi/input.go @@ -46,6 +46,7 @@ type Controller struct { ProtectThumbnails *bool `json:"protect_thumbnails" toml:"protect_thumbnails" xml:"protect_thumbnails" yaml:"protect_thumbnails"` SaveProtectDevices *bool `json:"save_protect_devices" toml:"save_protect_devices" xml:"save_protect_devices" yaml:"save_protect_devices"` ProtectAPIKey string `json:"protect_api_key" toml:"protect_api_key" xml:"protect_api_key" yaml:"protect_api_key"` + DisableNetwork *bool `json:"disable_network" toml:"disable_network" xml:"disable_network" yaml:"disable_network"` SaveIDs *bool `json:"save_ids" toml:"save_ids" xml:"save_ids" yaml:"save_ids"` SaveDPI *bool `json:"save_dpi" toml:"save_dpi" xml:"save_dpi" yaml:"save_dpi"` SaveTraffic *bool `json:"save_traffic" toml:"save_traffic" xml:"save_traffic" yaml:"save_traffic"` @@ -190,7 +191,16 @@ func (u *InputUnifi) getUnifi(c *Controller) error { backoff := 30 * time.Second for attempt := 0; attempt < maxAuthRetries; attempt++ { - c.Unifi, lastErr = unifi.NewUnifi(cfg) + // A Protect-only console (UNVR) runs no Network application, so NewUnifi's closing + // GetServerData() -- a GET of /proxy/network/status -- gets the UniFi OS SPA HTML back + // and fails the whole controller. NewProtectClient skips that probe and validates the + // Protect Integration API instead. See unpoller/unpoller#1066. + if *c.DisableNetwork { + c.Unifi, lastErr = unifi.NewProtectClient(cfg) + } else { + c.Unifi, lastErr = unifi.NewUnifi(cfg) + } + if lastErr == nil { u.LogDebugf("Authenticated with controller successfully, %s", c.URL) @@ -350,6 +360,10 @@ func (u *InputUnifi) setDefaults(c *Controller) { //nolint:cyclop c.SaveProtectDevices = &f } + if c.DisableNetwork == nil { + c.DisableNetwork = &f + } + if c.SaveAlarms == nil { c.SaveAlarms = &f } @@ -476,6 +490,10 @@ func (u *InputUnifi) setControllerDefaults(c *Controller) *Controller { //nolint c.SaveProtectDevices = u.Default.SaveProtectDevices } + if c.DisableNetwork == nil { + c.DisableNetwork = u.Default.DisableNetwork + } + if c.SaveAlarms == nil { c.SaveAlarms = u.Default.SaveAlarms } diff --git a/pkg/inputunifi/input_test.go b/pkg/inputunifi/input_test.go new file mode 100644 index 00000000..116ba4be --- /dev/null +++ b/pkg/inputunifi/input_test.go @@ -0,0 +1,377 @@ +package inputunifi_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/unpoller/unifi/v6" + "github.com/unpoller/unpoller/pkg/inputunifi" + "github.com/unpoller/unpoller/pkg/poller" + "golift.io/cnfg" + "golift.io/cnfgfile" +) + +const ( + metaInfoJSON = `{"applicationVersion":"7.2.105"}` + camerasJSON = `[{"id":"cam-1","modelKey":"camera","state":"CONNECTED","name":"Driveway","mac":"001122334455"}]` + sensorsJSON = `[{"id":"sen-1","modelKey":"sensor","state":"CONNECTED","name":"Garage","mac":"001122334466"}]` + nvrJSON = `{"id":"nvr-1","modelKey":"nvr","state":"CONNECTED","name":"UNVR4","mac":"001122334477"}` + emptyListJSON = `[]` + protectLogsJSON = `{"items":[{"id":"evt-1","modelKey":"event","type":"motion","camera":"cam-1","timestamp":1735689600000}]}` + unifiOSSPA = `` + protectAPIKeyStr = "protect-key-abc" +) + +// unvr is a fake Protect-only console. It answers the Protect Integration paths and the +// UniFi OS login, and serves the console's own SPA HTML for everything else -- which is +// exactly what a UNVR does with /proxy/network/*, there being no Network application to +// route to. That HTML is what breaks NewUnifi in unpoller/unpoller#1066. +type unvr struct { + mu sync.Mutex + requested []string +} + +func (s *unvr) start(t *testing.T) *httptest.Server { + t.Helper() + + record := func(r *http.Request) { + s.mu.Lock() + defer s.mu.Unlock() + + s.requested = append(s.requested, r.URL.Path) + } + + mux := http.NewServeMux() + + for path, body := range map[string]string{ + unifi.APIProtectMetaInfoPath: metaInfoJSON, + unifi.APIProtectCamerasPath: camerasJSON, + unifi.APIProtectSensorsPath: sensorsJSON, + unifi.APIProtectNVRPath: nvrJSON, + unifi.APIProtectLightsPath: emptyListJSON, + unifi.APIProtectBridgesPath: emptyListJSON, + unifi.APIProtectLinkStationsPath: emptyListJSON, + unifi.APIProtectLogPath: protectLogsJSON, + } { + mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { + record(r) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(body)) + }) + } + + mux.HandleFunc(unifi.APILoginPathNew, func(w http.ResponseWriter, r *http.Request) { + record(r) + w.Header().Set("x-csrf-token", "csrf-token") + w.WriteHeader(http.StatusOK) + }) + + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + record(r) + w.Header().Set("Content-Type", "text/html") + _, _ = w.Write([]byte(unifiOSSPA)) + }) + + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + + return srv +} + +// paths returns every path requested so far, and networkPaths only those under /proxy/network. +func (s *unvr) paths() []string { + s.mu.Lock() + defer s.mu.Unlock() + + return append([]string(nil), s.requested...) +} + +func (s *unvr) networkPaths() []string { + var network []string + + for _, p := range s.paths() { + if strings.HasPrefix(p, "/proxy/network") || p == unifi.APIStatusPath { + network = append(network, p) + } + } + + return network +} + +// newProtectOnlyInput builds an input with a single Protect-only controller. Options are +// applied before defaults are filled in, so a test can flip any flag it needs. +func newProtectOnlyInput(url string, opts ...func(*inputunifi.Controller)) *inputunifi.InputUnifi { + enabled := true + c := &inputunifi.Controller{ + URL: url, + User: "ro-user", + Pass: "secret", + ProtectAPIKey: protectAPIKeyStr, + DisableNetwork: &enabled, + SaveProtectDevices: &enabled, + } + + for _, opt := range opts { + opt(c) + } + + return &inputunifi.InputUnifi{Config: &inputunifi.Config{Controllers: []*inputunifi.Controller{c}}} +} + +// TestProtectOnlyControllerInitializes is the headline of unpoller/unpoller#1066: a UNVR is +// configured like any other controller and must come up. Before disable_network existed it +// died in NewUnifi and never even printed its config summary. +func TestProtectOnlyControllerInitializes(t *testing.T) { + t.Parallel() + + fake := &unvr{} + srv := fake.start(t) + + u := newProtectOnlyInput(srv.URL) + require.NoError(t, u.Initialize(nil)) + + require.Len(t, u.Controllers, 1) + c := u.Controllers[0] + + a := assert.New(t) + require.NotNil(t, c.Unifi, "controller client is nil, so initialization failed") + // NewProtectClient reports the Protect application version, there being no Network one. + a.Equal("7.2.105", c.Unifi.ServerVersion) + // Site discovery is skipped, so /proxy/network is never touched at all. + a.Empty(fake.networkPaths()) +} + +// The control case: the same console without disable_network still fails, so this test +// pins that the flag -- not some unrelated change -- is what makes the difference. +func TestProtectOnlyControllerFailsWithoutFlag(t *testing.T) { + t.Parallel() + + srv := (&unvr{}).start(t) + + disabled := false + u := newProtectOnlyInput(srv.URL, func(c *inputunifi.Controller) { c.DisableNetwork = &disabled }) + require.NoError(t, u.Initialize(nil)) + + assert.Nil(t, u.Controllers[0].Unifi, "NewUnifi should not survive a console with no Network application") +} + +func TestProtectOnlyControllerCollectsMetrics(t *testing.T) { + t.Parallel() + + fake := &unvr{} + srv := fake.start(t) + + u := newProtectOnlyInput(srv.URL) + require.NoError(t, u.Initialize(nil)) + + m, err := u.Metrics(nil) + require.NoError(t, err) + require.NotNil(t, m) + require.Len(t, m.ProtectDevices, 1) + + pd, ok := m.ProtectDevices[0].(*unifi.ProtectDevices) + require.True(t, ok, "outputs type-assert to *unifi.ProtectDevices, so nothing else may be appended") + + a := assert.New(t) + a.Equal("7.2.105", pd.Version) + a.Len(pd.Cameras, 1) + a.Len(pd.Sensors, 1) + require.NotNil(t, pd.NVR) + a.Equal("UNVR4", pd.NVR.Name) + + // Nothing Network-shaped is collected, and nothing Network-shaped is requested. + a.Empty(m.Devices) + a.Empty(m.Clients) + a.Empty(m.Sites) + a.Empty(fake.networkPaths()) + + require.Len(t, m.ControllerStatuses, 1) + a.True(m.ControllerStatuses[0].Up) +} + +// A Prometheus scrape of one target passes filter.Path. That path counts a poll as successful +// only if it produced devices or clients, and a Protect-only console produces neither -- so +// without ProtectDevices in that check a working console reports ErrDynamicLookupsDisabled. +func TestProtectOnlyControllerSucceedsOnFilteredScrape(t *testing.T) { + t.Parallel() + + srv := (&unvr{}).start(t) + + u := newProtectOnlyInput(srv.URL) + require.NoError(t, u.Initialize(nil)) + + m, err := u.Metrics(&poller.Filter{Path: srv.URL}) + require.NoError(t, err) + require.NotNil(t, m) + assert.Len(t, m.ProtectDevices, 1) +} + +// Protect event logs come from the legacy Protect endpoint, which is site-independent. Every +// other event collector is per-site, so all of them have to be skipped for this to work. +func TestProtectOnlyControllerCollectsEvents(t *testing.T) { + t.Parallel() + + enabled := true + fake := &unvr{} + srv := fake.start(t) + + u := newProtectOnlyInput(srv.URL, func(c *inputunifi.Controller) { c.SaveProtectLogs = &enabled }) + require.NoError(t, u.Initialize(nil)) + + e, err := u.Events(nil) + require.NoError(t, err) + require.NotNil(t, e) + assert.Len(t, e.Logs, 1) + assert.Empty(t, fake.networkPaths()) +} + +// RawMetrics' device/client kinds are all site-scoped. Returning an empty result for them +// would look like "this console has no devices"; the error says why instead. +func TestRawMetricsRejectsNetworkKindsWhenDisabled(t *testing.T) { + t.Parallel() + + srv := (&unvr{}).start(t) + + u := newProtectOnlyInput(srv.URL) + require.NoError(t, u.Initialize(nil)) + + _, err := u.RawMetrics(&poller.Filter{Kind: "devices"}) + require.ErrorIs(t, err, inputunifi.ErrNetworkDisabled) + + // The raw-path kind still works: it is the only way to inspect a Protect-only console. + body, err := u.RawMetrics(&poller.Filter{Kind: "other", Path: unifi.APIProtectMetaInfoPath}) + require.NoError(t, err) + assert.JSONEq(t, metaInfoJSON, string(body)) +} + +// A Protect-only controller that saves nothing is always a mistake, and silently collecting +// nothing is the failure mode hardest to spot in a log, so it must be called out at startup. +func TestProtectOnlyWarnsWhenNothingWillBeCollected(t *testing.T) { + t.Parallel() + + srv := (&unvr{}).start(t) + + disabled := false + logger := &captureLogger{} + u := newProtectOnlyInput(srv.URL, func(c *inputunifi.Controller) { c.SaveProtectDevices = &disabled }) + require.NoError(t, u.Initialize(logger)) + + assert.Contains(t, logger.errors(), "nothing will be collected from it") +} + +// The same, for a console that wants Protect devices but has no key to ask for them with. +func TestProtectOnlyWarnsWithoutAPIKey(t *testing.T) { + t.Parallel() + + srv := (&unvr{}).start(t) + + logger := &captureLogger{} + u := newProtectOnlyInput(srv.URL, func(c *inputunifi.Controller) { c.ProtectAPIKey = "" }) + require.NoError(t, u.Initialize(logger)) + + assert.Contains(t, logger.errors(), "no protect_api_key") +} + +// disable_network has to survive four independent binding paths -- toml, json, yaml and the +// UP_ environment -- each driven by its own struct tag. A typo in any one tag leaves the +// option silently inert for users of that format. Note the env name derives from the xml tag. +func TestDisableNetworkBindsInEveryConfigFormat(t *testing.T) { + t.Parallel() + + write := func(t *testing.T, name, body string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), name) + require.NoError(t, os.WriteFile(path, []byte(body), 0o600)) + + return path + } + + for _, tc := range []struct { + name string + file string + body string + }{ + {"toml", "up.conf", "[[unifi.controller]]\n url = \"https://unvr\"\n disable_network = true\n"}, + {"json", "up.json", `{"unifi":{"controllers":[{"url":"https://unvr","disable_network":true}]}}`}, + {"yaml", "up.yaml", "unifi:\n controllers:\n - url: \"https://unvr\"\n disable_network: true\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + input := &inputunifi.InputUnifi{Config: &inputunifi.Config{}} + require.NoError(t, cnfgfile.Unmarshal(input, write(t, tc.file, tc.body))) + require.Len(t, input.Controllers, 1) + require.NotNil(t, input.Controllers[0].DisableNetwork, "disable_network did not bind from %s", tc.name) + assert.True(t, *input.Controllers[0].DisableNetwork) + }) + } +} + +// Not parallel: t.Setenv is incompatible with a parallel test or parent. +// +// The env variable name comes from the xml tag, not the json one, which is why this is worth +// asserting rather than assuming from the toml/json/yaml cases above. +func TestDisableNetworkBindsFromEnvironment(t *testing.T) { + t.Setenv("UP_UNIFI_CONTROLLER_0_URL", "https://unvr") + t.Setenv("UP_UNIFI_CONTROLLER_0_DISABLE_NETWORK", "true") + + input := &inputunifi.InputUnifi{Config: &inputunifi.Config{}} + _, err := cnfg.UnmarshalENV(input, "UP") + require.NoError(t, err) + require.Len(t, input.Controllers, 1) + require.NotNil(t, input.Controllers[0].DisableNetwork, "UP_UNIFI_CONTROLLER_0_DISABLE_NETWORK did not bind") + assert.True(t, *input.Controllers[0].DisableNetwork) +} + +// The shipped examples must keep the Network application enabled, so upgrading unpoller never +// silently stops collecting from a working controller. +func TestShippedExamplesDoNotDisableNetwork(t *testing.T) { + t.Parallel() + + for _, name := range []string{"up.conf.example", "up.json.example", "up.yaml.example"} { + t.Run(name, func(t *testing.T) { + t.Parallel() + + input := &inputunifi.InputUnifi{Config: &inputunifi.Config{}} + require.NoError(t, cnfgfile.Unmarshal(input, filepath.Join("..", "..", "examples", name))) + + for i, c := range input.Controllers { + if c.DisableNetwork != nil { + assert.False(t, *c.DisableNetwork, "%s controller %d ships with disable_network set", name, i) + } + } + }) + } +} + +// captureLogger records what Initialize logs, so the startup warnings can be asserted on. +type captureLogger struct { + mu sync.Mutex + errs []string +} + +func (l *captureLogger) Logf(string, ...any) {} +func (l *captureLogger) LogDebugf(string, ...any) {} + +func (l *captureLogger) LogErrorf(msg string, v ...any) { + l.mu.Lock() + defer l.mu.Unlock() + + l.errs = append(l.errs, fmt.Sprintf(msg, v...)) +} + +func (l *captureLogger) errors() string { + l.mu.Lock() + defer l.mu.Unlock() + + return strings.Join(l.errs, "\n") +} diff --git a/pkg/inputunifi/interface.go b/pkg/inputunifi/interface.go index 011e8bf8..fb1171be 100644 --- a/pkg/inputunifi/interface.go +++ b/pkg/inputunifi/interface.go @@ -15,6 +15,7 @@ var ( ErrDynamicLookupsDisabled = fmt.Errorf("filter path requested but dynamic lookups disabled") ErrControllerNumNotFound = fmt.Errorf("controller number not found") ErrNoFilterKindProvided = fmt.Errorf("must provide filter: devices, clients, other") + ErrNetworkDisabled = fmt.Errorf("controller has disable_network set: no Network application to query") ) // Initialize gets called one time when starting up. @@ -84,14 +85,19 @@ func (u *InputUnifi) Initialize(l poller.Logger) error { } for i, c := range u.Controllers { - if err := u.getUnifi(u.setControllerDefaults(c)); err != nil { + u.warnProtectOnly(u.setControllerDefaults(c)) + + if err := u.getUnifi(c); err != nil { u.LogErrorf("Controller %d of %d Auth or Connection Error, retrying: %v", i+1, len(u.Controllers), err) continue } - if err := u.checkSites(c); err != nil { - u.LogErrorf("checking sites on %s: %v", c.URL, err) + // A Protect-only console has no sites to check. + if !*c.DisableNetwork { + if err := u.checkSites(c); err != nil { + u.LogErrorf("checking sites on %s: %v", c.URL, err) + } } u.Logf("Configured UniFi Controller %d of %d:", i+1, len(u.Controllers)) @@ -137,18 +143,21 @@ func (u *InputUnifi) DebugInput() (bool, error) { continue } - if err := u.checkSites(c); err != nil { - u.LogErrorf("checking sites on %s: %v", c.URL, err) + // A Protect-only console has no sites to check. + if !*c.DisableNetwork { + if err := u.checkSites(c); err != nil { + u.LogErrorf("checking sites on %s: %v", c.URL, err) - allOK = false + allOK = false - if allErrors != nil { - allErrors = fmt.Errorf("%v: %w", err, allErrors) - } else { - allErrors = err + if allErrors != nil { + allErrors = fmt.Errorf("%v: %w", err, allErrors) + } else { + allErrors = err + } + + continue } - - continue } u.Logf("Valid UniFi Controller %d of %d:", i+1, len(u.Controllers)) @@ -167,6 +176,10 @@ func (u *InputUnifi) logController(c *Controller) { } } + if *c.DisableNetwork { + mode += " (Protect only: no Network application)" + } + u.Logf(" => Mode: %s", mode) u.Logf(" => URL: %s (verify SSL: %v, timeout: %v)", c.URL, *c.VerifySSL, c.Timeout.Duration) @@ -184,16 +197,44 @@ func (u *InputUnifi) logController(c *Controller) { u.Logf(" => Username: %s (has password: %v) (has api-key: %v)", c.User, c.Pass != "", c.APIKey != "") } - u.Logf(" => Hash PII %v / Drop PII %v / Poll Sites: %s", *c.HashPII, *c.DropPII, strings.Join(c.Sites, ", ")) + u.Logf(" => Hash PII %v / Drop PII %v", *c.HashPII, *c.DropPII) + u.Logf(" => Save Protect Devices: %v (has protect-api-key: %v)", *c.SaveProtectDevices, c.ProtectAPIKey != "") + u.Logf(" => Save Protect Logs %v (thumbnails: %v)", *c.SaveProtectLogs, *c.ProtectThumbnails) + + // The rest are all Network-application settings; printing them for a Protect-only + // console would only suggest data that is never collected. + if *c.DisableNetwork { + return + } + + u.Logf(" => Poll Sites: %s", strings.Join(c.Sites, ", ")) u.Logf(" => Save Sites %v / Save DPI %v (metrics)", *c.SaveSites, *c.SaveDPI) u.Logf(" => Save Events %v / Save Syslog %v / Save IDs %v (logs)", *c.SaveEvents, *c.SaveSyslog, *c.SaveIDs) - u.Logf(" => Save Alarms %v / Anomalies %v / Protect Logs %v (thumbnails: %v)", *c.SaveAlarms, *c.SaveAnomal, *c.SaveProtectLogs, *c.ProtectThumbnails) - u.Logf(" => Save Protect Devices: %v (has protect-api-key: %v)", *c.SaveProtectDevices, c.ProtectAPIKey != "") + u.Logf(" => Save Alarms %v / Anomalies %v", *c.SaveAlarms, *c.SaveAnomal) u.Logf(" => Save Rogue APs: %v", *c.SaveRogue) u.Logf(" => Save Traffic %v", *c.SaveTraffic) u.Logf(" => Save Speed Tests: %v", *c.SaveSpeedTest) } +// warnProtectOnly reports a disable_network controller that can never collect anything. +// Neither case is fatal -- the controller is still polled -- but both are always a mistake, +// and silently collecting nothing is the failure mode hardest to diagnose from a log. +func (u *InputUnifi) warnProtectOnly(c *Controller) { + if !*c.DisableNetwork { + return + } + + if !*c.SaveProtectDevices && !*c.SaveProtectLogs { + u.LogErrorf("Controller %s sets disable_network but neither save_protect_devices nor "+ + "save_protect_logs: nothing will be collected from it", c.URL) + } + + if *c.SaveProtectDevices && c.ProtectAPIKey == "" && c.APIKey == "" { + u.LogErrorf("Controller %s sets disable_network and save_protect_devices but no "+ + "protect_api_key: the Protect Integration API cannot be authenticated", c.URL) + } +} + // Events allows you to pull only events (and IDs) from the UniFi Controller. // This does not fully respect HashPII, but it may in the future! // Use Filter.Path to pick a specific controller, otherwise poll them all! @@ -294,8 +335,10 @@ func (u *InputUnifi) Metrics(filter *poller.Filter) (*poller.Metrics, error) { metrics = poller.AppendMetrics(metrics, m) } - // If we collected data from at least one controller, return success - if len(metrics.Devices) > 0 || len(metrics.Clients) > 0 { + // If we collected data from at least one controller, return success. ProtectDevices counts: + // a Protect-only console contributes no devices and no clients, and without it a filtered + // scrape of one would fall through to the dynamic-controller path and fail. + if len(metrics.Devices) > 0 || len(metrics.Clients) > 0 || len(metrics.ProtectDevices) > 0 { return metrics, nil } @@ -332,6 +375,17 @@ func (u *InputUnifi) RawMetrics(filter *poller.Filter) ([]byte, error) { } } + // Every site-scoped kind below needs a Network application. Only the raw-path kind can + // say anything about a Protect-only console, so answer that and reject the rest plainly + // rather than returning a confusing empty result. + if *c.DisableNetwork { + if filter.Kind == "other" || filter.Kind == "o" { + return c.Unifi.GetJSON(filter.Path) + } + + return nil, fmt.Errorf("%s: %w", c.URL, ErrNetworkDisabled) + } + if err := u.checkSites(c); err != nil { return nil, err } diff --git a/pkg/inputunifi/updateweb.go b/pkg/inputunifi/updateweb.go index bd163882..3a0af817 100644 --- a/pkg/inputunifi/updateweb.go +++ b/pkg/inputunifi/updateweb.go @@ -79,6 +79,7 @@ func formatControllers(controllers []*Controller) []*Controller { SaveSyslog: c.SaveSyslog, SaveProtectLogs: c.SaveProtectLogs, SaveProtectDevices: c.SaveProtectDevices, + DisableNetwork: c.DisableNetwork, SaveIDs: c.SaveIDs, SaveDPI: c.SaveDPI, HashPII: c.HashPII,