summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authoromnom62 <75066712+omnom62@users.noreply.github.com>2026-04-17 22:33:40 +1000
committerGitHub <noreply@github.com>2026-04-17 22:33:40 +1000
commit2a5f7b17d0d3b18ae73c338971f3e933f0768f15 (patch)
tree2a1987872ad1dbe5e46c8284f969144a21457452
parente79e6658fde365a783b35cd8f99345209d67c15a (diff)
parentdb645c3fc5a66f0fdadd03b5096937f70787e3b0 (diff)
downloadvyos.vyos-2a5f7b17d0d3b18ae73c338971f3e933f0768f15.tar.gz
vyos.vyos-2a5f7b17d0d3b18ae73c338971f3e933f0768f15.zip
Merge branch 'main' into dependabot/github_actions/ansible/team-devtools/dot-github/workflows/ah_token_refresh.yml-26.2.0
-rw-r--r--.github/copilot-instructions.md49
-rw-r--r--.github/instructions/modules.instructions.md37
-rw-r--r--.github/instructions/tests.instructions.md114
-rw-r--r--.gitignore6
-rw-r--r--changelogs/fragments/copilot-instructions.yml4
5 files changed, 210 insertions, 0 deletions
diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
new file mode 100644
index 00000000..fc25faa1
--- /dev/null
+++ b/.github/copilot-instructions.md
@@ -0,0 +1,49 @@
+# Copilot Review Instructions — vyos.vyos
+
+This is the `vyos.vyos` Ansible network collection for managing VyOS devices.
+Namespace `vyos`, name `vyos`, version `6.0.0`. All modules are prefixed `vyos_`.
+
+## Commit and PR standards
+
+- Every commit title must start with a Phorge task ID: `T<number>: description`.
+- Every PR must have exactly one changelog fragment in `changelogs/fragments/`.
+- PR descriptions that state a test count (e.g. "Add 8 unit tests") must match the actual number of test methods in the changed files. Flag mismatches.
+
+## Changelog fragments
+
+Fragments are YAML files under `changelogs/fragments/`. Valid top-level keys:
+
+| Key | Use for |
+|-----|---------|
+| `trivial` | Developer tooling, CI, housekeeping, formatting-only changes |
+| `bugfixes` | Bug fixes |
+| `minor_changes` | New features or user-visible improvements |
+| `major_changes` | Breaking changes |
+| `security_fixes` | Security fixes |
+| `doc_changes` | Documentation-only changes |
+
+Flag any fragment that uses `minor_changes` for what is actually developer tooling (linting, formatting, gitignore, test scaffolding). Those should use `trivial`.
+
+## Module architecture
+
+Two module families:
+
+**Resource modules** (`vyos_interfaces`, `vyos_firewall_rules`, `vyos_bgp_global`, etc.) follow a four-part structure under `plugins/module_utils/network/vyos/`:
+- `argspec/{resource}/` — argument spec
+- `config/{resource}/` — config builder
+- `facts/{resource}/` — facts parser
+- `rm_templates/{resource}.py` — regex/Jinja2 CLI templates
+
+Resource modules support all states: `merged`, `replaced`, `overridden`, `deleted`, `rendered`, `gathered`, `parsed`.
+
+**Legacy modules** (`vyos_vlan`, `vyos_config`, `vyos_command`, `vyos_user`, etc.) do not follow the resource module pattern.
+
+## VyOS CLI conventions
+
+- Set commands: `set interfaces ethernet eth0 address '192.0.2.1/24'`
+- Delete commands: `delete interfaces ethernet eth0 address '192.0.2.1/24'`
+- Quoting varies by context. In general, string values (descriptions, names, ELIN numbers) are single-quoted; boolean flags and bare keywords are not. However, address/prefix values may be quoted or unquoted depending on where they appear:
+ - Quoted: `address '192.0.2.1/24'`, `description 'my-iface'`, `elin '0000000911'`
+ - Unquoted: `address 192.0.2.1` (in firewall groups), `disable`, `mtu-ignore`, `vif 200`
+- When reviewing tests and fixtures, align with the quoting style used by surrounding fixtures rather than flagging a missing quote as an error.
+- Interface types: `ethernet`, `loopback`, `bonding`, `bridge`, `tunnel`, `wireguard`.
diff --git a/.github/instructions/modules.instructions.md b/.github/instructions/modules.instructions.md
new file mode 100644
index 00000000..d7782e1e
--- /dev/null
+++ b/.github/instructions/modules.instructions.md
@@ -0,0 +1,37 @@
+---
+applyTo: "plugins/**,meta/runtime.yml"
+---
+
+# Plugin conventions
+
+## Module option descriptions
+
+- Must be complete English sentences ending with a period.
+- No grammar errors. Common mistake: "the number hops" should be "the number of hops".
+- Deprecated options must include `removed_in_version` and `removed_from_collection` fields.
+- Do not add new options with `deprecated: true` — remove deprecated options entirely.
+
+## rm_templates files
+
+Files in `plugins/module_utils/network/vyos/rm_templates/` define regex parsers and Jinja2 generators for a resource module. Each entry has:
+- `name` — unique identifier
+- `getval` — compiled regex with named groups
+- `setval` — Jinja2 template or callable producing a VyOS CLI command
+- `result` — dict mapping regex groups to facts structure
+- `shared` (optional) — bool, whether the template applies to a shared config block
+
+## meta/runtime.yml redirects
+
+Short-name redirects must point to the actual module name with the `vyos_` prefix. For example:
+```yaml
+snmp_server:
+ redirect: vyos.vyos.vyos_snmp_server # correct
+ # NOT: vyos.vyos.vyos_snmp_servers # wrong (pluralized)
+```
+
+Verify any changed redirect target exists as a real module file under `plugins/modules/`.
+
+## cliconf / terminal plugins
+
+`plugins/cliconf/vyos.py` — do not modify without understanding edit-mode and commit semantics.
+`plugins/terminal/vyos.py` — handles prompt detection; regex changes require testing against all supported VyOS versions listed in README.md.
diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md
new file mode 100644
index 00000000..150db41e
--- /dev/null
+++ b/.github/instructions/tests.instructions.md
@@ -0,0 +1,114 @@
+---
+applyTo: "tests/unit/**"
+---
+
+# Unit test conventions
+
+## Base class and structure
+
+All test classes inherit from `TestVyosModule` in `tests/unit/modules/network/vyos/vyos_module.py`.
+
+```python
+class TestVyosFooModule(TestVyosModule):
+ module = vyos_foo
+
+ def setUp(self): ...
+ def tearDown(self): ...
+ def load_fixtures(self, commands=None, filename=None): ...
+ def test_...(self): ...
+```
+
+## Mocking: resource modules
+
+Resource module tests require two framework-level patches in `setUp` with corresponding cleanup in `tearDown`:
+
+```python
+# in setUp:
+self.mock_get_resource_connection_config = patch(
+ "ansible_collections.ansible.netcommon.plugins.module_utils.network.common.cfg.base.get_resource_connection"
+)
+self.get_resource_connection_config = self.mock_get_resource_connection_config.start()
+
+self.mock_get_resource_connection_facts = patch(
+ "ansible_collections.ansible.netcommon.plugins.module_utils.network.common.facts.facts.get_resource_connection"
+)
+self.get_resource_connection_facts = self.mock_get_resource_connection_facts.start()
+
+# in tearDown:
+self.mock_get_resource_connection_config.stop()
+self.mock_get_resource_connection_facts.stop()
+```
+
+Most resource module tests also patch the facts class's `get_device_data` method directly and use it in `load_fixtures`:
+
+```python
+# in setUp:
+self.mock_execute_show_command = patch(
+ "ansible_collections.vyos.vyos.plugins.module_utils.network.vyos."
+ "facts.{resource}.{resource}.{Resource}Facts.get_device_data"
+)
+self.execute_show_command = self.mock_execute_show_command.start()
+
+# in load_fixtures:
+def load_from_file(*args, **kwargs):
+ return load_fixture("vyos_{resource}_config.cfg")
+self.execute_show_command.side_effect = load_from_file
+```
+
+An alternative pattern sets `self.get_resource_connection_facts.return_value.get_config.return_value = fixture_data` in `load_fixtures` instead — both approaches work, but the `get_device_data` pattern is used by most existing tests.
+
+## Mocking: legacy modules
+
+Legacy modules (`vyos_vlan`, `vyos_config`, `vyos_command`, etc.) patch the specific helper the module calls, directly in the module under test. Which helper depends on the module:
+
+```python
+# vyos_command, vyos_facts, vyos_ping — patch run_commands
+self.mock_run_commands = patch("ansible_collections.vyos.vyos.plugins.modules.vyos_foo.run_commands")
+# vyos_config — patch get_config / load_config
+self.mock_load_config = patch("ansible_collections.vyos.vyos.plugins.modules.vyos_foo.load_config")
+```
+
+In `load_fixtures`, configure the mock using `side_effect` (for dynamic fixture loading) or `return_value` (for a fixed response):
+
+```python
+# side_effect — used by most existing legacy tests
+def load_from_file(*args, **kwargs):
+ return load_fixture("vyos_foo_config.cfg")
+self.run_commands.side_effect = load_from_file
+
+# return_value — acceptable for simple fixed responses
+self.run_commands.return_value = [SHOW_OUTPUT]
+```
+
+## load_fixtures and filename
+
+`execute_module()` always calls `load_fixtures()`. Never set mock return values inside a test method — they will be overwritten by the next `execute_module` call. Use the `filename` parameter to vary the fixture:
+
+```python
+def load_fixtures(self, commands=None, filename=None):
+ if filename == "empty":
+ self.run_commands.return_value = [EMPTY_OUTPUT]
+ else:
+ self.run_commands.return_value = [DEFAULT_OUTPUT]
+```
+
+Then call: `self.execute_module(changed=True, commands=commands, filename="empty")`
+
+## Fixture files
+
+Raw device CLI output lives in `tests/unit/modules/network/vyos/fixtures/` as `.cfg` files.
+Load with `load_fixture("vyos_foo_config.cfg")`.
+
+## Required test coverage for resource modules
+
+A complete resource module test file should cover all applicable states:
+- `merged` (including an idempotent case)
+- `replaced`
+- `overridden`
+- `deleted`
+- `rendered` — assert `result["rendered"]` matches expected CLI commands
+- `gathered` — assert `result["gathered"]` contains expected structured data
+- `parsed` — pass `running_config=raw_string` and assert `result["parsed"]`
+
+Flag test files that are missing `rendered`, `gathered`, or `parsed` tests without explanation.
+
diff --git a/.gitignore b/.gitignore
index cbe347c4..1dbe8b0e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -120,3 +120,9 @@ changelogs/.plugin-cache.yaml
inventory.network
*.bak
+
+# Git worktrees
+.worktrees/
+
+# Claude Code
+.claude/
diff --git a/changelogs/fragments/copilot-instructions.yml b/changelogs/fragments/copilot-instructions.yml
new file mode 100644
index 00000000..c7af5c74
--- /dev/null
+++ b/changelogs/fragments/copilot-instructions.yml
@@ -0,0 +1,4 @@
+---
+trivial:
+ - Add Copilot custom review instructions to guide automated code review with
+ project-specific conventions for tests, changelog fragments, and module patterns.