From 1378412a9e70dfbfe0ed4a0c42b43f4bcb166f9b Mon Sep 17 00:00:00 2001 From: Matt Davis <6775756+nitzmahone@users.noreply.github.com> Date: Wed, 24 Jun 2026 18:11:13 -0700 Subject: [PATCH] Deconstruct AGENTS.md to context dir (#87162) * Deconstruct AGENTS.md to context dir * Split guidelines applicable to both humans and agents to top-level `context` dir. * Instruct agents to read all context files for now- piecemeal/just-in-time context ingestion has been spotty and problematic to date. Co-authored-by: Matt Clay Assisted-by: Claude * update package-data sanity to ignore context * markdown line length sanity * Ignore context dir in change detection * Remove core plugin deprecation instructions * Inconsistent with published docs and docsite behavior- resolve those first. --------- Co-authored-by: Matt Clay --- AGENTS.md | 233 ++---------------- README.md | 4 +- context/README.md | 26 ++ context/ci.md | 7 + context/code-structure.md | 46 ++++ context/coding-style.md | 83 +++++++ context/contributing.md | 20 ++ context/data-tagging.md | 14 ++ context/deprecation.md | 13 + context/dev-environment.md | 28 +++ context/documentation-standards.md | 55 +++++ context/error-handling.md | 72 ++++++ context/licensing.md | 6 + context/public-api.md | 27 ++ context/running-tests.md | 70 ++++++ context/writing-tests.md | 8 + .../_internal/classification/__init__.py | 3 + test/sanity/code-smell/package-data.py | 1 + 18 files changed, 505 insertions(+), 211 deletions(-) create mode 100644 context/README.md create mode 100644 context/ci.md create mode 100644 context/code-structure.md create mode 100644 context/coding-style.md create mode 100644 context/contributing.md create mode 100644 context/data-tagging.md create mode 100644 context/deprecation.md create mode 100644 context/dev-environment.md create mode 100644 context/documentation-standards.md create mode 100644 context/error-handling.md create mode 100644 context/licensing.md create mode 100644 context/public-api.md create mode 100644 context/running-tests.md create mode 100644 context/writing-tests.md diff --git a/AGENTS.md b/AGENTS.md index 51f95f7d863..c0df9b2a9fd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,33 +9,30 @@ This file provides guidance to Claude Code (claude.ai/code) and other compatible **BEFORE starting any PR review or development task:** 1. **Read this file first** - Don't work from memory or assumptions -2. **Use TodoWrite** to create a task list and track progress systematically -3. **Follow the numbered steps** in the relevant process sections -4. **Reference Quick Reference** for correct commands and patterns +2. **Read all [context](context) files** for the project's coding conventions and policies +3. **Use TodoWrite** to create a task list and track progress systematically +4. **Follow the numbered steps** in the relevant process sections +5. **Reference Quick Reference** for correct commands and patterns ## ⚠️ CRITICAL: Licensing Requirements -**NEVER suggest, recommend, or approve code that violates these requirements:** +**NEVER suggest, recommend, or approve code that violates the project's [licensing requirements](context/licensing.md).** +Always verify any new dependencies or suggested libraries are license-compatible. +This is non-negotiable -- licensing violations can create serious legal issues for the project. -- **ansible-core**: All code must be **GPLv3 compatible** -- **lib/ansible/module_utils/**: Defaults to **BSD-2-Clause** (more permissive) -- **External dependencies**: Only recommend libraries compatible with these licenses -- **PR reviews**: Always verify any new dependencies or suggested libraries are license-compatible -- **When in doubt**: Ask about licensing compatibility rather than assuming +## General Principles -**This is non-negotiable** - licensing violations can create serious legal issues for the project. +When reviewing code, don't flag issues that `ansible-test sanity` already catches. +Focus review effort on things automated checks can't verify. + +## Attribution + +Agents should disclose their involvement in contributions. +We recommend using an `Assisted-by:` commit trailer identifying the AI tool that assisted. ## Quick Reference -Most commonly used commands and patterns: - ```bash -# Testing -ansible-test sanity -v --docker default # Run all sanity tests -ansible-test sanity -v --docker default --test # Run specific sanity test -ansible-test units -v --docker default # Run unit tests -ansible-test integration -v --docker ubuntu2404 # Run integration tests - # PR Review and CI gh pr view # Get PR details gh pr view --comments # Check for ansibot CI failures @@ -45,72 +42,12 @@ gh pr diff # See all changes /azp-logs # Download CI logs for PR ``` -**Container Selection:** - -- Sanity/Unit tests: `--docker default` -- Integration tests: `--docker ubuntu2204`, `--docker ubuntu2404`, etc. (NOT default/base) - **Critical Reminders:** -- **Licensing**: See [Licensing Requirements](#️-critical-licensing-requirements) - GPLv3/BSD-2-Clause only +- **Licensing**: See [Licensing Requirements](context/licensing.md) - GPLv3/BSD-2-Clause only +- **Testing**: See [running-tests](context/running-tests.md) for `ansible-test` commands and container selection -## Development Environment Setup - -Ansible development typically uses an editable install after forking and cloning: - -```bash -# After forking and cloning the repository -pip install -e . -``` - -**Note:** ansible-core and all CLIs (including ansible-test) require a POSIX OS. On Windows, use WSL (Windows Subsystem for Linux). - -## Testing and CI - -### Basic Testing Commands - -```bash -# Run sanity tests - these are linting/static analysis (pylint, mypy, pep8, etc.) -ansible-test sanity -v --docker default - -# List available sanity tests -ansible-test sanity --list-tests - -# Run specific sanity tests -ansible-test sanity -v --docker default --test pep8 --test pylint - -# Run sanity on specific files (paths relative to repo root) -ansible-test sanity -v --docker default lib/ansible/modules/command.py - -# Run unit tests (recommended with Docker) -ansible-test units -v --docker default - -# Run specific unit test (paths relative to repo root, targets in test/units/) -ansible-test units -v --docker default test/units/modules/test_command.py - -# Run integration tests (choose appropriate container - NOT base/default) -ansible-test integration -v --docker ubuntu2404 - -# Run specific integration target (directory name in test/integration/targets/) -ansible-test integration -v --docker ubuntu2404 setup_remote_tmp_dir - -# Run with coverage -ansible-test units -v --docker default --coverage - -# Alternative: use --venv if Docker/Podman unavailable (less reliable for units/integration) -ansible-test sanity -v --venv -``` - -Available Docker containers for testing can be found in `./test/lib/ansible_test/_data/completion/docker.txt`. -The `base` and `default` containers are for sanity/unit tests only. For integration tests, use distro-specific -containers, depending on the modules being tested. - -**Test isolation options:** - -- `--docker` (supports Docker or Podman) - preferred for reliable, isolated testing -- `--venv` - fallback when containers unavailable, but unit tests may be unreliable due to host environment differences - -### Helping Developers with CI Failures +## Helping Developers with CI Failures When developers submit PRs and encounter CI failures, use these approaches to help diagnose and resolve issues: @@ -138,13 +75,7 @@ This shows: - Direct links to Azure DevOps build results - Individual job results (Sanity Test 1/2, Docker tests, Units, etc.) -**3. Common CI failure patterns:** - -- **Sanity failures**: Usually have specific fixes (trailing whitespace, import errors, etc.) -- **Integration test failures**: May require platform-specific containers or test adjustments -- **Unit test failures**: Often indicate actual code issues that need debugging - -**4. CI failure analysis workflow:** +**3. CI failure analysis workflow:** 1. Check ansibot comments first for immediate error details 2. Use `gh pr checks ` to get Azure Pipelines URLs for detailed logs @@ -152,7 +83,7 @@ This shows: 4. For sanity test failures, the error messages usually indicate exactly what needs to be fixed 5. For test failures, run the same tests locally using `ansible-test` to reproduce and debug -**5. Downloading Azure Pipelines logs for analysis:** +**4. Downloading Azure Pipelines logs for analysis:** When CI failures need deeper investigation beyond what's visible in ansibot comments or the web UI, use the `/azp-logs` skill: @@ -209,13 +140,8 @@ Use this checklist for EVERY PR review: When assisting with PR reviews, verify: -### Required Components - -- Changelog fragment exists in `changelogs/fragments/` -- Appropriate tests are included and cover the changed code - - Unit tests should be pytest style, and functional rather than tightly coupled to mocking - - Integration tests required for almost all plugin changes (tests the public API) - - Tests should exercise the actual changed code, not just add random coverage +- Changelog fragment exists (see [changelog requirements](context/documentation-standards.md#changelog-requirements)) +- Appropriate tests are included (see [test expectations](context/writing-tests.md#test-expectations-for-pull-requests)) ### Review Process @@ -223,21 +149,12 @@ Follow these steps in order for thorough PR reviews: 1. **Get PR details**: Use `gh pr view ` to understand the PR scope and description 2. **Get PR diff**: Use `gh pr diff ` to see all changes -3. **Check required components FIRST**: - - Verify changelog fragment exists and uses correct section (check `changelogs/config.yaml` for valid sections) - - Verify tests exist and specifically cover the changed code paths +3. **Check required components FIRST** (changelog and tests -- see links above) 4. **Checkout PR branch**: Use `gh pr checkout ` to examine code holistically with changes applied 5. **Review existing feedback**: Use `gh pr view --comments` to see all comments and previous review feedback 6. **Verify all issues addressed**: Ensure all bot failures, reviewer requests, and discussion points are resolved 7. **Call out any unresolved review feedback**: Explicitly mention any discussions or requests that remain unaddressed -### Common Review Issues to Check - -- **Changelog section errors**: Verify changelog uses valid section from `changelogs/config.yaml`. Fragment structure follows sections defined there. -- **Test scope**: Ensure tests exercise the actual changed code, not just add random coverage. - Integration tests required for almost all plugin changes (tests the public API). - Tests should be functional rather than tightly coupled to mocking. - ### Review Task Management - Use TodoWrite tool to track review steps for complex PRs @@ -253,107 +170,3 @@ Follow these steps in order for thorough PR reviews: - `gh pr checkout ` - Switch to PR branch for holistic examination - `Read` tool - Examine specific changed files in detail - `Grep` tool - Search for related code patterns or test coverage (uses ripgrep/rg) - -## Development Guidelines - -### Code Style Notes - -- Line limit is 160 characters (not 80) -- E402 (module level import not at top) is ignored -- In `lib/ansible/modules/`, imports must come after DOCUMENTATION, EXAMPLES, and RETURN definitions -- Don't add obvious comments about code -- Use native type hints with `from __future__ import annotations` (converts to strings at runtime) -- Don't document module parameters in docstrings - migrate to type hints instead -- **No trailing whitespace**: Always clean up trailing spaces on lines, especially when editing existing files - -### Python Version Support - -- Controller code: support range defined in `pyproject.toml` -- Modules/module_utils: minimum version in `lib/ansible/module_utils/basic.py` (`_PY_MIN`) up to max from `pyproject.toml` -- Modules support a wider Python version range than controller code - -### Dependencies and Imports - -- Prefer Python stdlib over external dependencies -- Use existing code from within the Ansible project -- `lib/ansible/modules/` can only import from `lib/ansible/module_utils/` (modules are packaged for remote execution) -- `lib/ansible/module_utils/` cannot import from outside itself - -## Documentation Standards - -### Module and Plugin Documentation - -- Modules and plugins require DOCUMENTATION, EXAMPLES, and RETURN blocks as static YAML string variables -- These blocks cannot be dynamically generated - they are parsed via AST/token parsing -- Alternative: "sidecar" documentation as `.yml` files with same stem name adjacent to plugin files -- All modules should have a `main()` function and `if __name__ == '__main__':` block -- Use `version_added` fields in documentation following existing version format patterns - -### Changelog Requirements - -- Changes require entries in `changelogs/fragments/` as YAML files -- Create a new fragment file per PR (never reuse existing fragments to avoid merge conflicts) -- Fragment structure follows sections defined in `changelogs/config.yaml` under the `sections` key -- Naming: `{issue_number}-{short-description}.yml` or `{component}-{description}.yml` if no issue -- Format: `- {component} - {description} ({optional URL to GH issue})` -- Content supports Sphinx markup (use double backticks for code references) - -## Repository Management - -### Plugin Development - -- New plugins should go into collections, not ansible-core -- ansible-core rarely accepts new plugins; core team makes these decisions - -### Branch and Release Management - -- All PRs target the `devel` branch -- Use GitHub templates when creating issues/PRs (`.github/ISSUE_TEMPLATE/` and `.github/PULL_REQUEST_TEMPLATE/`) -- For issues: fill out the `component` field with project root relative file path -- For PRs: adjust the issue type in the template as listed in `.github/PULL_REQUEST_TEMPLATE/PULL_REQUEST_TEMPLATE.md` -- Validate issues are fixed in `devel` before reporting against stable releases -- Bug fixes: backported to latest stable only -- Critical bug fixes: backported to latest and previous stable -- Security issues: contact security@ansible.com privately, not via GitHub - -### Backwards Compatibility - -- Backwards compatibility is prioritized over most other concerns -- Deprecation cycle: 4 releases (deprecation + 2 releases + removal) -- Use `Display.deprecated` or `AnsibleModule.deprecate` with version from `lib/ansible/release.py` plus 3 -- Example: deprecating in 2.19 means removal in 2.22 - -## Code Structure Reference - -### Core Structure - -- `lib/ansible/` - Main Ansible library code - - `cli/` - Command-line interface implementations (ansible, ansible-playbook, etc.) - - `executor/` - Task execution engine and strategies (includes PowerShell support in `powershell/`) - - `inventory/` - Inventory management and parsing - - `modules/` - Core modules (built-in automation modules) - - `module_utils/` - Shared utilities for modules (includes C# in `csharp/` and PowerShell in `powershell/`) - - `plugins/` - Plugin framework (filters, tests, lookups, etc.) - - `vars/` - Variable management - - `config/` - Configuration handling - - `collections/` - Ansible Collections framework - -### Key Components - -- **CLI Layer**: Entry points in `lib/ansible/cli/` handle command parsing and dispatch -- **Executor**: `lib/ansible/executor/` contains the core execution engine that runs tasks and plays -- **Module System**: Modules in `lib/ansible/modules/` are the units of work; they're executed remotely -- **Plugin Architecture**: `lib/ansible/plugins/` provides extensibility through filters, tests, lookups, etc. -- **Inventory**: `lib/ansible/inventory/` manages host and group definitions -- **Collections**: Modern packaging format for distributing Ansible content - -### Testing Infrastructure - -- `test/units/` - Unit tests mirroring the lib structure -- `test/integration/` - Integration tests organized by target (named after plugin/functionality being tested) - - Some targets have `context/controller` or `context/target` in their `aliases` file when not easily inferable - - Only modules run on target hosts; all other plugins execute locally in the ansible process -- `test/lib/` - Test utilities and frameworks -- `ansible-test` - Unified testing tool for all test types - -For CI failure debugging, see [Helping Developers with CI Failures](#helping-developers-with-ci-failures). diff --git a/README.md b/README.md index ca2cf8d5af1..ed464e118bf 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,9 @@ For more ways to get in touch, see [Communicating with the Ansible community](ht ## Coding Guidelines -We document our Coding Guidelines in the [Developer Guide](https://docs.ansible.com/ansible/devel/dev_guide/). We particularly suggest you review: +Development context for ansible-core can be found in the [context](context/) directory. + +We also document our Coding Guidelines in the [Developer Guide](https://docs.ansible.com/ansible/devel/dev_guide/). We particularly suggest you review: * [Contributing your module to Ansible](https://docs.ansible.com/ansible/devel/dev_guide/developing_modules_checklist.html) * [Conventions, tips, and pitfalls](https://docs.ansible.com/ansible/devel/dev_guide/developing_modules_best_practices.html) diff --git a/context/README.md b/context/README.md new file mode 100644 index 00000000000..8bb98fdfd2e --- /dev/null +++ b/context/README.md @@ -0,0 +1,26 @@ +# Context for Humans and Agents + +This directory contains information about developing ansible-core. +It should be equally applicable to both humans and agents. + +This is **not** the place for: + +- **Documentation on how to use Ansible** -- see the [ansible-documentation](https://github.com/ansible/ansible-documentation/) repository. +- **Learning how to develop content for Ansible** (modules, plugins, collections, roles, etc.) -- see the [Ansible Developer Guide](https://docs.ansible.com/ansible/devel/dev_guide/index.html). +- **Agent-specific instructions or guidance** -- see [AGENTS.md](../AGENTS.md). + +## Table of Contents + +- [contributing](contributing.md) - Contributing changes to ansible-core. +- [licensing](licensing.md) - Licensing requirements. +- [dev-environment](dev-environment.md) - Development environment setup. +- [code-structure](code-structure.md) - Directory layout, key components, import restrictions, and plugin policy. +- [coding-style](coding-style.md) - Python versions, dependencies, code formatting, and syntax conventions. +- [error-handling](error-handling.md) - Error and exception handling patterns. +- [data-tagging](data-tagging.md) - Working with data tags. +- [public-api](public-api.md) - Public surface area and internal-by-default conventions. +- [documentation-standards](documentation-standards.md) - Module/plugin documentation and changelog requirements. +- [running-tests](running-tests.md) - Running tests with ansible-test. +- [writing-tests](writing-tests.md) - Test expectations for pull requests. +- [deprecation](deprecation.md) - Backward compatibility and deprecation. +- [ci](ci.md) - Continuous integration. diff --git a/context/ci.md b/context/ci.md new file mode 100644 index 00000000000..f927a4ce240 --- /dev/null +++ b/context/ci.md @@ -0,0 +1,7 @@ +# Continuous integration + +## Common failure patterns + +- **Sanity failures**: Usually have specific fixes (trailing whitespace, import errors, etc.). +- **Integration test failures**: May require platform-specific containers or test adjustments. +- **Unit test failures**: Often indicate actual code issues that need debugging. diff --git a/context/code-structure.md b/context/code-structure.md new file mode 100644 index 00000000000..14ab65df11b --- /dev/null +++ b/context/code-structure.md @@ -0,0 +1,46 @@ +# Core structure + +- `lib/ansible/` - Main Ansible library code. + - `cli/` - Command-line interface implementations (ansible, ansible-playbook, etc.). + - `executor/` - Task execution engine and strategies (includes PowerShell support in `powershell/`). + - `inventory/` - Inventory management and parsing. + - `modules/` - Core modules (built-in automation modules). + - `module_utils/` - Shared utilities for modules (includes C# in `csharp/` and PowerShell in `powershell/`). + - `plugins/` - Plugin framework (filters, tests, lookups, etc.). + - `vars/` - Variable management. + - `config/` - Configuration handling. + - `collections/` - Ansible Collections framework. + +## Key components + +- **CLI Layer**: Entry points in `lib/ansible/cli/` handle command parsing and dispatch. +- **Executor**: `lib/ansible/executor/` contains the core execution engine that runs tasks and plays. +- **Module System**: Modules in `lib/ansible/modules/` are the units of work; they're executed remotely. +- **Plugin Architecture**: `lib/ansible/plugins/` provides extensibility through filters, tests, lookups, etc. +- **Inventory**: `lib/ansible/inventory/` manages host and group definitions. +- **Collections**: Modern packaging format for distributing Ansible content. + +## Plugin development + +- New plugins should go into collections, not ansible-core. +- ansible-core rarely accepts new plugins; core team makes these decisions. + +## Import restrictions + +- `lib/ansible/modules/` can only import from `lib/ansible/module_utils/` (modules are packaged for remote execution). +- `lib/ansible/module_utils/` cannot import from outside itself. + +## Resource embedding + +Modules that need to execute code on Python versions outside the normal module_utils range +can use `EmbedManager.embed()` from `ansible.module_utils.embed` to bundle standalone scripts into the AnsiballZ payload. +Embedded resources go in `lib/ansible/module_utils/_embed/`. + +## Testing infrastructure + +- `test/units/` - Unit tests mirroring the lib structure. +- `test/integration/` - Integration tests organized by target (named after plugin/functionality being tested). + - Some targets have `context/controller` or `context/target` in their `aliases` file when not easily inferable. + - Only modules run on target hosts; all other plugins execute locally in the ansible process. +- `test/lib/` - Test utilities and frameworks. +- `ansible-test` - Unified testing tool for all test types. diff --git a/context/coding-style.md b/context/coding-style.md new file mode 100644 index 00000000000..4d417b2cff3 --- /dev/null +++ b/context/coding-style.md @@ -0,0 +1,83 @@ +# Coding style and syntax + +For new code and updates to existing code, including unit tests. + +In general, prefer newer Python features as they become available. +A feature is available when the minimum supported Python version for the code being written supports it, +and it doesn't conflict with other supported versions. + +## Python version support + +- Controller code: minimum version defined in `pyproject.toml` (`requires-python`). +- Modules/module_utils: minimum version in `lib/ansible/module_utils/basic.py` (`_PY_MIN`). +- Modules support a wider Python version range than controller code. +- Supported Python versions for testing are defined within `ansible-test`. + +## Dependencies + +- Prefer Python stdlib over external dependencies. +- Use existing code from within the Ansible project. + +## Markdown + +Markdown files use [GitHub Flavored Markdown](https://github.github.com/gfm/) (verified by the "pymarkdown" sanity test). + +Use dashes (`-`) for unordered list items, not asterisks. +End list items with periods. + +## ASCII characters + +- Use ASCII quotes (`'` and `"`) instead of Unicode smart quotes (verified by the "no-smart-quotes" sanity test). +- Use ASCII dashes (`-` or `--`) instead of em dashes. + +## Line length + +The line limit is 160 characters. + +## Trailing whitespace + +Don't leave trailing whitespace on lines. + +## Docstrings + +Explain what the annotated code does, but don't create structured entries for parameters. +Don't document parameter types in docstrings -- use type hints instead. + +Anything considered a public API must have a docstring. +Internal code should, and it often makes sense for unit tests, too. + +## Line breaks in source text + +Try to stick to one sentence per line in text like docstrings, comments and changelog fragments. + +## Native type annotations + +Use native type hints with `from __future__ import annotations` (verified by the "boilerplate" sanity test). +Include type annotations on function/method arguments and return types, unless the annotation becomes too complex (e.g. `TypedDict`). +The "mypy" sanity test only performs type checking on annotated functions/methods. + +Prefer PEP 695 type parameter syntax over `TypeVar` and `ParamSpec` declarations. +For example, use `def foo[T](x: T) -> T` instead of declaring `T = TypeVar('T')` separately. +This does not apply to `module_utils/` code, which must support older Python versions that lack the feature. + +## Format strings + +Use f-strings instead of `%` strings or `str.format`, except for logging where formatting is deferred. + +## Quoting strings + +Use the `!r` format qualifier to quote a value instead of manually quoting. + +Example: `f"A string with a {quoted!r} value."` + +## Code formatting check/fix for internals + +The `black` sanity test runs against all `_internal` packages, using default settings with an increased line length of 160 and no quote conversion. +Use `ansible-test` to automatically apply required formatting changes. + +Example: `ansible-test sanity --test black --fix` + +## Import ordering in modules + +The E402 pycodestyle rule (module level import not at top) is ignored. +In `lib/ansible/modules/`, imports must come after the `DOCUMENTATION`, `EXAMPLES`, and `RETURN` definitions. diff --git a/context/contributing.md b/context/contributing.md new file mode 100644 index 00000000000..ce88386b6c6 --- /dev/null +++ b/context/contributing.md @@ -0,0 +1,20 @@ +# Contributing + +## Keep changes focused + +Changes should be limited to what's necessary to solve the problem at hand. +Avoid unrelated reformatting, refactoring, or style adjustments in the same change. + +Unfocused changes make review harder by increasing the diff size and obscuring the important parts. +They can also lead to unnecessary back-and-forth around style and preferences. + +## Branch and release management + +- All PRs target the `devel` branch. +- Use GitHub templates when creating issues/PRs (`.github/ISSUE_TEMPLATE/` and `.github/PULL_REQUEST_TEMPLATE/`). +- For issues: fill out the `component` field with project root relative file path. +- For PRs: adjust the issue type in the template as listed in `.github/PULL_REQUEST_TEMPLATE/PULL_REQUEST_TEMPLATE.md`. +- Validate issues are fixed in `devel` before reporting against stable releases. +- Bug fixes: backported to latest stable only. +- Critical bug fixes: backported to latest and previous stable. +- Security issues: contact security@ansible.com privately, not via GitHub. diff --git a/context/data-tagging.md b/context/data-tagging.md new file mode 100644 index 00000000000..a99f8d32298 --- /dev/null +++ b/context/data-tagging.md @@ -0,0 +1,14 @@ +# Data tagging + +## How not to break things + +Avoid unnecessary mutation of values, such as calling `str.strip` or `to_text`, etc. as these will drop tags, losing things like origin and trust for templating. + +When mutation of a value is necessary, carefully consider which tags, if any, need to be propagated to the resulting value. + +## When tags must be removed + +Some APIs don't understand tagged types and can't automatically treat them as their untagged equivalents. +This includes C-implemented Python APIs that perform exact type checks, serialization libraries that reject derived types, +and any interface where the consumer expects or requires plain types. +In these cases, use `transform_to_native_types()` from `ansible.utils.vars` to convert tagged values to native types before passing them to the API. diff --git a/context/deprecation.md b/context/deprecation.md new file mode 100644 index 00000000000..804aee92229 --- /dev/null +++ b/context/deprecation.md @@ -0,0 +1,13 @@ +# Backward compatibility and deprecation + +Backward compatibility is prioritized over most other concerns. + +## Deprecation cycle + +- Deprecation cycle: 4 releases (deprecation + 2 releases + removal). +- Removal version: current version in `lib/ansible/release.py` plus 3. +- Example: deprecating in 2.19 means removal in 2.22. + +## Deprecating code + +Use `Display.deprecated` or `AnsibleModule.deprecate` with the removal version. diff --git a/context/dev-environment.md b/context/dev-environment.md new file mode 100644 index 00000000000..f704026f7e2 --- /dev/null +++ b/context/dev-environment.md @@ -0,0 +1,28 @@ +# Development environment setup + +ansible-core and all CLIs (including ansible-test) require a POSIX OS. +On Windows, use WSL (Windows Subsystem for Linux). + +## Editable install + +Ansible development typically uses an editable installation after forking and cloning: + +```bash +pip install -e . +``` + +## Hacking script + +An alternative to an editable installation is to source the `hacking/env-setup` script: + +```bash +source hacking/env-setup +``` + +## Running tests without installation + +If you only need to run tests, you can invoke `ansible-test` directly without installing or sourcing the hacking script: + +```bash +bin/ansible-test sanity -v --docker default +``` diff --git a/context/documentation-standards.md b/context/documentation-standards.md new file mode 100644 index 00000000000..c466d3234bd --- /dev/null +++ b/context/documentation-standards.md @@ -0,0 +1,55 @@ +# Documentation standards + +## Module and plugin documentation + +- Modules and plugins require `DOCUMENTATION`, `EXAMPLES`, and `RETURN` blocks as static YAML string variables. +- These blocks cannot be dynamically generated -- they are parsed via AST/token parsing. +- Alternative: "sidecar" documentation as `.yml` files with same stem name adjacent to plugin files. +- All modules should have a `main()` function and `if __name__ == '__main__':` block. +- Use `version_added` fields in documentation following existing version format patterns. +- Filter, test, and lookup plugins should declare positional arguments using the `positional` key in `DOCUMENTATION`. + +## Markup in plugin documentation blocks + +Plugin `DOCUMENTATION`, `EXAMPLES`, and `RETURN` blocks use Ansible-specific markup, not reStructuredText or Markdown. + +### Semantic markup + +- `O(name)` -- option name, optionally with a value: `O(state=present)`. +- `V(value)` -- option value mentioned alone: `V(present)`. +- `RV(name)` -- return value name, optionally with a value: `RV(changed=true)`. +- `E(name)` -- environment variable: `E(ANSIBLE_CONFIG)`. + +Suboptions use dot-separated paths: `O(foo.bar)`. +Backslash escaping is supported inside these macros. + +### Formatting + +- `C()` -- monospace/code text. Example: `This works like the unix command C(grep).`. +- `B()` -- bold text. +- `I()` -- italic text. + +`C()`, `B()`, and `I()` do not support escaping, so they cannot contain `)`. + +Use `V()` instead of `C()` when the value may contain `)`. + +### Linking + +- `M(ansible.builtin.copy)` -- link to a module (FQCN required). +- `P(ansible.builtin.file#lookup)` -- link to a plugin (FQCN and type required). +- `L(title,https://example.com)` -- link with custom title. +- `U(https://example.com)` -- bare URL. +- `R(title,rst_anchor)` -- cross-reference to an RST anchor. + +`O()` and `RV()` can also link to options/return values in other plugins using the syntax +`O(ansible.builtin.copy#module:src)`. + +## Changelog requirements + +- Changes require entries in `changelogs/fragments/` as YAML files. +- Create a new fragment file per PR (never reuse existing fragments to avoid merge conflicts). +- Fragment structure follows sections defined in `changelogs/config.yaml` under the `sections` key. +- Verify changelog fragments use a valid section from `changelogs/config.yaml`. +- Naming: `{issue_number}-{short-description}.yml` or `{component}-{description}.yml` if no issue. +- Format: `- {component} - {description} ({optional URL to GH issue})`. +- Content supports Sphinx markup (use double backticks for code references). diff --git a/context/error-handling.md b/context/error-handling.md new file mode 100644 index 00000000000..be2e2cb1e04 --- /dev/null +++ b/context/error-handling.md @@ -0,0 +1,72 @@ +# Error handling + +## Tracebacks + +Do not manually generate tracebacks for errors or warnings. +Standardized traceback capture is included for errors, warnings and deprecation warnings for both controller and module code (Python). +See the `DISPLAY_TRACEBACK` config option for details on how to enable traceback capture and display. + +## In modules + +In most cases, just raise an exception. +The AnsiballZ wrapper now provides a general exception handler for Python modules, making use of `fail_json` unnecessary, unless the module result needs to be customized. +Calls to the various Python-module-side `warn` and `deprecate` methods/functions will also capture and marshal tracebacks to the controller when enabled. +When using `fail_json` to customize a module failure result, passing the `exception` argument to provide the currently active exception is unnecessary. + +## Deferred exceptions in modules + +When deferring exceptions in modules using `try/except` and `fail_json`, pass the captured `Exception` instance to `fail_json` using the `exception` argument. +The error handling infrastructure will handle collection of error details and traceback formatting. + +## Exception context + +Raising exceptions while another exception is active will cause the active exception to become the `__context__` for the newly raised exception. +This is usually not the intended behavior, and should be reserved for unexpected errors while handling the original exception. +In most cases a `raise from` is desired, either with `None` to suppress the original exception if it's not helpful, +or with the captured exception to set it as the `__cause__` for the newly raised exception. + +Suppression example: `raise Exception("something") from None` + +Cause example: `raise Exception("something") from ex` + +## Raising new exceptions + +Don't catch exceptions just to re-raise them, unless there's additional information that can be added in the newly raised exception. +In most cases, particularly for plugin/module failures, contextual information is automatically added, +making granular `try/except/raise` within the plugin or module unnecessary. + +## Error messages + +Do not repeat previous exception messages when constructing new exceptions. + +Anti-pattern: `raise Exception("it broke: {ex}") from ex` + +The built-in error chain handling mechanisms in Ansible will include the messages from cause/context exceptions automatically. + +In general, error messages passed to exceptions should be a fairly terse description of what happened, +and not contain extra diagnostic, contextual, or prescriptive correction advice (see `obj` and `help_text` below for more on that). + +## When and how to use AnsibleError + +The `AnsibleError` exception type provides support for improved error reporting. +However, if no arguments other than a message are given, there's usually no benefit over using built-in exception types. +So what are those other arguments? + +- `obj` - Usually a variable responsible for the error being raised -- not an `Exception` instance. +If this value is `Origin` tagged, then the error message shown to the user will be able to provide context showing what content triggered the error. +- `help_text` - Instructions and additional detail that helps the user understand how to resolve the error. +By putting that information here, it allows the `message` to be shorter and focus on the problem. +This information will be shown *after* the contextual error details provided by `obj`, if any. + +## Display warnings and errors + +The existing `warning` and `deprecated` methods on the `Display` object now support passing optional `help_text` and `obj` arguments, +matching those on `AnsibleError` for prescriptive guidance and source context from `Origin`-tagged values. +Additionally, the new `error_as_warning` method accepts an exception object and an optional contextual message directly, +allowing for a caught exception to be converted to a warning automatically +while still preserving the exception detail, traceback, and source object context (where applicable). + +## Jinja plugin errors + +In Jinja plugins, the `AnsibleFilterError` and `AnsibleLookupError` exception types are no longer needed. +Instead, use whatever exception type is appropriate for the error condition. diff --git a/context/licensing.md b/context/licensing.md new file mode 100644 index 00000000000..991155a12e8 --- /dev/null +++ b/context/licensing.md @@ -0,0 +1,6 @@ +# Licensing requirements + +- **ansible-core**: All code must be **GPLv3 compatible**. +- **lib/ansible/module_utils/**: Defaults to **BSD-2-Clause** (more permissive). +- **External dependencies**: Only use libraries compatible with these licenses. +- **When in doubt**: Ask about licensing compatibility rather than assuming. diff --git a/context/public-api.md b/context/public-api.md new file mode 100644 index 00000000000..667e0ab4234 --- /dev/null +++ b/context/public-api.md @@ -0,0 +1,27 @@ +# Public surface area + +Starting with ansible-core 2.19 we're trying to be more intentional about what features are part of our public API +and other public surface areas (e.g. CLI, configuration options, module arguments, ansible-core provided Jinja globals, etc.). + +## Imports in public Python modules + +To make public Python modules more self-documenting: + +- Imports in any file considered public API must be sunder-prefixed (e.g. `_module_name`), +to avoid confusion about imported objects being part of the public module API. +- Prefer module-level imports (e.g. `from ansible._internal import _amodule`) with dotted usage (e.g. `foo: _amodule.thing`). +This solves many circular import issues and reduces the need for sunder-prefix aliasing on internal imports in public API. +Hot code paths can use locals or aliased objects, but sparingly and only where it really matters. + +## Internal by default + +New feature implementations in Python code should always be: + +- Added to a module beneath the `ansible._internal` or `ansible.module_utils._internal` package. +- In a sunder-prefixed module (easier public module imports without aliasing). + +Only public types and functions should be added to new modules outside `_internal` packages. +Small sunder-prefixed utility types and functions are okay in public modules, +but in general, non-public API surface area for most new implementations should live completely under an `_internal` package. + +Internal-only types/functions/methods beneath an `_internal` package do not generally require sunder-prefixing on their names. diff --git a/context/running-tests.md b/context/running-tests.md new file mode 100644 index 00000000000..b75a4675947 --- /dev/null +++ b/context/running-tests.md @@ -0,0 +1,70 @@ +# Running tests + +All testing uses `ansible-test`, which supports sanity tests (linting/static analysis), unit tests, and integration tests. + +## Sanity tests + +Sanity tests don't require `--docker`. +Some tests may be skipped if their dependencies aren't installed locally (e.g. additional Python versions, shellcheck, PowerShell), +but this rarely matters when testing your own changes. + +Run sanity tests against all files in your change set, not just the ones you're actively editing. + +```bash +# Run all sanity tests +ansible-test sanity -v + +# List available sanity tests +ansible-test sanity --list-tests + +# Run specific sanity tests +ansible-test sanity -v --test pep8 --test pylint + +# Run sanity on specific files (paths relative to repo root) +ansible-test sanity -v lib/ansible/modules/command.py + +# Run all sanity tests in a container (for full coverage) +ansible-test sanity -v --docker +``` + +> [!NOTE] +> `--docker` without an argument defaults to the `default` container. +> Avoid placing a non-container argument immediately after `--docker`, as it will be interpreted as an image name. + +## Unit tests + +```bash +# Run all unit tests +ansible-test units -v --docker + +# Run a specific unit test (paths relative to repo root, targets in test/units/) +ansible-test units -v --docker test/units/modules/test_command.py + +# Run with coverage +ansible-test units -v --docker --coverage +``` + +## Integration tests + +```bash +# Run all integration tests +ansible-test integration -v --docker ubuntu + +# Run a specific integration target (directory name in test/integration/targets/) +ansible-test integration -v --docker ubuntu ping +``` + +## Container selection + +- Sanity/Unit tests: `--docker` (defaults to the `default` container). +- Integration tests: `--docker ubuntu`, `--docker fedora`, etc. (NOT default/base). + +The `base` and `default` containers are for sanity/unit tests only. +For integration tests, use distro-specific containers, depending on the modules being tested. + +Available containers and their supported Python versions are listed in the `--help` output for each test command. + +## Test isolation options + +- `--docker` (supports Docker or Podman) -- preferred for reliable, isolated testing. +- `--venv` -- fallback when containers are unavailable, but unit tests may be unreliable due to host environment differences. diff --git a/context/writing-tests.md b/context/writing-tests.md new file mode 100644 index 00000000000..26db0211209 --- /dev/null +++ b/context/writing-tests.md @@ -0,0 +1,8 @@ +# Writing tests + +## Test expectations for pull requests + +- Appropriate tests are required and should cover the changed code. +- Unit tests should be pytest style, and functional rather than tightly coupled to mocking. +- Integration tests are required for almost all plugin changes (tests the public API). +- Tests should exercise the actual changed code, not just add random coverage. diff --git a/test/lib/ansible_test/_internal/classification/__init__.py b/test/lib/ansible_test/_internal/classification/__init__.py index 2335070f940..0aa60d15e2e 100644 --- a/test/lib/ansible_test/_internal/classification/__init__.py +++ b/test/lib/ansible_test/_internal/classification/__init__.py @@ -710,6 +710,9 @@ class PathMapper: if path.startswith('changelogs/'): return minimal + if path.startswith('context/'): + return minimal + if path.startswith('hacking/'): return minimal diff --git a/test/sanity/code-smell/package-data.py b/test/sanity/code-smell/package-data.py index ae813d5bc8b..88bbe4eaad1 100644 --- a/test/sanity/code-smell/package-data.py +++ b/test/sanity/code-smell/package-data.py @@ -35,6 +35,7 @@ def collect_sdist_files(complete_file_list: list[str]) -> list[str]: 'changelogs/fragments/*', 'AGENTS.md', 'CLAUDE.md', + 'context/*', 'hacking/*', )