diff options
| author | Yuriy Andamasov <yuriy@vyos.io> | 2026-04-26 18:37:37 +0300 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2026-04-26 18:37:37 +0300 |
| commit | 64d89ede23ee0b1a95ab0b868637e77c450994d2 (patch) | |
| tree | cc0843a4111a305caee80895b8a647ea99e9c2a2 | |
| parent | bbd0f4163d601e0d62e95c148fe91bbb92b39dd5 (diff) | |
| parent | 80035465ca7345e9ba5aae8e6d49cf536bd75aa0 (diff) | |
| download | vyos.vyos-fix/isort-violations.tar.gz vyos.vyos-fix/isort-violations.zip | |
Merge branch 'main' into fix/isort-violationsfix/isort-violations
| -rw-r--r-- | .coderabbit.yaml | 591 | ||||
| -rw-r--r-- | .github/workflows/ah_token_refresh.yml | 2 | ||||
| -rw-r--r-- | changelogs/fragments/coderabbit-config.yml | 3 |
3 files changed, 595 insertions, 1 deletions
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_<module>). + - 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_<resource>_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 <asn> neighbor <addr> + 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_<module>_<state>_<scenario>). + + 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_<module>.py. + Inherit from TestVyosModule (import from vyos_module.py in same directory). + + Structure: + ```python + class TestVyos<Module>Module(TestVyosModule): + module = vyos_<module> + + 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.<resource>.<resource>.<FactsClass>.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_<module>_config.cfg") + self.execute_show_command.side_effect = load_from_file + ``` + + Required test methods for each resource module: + - test_vyos_<module>_merged: config change, changed=True, verify commands list + - test_vyos_<module>_merged_idempotent: no-op, changed=False, commands=[] + - test_vyos_<module>_replaced: replaced state, changed=True + - test_vyos_<module>_replaced_idempotent: replaced no-op, changed=False + - test_vyos_<module>_overridden: full override, changed=True + - test_vyos_<module>_deleted: deletion, changed=True + - test_vyos_<module>_gathered: state=gathered, verify result["gathered"] dict + - test_vyos_<module>_rendered: state=rendered, verify result["rendered"] commands + - test_vyos_<module>_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="<state>")) before execute_module. + Create fixture files in tests/unit/modules/network/vyos/fixtures/ named + vyos_<module>_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_<module>.py. + Inherit from TestVyosModule. + + Legacy modules mock differently from resource modules: + ```python + class TestVyos<Module>Module(TestVyosModule): + module = vyos_<module> + + def setUp(self): + super().setUp() + # Mock run_commands directly on the module: + self.mock_run_commands = patch( + "ansible_collections.vyos.vyos.plugins.modules." + "vyos_<module>.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_<module>.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/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 @@ -7,7 +7,7 @@ on: 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: 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. |
