public_git/unpoller_unpoller
1
0
Fork 0
mirror of https://github.com/unpoller/unpoller.git synced 2026-10-04 21:41:31 +02:00
Code Issues Projects Releases Wiki Activity

refactor(unas): replace disable flag with enable, defaulting to off

Browse Source 
`disable = false` is a double negative, and a bool named disable cannot
express opt-in anyway: it zero-values to false, so the flag was inert and
opt-in rested entirely on the device list being empty.

`enable` defaults to false and is now the real gate -- Initialize, Metrics
and DebugInput all return early unless it is set. The two existing guards
remain: an empty device list is still a no-op, and no default URL is ever
synthesized.

Configuring devices while enable is false is always a mistake, so that
combination logs one error instead of silently collecting nothing.

Adds binding tests for the flag across toml, json, yaml and UP_UNAS_ENABLE
(the env name derives from the xml tag, not the json one), plus a test that
all three shipped examples default to off. Both were verified by mutation:
breaking a struct tag or flipping an example fails the suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Cody Lee
2026-08-19 09:34:02 -05:00
co-authored by Claude Opus 5
parent 3d178bed8d
commit 01ac7ca2b7
8 changed files with 129 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/plan-unas-support.md
+10 -8
View File
@@ -9,8 +9,8 @@ alexgreenbank in the package header and README.
UNAS Pro is a standalone UniFi OS console with **no Network application**: no sites, no
`/status`, its own credentials, its own host. `inputunifi`'s sites → devices → per-site flow
has nothing to offer it, and folding it in would mean bolting a second credential set onto
`Controller`. So: a new `pkg/inputunas` plugin with its own `[unas]` config block, disabled
by default and a no-op when unconfigured.
`Controller`. So: a new `pkg/inputunas` plugin with its own `[unas]` config block, off by
default and a no-op when unconfigured.
### The one real blocker
@@ -130,11 +130,13 @@ in all three example configs. `go test ./...` and `golangci-lint run` are clean.
Refinements over the plan below:
1. **The opt-in mechanism is "no devices configured", not `disable`.** `Disable` zero-values
to false, so a field named `disable` cannot make a plugin default-off. `Initialize` returns
silently when the device list is empty, and unlike `inputunifi` it does **not** synthesize
a default device from `[unas.defaults]` — doing so would poll a host the operator never
named. `disable` remains as an explicit off switch.
1. **Opt-in is `enable`, defaulting to false — not `disable`.** A field named `disable`
zero-values to false, so it cannot make a plugin default-off and reads as a double
negative; `enable bool` defaults to off and says what it does. Two further guards:
`Initialize` returns silently when the device list is empty, and unlike `inputunifi` it
does **not** synthesize a default device from `[unas.defaults]` — doing so would poll a
host the operator never named. Devices configured while `enable` is false log one error,
since that combination is always a mistake.
2. **`Metrics` returns `(metrics, nil)` whenever any console was collected.** Not politeness:
`poller.collectMetrics` uses `if result.err != nil { ... } else if result.metric != nil`,
so returning both discards every metric in that cycle. One dead console out of three would
@@ -156,7 +158,7 @@ Refinements over the plan below:
```toml
[unas]
disable = false
enable = true
[unas.defaults]
user = "unpoller"
pass = ""
examples/up.conf.example
+3 -3
View File
@@ -275,13 +275,13 @@
# A UNAS Pro is a standalone UniFi OS console with no Network application, so it is polled
# by its own input plugin with its own credentials and host -- not as a UniFi controller.
#
# This section is entirely optional and does nothing unless you add at least one
# [[unas.device]] below: with no devices configured the plugin stays silent and inert.
# This section is entirely optional. The plugin is off unless you set enable = true and
# add at least one [[unas.device]] below; otherwise it stays silent and inert.
# Metrics land under the unifi_unas_ prefix in Prometheus, and in the unas_device,
# unas_pool, unas_disk, and unas_share measurements in InfluxDB.
#
#[unas]
# disable = false
# enable = true
#
# Defaults applied to every device that does not set its own value.
#[unas.defaults]
examples/up.json.example
+2 -2
View File
@@ -87,8 +87,8 @@
},
"unas": {
"//": "UNAS Pro consoles. JSON cannot carry comments, so this section ships disabled: it shows the shape, and flipping disable to false with a real url turns it on.",
"disable": true,
"//": "UNAS Pro consoles. JSON cannot carry comments, so this section ships off: it shows the shape, and setting enable to true with a real url turns it on.",
"enable": false,
"defaults": {
"user": "unpoller",
"pass": "",
examples/up.yaml.example
+1 -1
View File
@@ -91,7 +91,7 @@ unifi:
# by its own input plugin with its own credentials and host -- not as a UniFi controller.
# This section is optional: with no devices configured the plugin stays silent and inert.
#unas:
# disable: false
# enable: true
# defaults:
# user: "unpoller"
# pass: ""
pkg/inputunas/README.md
+6 -5
View File
@@ -2,9 +2,10 @@
Polls UniFi UNAS Pro storage consoles and hands their metrics to every configured output.
**This plugin is opt-in and does nothing until you configure at least one device.** With no
`[unas]` section, or a section with no devices, it stays silent and inert — it logs nothing
and polls nothing.
**This plugin is opt-in: `enable` defaults to `false`.** It does nothing until you set
`enable = true` *and* configure at least one device. With no `[unas]` section it stays silent
and inert — it logs nothing and polls nothing. If you configure devices but leave `enable`
unset, it logs one error saying so rather than failing quietly.
## Why a separate plugin?
@@ -17,7 +18,7 @@ rather than as another UniFi controller.
```toml
[unas]
disable = false
enable = true
# Applied to any device that does not set its own value.
[unas.defaults]
@@ -44,7 +45,7 @@ tables. See `examples/up.{conf,json,yaml}.example`.
| Variable | Meaning |
|---|---|
| `UP_UNAS_DISABLE` | Disable the plugin outright. |
| `UP_UNAS_ENABLE` | Turn the plugin on. Defaults to `false`. |
| `UP_UNAS_DEFAULT_USER` | Default username for every device. |
| `UP_UNAS_DEFAULT_PASS` | Default password. |
| `UP_UNAS_DEFAULT_VERIFY_SSL` | Default TLS verification. |
pkg/inputunas/config.go
+4 -4
View File
@@ -46,9 +46,9 @@ type Device struct {
// Config contains our configuration data.
type Config struct {
sync.RWMutex // locks a Device's client while it re-authenticates.
Default Device `json:"defaults" toml:"defaults" xml:"default" yaml:"defaults"`
Disable bool `json:"disable" toml:"disable" xml:"disable,attr" yaml:"disable"`
Devices []*Device `json:"devices" toml:"device" xml:"device" yaml:"devices"`
Default Device `json:"defaults" toml:"defaults" xml:"default" yaml:"defaults"`
Enable bool `json:"enable" toml:"enable" xml:"enable,attr" yaml:"enable"`
Devices []*Device `json:"devices" toml:"device" xml:"device" yaml:"devices"`
}
func init() { // nolint: gochecknoinits
@@ -64,7 +64,7 @@ func init() { // nolint: gochecknoinits
// setDefaults fills a device's unset fields from the defaults block, then from package
// defaults. It never invents a URL: an empty URL is how "not configured" is expressed, and
// synthesizing one (as inputunifi does with localhost:8443) would make the plugin poll a
// host the operator never named. That is what keeps UNAS support opt-in.
// host the operator never named. Opt-in rests on Config.Enable; this is the second guard.
func (u *InputUNAS) setDefaults(d *Device) *Device {
if d.User == "" {
d.User = u.Default.User
pkg/inputunas/input.go
+16 -8
View File
@@ -21,18 +21,26 @@ var ErrNoDevices = errors.New("no UNAS devices configured")
// Satisfies poller.Input interface.
func (u *InputUNAS) Initialize(l poller.Logger) error {
if u.Config == nil {
u.Config = &Config{Disable: true}
u.Config = &Config{}
}
if u.Logger = l; u.Disable {
u.Logger = l
// enable defaults to false, so the plugin is off until an operator asks for it. Warn when
// consoles are configured but the switch was never flipped -- that combination is always a
// mistake, and silence would leave the operator hunting for missing metrics. An operator
// who has never heard of UNAS has no devices either, and still sees nothing.
if !u.Enable {
if len(u.configuredDevices()) > 0 {
u.LogErrorf("UNAS devices are configured but unas.enable is false; not polling them")
}
return nil
}
u.Devices = u.configuredDevices()
// No [unas] block means no devices, which means nothing to say. Staying silent here is
// what makes the plugin opt-in: an operator who has never heard of UNAS should see no
// trace of it in the log.
// Enabled with nothing to poll: nothing to say, and nothing to do.
if len(u.Devices) == 0 {
return nil
}
@@ -96,7 +104,7 @@ func (u *InputUNAS) logDevice(d *Device) {
// returning `(metrics, err)` after one console of three fails would throw away the two that
// worked. Satisfies poller.Input interface.
func (u *InputUNAS) Metrics(filter *poller.Filter) (*poller.Metrics, error) {
if u.Disable {
if !u.Enable {
return nil, nil
}
@@ -234,7 +242,7 @@ func (u *InputUNAS) RawMetrics(filter *poller.Filter) ([]byte, error) {
// DebugInput checks that every configured console can be reached and authenticated against.
// Satisfies poller.Input interface.
func (u *InputUNAS) DebugInput() (bool, error) {
if u == nil || u.Config == nil || u.Disable {
if u == nil || u.Config == nil || !u.Enable {
return true, nil
}
@@ -294,7 +302,7 @@ func formatConfig(config *Config) *Config {
CertPaths: config.Default.CertPaths,
Timeout: config.Default.Timeout,
},
Disable: config.Disable,
Enable: config.Enable,
Devices: devices,
}
}
pkg/inputunas/input_test.go
+87 -6
View File
@@ -3,6 +3,8 @@ package inputunas_test
import (
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"sync/atomic"
"testing"
@@ -11,6 +13,8 @@ import (
"github.com/unpoller/unifi/v5"
"github.com/unpoller/unpoller/pkg/inputunas"
"github.com/unpoller/unpoller/pkg/poller"
"golift.io/cnfg"
"golift.io/cnfgfile"
)
const (
@@ -76,7 +80,7 @@ func newInput(t *testing.T, urls ...string) *inputunas.InputUNAS {
devices[i] = &inputunas.Device{URL: u, User: "unpoller", Pass: "secret"}
}
return &inputunas.InputUNAS{Config: &inputunas.Config{Devices: devices}}
return &inputunas.InputUNAS{Config: &inputunas.Config{Enable: true, Devices: devices}}
}
func TestMetricsCollectsConsole(t *testing.T) {
@@ -155,15 +159,25 @@ func TestMetricsPartialFailureStillReportsHealthyConsole(t *testing.T) {
require.Len(t, m.UNASDevices, 1)
}
func TestMetricsDisabled(t *testing.T) {
// enable defaults to false, so a config that names consoles but never sets it must not poll
// them. Configuring the devices here is the point: an empty config would pass even if the
// switch were ignored entirely.
func TestNotEnabled(t *testing.T) {
t.Parallel()
u := &inputunas.InputUNAS{Config: &inputunas.Config{Disable: true}}
srv := (&unasServer{}).start(t)
u := &inputunas.InputUNAS{Config: &inputunas.Config{
Devices: []*inputunas.Device{{URL: srv.URL, User: "unpoller", Pass: "secret"}},
}}
require.NoError(t, u.Initialize(nil))
m, err := u.Metrics(nil)
require.NoError(t, err)
require.Nil(t, m)
require.Nil(t, m, "enable is false, so nothing is polled")
ok, err := u.DebugInput()
require.NoError(t, err)
require.True(t, ok, "--debugio must not reach a console the operator has not enabled")
}
// With no [unas] block the plugin must be silent and inert: that is what makes UNAS support
@@ -176,7 +190,7 @@ func TestUnconfiguredIsInert(t *testing.T) {
m, err := u.Metrics(nil)
require.NoError(t, err)
require.Nil(t, m, "an unconfigured plugin disables itself")
require.Nil(t, m, "an unconfigured plugin stays off")
ok, err := u.DebugInput()
require.NoError(t, err)
@@ -186,7 +200,7 @@ func TestUnconfiguredIsInert(t *testing.T) {
func TestDeviceWithNoURLIsIgnored(t *testing.T) {
t.Parallel()
u := &inputunas.InputUNAS{Config: &inputunas.Config{Devices: []*inputunas.Device{{}, nil}}}
u := &inputunas.InputUNAS{Config: &inputunas.Config{Enable: true, Devices: []*inputunas.Device{{}, nil}}}
require.NoError(t, u.Initialize(nil))
m, err := u.Metrics(nil)
@@ -217,3 +231,70 @@ func TestRawMetrics(t *testing.T) {
_, err = u.RawMetrics(&poller.Filter{Unit: 9})
require.ErrorIs(t, err, inputunas.ErrNoDevices)
}
// The enable switch 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
// plugin silently off for users of that format, which is the same class of invisible failure
// as a missing tag anywhere else in this config. Note the env name derives from the xml tag,
// not the json one.
func TestEnableBindsInEveryConfigFormat(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", "[unas]\n enable = true\n[[unas.device]]\n url = \"https://nas\"\n"},
{"json", "up.json", `{"unas":{"enable":true,"devices":[{"url":"https://nas"}]}}`},
{"yaml", "up.yaml", "unas:\n enable: true\n devices:\n - url: \"https://nas\"\n"},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
input := &inputunas.InputUNAS{Config: &inputunas.Config{}}
require.NoError(t, cnfgfile.Unmarshal(input, write(t, tc.file, tc.body)))
assert.True(t, input.Enable, "enable did not bind from %s", tc.name)
require.Len(t, input.Devices, 1)
assert.Equal(t, "https://nas", input.Devices[0].URL)
})
}
}
// 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 TestEnableBindsFromEnvironment(t *testing.T) {
t.Setenv("UP_UNAS_ENABLE", "true")
input := &inputunas.InputUNAS{Config: &inputunas.Config{}}
_, err := cnfg.UnmarshalENV(input, "UP")
require.NoError(t, err)
assert.True(t, input.Enable, "UP_UNAS_ENABLE did not bind")
}
// The shipped examples must all default to off, so that installing unpoller never starts
// polling a storage console nobody asked for.
func TestShippedExamplesDoNotEnableUNAS(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 := &inputunas.InputUNAS{Config: &inputunas.Config{}}
require.NoError(t, cnfgfile.Unmarshal(input, filepath.Join("..", "..", "examples", name)))
assert.False(t, input.Enable, "%s ships with UNAS enabled", name)
})
}
}