T8220: Firewall Zone Policy support by omnom62 · Pull Request #447 · vyos/vyos.vyos

omnom62 · 2026-02-10T12:40:31Z

Change Summary

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Code style update (formatting, renaming)
Refactoring (no functional changes)
Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
Other (please describe):

Related Task(s)

https://vyos.dev/T8220

Related PR(s)

Component(s) name

vyos_firewall_global

Proposed changes

VyOS Ansible module that implements Firewall Zone Policy.

Provide new functionality in the VyOS Ansible module that VyOS doesn't currently support. Our module distinguishes between group name and zone name, but does not set the latter.

How to test

Test results

Unit tests are successful:
`============================= test session starts ==============================
platform linux -- Python 3.13.0, pytest-8.3.3, pluggy-1.5.0 -- /usr/bin/python3.13
rootdir: /root/ansible_collections/vyos/vyos
configfile: ../../../ansible/test/lib/ansible_test/_data/pytest/config/default.ini
plugins: xdist-3.6.1, mock-3.14.0
created: 8/8 workers
8 workers [6 items]

scheduling tests via LoadScheduling

tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_deleted
tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_merged
tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_replaced_idem
tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_replaced
tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_02_replaced
tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_merged_idem
[gw2] [ 16%] PASSED tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_merged_idem
[gw0] [ 33%] PASSED tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_deleted
[gw3] [ 50%] PASSED tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_replaced
[gw1] [ 66%] PASSED tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_merged
[gw5] [ 83%] PASSED tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_02_replaced
[gw4] [100%] PASSED tests/unit/modules/network/vyos/test_vyos_firewall_global.py::TestVyosFirewallGlobalModule::test_vyos_firewall_global_set_01_replaced_idem

generated xml file: /root/ansible_collections/vyos/vyos/tests/output/junit/python3.13-controller-units.xml -
============================== 6 passed in 2.36s ===============================
`
Sanity tests passed
Unit tests passed

Tested against VyOS versions:

1.5q2

Checklist:

I have read the CONTRIBUTING document
I have linked this PR to one or more Phabricator Task(s)
I have run the ansible sanity and unit tests
My commit headlines contain a valid Task id
My change requires a change to the documentation
I have updated the documentation accordingly
I have added unit tests to cover my changes
I have added a file to changelogs/fragments to describe the changes

gaige · 2026-03-08T22:47:35Z

The PR here is pretty devoid of information, can you fix that?

omnom62 · 2026-03-09T10:36:05Z

The PR here is pretty devoid of information, can you fix that?

@gaige - I tried and added more information - please check

gaige

Looks good. A couple of small changes (removal of commented out code).

gaige · 2026-04-14T17:53:19Z

        :rtype: dictionary
        :returns: The generated config
        """
+        # conf = "\n".join(


Is this ready for removal?

gaige · 2026-04-14T17:53:55Z

 set firewall global-options state-policy established action 'accept'
 set firewall global-options state-policy established log 'enable'
-set firewall global-options state-policy invalid action 'reject'
+# set firewall global-options state-policy invalid acti. .on 'reject'


Can we delete this?

Copilot

Pull request overview

Adds Firewall Zone Policy (“zone” / zone-based firewall) support to the vyos_firewall_global resource module, including parsing facts, rendering CLI commands, and extending unit/integration fixtures and documentation.

Changes:

Extend module argspec + documentation to accept config.zone definitions (interfaces, default action/log, intra-zone filtering, sources).
Add config rendering and facts parsing support for set firewall zone ... statements.
Update unit/integration tests + changelog fragment to cover the new behavior.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`plugins/modules/vyos_firewall_global.py`	Exposes `zone` option in module DOCUMENTATION.
`plugins/module_utils/network/vyos/argspec/firewall_global/firewall_global.py`	Adds `zone` structure to the argspec.
`plugins/module_utils/network/vyos/config/firewall_global/firewall_global.py`	Renders CLI commands for zone configuration (set/delete/replace paths).
`plugins/module_utils/network/vyos/facts/firewall_global/firewall_global.py`	Parses `set firewall zone ...` lines into facts.
`tests/unit/modules/network/vyos/test_vyos_firewall_global14.py`	Adds/updates unit expectations for zone-related commands.
`tests/integration/targets/vyos_firewall_global/vars/main.yaml`	Adds zone config to integration test input data.
`tests/integration/targets/vyos_firewall_global/vars/v1_4.yaml`	Updates expected command lists for v1.4 integration runs.
`tests/integration/targets/vyos_firewall_global/tests/cli/_parsed_config_1_4.cfg`	Updates parsed-config fixture with zone commands (currently contains invalid lines).
`docs/vyos.vyos.vyos_firewall_global_module.rst`	Updates published module docs to include `zone` options.
`changelogs/fragments/t882-fpz.yaml`	Adds changelog fragment announcing firewall zone support.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-17T12:08:27Z

+                                want,
+                                "name",
+                            ):
+                                commands.append(cmd + " D-" + want["name"] + " " + key)


In the delete/render phase (opr=False), this branch builds a command with a literal D- prefix and uses the raw key name (e.g. default_action) instead of the CLI token (default-action). This will generate invalid VyOS CLI (e.g. delete firewall zone D-<name> default_action). Please generate proper delete commands using the zone name + mapped CLI attribute (hyphenated), without the D- prefix, and ensure booleans vs valued attributes are handled correctly.

Suggested change

commands.append(cmd + " D-" + want["name"] + " " + key)

cli_attr = key.replace("_", "-")

if isinstance(val, bool):

commands.append(

cmd + " " + want["name"] + " " + cli_attr,

)

else:

commands.append(

cmd + " " + want["name"] + " " + cli_attr,

)

Copilot · 2026-04-17T12:08:28Z

+                    )
+            elif not opr and not have:
+                commands.append(
+                    cmd + " " + name + " intra-zone-filtering " + attr,


When deleting intra_zone_filtering (e.g. in state=replaced where the target zone removes this block), the not opr and not have branch appends ... intra-zone-filtering intra_zone_filtering, which is not a valid path. This should delete the intra-zone-filtering node (or the specific leaf keys) using the correct CLI syntax, not the Python attribute name.

Suggested change

cmd + " " + name + " intra-zone-filtering " + attr,

cmd + " " + name + " intra-zone-filtering",

Copilot · 2026-04-17T12:08:28Z

+        for item1 in want:
+            zone = item1["zone"]
+
+            if zone in have_index:
+                item2 = have_index[zone]
+
+                wfw = item1.get("firewall", {})
+                hfw = item2.get("firewall", {})
+                if wfw:
+                    if opr:
+                        sources = self._dict_diff(wfw, hfw)
+                        for source in sources:
+                            commands.append(
+                                cmd
+                                + " "
+                                + name
+                                + " from "
+                                + zone
+                                + " firewall "
+                                + source[0].replace("_", "-")
+                                + " "
+                                + source[1],
+                            )
+                    elif not opr and hfw:
+                        sources = self._dict_diff(wfw, hfw)
+                        for source in sources:
+                            commands.append(
+                                cmd
+                                + " "
+                                + name
+                                + " from "
+                                + zone
+                                + " firewall "
+                                + source[0].replace("_", "-")
+                                + " "
+                                + source[1],
+                            )
+                    elif not opr and not have:
+                        sources = self._dict_diff(wfw, hfw)
+                        for source in sources:
+                            commands.append(
+                                cmd + " " + name + " from " + zone,
+                            )
+            elif opr:
+                wfw = item1.get("firewall", {})


_render_sources() never generates delete commands for sources that exist in want but are absent from have when opr=False (e.g. removing a from <zone> entry during state=replaced). The elif opr: guard means removed sources are silently left behind. Add an elif not opr: path for zone not in have_index to delete the entire from <zone> subtree (or relevant leaves) so replace/delete operations are correct.

Copilot · 2026-04-17T12:08:28Z

+        # conf = "\n".join(
+        #     filter(
+        #         lambda x: ("firewall ipv6-name" and "firewall name" not in x),
+        #         conf,
+        #     ),
+        # )
+
        conf = "\n".join(
            filter(
-                lambda x: ("firewall ipv6-name" and "firewall name" not in x),
+                lambda x: not (
+                    x.startswith("set firewall ipv6 name") or x.startswith("set firewall ipv6 name")
+                ),
                conf,
            ),
        )


This filter predicate looks incorrect: it checks the same prefix twice and the prefix itself (set firewall ipv6 name) doesn’t match typical VyOS firewall commands. As written it effectively does nothing and leaves commented-out dead code above. Please either remove this filtering entirely (preferred if no longer needed) or fix it to match the actual lines that must be excluded, with a test that demonstrates the need.

Copilot · 2026-04-17T12:08:29Z

+                        "sources": {
+                            "elements": "dict",
+                            "type": "list",
+                            "options": {
+                                "zone": {"type": "str"},
+                                "firewall": {
+                                    "type": "dict",
+                                    "options": {
+                                        "name": {"type": "str"},
+                                        "ipv6_name": {"type": "str"},
+                                    },
+                                },
+                            },
+                        },


sources entries are assumed to always include a zone key (e.g. _render_sources() indexes item["zone"]). In the argspec it’s currently optional, which can lead to runtime KeyError rather than a validation error. Mark sources.options.zone as required: true (and consider doing the same for any other required subkeys used with direct indexing).

Copilot · 2026-04-17T12:08:29Z

+# set firewall global-options state-policy invalid acti. .on 'reject'
 set firewall global-options syn-cookies 'enable'
 set firewall global-options twa-hazards-protection 'enable'
+set firewall global-options twa-hazards-protection 'enable'


This parsed-config fixture now comments out the state-policy invalid action 'reject' line and introduces a typo (acti. .on), which will break parsing/round-trip expectations. It also duplicates the twa-hazards-protection command. Please restore the correct set firewall global-options state-policy invalid action 'reject' line (uncommented) and remove unintended duplicates so the fixture reflects valid device output.

Suggested change

# set firewall global-options state-policy invalid acti. .on 'reject'

set firewall global-options syn-cookies 'enable'

set firewall global-options twa-hazards-protection 'enable'

set firewall global-options twa-hazards-protection 'enable'

set firewall global-options state-policy invalid action 'reject'

set firewall global-options syn-cookies 'enable'

set firewall global-options twa-hazards-protection 'enable'

Init commit for FZP work

9399595

omnom62 requested a review from a team as a code owner February 10, 2026 12:40

omnom62 requested review from andamasov, dmbaturin and gaige February 10, 2026 12:40

Merge branch 'main' into t8220_fzp

b7d9f08

omnom62 marked this pull request as draft February 10, 2026 12:41

omnom62 and others added 15 commits February 10, 2026 22:46

changelog

2a41b02

Sanity fixes for FZP

fe5a0f2

unit test temp fix

4ffd6e6

Mid-result facts for FZP

e4f365e

Mid result for FZP

59fd48a

Mid-results intra-zone-filtering for fzp

1ce37b6

Intra-zone filtering mid-results

41d8bb6

IZP processing

7b49e1b

FZP config mid-results

38e5c06

Minor fixes to 'from' processing

eeb66a7

Mid results for FZP FR

6c750f3

UAT FZP ready

d91aaca

Merge branch 'main' into t8220_fzp

63cbde7

Preparing FZP SIT

c22eef0

FZP SIT is ready

01595cc

omnom62 changed the title ~~WIP T8220 Firewall Zone Policy support~~ T8220 Firewall Zone Policy support Feb 24, 2026

omnom62 marked this pull request as ready for review February 24, 2026 11:59

sever-sever changed the title ~~T8220 Firewall Zone Policy support~~ T8220: Firewall Zone Policy support Feb 25, 2026

omnom62 added 2 commits March 5, 2026 20:47

Merge branch 'main' into t8220_fzp

82e56e0

Merge branch 'main' into t8220_fzp

4403c1c

Merge branch 'main' into t8220_fzp

c2ef18e

Merge branch 'main' into t8220_fzp

615b3ea

gaige requested changes Apr 14, 2026

View reviewed changes

omnom62 added 2 commits April 15, 2026 05:31

Merge branch 'main' into t8220_fzp

a4a16df

Merge branch 'main' into t8220_fzp

5a5c702

Copilot AI review requested due to automatic review settings April 17, 2026 12:02

Copilot started reviewing on behalf of omnom62 April 17, 2026 12:03 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Merge branch 'main' into t8220_fzp

9684dbe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

T8220: Firewall Zone Policy support#447

T8220: Firewall Zone Policy support#447
omnom62 wants to merge 24 commits intovyos:mainfrom
omnom62:t8220_fzp

omnom62 commented Feb 10, 2026 •

edited

Loading

Uh oh!

gaige commented Mar 8, 2026

Uh oh!

omnom62 commented Mar 9, 2026

Uh oh!

gaige left a comment

Uh oh!

gaige Apr 14, 2026

Uh oh!

gaige Apr 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Copilot AI Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

-                                commands.append(cmd + " D-" + want["name"] + " " + key)
+                                cli_attr = key.replace("_", "-")
+                                if isinstance(val, bool):
+                                    commands.append(
+                                        cmd + " " + want["name"] + " " + cli_attr,
+                                    )
+                                else:
+                                    commands.append(
+                                        cmd + " " + want["name"] + " " + cli_attr,
+                                    )

	cmd + " " + name + " intra-zone-filtering " + attr,
	cmd + " " + name + " intra-zone-filtering",

Conversation

omnom62 commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Summary

Types of changes

Related Task(s)

Related PR(s)

Component(s) name

Proposed changes

How to test

Test results

Tested against VyOS versions:

Checklist:

Uh oh!

gaige commented Mar 8, 2026

Uh oh!

omnom62 commented Mar 9, 2026

Uh oh!

gaige left a comment

Choose a reason for hiding this comment

Uh oh!

gaige Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

gaige Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

omnom62 commented Feb 10, 2026 •

edited

Loading