From eeceb3f39ecdafc66e5f9f47b249242bb36dd209 Mon Sep 17 00:00:00 2001 From: Rundeck CI Date: Wed, 16 Sep 2026 11:11:36 -0700 Subject: [PATCH] Harden rundeck-plugin-versions skill after two real misses Three fixes, all found by revisiting this skill after the ua-runner ansible-plugin gap: 1. mapping.tsv: ua-runner now tracks a real ansiblePluginVersion, pyWinrmPluginVersion, opensshNodeExecutionVersion, sshjPluginVersion property (Forrest's call - track a real property rather than rely on runner-agent/build.gradle's submodule-read mechanism as the source of truth). Retired the VIA-SUBMODULE token from the data (kept supported in the scripts in case a future case needs it). 2. Fixed a real bug in both check-versions.sh and bump-versions-pr.sh: prop_ver()'s grep|head|sed pipeline exits 1 when a property genuinely isn't present yet, and under this script's set -euo pipefail, that silently killed the whole script (no error, just stopped) instead of reporting 'not found' like every other missing-property case. Found for real running check-versions.sh --plugin ansible-plugin right after adding it to ua-runner's mapping.tsv column, before the PR adding the actual property had merged. 3. Added scripts/audit-consumption.sh (Workflow D): read-only sweep that greps every '-' cell's repo for a real dependency reference to that plugin, in both compact and verbose Gradle syntax. Verified it catches both kinds of gap found this week (a plain compact-syntax grep alone would have missed rundeck-ec2-nodes-plugin's verbose-syntax dependency, same as the first time this was tried by hand) by deliberately reverting mapping.tsv to the old wrong state and confirming it flags exactly those rows, then restoring the correct file. --- skills/rundeck-plugin-versions/SKILL.md | 21 ++++ skills/rundeck-plugin-versions/mapping.tsv | 20 ++-- skills/rundeck-plugin-versions/reference.md | 10 +- .../scripts/audit-consumption.sh | 95 +++++++++++++++++++ .../scripts/bump-versions-pr.sh | 6 +- .../scripts/check-versions.sh | 11 ++- 6 files changed, 144 insertions(+), 19 deletions(-) create mode 100755 skills/rundeck-plugin-versions/scripts/audit-consumption.sh diff --git a/skills/rundeck-plugin-versions/SKILL.md b/skills/rundeck-plugin-versions/SKILL.md index 1fbc32f..b5eafa5 100644 --- a/skills/rundeck-plugin-versions/SKILL.md +++ b/skills/rundeck-plugin-versions/SKILL.md @@ -103,6 +103,22 @@ Unlike Workflows A/B, this one *does* push a branch and open a PR (not to `main` Each repo gets one stable branch (`bump-plugin-versions`, no date suffix). Re-running the script rebuilds that branch fresh off current `main` and force-pushes it every time, so it keeps **updating the same open PR in place** (via `gh pr edit`, same as Renovate's own PRs) instead of piling up a new dated branch/PR per run. If the previous PR for that branch was merged or closed, the next run starts a clean new one. Don't hand-edit the `bump-plugin-versions` branch between runs - it gets discarded and recreated. +## Workflow D - audit mapping.tsv itself + +Use before trusting a "not consumed" (`-`) cell, when adding a new plugin, or after a report seems to be missing a bump you expected (that's exactly how the two real gaps below were found - the hard way, one plugin at a time, after something else already went wrong). + +``` +- [ ] 1. Confirm/resolve the rundeckpro and ua-runner paths +- [ ] 2. Run: scripts/audit-consumption.sh +- [ ] 3. For each POSSIBLE GAP printed, read the surrounding line in that + file and decide: real gap (fix mapping.tsv, see reference.md's + Gotchas for the pattern) or false positive (e.g. a comment) +``` + +Read-only, same spirit as `check-versions.sh`'s drift report but one level up: it checks whether the *mapping itself* is right, not whether values are current. It greps each repo's whole tracked tree (not just root `gradle.properties`/`build.gradle`) for both compact (`"group:artifact:version"`) and verbose (`group: '...', name: '...'`) Gradle dependency syntax - a plain grep for the compact form alone is exactly what missed `rundeck-ec2-nodes-plugin`'s real (verbose-syntax) dependency the first time this kind of sweep was tried by hand. + +Two real gaps this would have caught immediately instead of after a missed bump (both 2026-09-16/17): 7 plugins `mapping.tsv` called vestigial/unconsumed in `rundeckpro` that were actually bundled via a real dependency list in its root `build.gradle`, and `rundeck-ec2-nodes-plugin`'s verbose-syntax dependency in `rundeckpro/plugins/cloud-aws-plugins/build.gradle`. See `reference.md`'s Gotchas section for the full list of non-obvious consumption patterns found this way. + ## Do not auto-push Never push directly to `main`, and never merge a PR this skill opens - a human reviews and merges. Note some consuming repos may enforce PR rulesets (direct pushes to `main` rejected). Never add Cursor/agent co-author trailers to any commit. Workflows A and B additionally stop before even opening a PR (diffs only, human opens the PR); Workflow C opens the PR itself but still leaves merging to a human. @@ -111,8 +127,13 @@ Never push directly to `main`, and never merge a PR this skill opens - a human r `check-versions.sh` and `bump-versions-pr.sh` both snapshot `origin/main`'s `gradle.properties` via `git show origin/main:gradle.properties` rather than reading the working-tree file directly. Reading the working tree is wrong whenever a repo is checked out on an in-progress feature branch that's stale relative to `main` (common - these are active repos) - it can report false drift, miss real drift, or (as happened once for real) make a proactive PR's commit message claim more changes than actually happened. Keep this pattern if you're modifying either script. +## Gotcha: a missing property must not crash the whole script + +Both scripts' `prop_ver()` ends in a `grep | head | sed` pipeline. Under this repo's `set -euo pipefail`, `grep` finding no match (a property that's genuinely not there yet - e.g. added to `mapping.tsv` but the PR adding the actual property line hasn't merged) exits 1, and with `pipefail` that becomes the whole pipeline's exit status - which kills the *entire script* silently (no error message, just stops) when it happens inside a plain `var=$(...)` assignment, rather than being treated as the normal "not found" case it actually is. Found for real 2026-09-17: checking `ansible-plugin` right after adding it to `ua-runner`'s `mapping.tsv` column, before the PR adding the property had merged, silently killed `check-versions.sh` with zero output. Both copies of `prop_ver()` now end the pipeline with `|| true` - keep that if you touch either one. + ## Scripts - `scripts/plugin-latest.sh ` - latest released version (gh release, clean-semver tag fallback). - `scripts/check-versions.sh [--root DIR | --rundeck DIR --rundeckpro DIR --ua-runner DIR] [--plugin NAME]` - read-only drift report across the three repos. - `scripts/bump-versions-pr.sh [--root DIR | --rundeck DIR --rundeckpro DIR --ua-runner DIR] [--dry-run]` - opens one PR per repo bundling every bump it needs. +- `scripts/audit-consumption.sh [--root DIR | --rundeckpro DIR --ua-runner DIR]` - read-only check that every `-` cell in `mapping.tsv` is actually right; see Workflow D. diff --git a/skills/rundeck-plugin-versions/mapping.tsv b/skills/rundeck-plugin-versions/mapping.tsv index f35f4d3..0224ad0 100644 --- a/skills/rundeck-plugin-versions/mapping.tsv +++ b/skills/rundeck-plugin-versions/mapping.tsv @@ -9,19 +9,19 @@ # them into bundledPlugins and testbuild.groovy reads them. build.yaml no longer carries versions. # - kubernetes uses kubernetesVersion in rundeckpro but kubernetesPluginVersion in ua-runner. # - nixy-step-plugins publishes one release that drives nixystepVersion (4 artifacts share it). -# - ua_runner_prop = "VIA-SUBMODULE" means ua-runner has no property of its own to bump: it reads -# the version straight out of the nested rundeck/ submodule's gradle.properties at build time -# (runner-agent/build.gradle's bundledCorePlugins map). It cannot drift the way a duplicated -# property can, so there is nothing for bump-versions-pr.sh to do there - do not treat this as -# "not consumed" (that was the exact ambiguity that hid the rundeckpro gap this mapping used to -# have; verified 2026-09-17). -ansible-plugin ansiblePluginVersion ansiblePluginVersion VIA-SUBMODULE +# - VIA-SUBMODULE (deprecated 2026-09-17, kept supported in the scripts in case it's needed again) +# meant a repo had no property of its own to bump because it read a version straight out of a +# nested git submodule's gradle.properties at build time. Forrest's call: track a real property +# in ua-runner's own gradle.properties instead, even where a submodule-read path also exists in +# runner-agent/build.gradle's bundledCorePlugins map - don't rely on the submodule mechanism as +# the source of truth, keep an explicit, bumpable property like everywhere else. +ansible-plugin ansiblePluginVersion ansiblePluginVersion ansiblePluginVersion aws-s3-model-source awsS3ModelSourceVersion awsS3ModelSourceVersion - -py-winrm-plugin pyWinrmPluginVersion pyWinrmPluginVersion VIA-SUBMODULE -openssh-node-execution opensshNodeExecutionVersion opensshNodeExecutionVersion VIA-SUBMODULE +py-winrm-plugin pyWinrmPluginVersion pyWinrmPluginVersion pyWinrmPluginVersion +openssh-node-execution opensshNodeExecutionVersion opensshNodeExecutionVersion opensshNodeExecutionVersion multiline-regex-datacapture-filter multilineRegexDatacaptureFilterVersion multilineRegexDatacaptureFilterVersion - attribute-match-node-enhancer attributeMatchNodeEnhancerVersion attributeMatchNodeEnhancerVersion - -sshj-plugin sshjPluginVersion sshjPluginVersion VIA-SUBMODULE +sshj-plugin sshjPluginVersion sshjPluginVersion sshjPluginVersion http-step - httpStepVersion httpStepVersion slack-incoming-webhook-plugin - slackWebhookVersion - aws-s3-steps - awsS3StepsVersion awsS3StepsVersion diff --git a/skills/rundeck-plugin-versions/reference.md b/skills/rundeck-plugin-versions/reference.md index b6e8e99..02067a8 100644 --- a/skills/rundeck-plugin-versions/reference.md +++ b/skills/rundeck-plugin-versions/reference.md @@ -14,13 +14,13 @@ Machine-readable source of truth: [`mapping.tsv`](mapping.tsv). This file is the | Plugin repo | Core prop (`gradle.properties`) | rundeckpro prop | ua-runner prop | |-------------|--------------------------------|-----------------|----------------| -| ansible-plugin | `ansiblePluginVersion` | `ansiblePluginVersion` (corePlugins bundle + testbuild.groovy) | via submodule, no prop | +| ansible-plugin | `ansiblePluginVersion` | `ansiblePluginVersion` (corePlugins bundle + testbuild.groovy) | `ansiblePluginVersion` | | aws-s3-model-source | `awsS3ModelSourceVersion` | `awsS3ModelSourceVersion` (corePlugins bundle + testbuild.groovy) | - | -| py-winrm-plugin | `pyWinrmPluginVersion` | `pyWinrmPluginVersion` (corePlugins bundle + testbuild.groovy) | via submodule, no prop | -| openssh-node-execution | `opensshNodeExecutionVersion` | `opensshNodeExecutionVersion` (corePlugins bundle + testbuild.groovy) | via submodule, no prop | +| py-winrm-plugin | `pyWinrmPluginVersion` | `pyWinrmPluginVersion` (corePlugins bundle + testbuild.groovy) | `pyWinrmPluginVersion` | +| openssh-node-execution | `opensshNodeExecutionVersion` | `opensshNodeExecutionVersion` (corePlugins bundle + testbuild.groovy) | `opensshNodeExecutionVersion` | | multiline-regex-datacapture-filter | `multilineRegexDatacaptureFilterVersion` | `multilineRegexDatacaptureFilterVersion` (corePlugins bundle + testbuild.groovy) | - | | attribute-match-node-enhancer | `attributeMatchNodeEnhancerVersion` | `attributeMatchNodeEnhancerVersion` (corePlugins bundle + testbuild.groovy) | - | -| sshj-plugin | `sshjPluginVersion` | `sshjPluginVersion` (corePlugins bundle + testbuild.groovy) | via submodule, no prop | +| sshj-plugin | `sshjPluginVersion` | `sshjPluginVersion` (corePlugins bundle + testbuild.groovy) | `sshjPluginVersion` | | http-step | - | `httpStepVersion` | `httpStepVersion` | | slack-incoming-webhook-plugin | - | `slackWebhookVersion` | - | | aws-s3-steps | - | `awsS3StepsVersion` | `awsS3StepsVersion` | @@ -43,7 +43,7 @@ Machine-readable source of truth: [`mapping.tsv`](mapping.tsv). This file is the - **Core reads plugin versions from `gradle.properties`.** `rundeck/gradle.properties` defines `ansiblePluginVersion`, `awsS3ModelSourceVersion`, `pyWinrmPluginVersion`, `opensshNodeExecutionVersion`, `multilineRegexDatacaptureFilterVersion`, `attributeMatchNodeEnhancerVersion`, `sshjPluginVersion`. `build.gradle` interpolates these into `bundledPlugins` and `testbuild.groovy` reads them; `build.yaml` no longer carries versions (it is a pointer comment). Update the property. - **kubernetes property name differs by repo:** `kubernetesVersion` in rundeckpro, `kubernetesPluginVersion` in ua-runner. rundeckpro also defines `kubernetesPluginVersion`, which is vestigial there. - **rundeckpro's Core-overlap props are NOT vestigial** (`ansiblePluginVersion`, `sshjPluginVersion`, `opensshNodeExecutionVersion`, `pyWinrmPluginVersion`, `awsS3ModelSourceVersion`, `multilineRegexDatacaptureFilterVersion`, `attributeMatchNodeEnhancerVersion`) - all seven are genuinely read twice: by `testbuild.groovy`'s expected-plugin-version map, and by a real `corePlugins` dependency list in rundeckpro's root `build.gradle` (`"org.rundeck.plugins::${}"`, mirroring Core's own bundling, comment references RUN-4569) that actually bundles the jars. A prior version of this note claimed only `ansiblePluginVersion` was real and the rest were vestigial; that was wrong and caused `bump-versions-pr.sh` to skip 5 real bumps in a rundeckpro PR (caught 2026-09-16). Bump these in rundeckpro whenever their Core value changes, same as any other consumed prop. -- **ua-runner bundles ansible-plugin, py-winrm-plugin, openssh-node-execution, and sshj-plugin too, but with no property of its own.** `runner-agent/build.gradle`'s `bundledCorePlugins` map reads the version straight out of the nested `rundeck/` submodule's `gradle.properties` at build time (`file("${rootProject.projectDir}/rundeck/gradle.properties")`), rather than duplicating it into ua-runner's own `gradle.properties`. It structurally cannot drift the way a duplicated property can, so there's nothing to bump - `mapping.tsv` marks these `VIA-SUBMODULE` in the ua-runner column rather than `-`, specifically so it doesn't read as "not consumed" (verified 2026-09-17, after the same ambiguity nearly hid this too). +- **ua-runner bundles ansible-plugin, py-winrm-plugin, openssh-node-execution, and sshj-plugin two different ways at once.** `runner-agent/build.gradle`'s `bundledCorePlugins` map reads the version straight out of the nested `rundeck/` submodule's `gradle.properties` at build time (`file("${rootProject.projectDir}/rundeck/gradle.properties")`) - that path alone can't drift, since it's not a duplicated value. But Forrest's call (2026-09-17) was to *also* track a real, independent property for each in ua-runner's own `gradle.properties` (added there for the first time that day) rather than rely on the submodule read as the source of truth - so these are tracked normally in `mapping.tsv` like any other consumed prop, not specially. `check-versions.sh`/`bump-versions-pr.sh` still support a `VIA-SUBMODULE` token for a future case like this, but it isn't used by anything right now. - **nixy-step-plugins is multi-module:** one release drives `nixystepVersion`, which feeds four artifacts (`waitfor`, `file`, `local-script`, `command`). - **rundeck-azure-plugin** is consumed in `rundeckpro/plugins/azure-plugins/build.gradle` (not `enterprise/build.gradle`), but its property still lives in `rundeckpro/gradle.properties`. - **rundeck-ec2-nodes-plugin** is consumed in `rundeckpro/plugins/cloud-aws-plugins/build.gradle` as a real `pluginLibs` dependency (verified 2026-08-18, grep for `rundeck-ec2-nodes-plugin` in that file). It is not vestigial - keep it in the operative dependency list. diff --git a/skills/rundeck-plugin-versions/scripts/audit-consumption.sh b/skills/rundeck-plugin-versions/scripts/audit-consumption.sh new file mode 100755 index 0000000..34d15ed --- /dev/null +++ b/skills/rundeck-plugin-versions/scripts/audit-consumption.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +# +# audit-consumption.sh +# +# Read-only sanity check for mapping.tsv itself: for every plugin marked +# "-" (not consumed) in rundeckpro or ua-runner, search that repo's whole +# tree for a real dependency reference to it. If one is found, mapping.tsv +# is probably wrong - print a warning instead of silently trusting the +# existing "-". +# +# Why this exists: mapping.tsv had two real gaps found the hard way +# (2026-09-16/17) - 7 plugins marked vestigial/unconsumed in rundeckpro +# that were actually bundled via a real dependency list in its root +# build.gradle, and 4 plugins ua-runner bundles via a submodule-read +# mechanism no doc mentioned. Both were found by manually grepping one +# plugin at a time after something else went wrong. This script does that +# sweep for every "-" cell up front, so a gap surfaces before it causes a +# missed bump rather than after. +# +# Deliberately covers BOTH Gradle dependency syntaxes: +# - compact: "group:artifact:version" e.g. "org.rundeck.plugins:docker:2.0.1" +# - verbose: group: '...', name: 'artifact', ... e.g. rundeck-ec2-nodes-plugin's real declaration +# A grep for only the compact form is exactly what missed rundeck-ec2-nodes-plugin +# the first time this kind of sweep was tried. +# +# This is a heuristic, not a precise dependency-graph tool: it flags +# candidates for a human to look at, same spirit as check-versions.sh's +# DRIFT report. False positives are possible (e.g. a plugin name +# mentioned only in a comment); read the surrounding line before editing +# mapping.tsv or bump-versions-pr.sh off of it. +# +# Usage: +# audit-consumption.sh [--root DIR | --rundeckpro DIR --ua-runner DIR] +# +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +MAPPING="$SCRIPT_DIR/../mapping.tsv" + +ROOT="${GH_ROOT:-$HOME/Documents/GitHub}" +RUNDECKPRO="" ; UARUNNER="" +while [ $# -gt 0 ]; do + case "$1" in + --root) [ $# -ge 2 ] || { echo "Missing value for --root" >&2; exit 2; }; ROOT="$2"; shift 2 ;; + --rundeckpro) [ $# -ge 2 ] || { echo "Missing value for --rundeckpro" >&2; exit 2; }; RUNDECKPRO="$2"; shift 2 ;; + --ua-runner) [ $# -ge 2 ] || { echo "Missing value for --ua-runner" >&2; exit 2; }; UARUNNER="$2"; shift 2 ;; + -h|--help) sed -n '2,30p' "$0"; exit 0 ;; + *) echo "Unknown arg: $1" >&2; exit 2 ;; + esac +done +RUNDECKPRO="${RUNDECKPRO:-$ROOT/rundeckpro}" +UARUNNER="${UARUNNER:-$ROOT/ua-runner}" + +[ -f "$MAPPING" ] || { echo "mapping.tsv not found at $MAPPING" >&2; exit 1; } + +# Search a repo's whole tracked tree (git-tracked only, so build output and +# the nested rundeck/ submodule checkout - which is its own repo - don't +# produce noise) for either Gradle dependency syntax referencing a plugin. +find_reference() { + local repo_dir="$1" plugin="$2" + [ -d "$repo_dir/.git" ] || return 1 + git -C "$repo_dir" grep -lE \ + "org\.rundeck[.a-zA-Z]*:${plugin}[:'\"]|name:[[:space:]]*['\"]${plugin}['\"]" \ + -- '*.gradle' '*.gradle.kts' 2>/dev/null | grep -v '^rundeck/' || true +} + +found_any=0 +while IFS=$'\t' read -r plugin core_prop pro_prop ua_prop; do + case "$plugin" in ''|\#*) continue ;; esac + + if [ "$pro_prop" = "-" ]; then + hits="$(find_reference "$RUNDECKPRO" "$plugin")" + if [ -n "$hits" ]; then + echo "POSSIBLE GAP: $plugin marked '-' for rundeckpro, but referenced in:" + echo "$hits" | sed 's/^/ rundeckpro\//' + found_any=1 + fi + fi + + if [ "$ua_prop" = "-" ]; then + hits="$(find_reference "$UARUNNER" "$plugin")" + if [ -n "$hits" ]; then + echo "POSSIBLE GAP: $plugin marked '-' for ua-runner, but referenced in:" + echo "$hits" | sed 's/^/ ua-runner\//' + found_any=1 + fi + fi +done < "$MAPPING" + +if [ "$found_any" -eq 0 ]; then + echo "No gaps found: every plugin marked '-' in mapping.tsv has no dependency reference in the corresponding repo tree." +else + echo + echo "Review each hit above - it may be a real gap (fix mapping.tsv), or a false positive (e.g. a comment mentioning the plugin name)." +fi diff --git a/skills/rundeck-plugin-versions/scripts/bump-versions-pr.sh b/skills/rundeck-plugin-versions/scripts/bump-versions-pr.sh index 538dca8..bb18718 100755 --- a/skills/rundeck-plugin-versions/scripts/bump-versions-pr.sh +++ b/skills/rundeck-plugin-versions/scripts/bump-versions-pr.sh @@ -46,8 +46,10 @@ UARUNNER="${UARUNNER:-$ROOT/ua-runner}" prop_ver() { local prop="$1" f="$2" - [ -f "$f" ] || { echo ""; return; } - grep -E "^[[:space:]]*${prop}[[:space:]]*=" "$f" 2>/dev/null | head -1 | sed -E 's/^[^=]*=[[:space:]]*//; s/[[:space:]]*$//' + [ -f "$f" ] || { echo ""; return 0; } + # See check-versions.sh's copy of this function for why the `|| true` matters: + # a property that isn't present yet is a normal outcome, not a script-ending error. + grep -E "^[[:space:]]*${prop}[[:space:]]*=" "$f" 2>/dev/null | head -1 | sed -E 's/^[^=]*=[[:space:]]*//; s/[[:space:]]*$//' || true } # col: 2=core prop column, 3=rundeckpro prop column, 4=ua-runner prop column diff --git a/skills/rundeck-plugin-versions/scripts/check-versions.sh b/skills/rundeck-plugin-versions/scripts/check-versions.sh index a76fd6d..394dc96 100755 --- a/skills/rundeck-plugin-versions/scripts/check-versions.sh +++ b/skills/rundeck-plugin-versions/scripts/check-versions.sh @@ -49,8 +49,15 @@ warn_missing "$RUNDECK"; warn_missing "$RUNDECKPRO"; warn_missing "$UARUNNER" # Value of a property in a gradle.properties file. prop_ver() { local prop="$1" f="$2" - [ -f "$f" ] || { echo ""; return; } - grep -E "^[[:space:]]*${prop}[[:space:]]*=" "$f" 2>/dev/null | head -1 | sed -E 's/^[^=]*=[[:space:]]*//; s/[[:space:]]*$//' + [ -f "$f" ] || { echo ""; return 0; } + # A property that isn't present yet is a normal, expected outcome (e.g. + # newly added to mapping.tsv but not yet merged anywhere) - grep exits 1 + # for "no match", and under this script's `set -e`/pipefail, that would + # otherwise silently kill the whole script rather than just report "not + # found" (found for real 2026-09-17: checking ansible-plugin against + # ua-runner's origin/main, which didn't have the property yet, aborted + # the script with zero output and no error message). + grep -E "^[[:space:]]*${prop}[[:space:]]*=" "$f" 2>/dev/null | head -1 | sed -E 's/^[^=]*=[[:space:]]*//; s/[[:space:]]*$//' || true } # Snapshot origin/main's actual gradle.properties rather than reading the