WIP T8323 vyos_nat module by omnom62 · Pull Request #451 · vyos/vyos.vyos

omnom62 · 2026-03-14T11:08:42Z

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)

Related PR(s)

Component(s) name

Proposed changes

How to test

Test results

Sanity tests passed
Unit tests passed

Tested against VyOS versions:

1.3.8
1.4-rolling-202201010100

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

Copilot

Pull request overview

This PR introduces a new (WIP) vyos_nat resource module to the vyos.vyos Ansible collection, including its argspec, config implementation skeleton, facts gathering, and parsing/rendering templates, along with runtime/README/docs/changelog updates.

Changes:

Added vyos_nat module entrypoint plus collection routing/docs/README/changelog entries.
Added NAT resource-module plumbing: argspec, config class, facts class, and rm_template parsers.
Removed an old/unused vrf.old config implementation file.

Reviewed changes

Copilot reviewed 11 out of 15 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`plugins/modules/vyos_nat.py`	New module entrypoint + DOCUMENTATION/EXAMPLES/RETURN for `vyos_nat`.
`plugins/module_utils/network/vyos/argspec/nat/nat.py`	Adds NAT argspec (currently not shaped like other resource argspecs).
`plugins/module_utils/network/vyos/config/nat/nat.py`	Adds NAT ResourceModule implementation (currently contains a hard `fail_json` debug stub + commented code).
`plugins/module_utils/network/vyos/facts/nat/nat.py`	Adds NAT facts gathering/parsing via `NatTemplate` (needs normalization + docstring fixes).
`plugins/module_utils/network/vyos/rm_templates/nat.py`	Adds NAT regex/Jinja template parsers (key mismatches vs argspec + quoting issues).
`plugins/module_utils/network/vyos/facts/facts.py`	Registers `nat` as a new resource facts subset.
`meta/runtime.yml`	Adds short-name redirect `nat -> vyos.vyos.vyos_nat`.
`docs/vyos.vyos.vyos_nat_module.rst`	Generated docs for the new module.
`changelogs/fragments/t8323-vyos_nat.yml`	Changelog fragment for the new module (currently categorized as `trivial`).
`README.md`	Adds `vyos_nat` to the module list.
`plugins/module_utils/network/vyos/config/vrf/vrf.old`	Removes old VRF config implementation file.

Copilot · 2026-04-17T12:42:07Z

+    def generate_commands(self):
+        """Generate configuration commands to send based on
+        want, have and desired state.
+        """
+        wantd = {}
+        haved = {}
+        wantd = deepcopy(self.want)
+        haved = deepcopy(self.have)
+
+        self._module.fail_json(msg={"want": wantd, "have": haved})
+
+    #     wantd = self._ntp_list_to_dict(self.want)


generate_commands() currently calls self._module.fail_json(...), which will make the module fail for every state except parsed/gathered and prevents any configuration changes from being applied. Please remove this debug fail_json and implement the normal want/have comparison + command generation (similar to other resource modules like ntp_global).

Copilot · 2026-04-17T12:42:07Z

+    #     wantd = self._ntp_list_to_dict(self.want)
+    #     haved = self._ntp_list_to_dict(self.have)
+
+    #     # if state is merged, merge want onto have and then compare
+    #     if self.state == "merged":
+    #         wantd = dict_merge(haved, wantd)
+
+    #     # if state is deleted, empty out wantd and set haved to wantd
+    #     if self.state == "deleted":
+    #         haved = {k: v for k, v in haved.items() if k in wantd or not wantd}
+    #         wantd = {}
+
+    #         commandlist = self._commandlist(haved)
+    #         servernames = self._servernames(haved)
+    #         # removing the servername and commandlist from the list after deleting it from haved
+    #         # iterate through the top-level items to delete
+    #         for k, have in haved.items():
+    #             if k not in wantd:
+    #                 for hk, hval in have.items():
+    #                     if hk == "allow_clients" and hk in commandlist:
+    #                         self.commands.append(
+    #                             self._tmplt.render({"": hk}, "allow_clients_delete", True),
+    #                         )
+    #                         commandlist.remove(hk)
+    #                     elif hk == "listen_addresses" and hk in commandlist:
+    #                         self.commands.append(
+    #                             self._tmplt.render({"": hk}, "listen_addresses_delete", True),
+    #                         )
+    #                         commandlist.remove(hk)
+    #                     elif hk == "server" and have["server"] in servernames:
+    #                         self._compareoverride(want={}, have=have)
+    #                         servernames.remove(have["server"])
+    #         # if everything is deleted add the delete command for {path} ntp
+    #         # this should be equiv: servernames == [] and commandlist == ["server"]:
+    #         if wantd == {} and haved != {}:
+    #             self.commands.append(
+    #                 self._tmplt.render({}, "service_delete", True),
+    #             )
+
+    #     # remove existing config for overridden and replaced
+    #     # Getting the list of the server names from haved
+    #     #   to avoid the duplication of overridding/replacing the servers
+    #     if self.state in ["overridden", "replaced"]:
+    #         commandlist = self._commandlist(haved)
+    #         servernames = self._servernames(haved)
+


This file still contains a large block of commented-out NTP-specific code. Keeping unrelated commented code makes the NAT resource module harder to maintain and review; please remove it (or replace it with NAT-specific command-generation logic/tests) before merging.

Copilot · 2026-04-17T12:42:07Z

+
+    argument_spec = {
+        "config": {
+            "type": "dict",
+            "nat": {
+                "type": "dict",
+                "options": {
+                    "cgnat": {
+                        "type": "dict",
+                        "options": {
+                            "log_allocation": {
+                                "type": "bool",
+                            },
+                            "pool": {
+                                "type": "dict",
+                                "options": {


The NAT argspec structure under config is not using the standard Ansible argument-spec shape (config: {type: dict, options: {...}}). As written, keys like nat, nat64, and nat66 are not under an options dict, which will prevent Ansible from validating/accepting those suboptions correctly. Please restructure config to use an options mapping (see argspec/ntp_global/ntp_global.py for the expected pattern).

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

+EXAMPLES = """
+# Using merged
+- name: Merge NAT source rule
+  vyos.vyos.vyos_nat:
+    config:
+      source:
+        rule:
+          - id: 100
+            description: "Outbound masquerade"
+    state: merged
+
+# Using gathered
+- name: Gather NAT config
+  vyos.vyos.vyos_nat:
+    state: gathered
+
+# Using deleted
+- name: Delete NAT config
+  vyos.vyos.vyos_nat:
+    state: deleted
+
+# Using replaced
+- name: Replace NAT config
+  vyos.vyos.vyos_nat:
+    config:
+      source:
+        rule:
+          - id: 100
+            description: "Replaced rule"
+    state: replaced
+
+# Using parsed
+- name: Parse NAT config
+  vyos.vyos.vyos_nat:
+    running_config: "{{ lookup('file', './nat_config.cfg') }}"
+    state: parsed
+
+# Using rendered
+- name: Render NAT config offline
+  vyos.vyos.vyos_nat:
+    config:
+      source:
+        rule:
+          - id: 100
+            description: "Rendered rule"
+    state: rendered
+"""


The module DOCUMENTATION/EXAMPLES are inconsistent with the argspec: DOCUMENTATION defines config.nat.* (and config.nat64/nat66), but the EXAMPLES use config: { source: ... } at the top level. This will confuse users and (depending on the final argspec) may not work at all; please update examples and/or the DOCUMENTATION options layout to match the actual module parameters.

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

+            "result": {
+                "{{ nat }}": {
+                    "cgnat": {
+                        "pool": {
+                            "external": [
+                                {
+                                    "name": "{{ name }}",
+                                    "ranges": ["{{ range }}"],
+                                    "seq": "{{ seq }}",
+                                },
+                            ],
+                        },
+                    },


The external CGNAT pool parser populates ranges (plural) and a seq field in the parsed result, but the module argspec/DOCUMENTATION defines the field as range and does not define seq. This mismatch will cause validate_config() to drop/empty the parsed data (and can break idempotency). Please align the parsed result keys with the argspec (and only emit fields that exist in the argspec).

Copilot · 2026-04-17T12:42:09Z

+author:
+- Evgeny Molotkov (@omnom62)
+notes:
+- Tested against VyOS 1.3.8, 1.4.2, the upcoming 1.5, and the rolling release of spring 2025


In notes, the first bullet does not end with a period. Option/notes text in module docs should be complete sentences ending with a period for consistency with the rest of the collection documentation.

Suggested change

- Tested against VyOS 1.3.8, 1.4.2, the upcoming 1.5, and the rolling release of spring 2025

- Tested against VyOS 1.3.8, 1.4.2, the upcoming 1.5, and the rolling release of spring 2025.

Copilot · 2026-04-17T12:42:09Z

+# -*- coding: utf-8 -*-
+from __future__ import absolute_import, division, print_function
+
+
+__metaclass__ = type
+


rm_templates files in this collection typically include the standard copyright header and a module-level docstring describing the template purpose (e.g. rm_templates/ntp_global.py, rm_templates/route_maps.py). This new template file is missing that header/docstring, which makes it inconsistent and harder to maintain.

Copilot · 2026-04-17T12:42:09Z

+                \s+(?P<description>.+)
+                $""",
+                re.VERBOSE,
+            ),
+            "setval": "{{ nat }} {{ type }} rule {{ rule }} description {{ description }}",
+            "result": {
+                "{{ nat }}": {
+                    "{{ type }}": {
+                        "rule": {
+                            "{{ rule }}": {"description": "{{ description }}"},


The NAT rule description command rendering does not quote the description value (... description {{ description }}). This will break for descriptions containing spaces/special characters and is inconsistent with other templates that render descriptions as single-quoted strings. Please ensure the rendered command wraps the description in quotes and that parsing/rendering round-trips correctly.

Suggested change

\s+(?P<description>.+)

$""",

re.VERBOSE,

),

"setval": "{{ nat }} {{ type }} rule {{ rule }} description {{ description }}",

"result": {

"{{ nat }}": {

"{{ type }}": {

"rule": {

"{{ rule }}": {"description": "{{ description }}"},

\s+(?:'(?P<description>[^']*)'|(?P<description_unquoted>.+))

$""",

re.VERBOSE,

),

"setval": "{{ nat }} {{ type }} rule {{ rule }} description '{{ description }}'",

"result": {

"{{ nat }}": {

"{{ type }}": {

"rule": {

"{{ rule }}": {"description": "{{ description | default(description_unquoted) }}"},

Copilot · 2026-04-17T12:42:10Z

@@ -0,0 +1,3 @@
+---
+trivial:


This changelog fragment is marked as trivial, but adding a new user-facing module is typically a minor_changes entry in this collection (e.g. changelogs/fragments/t6838_vrf.yml). Please move this entry under minor_changes so it is surfaced correctly in release notes.

Suggested change

trivial:

minor_changes:

Copilot · 2026-04-17T12:42:10Z

+def main():
+    """
+    Main entry point for module execution
+
+    :returns: the result form module invocation
+    """
+    module = AnsibleModule(
+        argument_spec=NatArgs.argument_spec,
+        mutually_exclusive=[["config", "running_config"]],
+        required_if=[
+            ["state", "merged", ["config"]],
+            ["state", "replaced", ["config"]],
+            ["state", "overridden", ["config"]],
+            ["state", "rendered", ["config"]],
+            ["state", "parsed", ["running_config"]],
+        ],
+        supports_check_mode=True,
+    )
+
+    result = Nat(module).execute_module()
+    module.exit_json(**result)
+


There are comprehensive unit tests for existing VyOS resource modules under tests/unit/modules/network/vyos/, but this PR adds a new vyos_nat module without any corresponding unit tests. Please add at least basic unit coverage for merged/deleted/rendered/parsed/gathered behavior and a couple of representative NAT rule renderings/parses to prevent regressions.

omnom62 added 3 commits March 8, 2026 20:46

VyOS NAT init

d1caa8b

init for vyos_nat

db0e0ef

Init for vyos_nat

b670f5c

omnom62 requested a review from a team as a code owner March 14, 2026 11:08

omnom62 requested review from andamasov, dmbaturin and gaige March 14, 2026 11:08

Merge branch 'main' into t8323_vyos_nat

531b648

omnom62 marked this pull request as draft March 14, 2026 11:09

changelog

2474a32

omnom62 changed the title ~~WIP T8323 vyos nat~~ WIP T8323 vyos_nat module Mar 14, 2026

omnom62 changed the title ~~WIP T8323 vyos_nat module~~ T8323 vyos_nat module Apr 2, 2026

omnom62 marked this pull request as ready for review April 2, 2026 20:35

omnom62 marked this pull request as draft April 2, 2026 20:36

omnom62 changed the title ~~T8323 vyos_nat module~~ WIP T8323 vyos_nat module Apr 2, 2026

omnom62 and others added 14 commits April 4, 2026 05:34

Merge branch 'main' into t8323_vyos_nat

e25ce20

Merge branch 'main' into t8323_vyos_nat

2e782e6

NAT WIP files

b0297b2

Generic nat rule rm_template

419df9b

NAT rm_template

d985e4e

rm_templates for NAT

6a70f28

Some sanity fixes for NAT module

3bc860f

NAT docs updates

6688c17

NAT doc update

10404c7

Module doc updates

d4cbf41

NAT facts

889a4da

nat64 and nat66 added

dd33971

Merge branch 'main' into t8323_vyos_nat

6d16d68

Merge branch 'main' into t8323_vyos_nat

e0b5118

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

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

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP T8323 vyos_nat module#451

WIP T8323 vyos_nat module#451
omnom62 wants to merge 19 commits intovyos:mainfrom
omnom62:t8323_vyos_nat

omnom62 commented Mar 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!

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!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

	- Tested against VyOS 1.3.8, 1.4.2, the upcoming 1.5, and the rolling release of spring 2025
	- Tested against VyOS 1.3.8, 1.4.2, the upcoming 1.5, and the rolling release of spring 2025.

Conversation

omnom62 commented Mar 14, 2026

Change Summary

Types of changes

Related Task(s)

Related PR(s)

Component(s) name

Proposed changes

How to test

Test results

Checklist:

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!

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!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants