Skip to content

Commit bd178b7

Browse files
markuslfebuerki-lf
authored andcommitted
fix(plugins): make ansible-doc render all in-house plugins + add guard (#267)
A description bullet containing a colon followed by a space is parsed by YAML as a mapping, which makes ansible-doc abort with 'expected str instance, AnsibleMapping found'. Rephrase the offending bullets in the nextcloud_occ_app_config / nextcloud_occ_system_config modules and the alert_contacts / mwindows / monitors option docs of the uptimerobot_monitor / uptimerobot_psp modules. Add tests/unit/test_plugin_docs.py, which parses every in-house plugin's DOCUMENTATION/RETURN and asserts each description is a string or list of strings, so this class of error fails at unit-test time instead of only at render time. Vendored plugins are out of scope (the ipa* modules also fail ansible-doc, but because their ansible-freeipa doc_fragment is not installed, which is unrelated).
1 parent 22c33f7 commit bd178b7

7 files changed

Lines changed: 121 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4242

4343
### Fixed
4444

45+
* **plugin:nextcloud_occ_app_config, plugin:nextcloud_occ_system_config, plugin:uptimerobot_monitor, plugin:uptimerobot_psp**: Fixed their documentation so `ansible-doc` renders them again. A unit-test guard now catches this class of error for every in-house plugin.
4546
* **plugin:bitwarden_item**: Fixed the lookup's documentation so `ansible-doc` renders it again.
4647
* **plugin:combine_lod**: The `combine_lod` filter now reports an error when an item is missing part of a composite `unique_key` (a list of keys), instead of silently grouping such items together. Inventories with incomplete composite keys that previously merged by accident now fail loudly and must be corrected. Also fixed its documentation so `ansible-doc` renders it again.
4748
* **role:kernel_settings**: The `systemd_cpu_affinity` setting is now actually applied. The value was computed and shown in the debug output but never passed to the underlying system role, so a configured CPU affinity had no effect.

CONTRIBUTING.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,7 +771,7 @@ In-house plugins live under `plugins/` following the standard Ansible collection
771771
```
772772

773773
* Use single quotes and f-strings consistently (vendored plugins keep their upstream style, see below).
774-
* Every plugin carries `DOCUMENTATION` (and `RETURN` / `EXAMPLES` where applicable). Keep it valid YAML: in a `description` list, a bullet containing a colon followed by a space is parsed as a mapping and makes `ansible-doc` fail, so rephrase or quote such bullets. Verify with `ansible-doc -t <filter|lookup|module> linuxfabrik.lfops.<name>`.
774+
* Every plugin carries `DOCUMENTATION` (and `RETURN` / `EXAMPLES` where applicable). Keep it valid YAML: in a `description` list, a bullet containing a colon followed by a space is parsed as a mapping and makes `ansible-doc` fail, so rephrase or quote such bullets. Verify with `ansible-doc -t <filter|lookup|module> linuxfabrik.lfops.<name>`; `tests/unit/test_plugin_docs.py` guards against this class of error for all in-house plugins.
775775
* Set `version_added` to the LFOps release the plugin first shipped in, and never change it afterwards.
776776
* `module_utils` holds code shared between plugins. Do not import the external Linuxfabrik Python Libraries (`lib`) into a plugin; copy what you need and note the origin in a comment.
777777

plugins/modules/nextcloud_occ_app_config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
- Drives C(occ config:app:set) and C(config:app:delete) to bring a single app config key into the desired state.
1818
- The current value and type are read from C(occ config:app:get --details --output=json) (or from a pre-fetched C(occ config:list --output=json --private) listing passed via I(installed_config_json)). C(occ config:app:set) is only called when the stored value or type does not already match.
1919
- When I(name) contains spaces, each whitespace-separated token is passed as a separate argument to C(occ), matching how Nextcloud addresses nested keys (e.g. C(name="endpoint enabled")).
20-
- Booleans are normalized for Nextcloud's storage: I(value) values C(true)/C(1)/C(on)/C(yes) (case-insensitive) become C(1) in the database; everything else becomes C(0). When reading via I(installed_config_json), the type is inferred from the JSON value type (Python C(bool)/C(int)/C(float)/C(list)/C(str)), since C(occ config:list) returns values already cast by C(convertTypedValue()).
20+
- Booleans are normalized for Nextcloud's storage. I(value) values C(true)/C(1)/C(on)/C(yes) (case-insensitive) become C(1) in the database; everything else becomes C(0). When reading via I(installed_config_json), the type is inferred from the JSON value type (Python C(bool)/C(int)/C(float)/C(list)/C(str)), since C(occ config:list) returns values already cast by C(convertTypedValue()).
2121
2222
requirements:
2323
- A working Nextcloud installation with the C(occ) command available.

plugins/modules/nextcloud_occ_system_config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
- Drives C(occ config:system:set) and C(config:system:delete) to bring a single system config key into the desired state.
1818
- The current value is read from C(occ config:system:get) (or from a pre-fetched C(occ config:list --output=json --private) listing passed via I(installed_config_json)). C(occ config:system:set) is only called when the stored value does not already match I(value).
1919
- When I(name) contains spaces, each whitespace-separated token is passed as a separate argument to C(occ), matching how Nextcloud addresses nested keys (e.g. C(name="trusted_domains 0"), C(name="forbidden_filename_characters 0")).
20-
- Booleans are normalized for C(occ): I(value) values C(true)/C(1)/C(on)/C(yes) (case-insensitive) become the literal string C(true); everything else becomes C(false). This matches what Nextcloud's CastHelper accepts on C(config:system:set). Note that this differs from C(nextcloud_occ_app_config), which stores booleans as C(1)/C(0).
20+
- Booleans are normalized for C(occ). I(value) values C(true)/C(1)/C(on)/C(yes) (case-insensitive) become the literal string C(true); everything else becomes C(false). This matches what Nextcloud's CastHelper accepts on C(config:system:set). Note that this differs from C(nextcloud_occ_app_config), which stores booleans as C(1)/C(0).
2121
2222
requirements:
2323
- A working Nextcloud installation with the C(occ) command available.

plugins/modules/uptimerobot_monitor.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@
167167
alert_contacts:
168168
description:
169169
- Alert contacts to attach to the monitor. Each item references an existing alert contact, plus the per-monitor I(threshold) and I(recurrence). The list is replaced on every run; pass an empty list to clear all attached contacts.
170-
- Resolution: when an item has I(id), it is used directly. Otherwise I(friendly_name) is resolved against C(getAlertContacts); an unknown name fails the play.
170+
- When an item has I(id), it is used directly. Otherwise I(friendly_name) is resolved against C(getAlertContacts); an unknown name fails the play.
171171
type: list
172172
elements: dict
173173
required: false
@@ -189,7 +189,7 @@
189189
mwindows:
190190
description:
191191
- Maintenance windows to attach to the monitor. Each item references an existing maintenance window. The list is replaced on every run; pass an empty list to detach all windows.
192-
- Resolution: when an item has I(id), it is used directly. Otherwise I(friendly_name) is resolved against C(getMWindows); an unknown name fails the play.
192+
- When an item has I(id), it is used directly. Otherwise I(friendly_name) is resolved against C(getMWindows); an unknown name fails the play.
193193
type: list
194194
elements: dict
195195
required: false

plugins/modules/uptimerobot_psp.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
monitors:
4343
description:
4444
- Monitors to display on the status page. Each item references an existing monitor.
45-
- Resolution: when an item has I(id), it is used directly. Otherwise I(friendly_name) is resolved against C(getMonitors); an unknown name fails the play.
45+
- When an item has I(id), it is used directly. Otherwise I(friendly_name) is resolved against C(getMonitors); an unknown name fails the play.
4646
type: list
4747
elements: dict
4848
suboptions:

tests/unit/test_plugin_docs.py

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
#!/usr/bin/env python3
2+
# -*- coding: utf-8; py-indent-offset: 4 -*-
3+
#
4+
# Author: Linuxfabrik GmbH, Zurich, Switzerland
5+
# Contact: info (at) linuxfabrik (dot) ch
6+
# https://www.linuxfabrik.ch/
7+
# License: The Unlicense, see LICENSE file.
8+
9+
"""Guard against the DOCUMENTATION YAML bug that breaks `ansible-doc`.
10+
11+
In a `description` block (a YAML list), a bullet that contains a colon
12+
followed by a space is parsed as a mapping instead of a string, e.g.
13+
14+
description:
15+
- Useful for X: it does Y. # parsed as {"Useful for X": "it does Y."}
16+
17+
`ansible-doc` then aborts with "expected str instance, AnsibleMapping
18+
found". This test parses every in-house plugin's DOCUMENTATION (and
19+
RETURN) and asserts that every `description` is a string or a list of
20+
strings, catching the bug at unit-test time instead of at render time.
21+
22+
Vendored plugins keep their upstream docs and are out of scope.
23+
"""
24+
25+
from __future__ import absolute_import, division, print_function
26+
27+
__metaclass__ = type
28+
29+
import ast
30+
import glob
31+
import os
32+
import unittest
33+
34+
import yaml
35+
36+
_REPO_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
37+
_PLUGIN_GLOBS = [
38+
'plugins/filter/*.py',
39+
'plugins/lookup/*.py',
40+
'plugins/modules/*.py',
41+
]
42+
# Vendored plugins are kept in lockstep with upstream and not restyled here.
43+
_VENDORED_PREFIXES = ('ipa',)
44+
_VENDORED_NAMES = {'lvm_pv.py'}
45+
46+
47+
def _in_house_plugin_files():
48+
files = []
49+
for pattern in _PLUGIN_GLOBS:
50+
for path in glob.glob(os.path.join(_REPO_ROOT, pattern)):
51+
name = os.path.basename(path)
52+
if name.startswith(_VENDORED_PREFIXES) or name in _VENDORED_NAMES:
53+
continue
54+
files.append(path)
55+
return sorted(files)
56+
57+
58+
def _extract_doc_constants(source):
59+
"""Return {const_name: yaml_obj} for DOCUMENTATION/RETURN string assignments."""
60+
tree = ast.parse(source)
61+
docs = {}
62+
for node in tree.body:
63+
if not isinstance(node, ast.Assign):
64+
continue
65+
names = [t.id for t in node.targets if isinstance(t, ast.Name)]
66+
for wanted in ('DOCUMENTATION', 'RETURN'):
67+
if wanted in names and isinstance(node.value, ast.Constant) \
68+
and isinstance(node.value.value, str):
69+
docs[wanted] = yaml.safe_load(node.value.value)
70+
return docs
71+
72+
73+
def _iter_description_problems(obj, path=''):
74+
"""Yield human-readable paths where a `description` is not str / list[str]."""
75+
if isinstance(obj, dict):
76+
for key, value in obj.items():
77+
if key == 'description':
78+
if isinstance(value, str):
79+
pass
80+
elif isinstance(value, list):
81+
for i, item in enumerate(value):
82+
if not isinstance(item, str):
83+
yield f'{path}.description[{i}] is {type(item).__name__}, expected str'
84+
else:
85+
yield f'{path}.description is {type(value).__name__}, expected str or list[str]'
86+
yield from _iter_description_problems(value, f'{path}.{key}')
87+
elif isinstance(obj, list):
88+
for i, item in enumerate(obj):
89+
yield from _iter_description_problems(item, f'{path}[{i}]')
90+
91+
92+
class TestPluginDocs(unittest.TestCase):
93+
94+
def test_in_house_plugins_have_renderable_descriptions(self):
95+
files = _in_house_plugin_files()
96+
self.assertTrue(files, 'no in-house plugin files found')
97+
for path in files:
98+
with self.subTest(plugin=os.path.relpath(path, _REPO_ROOT)):
99+
with open(path, 'r') as f:
100+
source = f.read()
101+
docs = _extract_doc_constants(source)
102+
problems = []
103+
for const_name, doc in docs.items():
104+
problems += [f'{const_name}{p}' for p in _iter_description_problems(doc)]
105+
self.assertEqual(
106+
problems, [],
107+
'description fields must be str or list[str] '
108+
'(a colon + space in a bullet makes ansible-doc fail):\n '
109+
+ '\n '.join(problems),
110+
)
111+
112+
113+
if __name__ == '__main__':
114+
unittest.main()

0 commit comments

Comments
 (0)