diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 00000000..38eae56f --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,591 @@ +# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json +# CodeRabbit configuration for vyos/vyos.vyos Ansible network collection +# Docs: https://docs.coderabbit.ai/guides/configure-coderabbit + +language: en-US +early_access: false +tone_instructions: > + Concise, technical, no filler. Focus on correctness, security, idempotency, + and Ansible conventions. Cite file paths and line numbers. + +reviews: + profile: chill + request_changes_workflow: false + high_level_summary: true + high_level_summary_placeholder: '@coderabbitai summary' + auto_title_placeholder: '@coderabbitai' + review_status: true + poem: false + collapse_walkthrough: true + changed_files_summary: true + sequence_diagrams: false + assess_linked_issues: true + related_issues: true + related_prs: true + suggested_labels: false + auto_apply_labels: false + suggested_reviewers: false + + auto_review: + enabled: true + auto_incremental_review: true + drafts: false + base_branches: + - main + ignore_title_keywords: + - WIP + - DO NOT MERGE + - Bump + + path_filters: + - '!**/__pycache__/**' + - '!**/*.pyc' + - '!**/*.egg-info/**' + - '!changelogs/changelog.yaml' + - '!.venv/**' + - '!.collections/**' + - '!.worktrees/**' + + path_instructions: + # ── Global PR hygiene ────────────────────────────────────────────── + - path: '**' + instructions: | + This is the vyos.vyos Ansible network collection (namespace=vyos, name=vyos, + version=6.0.0). PR titles must follow the format `T{id}: description` referencing + a Phorge task at vyos.dev. Every PR must include a changelog fragment in + changelogs/fragments/ (YAML, valid keys: major_changes, minor_changes, + breaking_changes, deprecated_features, removed_features, security_fixes, + bugfixes, known_issues, doc_changes, trivial; plus release_summary as a + prelude section). Style: black line-length=100, isort + profile=black line_length=100, flake8 max-line-length=120. Do not suggest + 88-char wrapping. + + # ── Module entry points ──────────────────────────────────────────── + - path: 'plugins/modules/vyos_*.py' + instructions: | + Module entry points. Each file must contain three YAML triple-string blocks: + DOCUMENTATION, EXAMPLES, and RETURN — this is Ansible's documentation contract, + not Python docstrings. Verify: + - DOCUMENTATION includes: module, author, short_description, description, + version_added, extends_documentation_fragment (vyos.vyos.vyos), options with + types and descriptions, and a notes section listing tested VyOS versions. + - EXAMPLES has at least one working task per supported state. + - RETURN documents all return keys with description, returned, type, and sample. + - The module wires argspec, config, and facts classes correctly. + - State choices include the full set where applicable: merged, replaced, + overridden, deleted, gathered, parsed, rendered. + Do not add Python-style docstrings (def-level) to these files — the YAML blocks + are the canonical documentation. + + # ── Argspec (auto-generated) ─────────────────────────────────────── + - path: 'plugins/module_utils/network/vyos/argspec/**' + instructions: | + Auto-generated by the Ansible resource module builder. These files carry a + "DO NOT EDIT" warning header. Do not suggest modifications to auto-generated + argspec files — changes will be overwritten. If the schema needs updating, + the resource module builder must regenerate it. Only flag issues if the + argument_spec dict has obvious type mismatches or missing required fields + that would cause runtime failures. + + # ── Config classes ───────────────────────────────────────────────── + - path: 'plugins/module_utils/network/vyos/config/**' + instructions: | + Config builders extending ansible.netcommon ConfigBase or ResourceModule. + These generate VyOS CLI commands from desired state. Verify: + - execute_module() handles all declared states correctly. + - set_config() and _set_config() process gathered facts and desired config + without data loss. + - Command generation produces valid VyOS CLI syntax (set/delete prefixes, + proper quoting of values with spaces). + - No silent swallowing of unknown keys — unknown config should raise or warn. + - Methods that compare current vs desired state handle empty/None gracefully. + Some older config files have auto-generated headers — do not restructure those. + + # ── Facts classes ────────────────────────────────────────────────── + - path: 'plugins/module_utils/network/vyos/facts/**' + instructions: | + Facts classes parse raw VyOS CLI output into structured dicts. Verify: + - Regex patterns handle edge cases (missing fields, empty values, quoted strings). + - populate() returns a clean dict even when device output is incomplete. + - get_device_data() uses the correct show command for the resource. + - facts/facts.py FACT_RESOURCE_SUBSETS and FACT_LEGACY_SUBSETS stay in sync + with available fact classes. + - Legacy facts (facts/legacy/) use run_commands(); resource facts use + get_resource_connection(). + + # ── RM Templates ─────────────────────────────────────────────────── + - path: 'plugins/module_utils/network/vyos/rm_templates/*.py' + instructions: | + Parser templates mapping structured data to VyOS CLI commands and vice versa. + Files with a `_14` suffix target VyOS 1.4+ behavior — do not suggest merging + them with the base version. Verify: + - _tmplt_* helper functions produce syntactically valid VyOS commands. + - Regex patterns in PARSERS list correctly capture all variations of the CLI + output (quoted values, optional fields, nested hierarchies). + - New templates include both set and delete command generation. + - compval/getval paths match the argspec structure. + + # ── Cliconf plugin ───────────────────────────────────────────────── + - path: 'plugins/cliconf/vyos.py' + instructions: | + Low-level CLI abstraction for VyOS. Handles configure mode, commit, diff, + command execution. Changes here affect all modules. Verify: + - edit_config() enters configure mode and commits correctly. + - get_diff() returns accurate before/after config diffs. + - Error handling catches VyOS-specific error patterns (commit failures, + invalid commands). + - __rpc__ list matches actually implemented methods. + + # ── Terminal plugin ──────────────────────────────────────────────── + - path: 'plugins/terminal/vyos.py' + instructions: | + Terminal prompt detection and initialization. Changes affect connection + reliability. Verify regex patterns against actual VyOS prompt formats + (configure mode, operational mode, different shell variants). Do not + remove existing patterns without testing against all supported VyOS versions. + + # ── Action plugin ────────────────────────────────────────────────── + - path: 'plugins/action/vyos.py' + instructions: | + Auto-proxies all modules to the device. Must validate network_cli connection + type. Symlinks from each module name point here. Keep minimal — logic belongs + in config classes, not the action plugin. + + # ── Changelog fragments ──────────────────────────────────────────── + - path: 'changelogs/fragments/*.{yaml,yml}' + instructions: | + Changelog fragments for ansible-changelog. Valid top-level keys: + major_changes, minor_changes, breaking_changes, deprecated_features, + removed_features, security_fixes, bugfixes, known_issues, doc_changes, + trivial. release_summary is a prelude section (one per release). + Fragment filename should be descriptive (e.g., fix-bgp-neighbor-timers.yml). + Use `trivial` for tooling/housekeeping. Entries should be complete sentences. + + # ── CI workflows ─────────────────────────────────────────────────── + - path: '.github/workflows/**' + instructions: | + CI pipeline: tests.yml (main CI with changelog, build, lint, sanity, unit jobs), + codecoverage.yml, release.yml (Galaxy + Automation Hub publish), check_label.yaml, + cla-check.yml. Changes to release.yml or ah_token_refresh.yml affect publishing + credentials — review with extra care. Do not remove the `all_green` aggregation + job from tests.yml. + + # ── Unit tests ───────────────────────────────────────────────────── + - path: 'tests/unit/**' + instructions: | + Unit tests use pytest + unittest.TestCase via TestVyosModule base class. + Key patterns: + - All test classes inherit TestVyosModule (from vyos_module.py). + - setUp() creates and starts mock patches; tearDown() stops them. + - execute_module(failed, changed, commands, sort) is the primary assertion method. + - load_fixtures() is overridden per test class to wire mock return values. + - Fixture files (.cfg) go in tests/unit/modules/network/vyos/fixtures/. + - Use load_fixture(name) to read fixtures — never inline raw config strings. + - set_module_args(dict(...)) configures module input before execution. + Style: black line-length=100, assertions via self.assertEqual / self.assertIn / + execute_module kwargs. pytest-xdist runs tests in parallel (-n 2). + + # ── Test fixtures ────────────────────────────────────────────────── + - path: 'tests/unit/modules/network/vyos/fixtures/**' + instructions: | + Raw VyOS CLI output files (.cfg). These are loaded by load_fixture() and + cached in memory. Format is VyOS `set ...` configuration syntax or show + command output. Fixture filenames follow the pattern: + vyos_{module}_config.cfg (base) or vyos_{module}_config_v14.cfg (VyOS 1.4+). + New fixtures must be syntactically valid VyOS config. Do not add JSON fixtures + unless the test explicitly requires JSON parsing. + + # ── Collection metadata ──────────────────────────────────────────── + - path: 'galaxy.yml' + instructions: | + Collection metadata. namespace=vyos, name=vyos. Version bumps must be + coordinated with release process. Dependency on ansible.netcommon>=2.5.1 + is required. Do not add unnecessary dependencies. + + - path: 'meta/runtime.yml' + instructions: | + Module redirects and tombstones. Adding a new module requires a redirect + entry (short name → FQCN). Tombstoned modules (logging, vyos_logging) must + not be un-tombstoned. requires_ansible must stay >=2.15.0 unless explicitly + bumping minimum version. + + finishing_touches: + docstrings: + enabled: true + unit_tests: + enabled: true + + tools: + github-checks: + enabled: true + timeout_ms: 90000 + eslint: + enabled: false + biome: + enabled: false + actionlint: + enabled: true + yamllint: + enabled: true + markdownlint: + enabled: true + languagetool: + enabled: true + level: default + enabled_only: false + gitleaks: + enabled: true + checkov: + enabled: false + semgrep: + enabled: true + ast-grep: + essential_rules: true + ruff: + enabled: false + +chat: + auto_reply: true + +knowledge_base: + opt_out: false + learnings: + scope: auto + issues: + scope: auto + pull_requests: + scope: auto + linked_repositories: + - repository: "ansible/ansible" + instructions: > + Core Ansible framework. Reference for module_utils base classes, + plugin interfaces (cliconf, terminal, action), module documentation + conventions (DOCUMENTATION/EXAMPLES/RETURN YAML blocks), and + ansible-test sanity requirements. + - repository: "ansible-collections/ansible.netcommon" + instructions: > + Network common collection. Contains ConfigBase, ResourceModule, + FactsBase, NetworkTemplate, and get_resource_connection — the base + classes and utilities that vyos.vyos modules directly extend. + +code_generation: + docstrings: + language: en-US + path_instructions: + # ── Module entry points: YAML blocks, not Python docstrings ────── + - path: 'plugins/modules/vyos_*.py' + instructions: | + Do NOT generate Python-style docstrings for these files. Ansible modules + use YAML triple-string blocks: DOCUMENTATION, EXAMPLES, and RETURN. + If updating these blocks: + - DOCUMENTATION must include: module name, author, short_description, + description (list of strings), version_added, extends_documentation_fragment + (vyos.vyos.vyos), and a full options tree with type, description, and + choices/default where applicable. Include a notes section listing supported + VyOS versions (1.3.8, 1.4.1, 1.4.2, 1.5 rolling). + - EXAMPLES must show at least one task per supported state using FQCN + (vyos.vyos.vyos_). + - RETURN must document: commands (list, always), before (dict, always), + after (dict, when changed), and any module-specific return values. + Keep version_added accurate — do not backdate. + + # ── Argspec: skip auto-generated files ─────────────────────────── + - path: 'plugins/module_utils/network/vyos/argspec/**' + instructions: | + Skip — these files are auto-generated by the Ansible resource module builder + and carry a "DO NOT EDIT" header. Do not generate or modify docstrings. + + # ── Config classes ─────────────────────────────────────────────── + - path: 'plugins/module_utils/network/vyos/config/**' + instructions: | + Config builder classes extending ConfigBase or ResourceModule. Use + reStructuredText-style docstrings (Ansible/Sphinx convention): + def method(self, ...): + """Short description. + + :param name: description + :type name: type + :rtype: type + :returns: description + """ + Document: execute_module(), set_config(), get__facts(), and any + method that generates CLI commands. Focus on what state transitions the + method handles and what CLI commands it may produce. Do not document trivial + __init__ that just calls super(). Some files have auto-generated headers — + keep docstrings minimal in those to avoid noise on regeneration. + + # ── Facts classes ──────────────────────────────────────────────── + - path: 'plugins/module_utils/network/vyos/facts/**' + instructions: | + Facts parsers that convert VyOS CLI output to structured dicts. Use rST + docstrings. Document: + - populate(): what show commands it runs and the dict structure it returns. + - render_config() / get_device_data(): the CLI command used and expected + output format. + - Any regex-heavy parsing method: briefly note what CLI patterns it handles. + Skip __init__.py files. + + # ── RM Templates ───────────────────────────────────────────────── + - path: 'plugins/module_utils/network/vyos/rm_templates/*.py' + instructions: | + Parser template files with _tmplt_* helper functions and PARSERS lists. + Add a module-level docstring describing the resource and VyOS CLI hierarchy + covered. For _tmplt_* functions: one-line docstring stating the VyOS command + path generated (e.g., "Generate `set protocols bgp neighbor + timers ...` commands."). Do not document individual regex PARSERS entries — + the patterns are self-describing. Files with _14 suffix target VyOS 1.4+ — + note this in the module docstring. + + # ── Cliconf plugin ────────────────────────────────────────────── + - path: 'plugins/cliconf/vyos.py' + instructions: | + Uses Ansible DOCUMENTATION block for plugin-level docs. For Python methods + use rST docstrings. Document: get_device_info(), edit_config(), get_config(), + get_diff(), commit(), discard_changes(). Note which methods enter/exit + configure mode. + + # ── Terminal and action plugins ────────────────────────────────── + - path: 'plugins/terminal/vyos.py' + instructions: | + Minimal docstrings only. Document on_open_shell() and on_close_shell() + explaining what terminal parameters they set. Regex patterns are + self-documenting — do not add inline comments to each pattern. + + - path: 'plugins/action/vyos.py' + instructions: | + Skip — thin proxy inheriting ActionNetworkModule. No docstrings needed. + + # ── Utilities ──────────────────────────────────────────────────── + - path: 'plugins/module_utils/network/vyos/utils/*.py' + instructions: | + Utility functions shared across modules. Use rST docstrings for any + function with non-obvious behavior. Document parameters, return types, + and side effects. Skip trivial one-liners. + + # ── Test files ────────────────────────────────────────────────── + - path: 'tests/**' + instructions: | + Skip — test files do not need docstrings. Test method names should be + self-describing (test_vyos___). + + unit_tests: + path_instructions: + # ── Resource module tests ──────────────────────────────────────── + - path: 'plugins/modules/vyos_interfaces.py' + instructions: &resource_module_tests | + Generate tests in tests/unit/modules/network/vyos/test_vyos_.py. + Inherit from TestVyosModule (import from vyos_module.py in same directory). + + Structure: + ```python + class TestVyosModule(TestVyosModule): + module = vyos_ + + def setUp(self): + super().setUp() + # Mock get_resource_connection at BOTH levels: + self.mock_get_resource_connection_config = patch( + "ansible_collections.ansible.netcommon.plugins.module_utils." + "network.common.cfg.base.get_resource_connection" + ) + self.mock_get_resource_connection_facts = patch( + "ansible_collections.ansible.netcommon.plugins.module_utils." + "network.common.facts.facts.get_resource_connection" + ) + # Mock the facts get_device_data method: + self.mock_execute_show_command = patch( + "ansible_collections.vyos.vyos.plugins.module_utils.network." + "vyos.facts....get_device_data" + ) + # Start all patches and store references + self.execute_show_command = self.mock_execute_show_command.start() + + def tearDown(self): + super().tearDown() + # Stop ALL patches + + def load_fixtures(self, commands=None, filename=None): + def load_from_file(*args, **kwargs): + return load_fixture(filename or "vyos__config.cfg") + self.execute_show_command.side_effect = load_from_file + ``` + + Required test methods for each resource module: + - test_vyos__merged: config change, changed=True, verify commands list + - test_vyos__merged_idempotent: no-op, changed=False, commands=[] + - test_vyos__replaced: replaced state, changed=True + - test_vyos__replaced_idempotent: replaced no-op, changed=False + - test_vyos__overridden: full override, changed=True + - test_vyos__deleted: deletion, changed=True + - test_vyos__gathered: state=gathered, verify result["gathered"] dict + - test_vyos__rendered: state=rendered, verify result["rendered"] commands + - test_vyos__parsed: state=parsed with running_config, verify output + + Assertions use self.execute_module(changed=True/False, commands=[...]). + Commands lists contain exact VyOS CLI strings: "set interfaces ethernet eth0 ...". + Use set_module_args(dict(config=[...], state="")) before execute_module. + Create fixture files in tests/unit/modules/network/vyos/fixtures/ named + vyos__config.cfg with valid VyOS set-syntax configuration. + Use load_fixture() to read fixtures — never inline raw config. + + - path: 'plugins/modules/vyos_l3_interfaces.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_lag_interfaces.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_lldp_global.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_lldp_interfaces.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_static_routes.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_firewall_rules.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_firewall_global.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_firewall_interfaces.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_ospfv2.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_ospfv3.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_ospf_interfaces.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_bgp_global.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_bgp_address_family.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_prefix_lists.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_route_maps.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_snmp_server.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_logging_global.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_ntp_global.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_hostname.py' + instructions: *resource_module_tests + + - path: 'plugins/modules/vyos_vrf.py' + instructions: *resource_module_tests + + # ── Legacy module tests ────────────────────────────────────────── + - path: 'plugins/modules/vyos_command.py' + instructions: &legacy_module_tests | + Generate tests in tests/unit/modules/network/vyos/test_vyos_.py. + Inherit from TestVyosModule. + + Legacy modules mock differently from resource modules: + ```python + class TestVyosModule(TestVyosModule): + module = vyos_ + + def setUp(self): + super().setUp() + # Mock run_commands directly on the module: + self.mock_run_commands = patch( + "ansible_collections.vyos.vyos.plugins.modules." + "vyos_.run_commands" + ) + self.run_commands = self.mock_run_commands.start() + # Some also mock get_capabilities or get_config/load_config + + def tearDown(self): + super().tearDown() + self.mock_run_commands.stop() + + def load_fixtures(self, commands=None, filename=None): + # Set run_commands return_value or side_effect + self.run_commands.return_value = [load_fixture(filename)] + ``` + + Legacy modules (vyos_command, vyos_config, vyos_facts, vyos_banner, + vyos_ping, vyos_system, vyos_user, vyos_vlan) do not use + resource module states. Test: successful execution, error handling, + idempotency where applicable, and specific module features (e.g., + vyos_command wait_for/retries, vyos_config src/lines/match). + Use execute_module(changed=, commands=) for assertions. + + - path: 'plugins/modules/vyos_config.py' + instructions: *legacy_module_tests + + - path: 'plugins/modules/vyos_facts.py' + instructions: *legacy_module_tests + + - path: 'plugins/modules/vyos_banner.py' + instructions: *legacy_module_tests + + - path: 'plugins/modules/vyos_ping.py' + instructions: *legacy_module_tests + + - path: 'plugins/modules/vyos_system.py' + instructions: *legacy_module_tests + + - path: 'plugins/modules/vyos_user.py' + instructions: *legacy_module_tests + + - path: 'plugins/modules/vyos_vlan.py' + instructions: *legacy_module_tests + + # ── Test infrastructure — do not generate tests for these ──────── + - path: 'tests/unit/modules/utils.py' + instructions: | + Skip — test infrastructure (ModuleTestCase base, set_module_args, exception + classes). Do not generate tests for test utilities. + + - path: 'tests/unit/modules/conftest.py' + instructions: | + Skip — pytest fixtures (patch_ansible_module). Do not generate tests. + + - path: 'tests/unit/modules/network/vyos/vyos_module.py' + instructions: | + Skip — TestVyosModule base class with execute_module(), load_fixture(), + and mock setup. Do not generate tests for the test base class. + + - path: 'tests/unit/modules/network/vyos/fixtures/**' + instructions: | + Skip — raw VyOS CLI output fixtures. Not code, not testable. + + # ── Non-module plugin code ─────────────────────────────────────── + - path: 'plugins/module_utils/**' + instructions: | + Module utility code (argspec, config, facts, rm_templates, utils). + These are tested indirectly through module-level tests — the config + classes are exercised when test_vyos_.py calls execute_module(). + Do not generate separate unit tests for module_utils classes unless + a utility function in plugins/module_utils/network/vyos/utils/ has + complex standalone logic worth testing in isolation. + + - path: 'plugins/cliconf/vyos.py' + instructions: | + Skip — cliconf plugin is tested via integration tests and indirectly + through module tests. Unit testing requires complex CliconfBase mocking + that provides little value over integration coverage. + + - path: 'plugins/terminal/vyos.py' + instructions: | + Skip — terminal plugin regex patterns are validated through integration + tests against actual VyOS devices. + + - path: 'plugins/action/vyos.py' + instructions: | + Skip — thin action proxy. Tested indirectly via module tests. 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: 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/.github/workflows/ah_token_refresh.yml b/.github/workflows/ah_token_refresh.yml index 0346920e..0c8cba7b 100644 --- a/.github/workflows/ah_token_refresh.yml +++ b/.github/workflows/ah_token_refresh.yml @@ -1,14 +1,14 @@ name: Refresh the automation hub token # the token expires every 30 days, so we need to refresh it on: schedule: - cron: '0 12 1,15 * *' # run 12pm on the 1st and 15th of the month workflow_dispatch: jobs: refresh: - uses: ansible/team-devtools/.github/workflows/ah_token_refresh.yml@v26.1.0 + uses: ansible/team-devtools/.github/workflows/ah_token_refresh.yml@v26.2.0 with: environment: release secrets: ah_token: ${{ secrets.AH_TOKEN }} diff --git a/.github/workflows/codecoverage.yml b/.github/workflows/codecoverage.yml index 878ae247..8ce3af36 100644 --- a/.github/workflows/codecoverage.yml +++ b/.github/workflows/codecoverage.yml @@ -1,71 +1,71 @@ --- name: Code Coverage # cloned from ansible-network/github_actions/.github/workflows/coverage_network_devices.yml@main # in order to deal with token issue in codecov on: # yamllint disable-line rule:truthy push: pull_request: branches: [main] jobs: codecoverage: env: PY_COLORS: "1" source_directory: "./source" python_version: "3.10" ansible_version: "latest" os: "ubuntu-latest" collection_pre_install: >- git+https://github.com/ansible-collections/ansible.utils.git git+https://github.com/ansible-collections/ansible.netcommon.git runs-on: ubuntu-latest name: "Code Coverage | Python 3.10" steps: - name: Checkout the collection repository uses: ansible-network/github_actions/.github/actions/checkout_dependency@main with: path: ${{ env.source_directory }} ref: ${{ github.event.pull_request.head.sha }} fetch-depth: "0" - name: Set up Python ${{ env.python_version }} uses: actions/setup-python@v6 with: python-version: ${{ env.python_version }} - name: Install ansible-core (${{ env.ansible-version }}) run: python3 -m pip install ansible-core pytest pytest-cov pytest-ansible-units pytest-forked pytest-xdist - name: Read collection metadata from galaxy.yml id: identify uses: ansible-network/github_actions/.github/actions/identify_collection@main with: source_path: ${{ env.source_directory }} - name: Build and install the collection uses: ansible-network/github_actions/.github/actions/build_install_collection@main with: install_python_dependencies: true source_path: ${{ env.source_directory }} collection_path: ${{ steps.identify.outputs.collection_path }} tar_file: ${{ steps.identify.outputs.tar_file }} - name: Print the ansible version run: ansible --version - name: Print the python dependencies run: python3 -m pip list - name: Run Coverage tests run: | pytest tests/unit -v --cov-report xml --cov=./ working-directory: ${{ steps.identify.outputs.collection_path }} - name: Upload coverage report to Codecov - uses: codecov/codecov-action@v5 + uses: codecov/codecov-action@v6 with: directory: ${{ steps.identify.outputs.collection_path }} fail_ci_if_error: false token: ${{ secrets.CODECOV_TOKEN }} env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/.gitignore b/.gitignore index cbe347c4..1dbe8b0e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,122 +1,128 @@ # CML/virl lab cache .virl/ # A collection directory, resulting from the use of the pytest-ansible-units plugin collections/ # Byte-compiled / optimized / DLL files __pycache__/ *.py[cod] *$py.class # C extensions *.so # Distribution / packaging .Python build/ develop-eggs/ dist/ downloads/ eggs/ .eggs/ lib/ lib64/ parts/ sdist/ var/ wheels/ *.egg-info/ .installed.cfg *.egg MANIFEST # PyInstaller # Usually these files are written by a python script from a template # before PyInstaller builds the exe, so as to inject date/other infos into it. *.manifest *.spec # Installer logs pip-log.txt pip-delete-this-directory.txt # Unit test / coverage reports htmlcov/ .tox/ .coverage .coverage.* .cache nosetests.xml coverage.xml *.cover .hypothesis/ .pytest_cache/ # Translations *.mo *.pot # Django stuff: *.log local_settings.py db.sqlite3 # Flask stuff: instance/ .webassets-cache # Scrapy stuff: .scrapy # Sphinx documentation docs/_build/ # PyBuilder target/ # Jupyter Notebook .ipynb_checkpoints # pyenv .python-version # celery beat schedule file celerybeat-schedule # SageMath parsed files *.sage.py # Environments .env .venv env/ venv/ ENV/ env.bak/ venv.bak/ # Spyder project settings .spyderproject .spyproject # Rope project settings .ropeproject # mkdocs documentation /site # mypy .mypy_cache/ # ide *.code-workspace .vscode/ .DS_Store changelogs/.plugin-cache.yaml # inventory for testing inventory.network *.bak + +# Git worktrees +.worktrees/ + +# Claude Code +.claude/ diff --git a/changelogs/fragments/T8512-isort-fix.yml b/changelogs/fragments/T8512-isort-fix.yml new file mode 100644 index 00000000..5209017c --- /dev/null +++ b/changelogs/fragments/T8512-isort-fix.yml @@ -0,0 +1,3 @@ +--- +trivial: + - Fix isort import ordering violations across module_utils to satisfy pre-commit checks. diff --git a/changelogs/fragments/coderabbit-config.yml b/changelogs/fragments/coderabbit-config.yml new file mode 100644 index 00000000..9442337f --- /dev/null +++ b/changelogs/fragments/coderabbit-config.yml @@ -0,0 +1,3 @@ +trivial: + - Add CodeRabbit review configuration with path-specific guidelines for review, + docstring generation, and unit test generation. 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. diff --git a/changelogs/fragments/fix-vlan-purge.yml b/changelogs/fragments/fix-vlan-purge.yml new file mode 100644 index 00000000..bbc1d08a --- /dev/null +++ b/changelogs/fragments/fix-vlan-purge.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - vyos_vlan - fix purge generating invalid ``delete ... vif None`` commands for bare interfaces without VLAN sub-interfaces. diff --git a/plugins/module_utils/network/vyos/facts/bgp_address_family/bgp_address_family.py b/plugins/module_utils/network/vyos/facts/bgp_address_family/bgp_address_family.py index 3386bd66..31839c5d 100644 --- a/plugins/module_utils/network/vyos/facts/bgp_address_family/bgp_address_family.py +++ b/plugins/module_utils/network/vyos/facts/bgp_address_family/bgp_address_family.py @@ -1,99 +1,99 @@ # -*- coding: utf-8 -*- # Copyright 2021 Red Hat # GNU General Public License v3.0+ # (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import absolute_import, division, print_function __metaclass__ = type """ The vyos bgp_address_family fact class It is in this file the configuration is collected from the device for a given resource, parsed, and the facts tree is populated based on the configuration. """ import re from ansible_collections.ansible.netcommon.plugins.module_utils.network.common import utils from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.argspec.bgp_address_family.bgp_address_family import ( Bgp_address_familyArgs, ) from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.rm_templates.bgp_address_family import ( Bgp_address_familyTemplate, ) from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.rm_templates.bgp_address_family_14 import ( Bgp_address_familyTemplate14, ) - +from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.version import ( + LooseVersion, +) from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.vyos import get_os_version -from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.version import LooseVersion - class Bgp_address_familyFacts(object): """The vyos bgp_address_family facts class""" def __init__(self, module, subspec="config", options="options"): self._module = module self.argument_spec = Bgp_address_familyArgs.argument_spec def get_device_data(self, connection): return connection.get('show configuration commands | match "set protocols bgp"') def populate_facts(self, connection, ansible_facts, data=None): """Populate the facts for Bgp_address_family network resource :param connection: the device connection :param ansible_facts: Facts dictionary :param data: previously collected conf :rtype: dictionary :returns: facts """ facts = {} objs = [] config_lines = [] if not data: data = self.get_device_data(connection) for resource in data.splitlines(): if "address-family" in resource or "system-as" in resource: config_lines.append(re.sub("'", "", resource)) # parse native config using the Bgp_address_family template based on version if LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4"): bgp_address_family_parser = Bgp_address_familyTemplate14(lines=config_lines) else: bgp_address_family_parser = Bgp_address_familyTemplate(lines=config_lines) objs = bgp_address_family_parser.parse() if objs: if "address_family" in objs: objs["address_family"] = list(objs["address_family"].values()) for af in objs["address_family"]: if "networks" in af: af["networks"] = sorted(af["networks"], key=lambda k: k["prefix"]) if "aggregate_address" in af: af["aggregate_address"] = sorted( af["aggregate_address"], key=lambda k: k["prefix"], ) if "neighbors" in objs: objs["neighbors"] = list(objs["neighbors"].values()) objs["neighbors"] = sorted(objs["neighbors"], key=lambda k: k["neighbor_address"]) for neigh in objs["neighbors"]: if "address_family" in neigh: neigh["address_family"] = list(neigh["address_family"].values()) ansible_facts["ansible_network_resources"].pop("bgp_address_family", None) params = utils.remove_empties(utils.validate_config(self.argument_spec, {"config": objs})) facts["bgp_address_family"] = params.get("config", []) ansible_facts["ansible_network_resources"].update(facts) return ansible_facts diff --git a/plugins/module_utils/network/vyos/facts/bgp_global/bgp_global.py b/plugins/module_utils/network/vyos/facts/bgp_global/bgp_global.py index dd793681..2883cc2d 100644 --- a/plugins/module_utils/network/vyos/facts/bgp_global/bgp_global.py +++ b/plugins/module_utils/network/vyos/facts/bgp_global/bgp_global.py @@ -1,93 +1,92 @@ # -*- coding: utf-8 -*- # Copyright 2021 Red Hat # GNU General Public License v3.0+ # (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import absolute_import, division, print_function __metaclass__ = type """ The vyos bgp_global fact class It is in this file the configuration is collected from the device for a given resource, parsed, and the facts tree is populated based on the configuration. """ import re from ansible_collections.ansible.netcommon.plugins.module_utils.network.common import utils from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.argspec.bgp_global.bgp_global import ( Bgp_globalArgs, ) from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.rm_templates.bgp_global import ( Bgp_globalTemplate, ) - from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.rm_templates.bgp_global_14 import ( Bgp_globalTemplate14, ) - +from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.version import ( + LooseVersion, +) from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.vyos import get_os_version -from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.utils.version import LooseVersion - class Bgp_globalFacts(object): """The vyos bgp_global facts class""" def __init__(self, module, subspec="config", options="options"): self._module = module self.argument_spec = Bgp_globalArgs.argument_spec def get_device_data(self, connection): return connection.get('show configuration commands | match "set protocols bgp"') def populate_facts(self, connection, ansible_facts, data=None): """Populate the facts for Bgp_global network resource :param connection: the device connection :param ansible_facts: Facts dictionary :param data: previously collected conf :rtype: dictionary :returns: facts """ facts = {} objs = {} config_lines = [] if not data: data = self.get_device_data(connection) for resource in data.splitlines(): if "address-family" not in resource: config_lines.append(re.sub("'", "", resource)) if LooseVersion(get_os_version(self._module)) >= LooseVersion("1.4"): bgp_global_parser = Bgp_globalTemplate14(lines=config_lines, module=self._module) else: bgp_global_parser = Bgp_globalTemplate(lines=config_lines, module=self._module) objs = bgp_global_parser.parse() if "neighbor" in objs: objs["neighbor"] = list(objs["neighbor"].values()) objs["neighbor"] = sorted(objs["neighbor"], key=lambda k: k["address"]) if "network" in objs: objs["network"] = sorted(objs["network"], key=lambda k: k["address"]) if "aggregate_address" in objs: objs["aggregate_address"] = sorted(objs["aggregate_address"], key=lambda k: k["prefix"]) ansible_facts["ansible_network_resources"].pop("bgp_global", None) params = utils.remove_empties( bgp_global_parser.validate_config(self.argument_spec, {"config": objs}, redact=True), ) facts["bgp_global"] = params.get("config", []) ansible_facts["ansible_network_resources"].update(facts) return ansible_facts diff --git a/plugins/module_utils/network/vyos/rm_templates/ospf_interfaces_14.py b/plugins/module_utils/network/vyos/rm_templates/ospf_interfaces_14.py index 43fae1e9..0d3aa5a7 100644 --- a/plugins/module_utils/network/vyos/rm_templates/ospf_interfaces_14.py +++ b/plugins/module_utils/network/vyos/rm_templates/ospf_interfaces_14.py @@ -1,650 +1,651 @@ # -*- coding: utf-8 -*- # Copyright 2020 Red Hat # GNU General Public License v3.0+ # (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import absolute_import, division, print_function + __metaclass__ = type """ The Ospf_interfaces parser templates file. This contains a list of parser definitions and associated functions that facilitates both facts gathering and native command generation for the given network resource. """ import re from ansible_collections.ansible.netcommon.plugins.module_utils.network.common.rm_base.network_template import ( NetworkTemplate, ) def _get_parameters(data): if data["afi"] == "ipv6": val = ["ospfv3", "ipv6"] else: val = ["ospf", "ip"] return val def _tmplt_ospf_int_delete(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) ) return command def _tmplt_ospf_int_cost(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " cost {cost}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_auth_password(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " authentication plaintext-password {plaintext_password}".format( **config_data["address_family"]["authentication"] ) ) return command def _tmplt_ospf_int_auth_md5(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " authentication md5 key-id {key_id} ".format( **config_data["address_family"]["authentication"]["md5_key"] ) + "md5-key {key}".format(**config_data["address_family"]["authentication"]["md5_key"]) ) return command def _tmplt_ospf_int_auth_md5_delete(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " authentication" ) return command def _tmplt_ospf_int_bw(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " bandwidth {bandwidth}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_hello_interval(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " hello-interval {hello_interval}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_dead_interval(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " dead-interval {dead_interval}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_mtu_ignore(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " mtu-ignore" ) return command def _tmplt_ospf_int_network(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " network {network}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_priority(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " priority {priority}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_retransmit_interval(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " retransmit-interval {retransmit_interval}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_transmit_delay(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " transmit-delay {transmit_delay}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_ifmtu(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " ifmtu {ifmtu}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_instance(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " instance-id {instance}".format(**config_data["address_family"]) ) return command def _tmplt_ospf_int_passive(config_data): params = _get_parameters(config_data["address_family"]) command = ( "protocols " + params[0] + " interface {name}".format(**config_data) + " passive" ) return command class Ospf_interfacesTemplate14(NetworkTemplate): def __init__(self, lines=None, module=None): prefix = {"set": "set", "remove": "delete"} super(Ospf_interfacesTemplate14, self).__init__( lines=lines, tmplt=self, prefix=prefix, module=module ) # fmt: off PARSERS = [ { "name": "ip_ospf", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) *$""", re.VERBOSE, ), "remval": _tmplt_ospf_int_delete, "compval": "address_family", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', } } } }, { "name": "authentication_password", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+authentication \s+plaintext-password \s+(?P\S+) *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_auth_password, "compval": "address_family.authentication", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "authentication": { "plaintext_password": "{{ text }}" } } } } }, { "name": "authentication_md5", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+authentication \s+md5 \s+key-id \s+(?P\d+) \s+md5-key \s+(?P\S+) *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_auth_md5, "remval": _tmplt_ospf_int_auth_md5_delete, "compval": "address_family.authentication", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "authentication": { "md5_key": { "key_id": "{{ id }}", "key": "{{ text }}" } } } } } }, { "name": "bandwidth", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+bandwidth \s+(?P\'\d+\') *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_bw, "compval": "address_family.bandwidth", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "bandwidth": "{{ bw }}" } } } }, { "name": "cost", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+cost \s+(?P\'\d+\') *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_cost, "compval": "address_family.cost", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "cost": "{{ val }}" } } } }, { "name": "hello_interval", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+hello-interval \s+(?P\'\d+\') *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_hello_interval, "compval": "address_family.hello_interval", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "hello_interval": "{{ val }}" } } } }, { "name": "dead_interval", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+dead-interval \s+(?P\'\d+\') *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_dead_interval, "compval": "address_family.dead_interval", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "dead_interval": "{{ val }}" } } } }, { "name": "mtu_ignore", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+(?Pmtu-ignore) *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_mtu_ignore, "compval": "address_family.mtu_ignore", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "mtu_ignore": "{{ True if mtu is defined }}" } } } }, { "name": "network", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+network \s+(?P\S+) *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_network, "compval": "address_family.network", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "network": "{{ val }}" } } } }, { "name": "priority", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+priority \s+(?P\'\d+\') *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_priority, "compval": "address_family.priority", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "priority": "{{ val }}" } } } }, { "name": "retransmit_interval", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+retransmit-interval \s+(?P\'\d+\') *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_retransmit_interval, "compval": "address_family.retransmit_interval", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "retransmit_interval": "{{ val }}" } } } }, { "name": "transmit_delay", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+transmit-delay \s+(?P\'\d+\') *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_transmit_delay, "compval": "address_family.transmit_delay", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "transmit_delay": "{{ val }}" } } } }, { "name": "ifmtu", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+ifmtu \s+(?P\'\d+\') *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_ifmtu, "compval": "address_family.ifmtu", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "ifmtu": "{{ val }}" } } } }, { "name": "instance", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+instance-id \s+(?P\'\d+\') *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_instance, "compval": "address_family.instance", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "instance": "{{ val }}" } } } }, { "name": "passive", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) \s+(?Ppassive) *$""", re.VERBOSE, ), "setval": _tmplt_ospf_int_passive, "compval": "address_family.passive", "result": { "name": "{{ name }}", "address_family": { '{{ "ipv4" if proto == "ospf" else "ipv6" }}': { "afi": '{{ "ipv4" if proto == "ospf" else "ipv6" }}', "passive": "{{ True if pass is defined }}" } } } }, { "name": "interface_name", "getval": re.compile( r""" ^set \s+protocols \s+(?Pospf|ospfv3) \s+interface \s+(?P\S+) .*$""", re.VERBOSE, ), "setval": "set protocols {{ proto }} interface {{ name }}", "result": { "name": "{{ name }}", } }, ] # fmt: on diff --git a/plugins/module_utils/network/vyos/utils/version.py b/plugins/module_utils/network/vyos/utils/version.py index cc3028c3..6d84ef1c 100644 --- a/plugins/module_utils/network/vyos/utils/version.py +++ b/plugins/module_utils/network/vyos/utils/version.py @@ -1,13 +1,15 @@ # -*- coding: utf-8 -*- # Copyright (c) 2021, Felix Fontein # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) # SPDX-License-Identifier: GPL-3.0-or-later """Provide version object to compare version numbers.""" from __future__ import absolute_import, division, print_function + + __metaclass__ = type from ansible.module_utils.compat.version import LooseVersion # pylint: disable=unused-import diff --git a/plugins/modules/vyos_vlan.py b/plugins/modules/vyos_vlan.py index f0b68bc9..9d23cc7c 100644 --- a/plugins/modules/vyos_vlan.py +++ b/plugins/modules/vyos_vlan.py @@ -1,395 +1,391 @@ #!/usr/bin/python # -*- coding: utf-8 -*- # Copyright: (c) 2017, Ansible by Red Hat, inc # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) from __future__ import absolute_import, division, print_function __metaclass__ = type DOCUMENTATION = """ module: vyos_vlan author: Trishna Guha (@trishnaguha) short_description: Manage VLANs on VyOS network devices description: - This module provides declarative management of VLANs on VyOS network devices. version_added: 1.0.0 notes: - Tested against VyOS 1.3.8, 1.4.2, the upcoming 1.5, and the rolling release of spring 2025. - This module works with connection C(ansible.netcommon.network_cli). See L(the VyOS OS Platform Options,../network/user_guide/platform_vyos.html). options: name: description: - Name of the VLAN. type: str address: description: - Configure Virtual interface address. type: str vlan_id: description: - ID of the VLAN. Range 0-4094. type: int interfaces: description: - List of interfaces that should be associated to the VLAN. type: list elements: str associated_interfaces: description: - This is a intent option and checks the operational state of the for given vlan C(name) for associated interfaces. If the value in the C(associated_interfaces) does not match with the operational state of vlan on device it will result in failure. type: list elements: str delay: description: - Delay the play should wait to check for declarative intent params values. default: 10 type: int aggregate: description: List of VLANs definitions. type: list elements: dict suboptions: name: description: - Name of the VLAN. type: str address: description: - Configure Virtual interface address. type: str vlan_id: description: - ID of the VLAN. Range 0-4094. type: int required: true interfaces: description: - List of interfaces that should be associated to the VLAN. type: list elements: str required: true associated_interfaces: description: - This is a intent option and checks the operational state of the for given vlan C(name) for associated interfaces. If the value in the C(associated_interfaces) does not match with the operational state of vlan on device it will result in failure. type: list elements: str delay: description: - Delay the play should wait to check for declarative intent params values. type: int state: description: - State of the VLAN configuration. type: str choices: - present - absent purge: description: - Purge VLANs not defined in the I(aggregate) parameter. default: false type: bool state: description: - State of the VLAN configuration. default: present type: str choices: - present - absent extends_documentation_fragment: - vyos.vyos.vyos """ EXAMPLES = """ - name: Create vlan vyos.vyos.vyos_vlan: vlan_id: 100 name: vlan-100 interfaces: eth1 state: present - name: Add interfaces to VLAN vyos.vyos.vyos_vlan: vlan_id: 100 interfaces: - eth1 - eth2 - name: Configure virtual interface address vyos.vyos.vyos_vlan: vlan_id: 100 interfaces: eth1 address: 172.26.100.37/24 - name: vlan interface config + intent vyos.vyos.vyos_vlan: vlan_id: 100 interfaces: eth0 associated_interfaces: - eth0 - name: vlan intent check vyos.vyos.vyos_vlan: vlan_id: 100 associated_interfaces: - eth3 - eth4 - name: Delete vlan vyos.vyos.vyos_vlan: vlan_id: 100 interfaces: eth1 state: absent """ RETURN = """ commands: description: The list of configuration mode commands to send to the device returned: always type: list sample: - set interfaces ethernet eth1 vif 100 description VLAN 100 - set interfaces ethernet eth1 vif 100 address 172.26.100.37/24 - delete interfaces ethernet eth1 vif 100 """ import re import time from copy import deepcopy from ansible.module_utils._text import to_text from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.common.validation import check_required_one_of from ansible_collections.ansible.netcommon.plugins.module_utils.network.common.utils import ( remove_default_spec, ) from ansible_collections.vyos.vyos.plugins.module_utils.network.vyos.vyos import ( load_config, run_commands, ) def search_obj_in_list(vlan_id, lst): obj = list() for o in lst: if o["vlan_id"] == vlan_id: obj.append(o) return obj def map_obj_to_commands(updates, module): commands = list() want, have = updates purge = module.params["purge"] for w in want: vlan_id = w["vlan_id"] name = w["name"] address = w["address"] state = w["state"] obj_in_have = search_obj_in_list(vlan_id, have) if state == "absent": if obj_in_have: for obj in obj_in_have: for i in obj["interfaces"]: commands.append("delete interfaces ethernet {0} vif {1}".format(i, vlan_id)) elif state == "present": if not obj_in_have: if w["interfaces"] and w["vlan_id"]: for i in w["interfaces"]: cmd = "set interfaces ethernet {0} vif {1}".format(i, vlan_id) if w["name"]: commands.append(cmd + " description {0}".format(name)) elif w["address"]: commands.append(cmd + " address {0}".format(address)) else: commands.append(cmd) if purge: for h in have: obj_in_want = search_obj_in_list(h["vlan_id"], want) if not obj_in_want: for i in h["interfaces"]: commands.append( "delete interfaces ethernet {0} vif {1}".format(i, h["vlan_id"]), ) return commands def map_params_to_obj(module): obj = [] aggregate = module.params.get("aggregate") if aggregate: for item in aggregate: for key in item: if item.get(key) is None: item[key] = module.params[key] d = item.copy() if not d["vlan_id"]: module.fail_json(msg="vlan_id is required") d["vlan_id"] = str(d["vlan_id"]) try: check_required_one_of(module.required_one_of, item) except TypeError as exc: module.fail_json(to_text(exc)) obj.append(d) else: obj.append( { "vlan_id": str(module.params["vlan_id"]), "name": module.params["name"], "address": module.params["address"], "state": module.params["state"], "interfaces": module.params["interfaces"], "associated_interfaces": module.params["associated_interfaces"], }, ) return obj def map_config_to_obj(module): objs = [] output = run_commands(module, "show interfaces") lines = output[0].strip().splitlines()[3:] for line in lines: splitted_line = re.split(r"\s{2,}", line.strip()) obj = {} eth = splitted_line[0].strip("'") - if eth.startswith("eth"): + if eth.startswith("eth") and "." in eth: obj["interfaces"] = [] - if "." in eth: - interface = eth.split(".")[0] - obj["interfaces"].append(interface) - obj["vlan_id"] = eth.split(".")[-1] - else: - obj["interfaces"].append(eth) - obj["vlan_id"] = None + interface = eth.split(".")[0] + obj["interfaces"].append(interface) + obj["vlan_id"] = eth.split(".")[-1] if splitted_line[1].strip("'") != "-": obj["address"] = splitted_line[1].strip("'") if len(splitted_line) > 3: obj["name"] = splitted_line[3].strip("'") obj["state"] = "present" objs.append(obj) return objs def check_declarative_intent_params(want, module, result): have = None obj_interface = list() is_delay = False for w in want: if w.get("associated_interfaces") is None: continue if result["changed"] and not is_delay: time.sleep(module.params["delay"]) is_delay = True if have is None: have = map_config_to_obj(module) obj_in_have = search_obj_in_list(w["vlan_id"], have) if obj_in_have: for obj in obj_in_have: obj_interface.extend(obj["interfaces"]) for w in want: if w.get("associated_interfaces") is None: continue for i in w["associated_interfaces"]: if (set(obj_interface) - set(w["associated_interfaces"])) != set([]): module.fail_json( msg="Interface {0} not configured on vlan {1}".format(i, w["vlan_id"]), ) def main(): """main entry point for module execution""" element_spec = dict( vlan_id=dict(type="int"), name=dict(), address=dict(), interfaces=dict(type="list", elements="str"), associated_interfaces=dict(type="list", elements="str"), delay=dict(default=10, type="int"), state=dict(default="present", choices=["present", "absent"]), ) aggregate_spec = deepcopy(element_spec) aggregate_spec["vlan_id"].update(required=True) aggregate_spec["interfaces"].update(required=True) # remove default in aggregate spec, to handle common arguments remove_default_spec(aggregate_spec) argument_spec = dict( aggregate=dict(type="list", elements="dict", options=aggregate_spec), purge=dict(default=False, type="bool"), ) argument_spec.update(element_spec) required_one_of = [ ["vlan_id", "aggregate"], ["aggregate", "interfaces", "associated_interfaces"], ] mutually_exclusive = [["vlan_id", "aggregate"]] module = AnsibleModule( argument_spec=argument_spec, supports_check_mode=True, required_one_of=required_one_of, mutually_exclusive=mutually_exclusive, ) warnings = list() result = {"changed": False} if warnings: result["warnings"] = warnings want = map_params_to_obj(module) have = map_config_to_obj(module) commands = map_obj_to_commands((want, have), module) result["commands"] = commands if commands: commit = not module.check_mode load_config(module, commands, commit=commit) result["changed"] = True check_declarative_intent_params(want, module, result) module.exit_json(**result) if __name__ == "__main__": main() diff --git a/tests/unit/modules/network/vyos/test_vyos_vlan.py b/tests/unit/modules/network/vyos/test_vyos_vlan.py new file mode 100644 index 00000000..4f2ea69a --- /dev/null +++ b/tests/unit/modules/network/vyos/test_vyos_vlan.py @@ -0,0 +1,119 @@ +# (c) 2016 Red Hat Inc. +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +from __future__ import absolute_import, division, print_function + + +__metaclass__ = type + +from unittest.mock import patch + +from ansible_collections.vyos.vyos.plugins.modules import vyos_vlan +from ansible_collections.vyos.vyos.tests.unit.modules.utils import set_module_args + +from .vyos_module import TestVyosModule + + +SHOW_INTERFACES_OUTPUT = """\ +Codes: S - State, L - Link, u - Up, D - Down, A - Admin Down +Interface IP Address S/L Description +--------- ---------- --- ----------- +eth0 10.0.2.15/24 u/u +eth0.100 - u/u vlan-100 +eth1 - u/u +eth1.200 192.0.2.1/24 u/u vlan-200 +eth2 - u/u +lo 127.0.0.1/8 u/u + ::1/128 +""" + + +class TestVyosVlanModule(TestVyosModule): + module = vyos_vlan + + def setUp(self): + super(TestVyosVlanModule, self).setUp() + + self.mock_load_config = patch( + "ansible_collections.vyos.vyos.plugins.modules.vyos_vlan.load_config", + ) + self.load_config = self.mock_load_config.start() + + self.mock_run_commands = patch( + "ansible_collections.vyos.vyos.plugins.modules.vyos_vlan.run_commands", + ) + self.run_commands = self.mock_run_commands.start() + + def tearDown(self): + super(TestVyosVlanModule, self).tearDown() + self.mock_load_config.stop() + self.mock_run_commands.stop() + + def load_fixtures(self, commands=None, filename=None): + self.load_config.return_value = dict(diff=None, session="session") + self.run_commands.return_value = [SHOW_INTERFACES_OUTPUT] + + def test_vyos_vlan_purge_no_bare_interfaces(self): + """Purge should only delete VLANs, not bare interfaces without vlan_id.""" + set_module_args( + dict( + vlan_id=100, + interfaces=["eth0"], + state="present", + purge=True, + ), + ) + result = self.execute_module(changed=True) + # Should only delete eth1.200, not bare eth0/eth1/eth2 with vif None + for cmd in result.get("commands", []): + self.assertNotIn( + "vif None", + cmd, + "Purge generated 'vif None' command for bare interface: {0}".format(cmd), + ) + # eth1.200 should be purged since it's not in the desired state + self.assertIn( + "delete interfaces ethernet eth1 vif 200", + result.get("commands", []), + ) + + def test_vyos_vlan_present(self): + set_module_args( + dict( + vlan_id=300, + name="vlan-300", + interfaces=["eth2"], + state="present", + ), + ) + commands = [ + "set interfaces ethernet eth2 vif 300 description vlan-300", + ] + self.execute_module(changed=True, commands=commands) + + def test_vyos_vlan_absent(self): + set_module_args( + dict( + vlan_id=100, + interfaces=["eth0"], + state="absent", + ), + ) + commands = [ + "delete interfaces ethernet eth0 vif 100", + ] + self.execute_module(changed=True, commands=commands)