Skip to content

Commit 57894f4

Browse files
Syndicclaude
andcommitted
docs(netbox_audit): correct the policy config schema and pin it with a test
CONFIG.md documented five top-level sections — `wan`, `vrf`, `wireless`, `rack_placement`, `poe` — that the loader has never accepted. The real schema nests every knob under `rules`, keyed by check ID, as `auditConfig` in config.go and the committed netbox_audit.config.json both show. encoding/json drops unknown keys silently, so a config written from the old doc parsed clean, ran green, and left every knob at its default. `wan.device_roles` was doubly wrong: the field is `wan_device_roles`, under `rules.interface-vrf`. CHECKS.md carried the same paths in its thirteen "Configuration knobs" bullets. Rather than prefix each with `rules.<check-id>.`, which runs to 79 characters on the wireless suppression knob, the section is named once in the lead-in and the bullets keep bare field names. The docs are prose that hand-copies a schema, and nothing failed while they drifted — the same shape as the couplings //meta/scripts:test_precommit_docs and :test_codeql_toolchain already guard. config_docs_test.go closes it by decoding the docs' own JSON blocks into the real auditConfig with DisallowUnknownFields, turning that silent drop into a build failure; it also runs CONFIG.md's example through loadAuditConfig to prove the documented values land, resolves every CHECKS.md knob against the schema, and fails if a check grows rules with no CONFIG.md section. Each assertion was negative-controlled — reinstating the original bug fails with `unknown field "wan"`. Also corrected while rewriting the sections they live in: private-ip-vrf covers IPv6 ULA as well as RFC1918 (netip.Addr.IsPrivate), an unrecognized check ID is fatal rather than ignored, and exempt_device_tags replaces the default list rather than extending it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 512e332 commit 57894f4

4 files changed

Lines changed: 313 additions & 86 deletions

File tree

tools/network_infrastructure_maintenance/cmd/netbox_audit/BUILD.bazel

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,16 @@ go_binary(
2929
go_test(
3030
name = "netbox_audit_test",
3131
size = "small",
32-
srcs = ["main_test.go"],
32+
srcs = [
33+
"config_docs_test.go",
34+
"main_test.go",
35+
],
36+
# config_docs_test.go reads the docs as test data; they are prose that hand-copies the
37+
# policy schema, and a wrong copy fails nowhere else.
38+
data = [
39+
"CHECKS.md",
40+
"CONFIG.md",
41+
],
3342
embed = [":netbox_audit_lib"],
3443
deps = [
3544
"//tools/network_infrastructure_maintenance/internal/audit",

tools/network_infrastructure_maintenance/cmd/netbox_audit/CHECKS.md

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ recorded.
4141

4242
**Why it matters:** Without position data, rack diagrams and capacity planning are unreliable.
4343

44-
**Configuration knobs:**
44+
**Configuration knobs**, under `rules.rack-placement` (see [CONFIG.md](CONFIG.md)):
4545

46-
- `rack_placement.exempt_child_devices` — skip the check for devices that have a parent device
47-
(default: `true`; child devices don't occupy their own rack unit)
48-
- `rack_placement.exempt_device_tags` — device tags that exempt a device from this check (default:
46+
- `exempt_child_devices` — skip the check for devices that have a parent device (default: `true`;
47+
child devices don't occupy their own rack unit)
48+
- `exempt_device_tags` — device tags that exempt a device from this check (default:
4949
`["0u-rack-device"]`; use this for zero-U items like patch panels or PDUs)
5050

5151
---
@@ -97,14 +97,14 @@ subnets that no longer have a VLAN.
9797
**Why it matters:** Wireless clients that connect to a managed switch port need proper VLAN tagging
9898
for correct network placement. Missing MAC addresses prevent DHCP reservations from being created.
9999

100-
**Configuration knobs:**
100+
**Configuration knobs**, under `rules.wireless-normalization` (see [CONFIG.md](CONFIG.md)):
101101

102-
- `wireless.suppress_if_connected_wired_interface_is_complete` — skip the check for a device if it
103-
already has a fully-configured wired interface (default: `true`; avoids noise for dual-homed
104-
devices where the wired port is the primary)
105-
- `wireless.require_mode` — require 802.1Q mode (default: `true`)
106-
- `wireless.require_untagged_vlan` — require an untagged VLAN (default: `true`)
107-
- `wireless.require_primary_mac` — require a primary MAC (default: `true`)
102+
- `suppress_if_connected_wired_interface_is_complete` — skip the check for a device if it already
103+
has a fully-configured wired interface (default: `true`; avoids noise for dual-homed devices
104+
where the wired port is the primary)
105+
- `require_mode` — require 802.1Q mode (default: `true`)
106+
- `require_untagged_vlan` — require an untagged VLAN (default: `true`)
107+
- `require_primary_mac` — require a primary MAC (default: `true`)
108108

109109
---
110110

@@ -124,12 +124,12 @@ PoE type hierarchy from least to most capable:
124124
**Why it matters:** Mis-matched PoE types can cause devices to fail to power on or to operate in
125125
degraded mode.
126126

127-
**Configuration knobs:**
127+
**Configuration knobs**, under `rules.poe-power` (see [CONFIG.md](CONFIG.md)):
128128

129-
- `poe.check_powered_device_supply` — enable or disable the entire check (default: `true`)
130-
- `poe.require_pse_mode_on_peer` — flag the finding if the connected peer is not marked as PSE
129+
- `check_powered_device_supply` — enable or disable the entire check (default: `true`)
130+
- `require_pse_mode_on_peer` — flag the finding if the connected peer is not marked as PSE
131131
(default: `true`)
132-
- `poe.unknown_type_policy` — how to handle a PD or PSE with an unset PoE type: `"fail"` (flag as a
132+
- `unknown_type_policy` — how to handle a PD or PSE with an unset PoE type: `"fail"` (flag as a
133133
finding) or `"ignore"` (skip silently) (default: `"fail"`)
134134

135135
---
@@ -142,10 +142,10 @@ assigned, except interfaces on WAN-side device roles.
142142
**Why it matters:** VRF assignment determines routing context. An interface without a VRF is
143143
ambiguously placed and can cause routing misconfigurations in tooling that reads from NetBox.
144144

145-
**Configuration knobs:**
145+
**Configuration knobs**, under `rules.interface-vrf` (see [CONFIG.md](CONFIG.md)):
146146

147-
- `vrf.require_on_interfaces` — enable or disable this check (default: `true`)
148-
- `wan.device_roles` — device roles considered WAN-side, which are exempt from VRF requirements
147+
- `require_on_interfaces` — enable or disable this check (default: `true`)
148+
- `wan_device_roles` — device roles considered WAN-side, which are exempt from VRF requirements
149149
(default: `["ISP Equipment"]`)
150150

151151
---
@@ -159,10 +159,10 @@ ambiguously placed and can cause routing misconfigurations in tooling that reads
159159
VRF is ambiguous. Tools that export routes or generate firewall rules from NetBox will produce
160160
incorrect results.
161161

162-
**Configuration knobs:**
162+
**Configuration knobs**, under `rules.private-ip-vrf` (see [CONFIG.md](CONFIG.md)):
163163

164-
- `vrf.require_on_private_ips` — enforce VRF on RFC1918 addresses (default: `true`)
165-
- `vrf.require_on_public_ips` — also enforce VRF on public IP addresses (default: `false`)
164+
- `require_on_private_ips` — enforce VRF on private addresses (default: `true`)
165+
- `require_on_public_ips` — also enforce VRF on public IP addresses (default: `false`)
166166

167167
---
168168

tools/network_infrastructure_maintenance/cmd/netbox_audit/CONFIG.md

Lines changed: 99 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,33 @@
33
The policy config is a JSON file that controls which checks run and adjusts the behavior of
44
individual checks. Pass it with `-config <path>` or `NETBOX_AUDIT_CONFIG=<path>`.
55

6-
All fields are optional. Omitting a field keeps the documented default.
6+
All fields are optional; omitting one keeps the documented default. Unknown keys are ignored
7+
silently, so a misspelled section reads as "not configured" rather than as an error — a run that
8+
stays green is not evidence that a new section was picked up. Check it against the names below.
79

810
---
911

1012
## Top-level structure
1113

14+
Two keys: `checks` selects which checks run, `rules` tunes individual ones.
15+
1216
```json
1317
{
14-
"checks": { ... },
15-
"wan": { ... },
16-
"vrf": { ... },
17-
"wireless": { ... },
18-
"rack_placement": { ... },
19-
"poe": { ... }
18+
"checks": {},
19+
"rules": {
20+
"interface-vrf": {},
21+
"private-ip-vrf": {},
22+
"wireless-normalization": {},
23+
"rack-placement": {},
24+
"poe-power": {}
25+
}
2026
}
2127
```
2228

29+
Every key under `rules` is a **check ID** — the same string `checks.enabled` / `checks.disabled`
30+
take, and the same one [CHECKS.md](CHECKS.md) documents each check under. Only the five checks above
31+
read rules; the other thirteen have no knobs.
32+
2333
---
2434

2535
## `checks`
@@ -39,7 +49,8 @@ Controls which checks are included in a run.
3949
| `disabled` | `string[]` | `[]` | Check IDs to skip. Applied after `enabled`. |
4050

4151
`enabled` and `disabled` can be used together: `enabled` acts as an allowlist, then `disabled`
42-
removes specific entries from that list.
52+
removes specific entries from that list. An unrecognized ID in either list is a fatal error, so a
53+
typo here fails the run rather than silently skipping a check.
4354

4455
Valid check IDs: `required-device-fields`, `device-locations`, `parent-placement`, `rack-placement`,
4556
`device-type-drift`, `honeypots`, `wireless-normalization`, `poe-power`, `interface-vrf`,
@@ -48,53 +59,60 @@ Valid check IDs: `required-device-fields`, `device-locations`, `parent-placement
4859

4960
---
5061

51-
## `wan`
62+
## `rules.interface-vrf`
5263

53-
Identifies which device roles are considered WAN-side. Devices in these roles are excluded from VRF
54-
and interface checks that do not apply to ISP-managed equipment.
64+
Controls the `interface-vrf` check, including which device roles are treated as WAN-side. Devices
65+
in those roles — and interfaces cabled to them — are exempt from the VRF requirement, since
66+
ISP-managed equipment sits outside the routing contexts NetBox models.
5567

5668
```json
57-
"wan": {
58-
"device_roles": ["ISP Equipment"]
69+
"rules": {
70+
"interface-vrf": {
71+
"wan_device_roles": ["ISP Equipment"],
72+
"require_on_interfaces": true
73+
}
5974
}
6075
```
6176

62-
| Field | Type | Default | Description |
63-
| -------------- | ---------- | ------------------- | --------------------------------------- |
64-
| `device_roles` | `string[]` | `["ISP Equipment"]` | Device role names to treat as WAN-side. |
77+
| Field | Type | Default | Description |
78+
| ----------------------- | ---------- | ------------------- | ------------------------------------------------------------------------------ |
79+
| `wan_device_roles` | `string[]` | `["ISP Equipment"]` | Device role names to treat as WAN-side. |
80+
| `require_on_interfaces` | `bool` | `true` | Flag in-use interfaces that have no VRF assigned (excluding WAN-side devices). |
6581

6682
---
6783

68-
## `vrf`
84+
## `rules.private-ip-vrf`
6985

70-
Controls VRF enforcement across the `interface-vrf` and `private-ip-vrf` checks.
86+
Controls the `private-ip-vrf` check.
7187

7288
```json
73-
"vrf": {
74-
"require_on_private_ips": true,
75-
"require_on_public_ips": false,
76-
"require_on_interfaces": true
89+
"rules": {
90+
"private-ip-vrf": {
91+
"require_on_private_ips": true,
92+
"require_on_public_ips": false
93+
}
7794
}
7895
```
7996

80-
| Field | Type | Default | Description |
81-
| ------------------------ | ------ | ------- | ------------------------------------------------------------------------------ |
82-
| `require_on_private_ips` | `bool` | `true` | Flag RFC1918 IP addresses that have no VRF assigned. |
83-
| `require_on_public_ips` | `bool` | `false` | Also flag public IP addresses that have no VRF assigned. |
84-
| `require_on_interfaces` | `bool` | `true` | Flag in-use interfaces that have no VRF assigned (excluding WAN-side devices). |
97+
| Field | Type | Default | Description |
98+
| ------------------------ | ------ | ------- | ---------------------------------------------------------- |
99+
| `require_on_private_ips` | `bool` | `true` | Flag private IP addresses that have no VRF assigned. |
100+
| `require_on_public_ips` | `bool` | `false` | Also flag public IP addresses that have no VRF assigned. |
85101

86102
---
87103

88-
## `wireless`
104+
## `rules.wireless-normalization`
89105

90106
Controls the `wireless-normalization` check.
91107

92108
```json
93-
"wireless": {
94-
"suppress_if_connected_wired_interface_is_complete": true,
95-
"require_mode": true,
96-
"require_untagged_vlan": true,
97-
"require_primary_mac": true
109+
"rules": {
110+
"wireless-normalization": {
111+
"suppress_if_connected_wired_interface_is_complete": true,
112+
"require_mode": true,
113+
"require_untagged_vlan": true,
114+
"require_primary_mac": true
115+
}
98116
}
99117
```
100118

@@ -107,14 +125,16 @@ Controls the `wireless-normalization` check.
107125

108126
---
109127

110-
## `rack_placement`
128+
## `rules.rack-placement`
111129

112130
Controls the `rack-placement` check.
113131

114132
```json
115-
"rack_placement": {
116-
"exempt_child_devices": true,
117-
"exempt_device_tags": ["0u-rack-device"]
133+
"rules": {
134+
"rack-placement": {
135+
"exempt_child_devices": true,
136+
"exempt_device_tags": ["0u-rack-device"]
137+
}
118138
}
119139
```
120140

@@ -123,17 +143,23 @@ Controls the `rack-placement` check.
123143
| `exempt_child_devices` | `bool` | `true` | Skip the check for devices that are installed inside a parent device. Child devices do not occupy their own rack unit. |
124144
| `exempt_device_tags` | `string[]` | `["0u-rack-device"]` | Devices carrying any of these tags are exempt from U position and face requirements. Use this for zero-U items such as PDUs, patch panels, or cable managers. |
125145

146+
`exempt_device_tags` **replaces** the default rather than extending it, so keep `0u-rack-device` in
147+
the list when adding a tag. Entries are matched against tag slugs, trimmed of surrounding
148+
whitespace.
149+
126150
---
127151

128-
## `poe`
152+
## `rules.poe-power`
129153

130154
Controls the `poe-power` check.
131155

132156
```json
133-
"poe": {
134-
"check_powered_device_supply": true,
135-
"require_pse_mode_on_peer": true,
136-
"unknown_type_policy": "fail"
157+
"rules": {
158+
"poe-power": {
159+
"check_powered_device_supply": true,
160+
"require_pse_mode_on_peer": true,
161+
"unknown_type_policy": "fail"
162+
}
137163
}
138164
```
139165

@@ -143,6 +169,10 @@ Controls the `poe-power` check.
143169
| `require_pse_mode_on_peer` | `bool` | `true` | Flag a finding when a PD interface is connected to a peer that is not marked as PSE mode. |
144170
| `unknown_type_policy` | `string` | `"fail"` | How to handle a PD or PSE with no PoE type set. `"fail"` flags it as a finding; `"ignore"` skips silently. |
145171

172+
`unknown_type_policy` is the one value validated on load: anything other than `"fail"`, `"ignore"`,
173+
or the empty string aborts the run rather than falling back to a default. Case and surrounding
174+
whitespace are normalized.
175+
146176
### PoE type hierarchy
147177

148178
The check uses this ordering to determine whether a supply is sufficient for a demand:
@@ -165,28 +195,33 @@ A supply of type N satisfies any demand of type ≤ N.
165195
"checks": {
166196
"disabled": ["device-type-drift"]
167197
},
168-
"wan": {
169-
"device_roles": ["ISP Equipment", "ISP Router"]
170-
},
171-
"vrf": {
172-
"require_on_private_ips": true,
173-
"require_on_public_ips": false,
174-
"require_on_interfaces": true
175-
},
176-
"wireless": {
177-
"suppress_if_connected_wired_interface_is_complete": true,
178-
"require_mode": true,
179-
"require_untagged_vlan": true,
180-
"require_primary_mac": true
181-
},
182-
"rack_placement": {
183-
"exempt_child_devices": true,
184-
"exempt_device_tags": ["0u-rack-device", "wall-mount"]
185-
},
186-
"poe": {
187-
"check_powered_device_supply": true,
188-
"require_pse_mode_on_peer": true,
189-
"unknown_type_policy": "ignore"
198+
"rules": {
199+
"interface-vrf": {
200+
"wan_device_roles": ["ISP Equipment", "ISP Router"],
201+
"require_on_interfaces": true
202+
},
203+
"private-ip-vrf": {
204+
"require_on_private_ips": true,
205+
"require_on_public_ips": false
206+
},
207+
"wireless-normalization": {
208+
"suppress_if_connected_wired_interface_is_complete": true,
209+
"require_mode": true,
210+
"require_untagged_vlan": true,
211+
"require_primary_mac": true
212+
},
213+
"rack-placement": {
214+
"exempt_child_devices": true,
215+
"exempt_device_tags": ["0u-rack-device", "wall-mount"]
216+
},
217+
"poe-power": {
218+
"check_powered_device_supply": true,
219+
"require_pse_mode_on_peer": true,
220+
"unknown_type_policy": "ignore"
221+
}
190222
}
191223
}
192224
```
225+
226+
The repo's own policy file, [`netbox_audit.config.json`](../../netbox_audit.config.json), is the
227+
same shape with every value at its default.

0 commit comments

Comments
 (0)