From dd5070dceb9bc22ea73bb1c7af7371ea6dcd5dcf Mon Sep 17 00:00:00 2001 From: oddly Date: Fri, 7 Aug 2026 16:47:12 +0200 Subject: [PATCH 1/5] chore(ci): fail CI when defaults/main.yml drifts from argument_specs.yml MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds scripts/check_argspecs.py — for every role that ships a meta/argument_specs.yml, compare the set of top-level (non-underscored) variables in defaults/main.yml against the options declared in the spec. Any mismatch either way (var in defaults but not spec, option in spec but no longer in defaults) exits non-zero with the offending names and a pointer to scripts/gen_argspecs.py for regeneration. Wired into test_linting.yml as the last step of the whole-collection lint job (skipped for the per-role invocation so we do not run five duplicate checks per PR). Also promotes gen_argspecs.py from /tmp/ into scripts/ so the fix path is self-contained, and uncomments elasticstack_cert_pass in defaults (one commented line the initial argument_specs PR left as spec-only drift — flipping to elasticstack_cert_pass: "" matches the actual role behaviour and clears the check). --- .github/workflows/test_linting.yml | 8 + roles/elasticstack/defaults/main.yml | 2 +- scripts/check_argspecs.py | 101 +++++++++++++ scripts/gen_argspecs.py | 210 +++++++++++++++++++++++++++ 4 files changed, 320 insertions(+), 1 deletion(-) create mode 100755 scripts/check_argspecs.py create mode 100755 scripts/gen_argspecs.py diff --git a/.github/workflows/test_linting.yml b/.github/workflows/test_linting.yml index 2a1b1785..6e25e6ad 100644 --- a/.github/workflows/test_linting.yml +++ b/.github/workflows/test_linting.yml @@ -84,3 +84,11 @@ jobs: env: ROLENAME: ${{ inputs.rolename }} if: ${{ inputs.rolename != '' }} + + # Fails if a role's meta/argument_specs.yml is out of sync with its + # defaults/main.yml — catches "I added a var but forgot to update the + # spec" and its inverse. See scripts/gen_argspecs.py for regeneration. + - name: Check argument_specs drift. + run: | + python3 scripts/check_argspecs.py + if: ${{ inputs.rolename == '' }} diff --git a/roles/elasticstack/defaults/main.yml b/roles/elasticstack/defaults/main.yml index cd2db40e..44cea273 100644 --- a/roles/elasticstack/defaults/main.yml +++ b/roles/elasticstack/defaults/main.yml @@ -42,7 +42,7 @@ elasticstack_ca_pass: PleaseChangeMe # logstash_tls_key_passphrase, and beats_tls_key_passphrase. # Leave empty to use the per-role passphrases. # @end -# elasticstack_cert_pass: +elasticstack_cert_pass: "" # @var elasticstack_ca_validity_period:description: Validity period in days for the CA certificate elasticstack_ca_validity_period: 1095 # === General Settings === diff --git a/scripts/check_argspecs.py b/scripts/check_argspecs.py new file mode 100755 index 00000000..db12e88e --- /dev/null +++ b/scripts/check_argspecs.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +""" +Verify that every role's meta/argument_specs.yml lists the same variables +its defaults/main.yml defines. Fails CI when the two drift. + +Runs across every role that has a meta/argument_specs.yml. Roles without +one (e.g. `repos`, which uses variables from `elasticstack` instead) are +skipped intentionally. + +Exit codes: + 0 All roles in sync + 1 At least one role has drift (details on stderr) + 2 Usage / setup error + +The parser only looks at the top-level `varname: value` lines in +defaults/main.yml. Internal-only vars (leading underscore) are ignored +so vars/main.yml-style facts don't need arg-spec entries. +""" +import sys +import re +from pathlib import Path + +try: + import yaml +except ImportError: + print("check_argspecs: PyYAML is required (pip install pyyaml)", file=sys.stderr) + sys.exit(2) + + +VAR_LINE = re.compile(r"^([A-Za-z][\w]*)\s*:") + + +def defaults_vars(path): + """Top-level variable names in a defaults/main.yml, minus internal underscore vars.""" + out = set() + with open(path) as f: + for line in f: + m = VAR_LINE.match(line) + if m: + name = m.group(1) + if not name.startswith("_"): + out.add(name) + return out + + +def argspec_options(path): + """Option names declared under any entry point (main + role-scoped + task-file entry points like node_maintenance_start / _end).""" + with open(path) as f: + spec = yaml.safe_load(f) or {} + out = set() + for entry in (spec.get("argument_specs") or {}).values(): + for name in (entry.get("options") or {}).keys(): + out.add(name) + return out + + +def main(): + repo = Path(__file__).resolve().parent.parent + roles = sorted(p for p in (repo / "roles").iterdir() if p.is_dir()) + exit_code = 0 + + for role in roles: + defaults = role / "defaults" / "main.yml" + specs = role / "meta" / "argument_specs.yml" + if not specs.exists(): + continue # role opts out + if not defaults.exists(): + print( + f"[{role.name}] has meta/argument_specs.yml but no defaults/main.yml", + file=sys.stderr, + ) + exit_code = 1 + continue + + vars_ = defaults_vars(defaults) + opts = argspec_options(specs) + missing_in_spec = vars_ - opts + extra_in_spec = opts - vars_ + + if missing_in_spec or extra_in_spec: + exit_code = 1 + print(f"\n[{role.name}] argument_specs drift:", file=sys.stderr) + for v in sorted(missing_in_spec): + print(f" - in defaults but missing from spec: {v}", file=sys.stderr) + for v in sorted(extra_in_spec): + print(f" - in spec but no longer in defaults: {v}", file=sys.stderr) + else: + print(f"[{role.name}] ok ({len(vars_)} vars)") + + if exit_code: + print( + "\nFix by editing meta/argument_specs.yml or regenerating with " + "`scripts/gen_argspecs.py `.", + file=sys.stderr, + ) + sys.exit(exit_code) + + +if __name__ == "__main__": + main() diff --git a/scripts/gen_argspecs.py b/scripts/gen_argspecs.py new file mode 100755 index 00000000..3237bec7 --- /dev/null +++ b/scripts/gen_argspecs.py @@ -0,0 +1,210 @@ +#!/usr/bin/env python3 +""" +Generate meta/argument_specs.yml for an Ansible role by parsing @var +docblocks in defaults/main.yml. + +Usage: gen_argspecs.py +Writes: /meta/argument_specs.yml +""" +import sys +import re +import yaml +from pathlib import Path + + +def parse_defaults(path): + """Returns list of dicts: [{'name': str, 'description': str, 'default': any}]""" + with open(path) as f: + text = f.read() + lines = text.splitlines() + entries = [] + i = 0 + pending_desc = None + pending_var = None + while i < len(lines): + line = lines[i] + + # Single-line @var: `# @var NAME:description: TEXT` + m = re.match(r"^# @var\s+([\w.]+):description:\s*(.*)$", line) + if m and not m.group(2).endswith('>'): + pending_var = m.group(1) + pending_desc = m.group(2).strip() + i += 1 + continue + + # Multi-line @var: `# @var NAME:description: >` then lines until `# @end` + # OR the first non-comment line (some docblocks in this repo omit @end). + m = re.match(r"^# @var\s+([\w.]+):description:\s*>\s*$", line) + if m: + pending_var = m.group(1) + desc_lines = [] + i += 1 + while i < len(lines): + stripped = lines[i].strip() + if stripped.startswith("# @end"): + i += 1 + break + if stripped.startswith("# @var"): + # A new @var starts (typically an :example: sibling) — + # description ends here, leave i pointing at that line. + break + if not stripped.startswith("#"): + # Docblock ended without @end — leave i pointing at + # the value line and let the value-line branch handle it. + break + desc_lines.append(lines[i].lstrip("# ").rstrip()) + i += 1 + pending_desc = " ".join(l for l in desc_lines if l).strip() + continue + + # Also skip `# @var ...:example:` blocks + if re.match(r"^# @var\s+[\w.]+:example:", line): + # skip the example block similarly + if line.rstrip().endswith('>'): + i += 1 + while i < len(lines) and not lines[i].strip().startswith("# @end"): + i += 1 + if i < len(lines): + i += 1 + else: + i += 1 + continue + + # Value line: NAME: VALUE (or NAME: on its own for multi-line YAML) + m = re.match(r"^(\w+):\s*(.*)$", line) + if m and pending_var == m.group(1): + varname = m.group(1) + # Try to YAML-parse the full block (may span lines for lists/dicts) + # Simple heuristic: take from here until next blank line or next @var + block_end = i + 1 + while block_end < len(lines): + nxt = lines[block_end] + if ( + nxt.strip() == "" + or nxt.startswith("# @var") + or nxt.startswith("# ==") + ): + break + block_end += 1 + block_text = "\n".join(lines[i:block_end]) + try: + parsed = yaml.safe_load(block_text) + if isinstance(parsed, dict) and varname in parsed: + default_val = parsed[varname] + else: + default_val = None + except Exception: + default_val = None + entries.append( + {"name": varname, "description": pending_desc or "", "default": default_val} + ) + pending_var = None + pending_desc = None + i = block_end + continue + + # Also handle: commented-out defaults (# varname:) — skip but record with unset default + m = re.match(r"^#\s*(\w+):\s*$", line) + if m and pending_var == m.group(1): + entries.append( + {"name": m.group(1), "description": pending_desc or "", "default": None} + ) + pending_var = None + pending_desc = None + i += 1 + continue + + i += 1 + return entries + + +def infer_type(value): + if isinstance(value, bool): + return "bool" + if isinstance(value, int): + return "int" + if isinstance(value, float): + return "float" + if isinstance(value, list): + return "list" + if isinstance(value, dict): + return "dict" + if isinstance(value, str): + # Jinja-templated strings still get type str + return "str" + return "str" + + +def build_argument_specs(role_name, entries, short_desc, long_desc): + options = {} + for e in entries: + opt = {"description": e["description"] or f"See defaults/main.yml for {e['name']}."} + if e["default"] is not None: + opt["type"] = infer_type(e["default"]) + opt["default"] = e["default"] + else: + opt["type"] = "raw" + options[e["name"]] = opt + return { + "argument_specs": { + "main": { + "short_description": short_desc, + "description": long_desc, + "options": options, + } + } + } + + +ROLE_METADATA = { + "elasticsearch": ( + "Install, configure, and manage Elasticsearch", + "Handles cluster formation, TLS certificate management, security setup (users, passwords, HTTPS), rolling upgrades (8.x to 9.x), JVM tuning, and systemd service management.", + ), + "kibana": ( + "Install, configure, and manage Kibana", + "Handles package install, TLS setup for the Kibana web UI, keystore-managed secrets, integration with an Elasticsearch backend, and systemd service management.", + ), + "logstash": ( + "Install, configure, and manage Logstash", + "Handles package install, pipeline configuration, TLS certificate management for input/output, JVM tuning, and systemd service management.", + ), + "beats": ( + "Install, configure, and manage Elastic Beats (filebeat, metricbeat, auditbeat)", + "Handles package install, ECS-schema output configuration to Elasticsearch or Logstash, TLS certificate distribution, and systemd service management per beat.", + ), + "elasticstack": ( + "Shared defaults and CA management for the oddly.elasticstack collection", + "Provides collection-wide variables (inventory group names, ports, CA host, certificate settings) and the internal certificate authority workflow used by the elasticsearch, kibana, logstash, and beats roles.", + ), + "repos": ( + "Manage Elastic package repositories", + "Installs the Elastic apt/yum repository configuration matching elasticstack_release and elasticstack_repo_base_url, keeping the elasticsearch/kibana/logstash/beats package installs pointed at the right major version.", + ), +} + + +def main(): + role_path = Path(sys.argv[1]).resolve() + role_name = role_path.name + defaults = role_path / "defaults" / "main.yml" + if not defaults.exists(): + print(f"no defaults/main.yml at {defaults}", file=sys.stderr) + sys.exit(1) + + entries = parse_defaults(defaults) + short_desc, long_desc = ROLE_METADATA.get( + role_name, (f"Role {role_name}", f"Role {role_name}.") + ) + spec = build_argument_specs(role_name, entries, short_desc, long_desc) + + (role_path / "meta").mkdir(exist_ok=True) + outpath = role_path / "meta" / "argument_specs.yml" + with open(outpath, "w") as f: + f.write("---\n") + yaml.dump(spec, f, sort_keys=False, default_flow_style=False, width=120) + print(f"wrote {outpath} with {len(entries)} options") + + +if __name__ == "__main__": + main() From 6ffecfc1e16baa94ae5c3918cd845e105b617400 Mon Sep 17 00:00:00 2001 From: oddly Date: Tue, 11 Aug 2026 09:12:10 +0200 Subject: [PATCH 2/5] chore(ci): scope drift check to main entry point + backfill maintenance defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups the #190 merge surfaced against #186's own drift check: scripts/check_argspecs.py: only compare the role's 'main' entry point options against defaults/main.yml. Task-file entry points (node_maintenance_start / _end) take per-invocation parameters like elasticsearch_maintenance_password and _api_url that don't need role-wide defaults — they're inputs to a specific action, not defaults. The wider check was flagging those as spurious drift. roles/elasticsearch/meta/argument_specs.yml: the six role-wide maintenance defaults (elasticsearch_maintenance_wait_status, _health_retries, _health_delay, _wait_health, _require_green, elasticsearch_drain_cluster_settings) were declared under the two task-file entry points but not under 'main'. defaults/main.yml lists them, so the (correctly narrower) drift check now catches that gap and this backfill closes it. Also drops the pre-existing 'role_prefix | Verb' name[casing] warnings that were tripping lint_full for any PR whose diff pulled in the cert_detect_content_mode tasks. The convention is intentional (it matches the commit-message style), so it's moved from warn_list to skip_list rather than papered over with a noqa on every task. --- .ansible-lint | 7 +++++- roles/elasticsearch/meta/argument_specs.yml | 27 +++++++++++++++++++++ scripts/check_argspecs.py | 17 +++++++------ 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/.ansible-lint b/.ansible-lint index e7ab3912..62b7a1cd 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -10,7 +10,6 @@ exclude_paths: warn_list: - experimental - key-order[task] - - name[casing] - name[missing] - package-latest - schema[meta] @@ -19,6 +18,12 @@ skip_list: - command-instead-of-module - galaxy[no-changelog] - line-length + # The repo uses `role_prefix | Verb` task names (lowercase prefix, uppercase + # verb) — matches the commit-message convention. This rule was in warn_list + # but ansible-lint still counts warnings against the moderate profile, so + # lint_full failed on the pre-existing cert_detect_content_mode tasks any + # time the changes filter marked their role for linting. + - name[casing] - no-handler - package-latest - role-name diff --git a/roles/elasticsearch/meta/argument_specs.yml b/roles/elasticsearch/meta/argument_specs.yml index 53b27658..0e6568df 100644 --- a/roles/elasticsearch/meta/argument_specs.yml +++ b/roles/elasticsearch/meta/argument_specs.yml @@ -340,6 +340,33 @@ argument_specs: set to true from a playbook to force renewal regardless of buffer. type: bool default: false + elasticsearch_maintenance_wait_status: + description: Minimum cluster health status the node maintenance entry points wait for. Use yellow or green. + type: str + default: green + choices: [green, yellow] + elasticsearch_maintenance_health_retries: + description: Number of cluster health polling attempts in the node maintenance entry points. + type: int + default: 60 + elasticsearch_maintenance_health_delay: + description: Delay in seconds between cluster health polling attempts in the node maintenance entry points. + type: int + default: 30 + elasticsearch_maintenance_wait_health: + description: Wait for cluster health at the end of node maintenance. Disable for a defensive state reset at the start of a run. + type: bool + default: true + elasticsearch_maintenance_require_green: + description: Fail node_maintenance_end unless the cluster returns to green. Default accepts yellow. + type: bool + default: false + elasticsearch_drain_cluster_settings: + description: Persistent cluster settings applied while a node is drained for maintenance (typically a recovery throughput + boost). node_maintenance_end restores every key listed here to its value in elasticsearch_cluster_settings, or removes + it when no baseline is declared there. + type: dict + default: {} node_maintenance_start: short_description: Prepare the cluster for taking this node down description: Health gate, voting exclusion, primaries-only allocation, ML upgrade mode, optional recovery boost and diff --git a/scripts/check_argspecs.py b/scripts/check_argspecs.py index db12e88e..ea94bcbc 100755 --- a/scripts/check_argspecs.py +++ b/scripts/check_argspecs.py @@ -44,15 +44,18 @@ def defaults_vars(path): def argspec_options(path): - """Option names declared under any entry point (main + role-scoped - task-file entry points like node_maintenance_start / _end).""" + """Option names declared under the role's `main` entry point. + + Task-file entry points (e.g. node_maintenance_start / _end) are + invoked with per-call parameters that don't need to appear in + defaults/main.yml — they're inputs to a specific action, not + role-wide defaults. Only `main`'s options are role-wide vars. + """ with open(path) as f: spec = yaml.safe_load(f) or {} - out = set() - for entry in (spec.get("argument_specs") or {}).values(): - for name in (entry.get("options") or {}).keys(): - out.add(name) - return out + entries = spec.get("argument_specs") or {} + main = entries.get("main") or {} + return set((main.get("options") or {}).keys()) def main(): From 13fd6e7973c317c570d7454d752b0fcd6c6a25c5 Mon Sep 17 00:00:00 2001 From: oddly Date: Tue, 11 Aug 2026 15:42:18 +0200 Subject: [PATCH 3/5] fix(elasticsearch): treat empty elasticstack_cert_pass as unset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'Set common password for common certificates' set_fact gated on elasticstack_cert_pass being defined, which was safe as long as the role's own default kept the variable commented out. When the drift check work uncommented it to a default of "" (so the arg spec and defaults stay in sync), the gate started firing and blanked elasticsearch_tls_key_passphrase. That empty string cascaded into elasticsearch-keystore.yml's http.ssl.keystore.secure_password Set step, which pipes it as stdin to `elasticsearch-keystore add` — and that binary rejects empty passphrases with a non-zero exit. Reproduced as: elasticsearch_custom passing on main (var commented, gate false) but failing on the drift branch (var defined empty, gate true, empty stdin) even though no keystore code changed between them. The docstring already promised 'Leave empty to use the per-role passphrases', so this changes the gate from 'is defined' to a non-empty check, which matches the documented behaviour and keeps downstream unaffected whether the default line is present or absent. --- roles/elasticsearch/tasks/main.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/roles/elasticsearch/tasks/main.yml b/roles/elasticsearch/tasks/main.yml index 010d59c1..9f24ebfe 100644 --- a/roles/elasticsearch/tasks/main.yml +++ b/roles/elasticsearch/tasks/main.yml @@ -63,8 +63,16 @@ - name: Set common password for common certificates ansible.builtin.set_fact: elasticsearch_tls_key_passphrase: "{{ elasticstack_cert_pass }}" + # `is defined` was the old gate — that worked as long as the default in + # roles/elasticstack/defaults/main.yml was left commented out, because + # setting it to "" then still counted as defined and silently blanked + # elasticsearch_tls_key_passphrase, which cascaded into an empty stdin + # to elasticsearch-keystore and a failed keystore.add. The docstring + # already says "Leave empty to use the per-role passphrases" — respect + # that: only apply the override when the user actually supplied a + # non-empty value. when: - - elasticstack_cert_pass is defined + - elasticstack_cert_pass | default('') | length > 0 tags: - certificates - renew_ca From 81ef6867fccce4d88c09d7a9fe0b406d56f92262 Mon Sep 17 00:00:00 2001 From: oddly Date: Tue, 11 Aug 2026 19:39:28 +0200 Subject: [PATCH 4/5] chore(ci): halve full_stack matrix concurrency to 3 to give heavy scenarios room Committed memory with 6 concurrent slots landed close to 100 GB out of ~120 GB usable on incus-ci. That was fine while the memory-gate timeout was 15 min, but repeated real-world runs with the 45-min timeout still expose the 20 GB elasticstack_default and 13.8 GB es_kibana scenarios to steady starvation: they lose the retry race against smaller peers that release+reacquire faster. Dropping to 3 slots keeps peak committed memory well within reach and lets the heavy jobs actually acquire on the first attempt. Wall clock roughly doubles for the full matrix but every scenario finishes. --- .github/workflows/test_full_stack.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test_full_stack.yml b/.github/workflows/test_full_stack.yml index 83a4f8a2..393778bc 100644 --- a/.github/workflows/test_full_stack.yml +++ b/.github/workflows/test_full_stack.yml @@ -95,9 +95,14 @@ jobs: # the shared incus-ci host. The memory-capacity gate in shared/create.yml # eventually admits every job, but without a matrix-level cap the full # 16-combo PR matrix would swamp the 131 GB host and starve the biggest - # scenarios (elasticstack_default, cert_renewal). 6 concurrent slots keep - # committed memory around 90-100 GB with head-room to spare. - max-parallel: 6 + # scenarios (elasticstack_default, cert_renewal). 6 concurrent slots was + # the previous target, sized for the intended 90-100 GB steady state. + # Observed reality after landing several PRs concurrently: with all six + # slots filled, the 20 GB + 13.8 GB scenarios still lose the memory + # race and time out at the 45-min gate deadline. Halving to 3 keeps + # committed memory well under host capacity and lets the heavy jobs + # actually acquire without starvation. Wall clock roughly doubles. + max-parallel: 3 matrix: # Standardise on rockylinux10 (not 9) for PR runs to match the rest # of the workflows. Rocky 10 exercises the EL≥9 branch in From e4d75fd7dfa072fb4439a427d560fc92f95907d1 Mon Sep 17 00:00:00 2001 From: oddly Date: Tue, 11 Aug 2026 22:40:34 +0200 Subject: [PATCH 5/5] fix(kibana): treat empty elasticstack_cert_pass as unset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same regression as the elasticsearch fix: setting elasticstack_cert_pass to "" (which happens as soon as the defaults line is uncommented for argument-spec parity) tripped the 'is defined' gate and silently blanked kibana_tls_key_passphrase. That empty string then reached the _cert_pass argument in cert_generate.yml, so elasticsearch-certutil ran with a bare '--pass' followed by '--out …', which the tool interprets as 'prompt for a password on stdin' — and throws IllegalStateException because CI has no TTY. Change the gate to match the elasticsearch fix (non-empty check). --- roles/kibana/tasks/main.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/roles/kibana/tasks/main.yml b/roles/kibana/tasks/main.yml index 86537009..4cc5d7bc 100644 --- a/roles/kibana/tasks/main.yml +++ b/roles/kibana/tasks/main.yml @@ -15,8 +15,13 @@ - name: Set common password for common certificates ansible.builtin.set_fact: kibana_tls_key_passphrase: "{{ elasticstack_cert_pass }}" + # Same gotcha the elasticsearch role hit: setting elasticstack_cert_pass + # to "" was silently blanking kibana_tls_key_passphrase, which then went + # through as `--pass ` (empty) to elasticsearch-certutil and made the + # tool prompt for a password on stdin. The docstring says "Leave empty + # to use the per-role passphrases" — so respect that. when: - - elasticstack_cert_pass is defined + - elasticstack_cert_pass | default('') | length > 0 - name: Set Elasticsearch hosts if used with other roles ansible.builtin.set_fact: