Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions changelogs/fragments/fix-policy-resource-rule-empty-groups.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
bugfixes:
- >-
configure role - a policy rule targeting a resource
(``source_resource``/``destination_resource``) no longer fails with
``422 specify either destinations or destination resources, not both``
(https://github.com/netbirdio/ansible-netbird/issues/67). The resolver
stamped ``sources: []`` and ``destinations: []`` onto every rule, so a
rule defined only with the resource form reached the API carrying both
a resource reference and an empty group list, which the API rejects.
Group references are now only resolved (and only present) when the rule
actually defines them.
39 changes: 25 additions & 14 deletions plugins/filter/netbird_resolve.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,31 @@ def _resolve_policy(policy, group_ids, posture_check_ids, peer_ids=None, missing
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,
kind='group',
context="policy '%s' rule '%s' sources" % (policy_name, rule_name),
missing=missing,
)
resolved_rule['destinations'] = _resolve_names(
rule.get('destinations', []),
group_ids,
kind='group',
context="policy '%s' rule '%s' destinations" % (policy_name, rule_name),
missing=missing,
)
# A rule targets either groups (sources/destinations) or a single
# resource (source_resource/destination_resource); the API rejects
# a rule carrying both, even when the group list is empty. Resolve
# group references only when present, and drop explicitly empty
# lists -- an empty group list is never valid to the API.
if rule.get('sources'):
resolved_rule['sources'] = _resolve_names(
rule.get('sources', []),
group_ids,
kind='group',
context="policy '%s' rule '%s' sources" % (policy_name, rule_name),
missing=missing,
)
else:
resolved_rule.pop('sources', None)
if rule.get('destinations'):
resolved_rule['destinations'] = _resolve_names(
rule.get('destinations', []),
group_ids,
kind='group',
context="policy '%s' rule '%s' destinations" % (policy_name, rule_name),
missing=missing,
)
else:
resolved_rule.pop('destinations', None)
if rule.get('source_resource') is not None:
resolved_rule['source_resource'] = _resolve_resource_ref(
rule['source_resource'], peer_ids,
Expand Down
150 changes: 150 additions & 0 deletions tests/unit/plugins/filter/test_netbird_resolve_policy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
# -*- coding: utf-8 -*-
# Copyright: (c) 2024-2026, NetBird and contributors
# GNU General Public License v3.0+ (see LICENSE or https://www.gnu.org/licenses/gpl-3.0.txt)
"""Unit tests for policy rule resolution in netbird_resolve.

Run via:
ansible-test units --docker default

A policy rule targets either groups (``sources``/``destinations``) or a
single resource (``source_resource``/``destination_resource``) — the API
rejects a rule that carries both, even when one side is an empty list.
_resolve_policy therefore must not inject ``sources: []`` or
``destinations: []`` into rules that only define the resource form.
"""

from __future__ import absolute_import, division, print_function
__metaclass__ = type

import pytest

from ansible.errors import AnsibleFilterError

from ansible_collections.community.ansible_netbird.plugins.filter.netbird_resolve import (
netbird_resolve_ids,
netbird_missing_refs,
)

GROUP_IDS = {
'All': 'd919v4e0tggg008irqbg',
'developers': 'grp-dev-001',
}

PEER_IDS = {
'server-1': 'peer-0001',
}


def resolve_one(policy, **kwargs):
kwargs.setdefault('group_ids', GROUP_IDS)
kwargs.setdefault('peer_ids', PEER_IDS)
return netbird_resolve_ids([policy], 'policy', **kwargs)[0]


class TestResourceTargetedRules:

def test_destination_resource_rule_gets_no_destinations_key(self):
policy = {
'name': 'TUI Traffic',
'rules': [{
'name': 'TUI SSH Traffic',
'sources': ['All'],
'destination_resource': {'id': 'd9ntt0e0tggg00au8b70', 'type': 'host'},
'protocol': 'tcp',
'ports': ['22'],
'action': 'accept',
}],
}
rule = resolve_one(policy)['rules'][0]
assert 'destinations' not in rule
assert rule['sources'] == ['d919v4e0tggg008irqbg']
assert rule['destination_resource'] == {'id': 'd9ntt0e0tggg00au8b70', 'type': 'host'}

def test_source_resource_rule_gets_no_sources_key(self):
policy = {
'name': 'peer-sourced',
'rules': [{
'name': 'r1',
'source_resource': {'name': 'server-1', 'type': 'peer'},
'destinations': ['developers'],
'action': 'accept',
}],
}
rule = resolve_one(policy)['rules'][0]
assert 'sources' not in rule
assert rule['source_resource'] == {'id': 'peer-0001', 'type': 'peer'}
assert rule['destinations'] == ['grp-dev-001']

def test_explicit_empty_destinations_is_dropped(self):
policy = {
'name': 'explicit-empty',
'rules': [{
'name': 'r1',
'sources': ['All'],
'destinations': [],
'destination_resource': {'id': 'res-1', 'type': 'host'},
'action': 'accept',
}],
}
rule = resolve_one(policy)['rules'][0]
assert 'destinations' not in rule

def test_explicit_empty_or_null_sources_is_dropped(self):
policy = {
'name': 'explicit-empty',
'rules': [{
'name': 'r1',
'sources': None,
'source_resource': {'id': 'res-1', 'type': 'host'},
'destinations': ['All'],
'action': 'accept',
}],
}
rule = resolve_one(policy)['rules'][0]
assert 'sources' not in rule

def test_non_peer_resource_ref_passes_through(self):
policy = {
'name': 'host-target',
'rules': [{
'name': 'r1',
'sources': ['developers'],
'destination_resource': {'id': 'res-123', 'type': 'domain'},
'action': 'accept',
}],
}
rule = resolve_one(policy)['rules'][0]
assert rule['destination_resource'] == {'id': 'res-123', 'type': 'domain'}


class TestGroupTargetedRules:

def test_group_rules_still_resolve_both_sides(self):
policy = {
'name': 'group-to-group',
'rules': [{
'name': 'r1',
'sources': ['All'],
'destinations': ['developers'],
'action': 'accept',
}],
}
rule = resolve_one(policy)['rules'][0]
assert rule['sources'] == ['d919v4e0tggg008irqbg']
assert rule['destinations'] == ['grp-dev-001']

def test_unknown_group_name_still_raises(self):
policy = {
'name': 'typo',
'rules': [{'name': 'r1', 'sources': ['no-such-group'], 'destinations': ['All'], 'action': 'accept'}],
}
with pytest.raises(AnsibleFilterError):
netbird_resolve_ids([policy], 'policy', group_ids=GROUP_IDS)

def test_unknown_group_name_still_collected_by_missing_refs(self):
policy = {
'name': 'typo',
'rules': [{'name': 'r1', 'sources': ['no-such-group'], 'destinations': ['All'], 'action': 'accept'}],
}
missing = netbird_missing_refs([policy], 'policy', group_ids=GROUP_IDS)
assert [m['name'] for m in missing] == ['no-such-group']
Loading