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.
The configure role's name resolver previously used dict.get(name, name)
fallbacks, so any unresolvable reference in YAML config was silently
passed through to the API as if it were a valid ID. On the server side,
the reference was discarded — producing half-applied policies whose
rules ended up with sources: null (or destinations/auto_groups wiped).
Common failure mode: a typo in a group name ships a broken firewall
rule that reports changed=true but has no source constraint.
This change makes the resolver strict:
- Unknown name that is ALSO not an existing ID -> AnsibleFilterError
with a message naming the resource, field, and unresolved value.
- Known name -> resolves to ID (unchanged).
- Value that matches an existing ID -> passes through (preserves
backward compatibility for YAML configs that use raw IDs).
Applies to groups in policy sources/destinations, setup_key auto_groups,
policy source_posture_checks, peer refs in policy source_resource/
destination_resource, and network router.peer / resource.groups.
Also: the /api/networks/{id}/routers endpoint returns router.peer as
the peer's HOSTNAME rather than its canonical name. The peer_ids map
(configure role) and peer_id_map (export role) now include hostname
aliases so round-trips stay idempotent.
Files:
- plugins/filter/netbird_resolve.py: strict _resolve_names,
_resolve_resource_ref, new _resolve_peer_id helper
- roles/configure/tasks/main.yml: peer_ids merges hostname->id
aliases with name->id
- roles/export/tasks/main.yml: peer_id_map merges hostname->name
aliases with id->name
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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).
PR#31 fixed export silently dropping peer-based policy sources, but
collapsed them into sources: [peer_name]. The configure role resolver
only maps sources against group_ids, so peer names fell through
unresolved. The netbird_policy module then sent literal peer names to
the API, which silently accepted them and wiped sourceResource --
turning the exported YAML into a policy with no source at all (reported
as changed: true).
Changes:
- Export template emits source_resource: {name, type: peer} for
peer-type resources (keeps host/domain/subnet as {id, type}).
- _resolve_policy in netbird_resolve accepts peer_ids and converts
{name, type: peer} to {id, type: peer}; non-peer refs pass through.
- configure/tasks/main.yml passes peer_ids to the policy resolve call
(the map is already built on line 214 for networks).
Roundtrip verified on live NetBird: sourceResource preserved; apply
reports ok (idempotent) instead of false changed: true.
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.
Misha BraginandGitHub
Jack Carter
Tomas Kloda