diff --git a/.github/update-best-practice-table.py b/.github/update-best-practice-table.py index 0bc76eb3f..17847ac3d 100755 --- a/.github/update-best-practice-table.py +++ b/.github/update-best-practice-table.py @@ -22,6 +22,11 @@ associated with them) and produce a suitable block of Markdown to include in the Ops documentation. +Each ``Best practice`` admonition may declare a ``:name:`` option, which is used +as a stable identifier for the practice. The generated output tags each bullet +with a trailing ```` HTML comment, so downstream automation +can match practices by ID. + ## Local Usage You'll need a clone of the `operator` repository (presumably the one you're running this command @@ -41,25 +46,51 @@ import pathlib import re +# Block at the top of the generated file, explaining how downstream automation +# can locate per-practice identifiers. The body cannot include a literal `-->` +# (it would close the surrounding HTML comment early and leak the remainder of +# the header into the rendered page), so the example syntax is described +# without showing the closing characters. +HEADER = """\ + +""" + def extract_best_practice_blocks_rst(file_path: pathlib.Path, content: str): """Extracts 'Best practice' blocks from a ReST file. - Returns a tuple of (heading, reference, content). + Returns a list of (heading, reference, content, name) tuples. ``name`` is + the value of the admonition's ``:name:`` option, or ``None`` if not set. """ lines = content.splitlines() - results: list[tuple[str | None, str | None, str]] = [] + results: list[tuple[str | None, str | None, str, str | None]] = [] current_heading = None current_ref = None previous_line = '' inside_admonition = False admonition_lines: list[str] = [] + admonition_name: str | None = None for line in lines: if line.strip() == ':class: hint': continue + name_match = inside_admonition and re.match(r'\s*:name:\s*(\S+)\s*$', line) + if name_match: + admonition_name = name_match.group(1) + continue + rst_match = re.match(r'^[-=]{3,}$', line.strip()) if rst_match and previous_line.strip(): current_heading = previous_line.strip() @@ -69,14 +100,21 @@ def extract_best_practice_blocks_rst(file_path: pathlib.Path, content: str): if inside_admonition: at_end = previous_line.strip() == '' and len(line) > 0 and line[0] != ' ' if at_end: - results.append((current_heading, current_ref, '\n'.join(admonition_lines))) + results.append(( + current_heading, + current_ref, + '\n'.join(admonition_lines), + admonition_name, + )) inside_admonition = False + admonition_name = None else: admonition_lines.append(line) if re.match(r'^\.\. admonition:: Best practice', line): inside_admonition = True admonition_lines.clear() + admonition_name = None previous_line = line continue @@ -87,27 +125,34 @@ def extract_best_practice_blocks_rst(file_path: pathlib.Path, content: str): previous_line = line - results.sort() + results.sort(key=lambda t: (t[0] or '', t[1] or '', t[2])) return results def extract_best_practice_blocks_md(file_path: pathlib.Path, content: str): """Extracts 'Best practice' blocks from a Markdown file. - Returns a tuple of (heading, reference, content). + Returns a list of (heading, reference, content, name) tuples. ``name`` is + the value of the admonition's ``:name:`` option, or ``None`` if not set. """ lines = content.splitlines() - results: list[tuple[str | None, str | None, str]] = [] + results: list[tuple[str | None, str | None, str, str | None]] = [] current_heading = None current_ref = None inside_admonition = False admonition_lines: list[str] = [] + admonition_name: str | None = None for line in lines: if line.strip() == ':class: hint': continue + name_match = inside_admonition and re.match(r'\s*:name:\s*(\S+)\s*$', line) + if name_match: + admonition_name = name_match.group(1) + continue + md_match = re.match(r'^(#{2,})\s+(.*)', line) if md_match: current_heading = md_match.group(2) @@ -116,14 +161,21 @@ def extract_best_practice_blocks_md(file_path: pathlib.Path, content: str): if inside_admonition: at_end = line.strip() == '```' if at_end: - results.append((current_heading, current_ref, '\n'.join(admonition_lines))) + results.append(( + current_heading, + current_ref, + '\n'.join(admonition_lines), + admonition_name, + )) inside_admonition = False + admonition_name = None else: admonition_lines.append(line) if re.match(r'^```{admonition} Best practice', line): inside_admonition = True admonition_lines.clear() + admonition_name = None continue ref_match = re.match(r'\((.+?)\)=', line) @@ -131,7 +183,7 @@ def extract_best_practice_blocks_md(file_path: pathlib.Path, content: str): current_ref = ref_match.group(1) continue - results.sort() + results.sort(key=lambda t: (t[0] or '', t[1] or '', t[2])) return results @@ -158,6 +210,7 @@ def main(): help='Path to a clone of canonical/charmcraft', ) args = parser.parse_args() + print(HEADER) path_to_ops = pathlib.Path(__file__).parent.parent for directory, base_url, make_ref, ref_sub in ( (path_to_ops / 'docs', 'https://documentation.ubuntu.com/ops/latest/', make_ops_ref, None), @@ -188,13 +241,26 @@ def main(): link = f'{base_url}{file_path.relative_to(directory).with_suffix("")}/' if len(practices): print(f'**[{title}]({link})**') - for heading, ref, practice in practices: - ref = make_ref(heading, ref) if ref and heading else ref - see_more = f' See {ref}.' if heading and ref else '' + # Practices with no enclosing section link fall back to linking to + # their own :name: anchor, but only when the link text (the + # heading, or the page title) appears once on this page: + # identically-named links with different destinations are + # confusing. + fallback_texts = [p[0] or title for p in practices if p[3] and not (p[0] and p[1])] + for heading, ref, practice, name in practices: + if heading and ref: + see_more = f' See {make_ref(heading, ref)}.' + elif name and fallback_texts.count(heading or title) == 1: + see_more = f' See {make_ref(heading or title, name)}.' + else: + see_more = '' practice = re.sub(r'\s+', ' ', practice).strip() if ref_sub: practice = re.sub(r':ref:', ref_sub, practice) - print(f'- {practice}{see_more}') + if name: + print(f'- {practice}{see_more} ') + else: + print(f'- {practice}{see_more}') if len(practices): print() diff --git a/docs/_static/best-practice-anchors.js b/docs/_static/best-practice-anchors.js new file mode 100644 index 000000000..5a0f67cb5 --- /dev/null +++ b/docs/_static/best-practice-anchors.js @@ -0,0 +1,12 @@ +document.addEventListener('DOMContentLoaded', () => { + document.querySelectorAll('div.admonition[id^="best-practice-"]').forEach((el) => { + const title = el.querySelector('.admonition-title'); + if (!title) return; + const a = document.createElement('a'); + a.className = 'headerlink'; + a.href = '#' + el.id; + a.title = 'Link to this best practice'; + a.textContent = '¶'; + title.appendChild(a); + }); +}); diff --git a/docs/_static/project_specific.css b/docs/_static/project_specific.css index 9f5b2d133..cea75f7f4 100644 --- a/docs/_static/project_specific.css +++ b/docs/_static/project_specific.css @@ -9,6 +9,17 @@ body { mask-image: var(--icon-star); } +/* Reveal a permalink anchor when hovering a best-practice admonition. The + anchor itself is injected by best-practice-anchors.js. */ +div.admonition[id^="best-practice-"] > .admonition-title > a.headerlink { + visibility: hidden; + margin-left: 0.5em; +} +div.admonition[id^="best-practice-"]:hover > .admonition-title > a.headerlink, +div.admonition[id^="best-practice-"] > .admonition-title > a.headerlink:focus { + visibility: visible; +} + /* Vertically align text in table cells and add an initial horizontal line */ table.docutils.top-aligned td { vertical-align: top; diff --git a/docs/conf.py b/docs/conf.py index f9b497fc7..2d5e7f2dd 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -279,6 +279,7 @@ # Adds custom JavaScript files, located remotely or in 'html_static_path'. html_js_files = [ "https://assets.ubuntu.com/v1/287a5e8f-bundle.js", + "best-practice-anchors.js", ] # Appends extra markup to the end of every document written in reST diff --git a/docs/howto/initialise-your-project.md b/docs/howto/initialise-your-project.md index e4c54e7b6..284d5bbd2 100644 --- a/docs/howto/initialise-your-project.md +++ b/docs/howto/initialise-your-project.md @@ -31,6 +31,7 @@ Create a repository with your source control of choice. ```{admonition} Best practice :class: hint +:name: best-practice-repository-naming Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold diff --git a/docs/howto/log-from-your-charm.md b/docs/howto/log-from-your-charm.md index feca8dcc7..4e8d69689 100644 --- a/docs/howto/log-from-your-charm.md +++ b/docs/howto/log-from-your-charm.md @@ -47,6 +47,7 @@ juju debug-log --debug --include-module juju.worker.uniter.operation --include-m ```{admonition} Best practice :class: hint +:name: best-practice-use-logging-not-print Capture output to `stdout` and `stderr` in your charm and use the logging and warning functionality to send messages to the charm user, rather than rely on @@ -72,12 +73,14 @@ logger.info(f"Got some more information {more_info}") ```{admonition} Best practice :class: hint +:name: best-practice-meaningful-log-messages Ensure that log messages are clear, meaningful, and provide enough information for the user to troubleshoot any issues. Avoid spurious logging. For instance, try not to log when event handlers are called, as the Juju controller does this automatically. ``` ```{admonition} Best practice :class: hint +:name: best-practice-no-sensitive-data-in-logs Never log credentials or other sensitive information. ``` diff --git a/docs/howto/manage-charms.md b/docs/howto/manage-charms.md index 6729baa45..1924a0fbf 100644 --- a/docs/howto/manage-charms.md +++ b/docs/howto/manage-charms.md @@ -44,6 +44,7 @@ The Charmcraft profile has configured some commands to help you develop your cha ```{admonition} Best practice :class: hint +:name: best-practice-charmcraft-profile-commands All charms should provide the commands configured by the Charmcraft profile, to allow easy testing across the charm ecosystem. It's fine to tweak the diff --git a/docs/howto/manage-configuration.md b/docs/howto/manage-configuration.md index 17845b97f..f40aeeafa 100644 --- a/docs/howto/manage-configuration.md +++ b/docs/howto/manage-configuration.md @@ -12,6 +12,7 @@ In the `charmcraft.yaml` file of the charm, under `config.options`, add a config ```{admonition} Best practice :class: hint +:name: best-practice-no-duplicate-model-config Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. ``` diff --git a/docs/howto/manage-libraries.md b/docs/howto/manage-libraries.md index 99b49e9b9..cd6a0c312 100644 --- a/docs/howto/manage-libraries.md +++ b/docs/howto/manage-libraries.md @@ -108,6 +108,7 @@ class DatabaseRequirer(ops.Object): ```{admonition} Best practice :class: hint +:name: best-practice-libraries-no-status-mutation Libraries should never mutate the status of a unit or application. Instead, use return values, or raise exceptions and let them bubble back up to the charm for diff --git a/docs/howto/run-workloads-with-a-charm-machines.md b/docs/howto/run-workloads-with-a-charm-machines.md index 9493a71b8..b97b114a9 100644 --- a/docs/howto/run-workloads-with-a-charm-machines.md +++ b/docs/howto/run-workloads-with-a-charm-machines.md @@ -103,6 +103,7 @@ def uninstall() -> None: ```{admonition} Best practice :class: hint +:name: best-practice-pin-workload-versions Pin workload versions rather than installing the latest available package. A charm that silently upgrades between reconciliations is hard to debug, and can break if upstream introduces a breaking change. ``` @@ -144,6 +145,7 @@ If no library is available for installing the workload, use `subprocess` to run ```{admonition} Best practice :class: hint +:name: best-practice-safe-subprocess When running subprocesses, use absolute paths (for example, `/usr/bin/apt` rather than `apt`) to avoid PATH-based attacks, and pass arguments as a list rather than a shell string so that the shell doesn't interpret them. Use `check=True` so that failures raise — generally you'll want to catch that exception and log the return code and possibly `stderr` — and `capture_output=True` so that output from the command doesn't leak into the Juju log. ``` diff --git a/docs/howto/set-up-continuous-integration-for-a-charm.md b/docs/howto/set-up-continuous-integration-for-a-charm.md index 1bc127c55..d77f5e251 100644 --- a/docs/howto/set-up-continuous-integration-for-a-charm.md +++ b/docs/howto/set-up-continuous-integration-for-a-charm.md @@ -9,6 +9,7 @@ myst: ```{admonition} Best practice :class: hint +:name: best-practice-automated-ci The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. ``` diff --git a/docs/howto/write-and-structure-charm-code.md b/docs/howto/write-and-structure-charm-code.md index 117ff0e3b..ae9084037 100644 --- a/docs/howto/write-and-structure-charm-code.md +++ b/docs/howto/write-and-structure-charm-code.md @@ -25,6 +25,7 @@ charm code that will run with the Python version of the oldest base you support. ```{admonition} Best practice :class: hint +:name: best-practice-requires-python Set the [`requires-python`](https://packaging.python.org/en/latest/specifications/pyproject-toml/#requires-python) version in your `pyproject.toml` so that tooling will detect any use of Python @@ -79,6 +80,7 @@ We recommend that you use `uv add` and `uv remove` instead of editing dependenci ```{admonition} Best practice :class: hint +:name: best-practice-automated-dependency-updates Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. @@ -86,6 +88,7 @@ particularly security releases, for all your dependencies. ```{admonition} Best practice :class: hint +:name: best-practice-commit-lock-file Ensure that the `pyproject.toml` *and* the lock file are committed to version control, so that exact versions of charms can be reproduced. @@ -93,6 +96,7 @@ control, so that exact versions of charms can be reproduced. ```{admonition} Best practice :class: hint +:name: best-practice-avoid-charm-plugin Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. ``` diff --git a/docs/reuse/best-practices.txt b/docs/reuse/best-practices.txt index fa397b01d..7f7bf7a34 100644 --- a/docs/reuse/best-practices.txt +++ b/docs/reuse/best-practices.txt @@ -1,32 +1,44 @@ + + **[How to initialise your project](https://documentation.ubuntu.com/ops/latest/howto/initialise-your-project/)** -- Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold multiple related charms. For the charm name, see [](#decide-your-charms-name). See [Create a repository and initialise it](#create-a-repository-and-initialise-it). +- Name the repository using the pattern ``-operator`` for a single charm, or ``-operators`` when the repository will hold multiple related charms. For the charm name, see [](#decide-your-charms-name). See [Create a repository and initialise it](#create-a-repository-and-initialise-it). **[How to log from your charm](https://documentation.ubuntu.com/ops/latest/howto/log-from-your-charm/)** -- Capture output to `stdout` and `stderr` in your charm and use the logging and warning functionality to send messages to the charm user, rather than rely on Juju capturing output. In particular, you should avoid `print()` calls, and ensure that any subprocess calls capture output. -- Ensure that log messages are clear, meaningful, and provide enough information for the user to troubleshoot any issues. Avoid spurious logging. For instance, try not to log when event handlers are called, as the Juju controller does this automatically. -- Never log credentials or other sensitive information. +- Capture output to `stdout` and `stderr` in your charm and use the logging and warning functionality to send messages to the charm user, rather than rely on Juju capturing output. In particular, you should avoid `print()` calls, and ensure that any subprocess calls capture output. +- Ensure that log messages are clear, meaningful, and provide enough information for the user to troubleshoot any issues. Avoid spurious logging. For instance, try not to log when event handlers are called, as the Juju controller does this automatically. +- Never log credentials or other sensitive information. **[How to manage charms](https://documentation.ubuntu.com/ops/latest/howto/manage-charms/)** -- All charms should provide the commands configured by the Charmcraft profile, to allow easy testing across the charm ecosystem. It's fine to tweak the configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides. See [Develop your charm](#develop-your-charm). +- All charms should provide the commands configured by the Charmcraft profile, to allow easy testing across the charm ecosystem. It's fine to tweak the configuration of individual tools, or to add additional commands, but keep the command names and meanings that the profile provides. See [Develop your charm](#develop-your-charm). **[How to manage configuration](https://documentation.ubuntu.com/ops/latest/howto/manage-configuration/)** -- Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. See [Define a configuration option](#define-a-configuration-option). +- Don't duplicate model-level configuration options that are controlled by {external+juju:ref}`juju model-config `. See [Define a configuration option](#define-a-configuration-option). **[How to manage libraries](https://documentation.ubuntu.com/ops/latest/howto/manage-libraries/)** -- Libraries should never mutate the status of a unit or application. Instead, use return values, or raise exceptions and let them bubble back up to the charm for the charm author to handle as they see fit. See [Write a library](#manage-libraries-write-a-library). +- Libraries should never mutate the status of a unit or application. Instead, use return values, or raise exceptions and let them bubble back up to the charm for the charm author to handle as they see fit. See [Write a library](#manage-libraries-write-a-library). **[How to run workloads with a machine charm](https://documentation.ubuntu.com/ops/latest/howto/run-workloads-with-a-charm-machines/)** -- Pin workload versions rather than installing the latest available package. A charm that silently upgrades between reconciliations is hard to debug, and can break if upstream introduces a breaking change. See [APT packages](#run-workloads-with-a-charm-machines-apt-packages). -- When running subprocesses, use absolute paths (for example, `/usr/bin/apt` rather than `apt`) to avoid PATH-based attacks, and pass arguments as a list rather than a shell string so that the shell doesn't interpret them. Use `check=True` so that failures raise — generally you'll want to catch that exception and log the return code and possibly `stderr` — and `capture_output=True` so that output from the command doesn't leak into the Juju log. See [When there's no library](#run-workloads-with-a-charm-machines-when-theres-no-library). +- Pin workload versions rather than installing the latest available package. A charm that silently upgrades between reconciliations is hard to debug, and can break if upstream introduces a breaking change. See [APT packages](#run-workloads-with-a-charm-machines-apt-packages). +- When running subprocesses, use absolute paths (for example, `/usr/bin/apt` rather than `apt`) to avoid PATH-based attacks, and pass arguments as a list rather than a shell string so that the shell doesn't interpret them. Use `check=True` so that failures raise — generally you'll want to catch that exception and log the return code and possibly `stderr` — and `capture_output=True` so that output from the command doesn't leak into the Juju log. See [When there's no library](#run-workloads-with-a-charm-machines-when-theres-no-library). **[How to set up continuous integration for a charm](https://documentation.ubuntu.com/ops/latest/howto/set-up-continuous-integration-for-a-charm/)** -- The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. +- The quality assurance pipeline of a charm should be automated using a continuous integration (CI) system. See [How to set up continuous integration for a charm](#best-practice-automated-ci). **[How to write and structure charm code](https://documentation.ubuntu.com/ops/latest/howto/write-and-structure-charm-code/)** -- Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -- Ensure that the `pyproject.toml` *and* the lock file are committed to version control, so that exact versions of charms can be reproduced. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -- Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). -- Set the [`requires-python`](https://packaging.python.org/en/latest/specifications/pyproject-toml/#requires-python) version in your `pyproject.toml` so that tooling will detect any use of Python features not available in the versions you support. See [Use the Python provided by the base](#define-the-required-dependencies). +- Avoid using Charmcraft's `charm` plugin if possible. Instead, {external+charmcraft:ref}`migrate to the uv plugin ` or the {external+charmcraft:ref}`poetry plugin `. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). +- Ensure that the `pyproject.toml` *and* the lock file are committed to version control, so that exact versions of charms can be reproduced. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). +- Ensure that tooling is configured to automatically detect new versions, particularly security releases, for all your dependencies. See [Add Python dependencies to pyproject.toml and update the lock file](#define-the-required-dependencies). +- Set the [`requires-python`](https://packaging.python.org/en/latest/specifications/pyproject-toml/#requires-python) version in your `pyproject.toml` so that tooling will detect any use of Python features not available in the versions you support. See [Use the Python provided by the base](#define-the-required-dependencies). **[Manage charms](https://documentation.ubuntu.com/charmcraft/latest/howto/manage-charms/)** - If you're setting up a ``git`` repository: name it using the pattern ``-operator``. For the charm name, see {external+charmcraft:ref}`specify-a-name`. See {external+charmcraft:ref}`Initialise a charm `.