mirror of
https://github.com/netbirdio/ansible-netbird.git
synced 2026-05-22 18:43:36 -07:00
fix: raise on unresolved group/peer/posture-check references instead of silent drop
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>
This commit is contained in:
Jack Carter
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
39066a36c2
commit
27595eee61
@@ -7,63 +7,128 @@
|
||||
from __future__ import absolute_import, division, print_function
|
||||
__metaclass__ = type
|
||||
|
||||
from ansible.errors import AnsibleFilterError
|
||||
|
||||
def _resolve_names(names, id_map):
|
||||
"""Resolve a list of names using an ID map, falling back to original."""
|
||||
|
||||
def _resolve_names(names, id_map, kind='group', context=''):
|
||||
"""Resolve a list of names using an ID map.
|
||||
|
||||
A value passes through unchanged if it is already a known API ID (i.e.
|
||||
appears as a value in id_map), which keeps YAML configs that reference
|
||||
groups/checks by raw ID working. An unknown value (neither a known name
|
||||
nor a known ID) raises AnsibleFilterError so that silent typos don't
|
||||
produce half-applied resources.
|
||||
"""
|
||||
if not names:
|
||||
return []
|
||||
return [id_map.get(name, name) for name in names]
|
||||
known_ids = set(id_map.values())
|
||||
resolved = []
|
||||
for name in names:
|
||||
if name in id_map:
|
||||
resolved.append(id_map[name])
|
||||
elif name in known_ids:
|
||||
resolved.append(name)
|
||||
else:
|
||||
raise AnsibleFilterError(
|
||||
"Unknown %s '%s'%s. It is neither a known %s name nor an "
|
||||
"existing %s ID. Check for typos or a missing %s definition."
|
||||
% (kind, name, (" in " + context) if context else '',
|
||||
kind, kind, kind)
|
||||
)
|
||||
return resolved
|
||||
|
||||
|
||||
def _resolve_setup_key(sk, group_ids):
|
||||
"""Resolve a single setup key's auto_groups."""
|
||||
result = dict(sk)
|
||||
if 'auto_groups' in sk:
|
||||
result['auto_groups'] = _resolve_names(sk['auto_groups'], group_ids)
|
||||
result['auto_groups'] = _resolve_names(
|
||||
sk['auto_groups'], group_ids,
|
||||
kind='group',
|
||||
context="setup key '%s' auto_groups" % sk.get('name', '<unnamed>'),
|
||||
)
|
||||
return result
|
||||
|
||||
|
||||
def _resolve_resource_ref(resource, peer_ids):
|
||||
def _resolve_resource_ref(resource, peer_ids, context=''):
|
||||
"""Resolve {name, type: peer} to {id, type: peer} using peer_ids map.
|
||||
|
||||
Non-peer resources (host/domain/subnet) pass through unchanged since their
|
||||
IDs in exported YAML are already concrete API IDs.
|
||||
|
||||
Raises AnsibleFilterError if a peer name cannot be resolved to an ID.
|
||||
"""
|
||||
if not isinstance(resource, dict):
|
||||
return resource
|
||||
if resource.get('type') == 'peer' and 'name' in resource and 'id' not in resource:
|
||||
name = resource['name']
|
||||
return {'id': peer_ids.get(name, name), 'type': 'peer'}
|
||||
known_ids = set(peer_ids.values())
|
||||
if name in peer_ids:
|
||||
return {'id': peer_ids[name], 'type': 'peer'}
|
||||
if name in known_ids:
|
||||
return {'id': name, 'type': 'peer'}
|
||||
raise AnsibleFilterError(
|
||||
"Unknown peer '%s'%s. It is neither a known peer name nor an "
|
||||
"existing peer ID." % (name, (" in " + context) if context else '')
|
||||
)
|
||||
return resource
|
||||
|
||||
|
||||
def _resolve_peer_id(peer_ref, peer_ids, context=''):
|
||||
"""Resolve a single peer name or ID to an ID. Raises on unknown."""
|
||||
if peer_ref is None:
|
||||
return peer_ref
|
||||
known_ids = set(peer_ids.values())
|
||||
if peer_ref in peer_ids:
|
||||
return peer_ids[peer_ref]
|
||||
if peer_ref in known_ids:
|
||||
return peer_ref
|
||||
raise AnsibleFilterError(
|
||||
"Unknown peer '%s'%s. It is neither a known peer name nor an "
|
||||
"existing peer ID." % (peer_ref, (" in " + context) if context else '')
|
||||
)
|
||||
|
||||
|
||||
def _resolve_policy(policy, group_ids, posture_check_ids, peer_ids=None):
|
||||
"""Resolve a single policy's group, posture check, and peer references."""
|
||||
peer_ids = peer_ids or {}
|
||||
result = dict(policy)
|
||||
policy_name = policy.get('name', '<unnamed>')
|
||||
|
||||
if 'source_posture_checks' in policy:
|
||||
result['source_posture_checks'] = _resolve_names(
|
||||
policy.get('source_posture_checks', []), posture_check_ids
|
||||
policy.get('source_posture_checks', []),
|
||||
posture_check_ids,
|
||||
kind='posture_check',
|
||||
context="policy '%s' source_posture_checks" % policy_name,
|
||||
)
|
||||
|
||||
if 'rules' in policy:
|
||||
resolved_rules = []
|
||||
for rule in policy.get('rules', []):
|
||||
rule_name = rule.get('name', '<unnamed>')
|
||||
resolved_rule = dict(rule)
|
||||
resolved_rule['sources'] = _resolve_names(
|
||||
rule.get('sources', []), group_ids
|
||||
rule.get('sources', []),
|
||||
group_ids,
|
||||
kind='group',
|
||||
context="policy '%s' rule '%s' sources" % (policy_name, rule_name),
|
||||
)
|
||||
resolved_rule['destinations'] = _resolve_names(
|
||||
rule.get('destinations', []), group_ids
|
||||
rule.get('destinations', []),
|
||||
group_ids,
|
||||
kind='group',
|
||||
context="policy '%s' rule '%s' destinations" % (policy_name, rule_name),
|
||||
)
|
||||
if rule.get('source_resource') is not None:
|
||||
resolved_rule['source_resource'] = _resolve_resource_ref(
|
||||
rule['source_resource'], peer_ids
|
||||
rule['source_resource'], peer_ids,
|
||||
context="policy '%s' rule '%s' source_resource" % (policy_name, rule_name),
|
||||
)
|
||||
if rule.get('destination_resource') is not None:
|
||||
resolved_rule['destination_resource'] = _resolve_resource_ref(
|
||||
rule['destination_resource'], peer_ids
|
||||
rule['destination_resource'], peer_ids,
|
||||
context="policy '%s' rule '%s' destination_resource" % (policy_name, rule_name),
|
||||
)
|
||||
resolved_rules.append(resolved_rule)
|
||||
result['rules'] = resolved_rules
|
||||
@@ -74,14 +139,19 @@ def _resolve_policy(policy, group_ids, posture_check_ids, peer_ids=None):
|
||||
def _resolve_network(network, group_ids, peer_ids):
|
||||
"""Resolve a single network's group, peer, and peer_group references."""
|
||||
result = dict(network)
|
||||
network_name = network.get('name', '<unnamed>')
|
||||
|
||||
if 'resources' in network:
|
||||
resolved_resources = []
|
||||
for resource in network.get('resources', []):
|
||||
resource_name = resource.get('name', resource.get('address', '<unnamed>'))
|
||||
resolved = dict(resource)
|
||||
if 'groups' in resource:
|
||||
resolved['groups'] = _resolve_names(
|
||||
resource.get('groups', []), group_ids
|
||||
resource.get('groups', []),
|
||||
group_ids,
|
||||
kind='group',
|
||||
context="network '%s' resource '%s' groups" % (network_name, resource_name),
|
||||
)
|
||||
resolved_resources.append(resolved)
|
||||
result['resources'] = resolved_resources
|
||||
@@ -90,11 +160,17 @@ def _resolve_network(network, group_ids, peer_ids):
|
||||
resolved_routers = []
|
||||
for router in network.get('routers', []):
|
||||
resolved = dict(router)
|
||||
if 'peer' in router:
|
||||
resolved['peer'] = peer_ids.get(router['peer'], router['peer'])
|
||||
if 'peer' in router and router['peer'] is not None:
|
||||
resolved['peer'] = _resolve_peer_id(
|
||||
router['peer'], peer_ids,
|
||||
context="network '%s' router peer" % network_name,
|
||||
)
|
||||
if 'peer_groups' in router:
|
||||
resolved['peer_groups'] = _resolve_names(
|
||||
router.get('peer_groups', []), group_ids
|
||||
router.get('peer_groups', []),
|
||||
group_ids,
|
||||
kind='group',
|
||||
context="network '%s' router peer_groups" % network_name,
|
||||
)
|
||||
resolved_routers.append(resolved)
|
||||
result['routers'] = resolved_routers
|
||||
@@ -111,7 +187,12 @@ def netbird_resolve_ids(resource_list, resource_type, **kwargs):
|
||||
**kwargs: group_ids, peer_ids, posture_check_ids
|
||||
|
||||
Returns:
|
||||
list of resource dicts with names replaced by IDs
|
||||
list of resource dicts with names replaced by IDs.
|
||||
|
||||
Raises:
|
||||
AnsibleFilterError: if any referenced group/posture-check/peer name
|
||||
cannot be resolved to an ID. This prevents silent half-applies
|
||||
where typos produce policies with dropped references.
|
||||
"""
|
||||
if not isinstance(resource_list, list):
|
||||
return []
|
||||
@@ -138,17 +219,19 @@ def netbird_resolve_ids(resource_list, resource_type, **kwargs):
|
||||
return result
|
||||
|
||||
|
||||
def netbird_resolve_names(name_list, id_map):
|
||||
def netbird_resolve_names(name_list, id_map, kind='group', context=''):
|
||||
"""Resolve a simple list of names to IDs.
|
||||
|
||||
Used for inline resolution (e.g., DNS zone distribution_groups).
|
||||
Falls back to original name if not found in map.
|
||||
|
||||
Raises AnsibleFilterError if a name is neither a known key nor a known
|
||||
value (ID) in the map.
|
||||
"""
|
||||
if not isinstance(name_list, list):
|
||||
return []
|
||||
if not isinstance(id_map, dict):
|
||||
return name_list
|
||||
return _resolve_names(name_list, id_map)
|
||||
return _resolve_names(name_list, id_map, kind=kind, context=context)
|
||||
|
||||
|
||||
class FilterModule(object):
|
||||
|
||||
@@ -209,9 +209,15 @@
|
||||
resource: peers
|
||||
register: api_peers
|
||||
|
||||
- name: Build peer name→ID map
|
||||
- name: Build peer name→ID map (hostname aliases included)
|
||||
# The /api/networks/{id}/routers endpoint returns router.peer as the
|
||||
# peer's HOSTNAME (not name). To let exported network configs resolve
|
||||
# cleanly, include hostname→id pairs as aliases for name→id.
|
||||
# The name map is combined last so peer.name takes precedence on collision.
|
||||
ansible.builtin.set_fact:
|
||||
peer_ids: "{{ dict(api_peers.data | map(attribute='name') | zip(api_peers.data | map(attribute='id'))) }}"
|
||||
peer_ids: >-
|
||||
{{ dict(api_peers.data | map(attribute='hostname') | zip(api_peers.data | map(attribute='id')))
|
||||
| combine(dict(api_peers.data | map(attribute='name') | zip(api_peers.data | map(attribute='id')))) }}
|
||||
|
||||
- name: Display ID maps
|
||||
ansible.builtin.debug:
|
||||
|
||||
@@ -175,7 +175,14 @@
|
||||
ansible.builtin.set_fact:
|
||||
group_id_map: "{{ dict(groups_data.data | map(attribute='id') | zip(groups_data.data | map(attribute='name'))) }}"
|
||||
posture_check_id_map: "{{ dict(posture_checks_data.data | map(attribute='id') | zip(posture_checks_data.data | map(attribute='name'))) }}"
|
||||
peer_id_map: "{{ dict(peers_data.data | map(attribute='id') | zip(peers_data.data | map(attribute='name'))) }}"
|
||||
# peer_id_map: lookup both peer.id AND peer.hostname -> peer.name.
|
||||
# The networks router API returns router.peer as hostname in some
|
||||
# cases, so we need to resolve either form to the canonical peer name
|
||||
# when exporting. peer.id map is combined last so an ID always wins
|
||||
# on the rare chance a hostname collides with an ID.
|
||||
peer_id_map: >-
|
||||
{{ dict(peers_data.data | map(attribute='hostname') | zip(peers_data.data | map(attribute='name')))
|
||||
| combine(dict(peers_data.data | map(attribute='id') | zip(peers_data.data | map(attribute='name')))) }}
|
||||
|
||||
# =====================================================================
|
||||
# Write clean config files (ready to use with configure role)
|
||||
|
||||
Reference in New Issue
Block a user