Code-review follow-up on bbea38e. Two small cleanups, no behavior change:
- plugins/module_utils/netbird_api.py: revert update_nameserver_group's
PUT-body construction for `domains` back to the partial-update pattern
(`if domains is not None`) used by every other update_* method. The
helper is now a pure HTTP primitive again, so future modules can rely
on consistent partial-update semantics for this surface.
- plugins/modules/netbird_dns.py: coerce `domains` and `groups` from
None to [] at the module layer, right before building update_params
and calling the API helper. This keeps the null-heal intent local to
the DNS module (where the dashboard-crash reason lives) and applies
symmetrically to both list fields, closing the asymmetry between
nsgroup_needs_update (already symmetric) and the PUT body.
Net effect is identical to bbea38e: installer-seeded `domains: null`
still heals to `[]` on apply, the dashboard still loads, the sister
repo's smoke/extended-CRUD assertions still pass. Only the layer at
which the coercion lives has moved.
Coexists cleanly with open PR #33 (fix/strict-name-resolution): both
branches touch roles/configure/tasks/main.yml at non-overlapping hunks.
A self-hosted NetBird dashboard crashed when loading the DNS → Nameservers
page because the installer-default "Public DNS" nameserver group was stored
with `domains: null` and the dashboard frontend does not null-guard that
field. The collection's apply cycle was preserving the null state rather
than healing it.
Four layers of fix:
- roles/export/templates/export/dns/nameservers.yml.j2 — always emit
`domains: []` on export instead of omitting the key when the API
returned null. Ensures the exported YAML carries an explicit list
into the apply cycle.
- roles/configure/tasks/main.yml — coerce `item.domains is null` to
`[]` via `default([], true)` before passing to the module, so that
a hand-edited YAML with `domains: null` still produces `[]`.
- plugins/module_utils/netbird_api.py — `update_nameserver_group` now
always includes `domains` in the PUT body and coerces `None → []`.
Previously it skipped the field on `None`, which preserved the
backend's null state.
- plugins/modules/netbird_dns.py — `nsgroup_needs_update` now treats
backend `domains: null` (or `groups: null`) as a heal-eligible change
against a list-valued desired state, so a PUT fires to coerce the
field. Previously it used `current.get('domains') or []` which
silently equated null with [] and returned "no change".
With these together, a `make safe-apply-test` cycle heals the
installer-seeded null state, and the dashboard loads. Verified against
netbird.cybersunset.net.
Two related crashes when a nameserver group has no domains or groups:
the API stores unset fields as null (not []), but the export template
and netbird_dns module both assumed list.
- roles/export/templates/export/dns/nameservers.yml.j2: add
`is not none` guards on ns.groups and ns.domains. Matches the pattern
already used on ns.description (line 14) and across zones.yml.j2.
Without this, `None | length` crashes the export template and blocks
every test that does a full export (iac, roundtrip, strict).
- plugins/modules/netbird_dns.py: in nsgroup_needs_update, use
`current.get('domains') or []` instead of `current.get('domains', [])`.
The .get default only applies when the key is missing, not when the
value is None. Without this, `set(None)` crashes when diffing against
a live API group that has `domains: null`.
Verified on live self-hosted NetBird: iac-selfhosted, roundtrip-selfhosted,
and strict-selfhosted now all pass (previously failed at the DNS export
or DNS update step).
list_dns_zone_records() returns null for zones with no records,
causing TypeError when iterating. Same pattern fixed in #20 for
other modules — this one was missed.
Changes module behavior so that omitting auto_groups, peers, or
similar list fields on update preserves the existing values instead
of wiping them to [].
Modules changed:
- netbird_setup_key: auto_groups default [] -> None, preserve on update
- netbird_group: peers default [] -> None, preserve on update,
normalize peer dicts to IDs
- netbird_user: auto_groups default [] -> None, preserve on update
Also:
- Reimplement get_current_user() since /api/users/me does not exist.
Now lists users and matches by is_current flag, with explicit error
on multi-user deployments without the flag.
- Configure role: use default(omit) for setup key auto_groups so
the module-level preservation works through the role.
- Configure role: conditional auto_groups resolution (skip when
auto_groups not defined in YAML config).
- Configure role: DNS zone distribution_groups now handles both
group names and raw IDs (falls back to original value when not
found in group_ids map).
- Updated DOCUMENTATION strings to remove stale default: [] and
document preservation behavior.
- Add (items or []) to all find_*_by_* functions across 12 modules.
The NetBird API returns null for empty collections, causing TypeError
when iterating. Affects find_group_by_name, find_policy_by_name,
find_setup_key_by_name, find_user_by_email, find_user_by_name,
find_network_by_name, find_route_by_network_id, find_nsgroup_by_name,
find_zone_by_name, find_posture_check_by_name, find_idp_by_name,
find_invite_by_email, find_token_by_name.
- Add (or []) to sync_routers and sync_resources in netbird_network
for the same reason (new networks have null router/resource lists).
- Fix export role network enrichment: .get('json', []) does not work
on Ansible uri module results. Changed to .json | default([]).
- Fix set_fact parsing in export and configure roles: {{ result | to_json }}
produces a JSON string, not a list. Added | from_json so downstream
loop directives receive proper lists (3 occurrences in configure,
1 in export).
- Use urllib.parse.urlencode for query parameter encoding instead of
raw f-string interpolation to prevent URL breakage with special chars.
Jack Carter
Tomas Kloda