diff --git a/roles/base/filter_plugins/firewall_rules.py b/roles/base/filter_plugins/firewall_rules.py index 4bb2c9d..c0d1fe3 100644 --- a/roles/base/filter_plugins/firewall_rules.py +++ b/roles/base/filter_plugins/firewall_rules.py @@ -25,14 +25,24 @@ def _host_cidr(host, hostvars): def _resolve_source(frm, catalog, zones, hostvars, groups): - """Resolve a symbolic `from` to a sorted list of source CIDRs.""" + """Resolve a symbolic `from` to a sorted, non-empty list of source CIDRs. + + Resolution order: zone -> catalog service -> inventory group -> host. A name present + in more than one namespace resolves to the first match in that order. Resolving to + zero hosts (e.g. an empty group) is an error, not a silently empty rule. + """ if frm in zones: return [zones[frm]] if frm in catalog: - return sorted(_host_cidr(h, hostvars) - for h in _placement_hosts(catalog[frm], groups)) + hosts = _placement_hosts(catalog[frm], groups) + if not hosts: + raise ValueError(f"firewall source service '{frm}' resolves to no hosts") + return sorted(_host_cidr(h, hostvars) for h in hosts) if frm in groups: - return sorted(_host_cidr(h, hostvars) for h in groups[frm]) + hosts = groups[frm] + if not hosts: + raise ValueError(f"firewall source group '{frm}' has no members") + return sorted(_host_cidr(h, hostvars) for h in hosts) if frm in hostvars: return [_host_cidr(frm, hostvars)] raise ValueError(f"unresolvable firewall source '{frm}'") @@ -43,12 +53,15 @@ def resolve_firewall_rules(catalog, zones, inventory_hostname, hostvars, groups) catalog = catalog or {} zones = zones or {} groups = groups or {} + hostvars = hostvars or {} rules = [] for _name, entry in sorted(catalog.items()): if inventory_hostname not in _placement_hosts(entry, groups): continue for ing in entry.get("ingress", []): + if "from" not in ing or "port" not in ing: + raise ValueError(f"ingress entry missing 'from' or 'port': {ing!r}") rules.append({ "proto": ing.get("proto", "tcp"), "port": int(ing["port"]), diff --git a/tests/test_firewall_rules.py b/tests/test_firewall_rules.py index d45cdd6..fe91c22 100644 --- a/tests/test_firewall_rules.py +++ b/tests/test_firewall_rules.py @@ -71,3 +71,29 @@ def test_missing_ansible_host_raises(): "ingress": [{"from": "docker02", "port": 80, "proto": "tcp"}]}} with pytest.raises(ValueError): fr.resolve_firewall_rules(cat, ZONES, "docker01", {"docker01": {}, "docker02": {}}, GROUPS) + + +def test_hosts_list_placement(): + cat = {"svc": {"hosts": ["docker01", "docker02"], + "ingress": [{"from": "lan", "port": 9090, "proto": "tcp"}]}} + out = fr.resolve_firewall_rules(cat, ZONES, "docker01", HOSTVARS, GROUPS) + assert out == [{"proto": "tcp", "port": 9090, "sources": ["10.30.0.0/24"]}] + + +def test_proto_defaults_to_tcp(): + cat = {"svc": {"host": "docker01", "ingress": [{"from": "lan", "port": 80}]}} + out = fr.resolve_firewall_rules(cat, ZONES, "docker01", HOSTVARS, GROUPS) + assert out == [{"proto": "tcp", "port": 80, "sources": ["10.30.0.0/24"]}] + + +def test_empty_group_source_raises(): + cat = {"svc": {"host": "docker01", + "ingress": [{"from": "empty_grp", "port": 80, "proto": "tcp"}]}} + with pytest.raises(ValueError): + fr.resolve_firewall_rules(cat, ZONES, "docker01", HOSTVARS, {"empty_grp": []}) + + +def test_ingress_missing_port_raises(): + cat = {"svc": {"host": "docker01", "ingress": [{"from": "lan"}]}} + with pytest.raises(ValueError): + fr.resolve_firewall_rules(cat, ZONES, "docker01", HOSTVARS, GROUPS)