anaeem/ansible-collection-discovery
1
0
Fork 0
Code Issues Pull requests Projects Releases 1 Packages Wiki Activity Actions

apme/v5-readability-bundle #7

Closed
anaeem wants to merge 0 commits from apme/v5-readability-bundle into main
pull from: apme/v5-readability-bundle
merge into: anaeem:main
Conversation 0 Commits - Files changed - +7103 -2342
anaeem commented 2026-05-23 22:37:10 +00:00
Owner
Copy link
No description provided. 
The b426741b scan against v5 surfaces ~1300 noise findings that
either don't apply (L046 free-form shell is project convention),
are confirmed parser bugs (L098 Jinja-in-block-scalar misread as
duplicate keys), or fire on loop-bound vars the scanner can't
trace (L039 across inspect_app, app, _analyze_entry, pair_entry,
detector, _va_*, _di_*, _pb_*).

Also refreshes the detector-files glob — v5 moved detectors from
roles/detect_apps/files/ to roles/discover/files/detectors/.
Keeps both globs so older scan baselines still suppress cleanly.

After this lands the actionable bucket should drop from ~1556 to
~200, leaving real work (M005 forward-compat, L022 pipefail, and
the inner-file refactors) visible in the dashboard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After v5 collapsed/renamed roles, APME flagged 213 task-name violations
on the new file set (L025 = lowercase first letter, L070 = Jinja
expression not at end of string). Prior sweeps (722eccf, 47f4b24) ran
pre-v5 so these weren't covered.

Mechanical rewrite: every name matching `{{ var }} <sep> text` becomes
`Text <sep> {{ var }}`. The verb is now the first thing the operator
reads in PLAY RECAP — `Set JVM facts — host1-tomcat` instead of
`host1-tomcat — Set JVM facts`. Same data, easier scan.

103 names rewritten by a regex sweep (/tmp/fix_l025_l070_v5.py); 7
outliers (mid-string Jinja, multi-Jinja names) hand-fixed in
validate_app.yml, validate_one_attempt.yml, pair_one.yml,
fetch_facts_for_app.yml, upload_binary.yml, paginate.yml.

Skipped:
  * playbooks/utils/apply-v5-to-aap.yml — PR 6 will handle alongside
    its AAP-staging --check.

Verified: `yaml.safe_load` passes on all 17 modified files. Names are
display-only; no task semantics change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three top-level discover task files were each a 250-550-line wall of
YAML+Jinja+shell where the reader had to scroll past nine implicit
phases to know what the file did. Splitting each into a short
orchestrator plus per-phase sub-files matches the v5 pattern set by
roles/discover/tasks/main.yml — read main, see the phases; read a
phase, see its logic.

  java_common.yml  533 → 48-line orchestrator + 10 phases in java/
    1. jvm                — heap, GC, agents, classpath, sysprops
    2. java_version       — version + vendor from /proc/<pid>/exe
    3. keystores          — find + default-password sweep
    4. jdbc               — config-file URL scrape + driver lookup
    5. system_packages    — Satellite-sourced (stub, kept for compat)
    6. environment        — /proc environ + setenv.sh + systemd unit
    7. cron               — system + user crontab search
    8. log_config         — log4j/logback/JUL classify + paths
    9. network            — live ss -tnp peers
   10. secrets            — pattern markers only, never values

  gather.yml       301 → 31-line orchestrator + 6 phases in gather/
    1. host_facts         — setup + package_facts + service_facts
    2. processes          — ps + ss/netstat parsing
    3. filesystem         — shallow dir scan + marker-file find
    4. java_envs          — enumerate JDK installs + versions
    5. host_probes        — env vars, init system, pkg manager
    6. assemble           — consolidate into discovered_facts + debug

  war_introspection.yml 261 → 32-line orchestrator + 3 in war/
    1. locate             — resolve deploy dir + list + pick tool
    2. introspect         — emit ===DEPLOYMENT=== protocol (shell)
    3. parse              — parse the protocol into _di_deployments
                            (with cross-reference comments to keep
                             shell + parser in lock-step)

Every new file opens with `# Input` and `# Output` lines naming the
upstream loop vars and downstream facts. Task names use the
"<subfile> | Verb ..." convention from AGENTS.md so PLAY RECAP lines
remain searchable. No task semantics change — every shell, every
set_fact, every register is preserved verbatim; only the file
boundaries move.

Verified: yaml.safe_load passes on all 21 files;
`ansible-playbook playbooks/discover.yml --syntax-check` succeeds.
Module-op count is identical to main except for the 10 new
include_tasks the orchestrators add.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mta_publish/main.yml was a 542-line waterfall where six different
concerns (auth, categories, tags, business-service, component prep,
analyzer wiring) shared the file with no visual separation. A reader
had to scroll past five copy-pasted uri+loop blocks to see what came
after the tag-creation pass. Split into a 151-line orchestrator with
phase imports plus five files under phases/, mirroring the v5 role
pattern set by roles/discover/tasks/main.yml.

The six tag-creation loops (verdict / host / engagement / finding /
confidence / jdk) collapsed into one `_ensure_tags.yml` helper called
via include_tasks + vars from phases/3_tags.yml. New patterns added
later only need a one-line `include_tasks` block instead of a 16-line
copy-paste.

upsert_application.yml — broke the 73-line tag-ref-builder set_fact
(15+ nested conditionals, no visual structure) into six named layers:

  (a) bucket tags     — verdict, host, engagement
  (b) runtime tag     — Tomcat / EAP / Spring Boot / …
  (c) framework tags  — Spring Boot, Hibernate, RHEL 8, Java
  (d) confidence tag  — HIGH / MEDIUM / LOW
  (e) JDK version tag — JDK 8 / 11 / 17 / …
  (f) finding tags    — pre-built list from fleet_report

Each layer is its own set_fact step. PLAY RECAP now shows which
category contributed which tags. File is 65 lines longer but every
line is independently understandable.

llm_generate_app.yml — the 14-line "Strip VM-specific paths from JVM
opts" set_fact had 12 individual regex patterns inlined into one
boolean conjunction. Moved patterns to a task-local `vars:` list
grouped by category (Tomcat / WildFly / JVM filesystem / Logging /
Misc) and rewrote the filter as a small for-loop over the list.
Verified against the old form with a side-by-side test playbook —
byte-identical output. Added a top-of-file docstring describing the
nine sub-phases.

No external surfaces change. Verified:
  - yaml.safe_load passes on every modified file
  - ansible-playbook playbooks/addons/fleet-report.yml --syntax-check OK
  - VM-strip filter equivalence verified by playbook diff

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ansible 2.19+ marks registered task results as "untrusted" when used
directly in Jinja templates. M005 fires 59 times in scan b426741b on
this collection. This commit handles the safe subset — every
`var.stdout` / `var.stdout_lines` reference inside a Jinja expression
gets a `(… | string)` wrap.

The wrap is a no-op on Ansible 2.18 (the value is already a string)
and is forward-compatible with 2.19's untrusted-variable check. Zero
behavioural change against the current Ansible.

41 sites wrapped across 9 files (see /tmp/fix_m005_stdout.py for the
regex used). Unaffected: `.content` references (need per-site lifting
to set_facts; many flow through `| from_json` which is already
trust-safe), and `.json.X[…]` accesses (need set_fact lifts upstream
of the access).

The remaining ~18 M005 sites are deferred to a follow-up scoped
against an actual Ansible 2.19 EE — mechanical guessing here risks
introducing real bugs when the wrap pattern isn't the right one.

Verified: yaml.safe_load passes on all 9 modified files. Behaviour
under 2.18 is identical (| string is identity on a string value).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both files are already structured around phase comments; this commit
promotes them to numbered "Phase N — Title" dividers in line with the
discover + mta_publish refactors so the whole collection has one
consistent way to read multi-phase task files.

playbooks/utils/apply-v5-to-aap.yml (load-bearing — touches AAP JT
97/98/107/108):
  - Six phase headers (resolve → JT-perapp-analyze → Survey → JT-E
    fan_out → re-point legacy JTs → workflow rewire), each with a
    one-paragraph "what this phase does and why" note.
  - Play name typo fixed: "Bootstrap v4" → "Bootstrap v5".
  - No logic changes. Task names, set_facts, uri calls, when guards
    are all untouched. Diff is comments only.

roles/deploy/tasks/git_pr.yml:
  - Expanded the header docstring: phase map, required inputs (with
    types), optional inputs, required defaults (with sourcing), the
    external side-effects this file performs, and what set_stats it
    emits. Reader can answer "what does this do, what does it touch,
    what does it produce" without scrolling.
  - Six phase headers with one-paragraph notes — Phase 5's note
    points at the inline force-with-lease comment so a reader knows
    where the gnarliest logic lives.

Verified: yaml.safe_load passes; ansible-playbook --syntax-check on
apply-v5-to-aap.yml succeeds. Diff is entirely additive comments;
behaviour is byte-identical. Safe to merge ahead of staging --check
since no AAP API calls changed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
35 L022 findings against discovery shells. APME's expectation —
prepend `set -o pipefail` to every shell-with-a-pipe — would
introduce real behavioural changes here: the discovery shells
intentionally tolerate empty grep results because absence of data is
the common case on most fleet hosts.

The collection's existing pattern (documented in AGENTS.md
§Idempotency / §Error Handling) is `failed_when: false` +
`changed_when: false` on every discovery shell. The scan confirms:
31 of 35 L022 sites already carry `failed_when: false` and 34 of 35
have `changed_when: false`. The failure-swallowing pipefail is meant
to expose is already routed through the Ansible-level guards.

Adding pipefail without also replacing each `failed_when: false`
with a specific failure condition would surface expected-empty
results as task failures on hosts that don't have Java/keystores/
JDBC config/cron jobs to find — which is almost every host in a
real migration scan.

Suppress with the reasoning inline so a future defence-in-depth
pass (proper per-site failure conditions + pipefail) has the
context it needs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR 2 (verb-first task names) and PR 5 (.stdout `| string` wraps)
touched the original 533/301/261-line java_common.yml / gather.yml /
war_introspection.yml files. PR 3 replaced those files wholesale
with short orchestrators pointing at new sub-files under java/,
gather/, war/.

Took PR 3's orchestrator versions (the new short files) and re-ran
PR 5's stdout-wrap sweep against the new sub-files so the bundle
ends in the same net state as landing PRs 1→2→3→5 sequentially.
PR 3's sub-files were already created with verb-first names so the
PR 2 sweep matched zero lines on the new content.

12 stdout references wrapped in `| string` across 9 sub-files.

Verified: yaml.safe_load passes on all touched files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Conflict in roles/containerize/tasks/llm_generate_app.yml: HEAD had
PR 2's verb-first rename of the VM-strip task, incoming PR 4 had
the vars-ified pattern block. Took PR 4's structure (comment + vars:
list) with PR 2's verb-first task name.

Also fixes a latent bug in PR 5's stdout-wrap regex that escaped
its YAML-load smoke check: `var.stdout_lines | default(…)` was
being rewritten to `(var.stdout | string)_lines | default(…)`
because the negative-lookahead suffix that excluded already-trusted
filter chains (`| default`) caused the engine to backtrack from
the longer `.stdout_lines` match to the shorter `.stdout` match
when `| default` followed. 18 sites fixed to
`(var.stdout_lines | string) | default(…)` across discover/,
sbom_scan/, containerize/, deploy/, and playbooks/utils/cleanup.yml.

Verified: yaml.safe_load passes on every modified file.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Conflict in .apme.yml: both PR 1 and PR 7 added new suppression
blocks to the same section. Kept both — PR 1's L039/L098/L046
entries followed by PR 7's L022 entry.
Same pattern as PR 3 (discover) and PR 4 (mta_publish): one short
orchestrator + per-phase sub-files under llm/, each with a header
docstring naming its inputs and outputs.

llm_generate_app.yml  319 → 50-line orchestrator + 8 phases in llm/
  1. context             — discovery context + runtime classification
  2. memory              — heap → memory request/limit
  3. jvm_opts            — strip VM-specific -D flags
  4. port                — pick primary application port
  5. packages            — pom.xml deps + Satellite packages
  6. starter             — deterministic Containerfile starter (template)
  7. prompts             — system + user prompts (render + slurp + decode)
  8. call_and_finalize   — POST OpenRouter + parse response + cleanup

No task semantics change. Every set_fact, every template render, every
include, every shell wraps unchanged. Only file boundaries move and
docstrings clarify the per-phase contract.

Verified: yaml.safe_load passes on all 9 files; per_app.yml
syntax-check succeeds (the orchestrator path containerize → llm_containerfile.yml
→ llm_generate_app.yml → llm/* resolves cleanly).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three load-bearing files in mta_analyze had brief one-line headers.
Replaced with the v5-pattern header block: phase map naming each step,
required inputs (with shape), outputs, and reasoning comments where
the logic isn't obvious.

  main.yml      — 23 L: phase map, mode contract, required vs optional inputs
  pair.yml      — 147 L: 7-phase map, full input/output contract, the
                          eventual-consistency rationale for the defensive
                          re-GET in pair_one.yml is now signposted from here
  pair_one.yml  — 183 L: per-app flow described step by step, the loop
                          variable's shape is documented, the cumulative
                          fact (_pair_analyzer_queued) is called out

Other mta_analyze files (analyze, ensure_identity, ensure_tags,
poll_one, process_one, queue_one, resolve_tags, tag_one) already have
serviceable headers — left as-is, no per-file restructure needed.

No task semantics change; headers and comments only.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four-tier docs (Overview / Architecture / Components / Runbooks) covering
the v5 three-role pipeline, the fact-file contract, the verdict tree, and
the known integration gaps. Multi-stage Containerfile (antora -> nginx)
and a Gitea Actions workflow that builds + pushes the image to
oci.arsalan.io/library/migration-discovery-showroom on push to main.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Same pattern as PRs 3/4 and the llm_generate_app split: one short
orchestrator + per-phase sub-files under phases/, each with a header
docstring naming its inputs and outputs.

fleet_report/main.yml  345 → 35-line orchestrator + 5 phases in phases/
  1_collect    — per-host data collection (apps + deep + configs;
                  raw config content stripped to prevent secret leakage)
  2_score      — Green/Yellow/Red/Gray verdict from 9 flag dimensions
                  (secrets, complex runtime, non-standard path,
                   keystores, JDBC, external deps, heap unset, Java EOL,
                   low-confidence)
  3_stats      — fleet-wide totals + runtime breakdown + risk roll-up
  4_render     — output dir + HTML + catalog data + JSON dump
  5_finalize   — operator summary debug + AAP set_stats artifact

Score phase stays as one big set_fact intentionally: each flag pushes
into the same accumulating `flags[]` list inside a per-host for-loop,
and Jinja for-loop scope makes splitting the dimensions across tasks
impractical. The 164-line file now opens with a docstring naming all
9 dimensions and the verdict scoring rule, so a reader can skim
without having to trace.

No task semantics change. Every set_fact, every template render is
preserved verbatim; only file boundaries move and docstrings clarify
the per-phase contract.

Verified: yaml.safe_load passes on all 6 files; addons/fleet-report.yml
syntax-check succeeds.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The header was a 2-line summary that left the per-file ===POM=== /
===GRADLE=== shell protocol entirely undocumented. A reader hitting
the parser-side set_fact had to reverse-engineer the markers.

Replaced with the v5-pattern docstring block: per-protocol section
list (markers + what each emits), loop var shape, role-internal
input, and the final _di_build_info dict's key set.

No task semantics change; docstring only.
The 5 embedded shells in git_pr.yml ranged from 4-12 lines and
mixed Ansible Jinja interpolation with bash control flow. Lifted
each to a standalone .sh under roles/deploy/files/ invoked via
ansible.builtin.script with positional args.

New files (each opens with a header explaining purpose + args):
  git_fetch_reset.sh     — fast-path refresh of an existing local clone
  git_checkout_branch.sh — switch to bundle/<slug> (amend or create)
  git_wipe_stale.sh      — clear stale manifest files (keep .gitkeep)
  git_commit.sh          — stage + commit, prints NOCHANGE/COMMITTED
  git_push.sh            — force-with-lease push with safe lease semantics

The long --force-with-lease commentary that used to live above the
push task now sits at the top of git_push.sh next to the code it
documents. git_pr.yml's task drops to a one-line script invocation
plus a backreference comment.

No task semantics change. Behaviour is identical: same arguments,
same exit codes, same NOCHANGE/COMMITTED token used by changed_when,
same no_log on the push.

Verified: yaml.safe_load passes; per_app.yml syntax-check OK; all
scripts marked executable (chmod 755).
Step-by-step validation guide for the v5 readability sweep:
  1. Push the locally-built bundle (SSH agent + push)
  2. Static checks (yaml safe_load + syntax-check + bash -n)
  3. APME re-scan + triage block read
  4. File-tree sanity check on the refactored roles
  5. End-to-end discover with byte-diff vs a pre-sweep baseline
  6. Per-app containerize + gitea PR verification
  7. MTA publish to a scratch hub + tag-category diff
  8. AAP apply-v5-to-aap dry-run + JT playbook-field check
  9. Full workflow run from AAP UI
  + rollback path if any step fails

Each step is independently meaningful — readers can stop at step 4
for static-only validation or carry through to step 9 for full
behavioural coverage.
- docs/V4_VALIDATION.md → docs/archive/V4_VALIDATION.md
  v4 validation report is historical now; matches ARCHITECTURE_V3.md +
  ARCHITECTURE_CRITIQUE.md which were archived in the same pattern.
- docs/APME_REVIEW.md
  Add a top-of-file disclaimer that path references predate v5 and
  point readers at ARCHITECTURE_V5.md + the new validation runbook.
  Refresh the .apme.yml suppression list summary to match the v5
  state (L022/L039/L046/L098 added; detector-files glob extended).
- playbooks/utils/cleanup.yml
  Play name was 'Step 3: Cleanup migrated artifacts' (v4 numbering).
  Renamed to 'Cleanup migrated artifacts (v5 add-on)' so PLAY RECAP
  doesn't misrepresent the v5 workflow.
docs: refresh APME_REVIEW disclaimer + cleanup play name for v5
Some checks failed
APME static analysis / apme-check (pull_request) Has been cancelled
Details
0c7c18a340 
- docs/APME_REVIEW.md
  Add a top-of-file disclaimer that path references predate v5 and
  point readers at ARCHITECTURE_V5.md + the new validation runbook.
  Refresh the .apme.yml suppression list summary to match the v5
  state (L022/L039/L046/L098 added; detector-files glob extended).
- playbooks/utils/cleanup.yml
  Play name was 'Step 3: Cleanup migrated artifacts' (v4 numbering).
  Renamed to 'Cleanup migrated artifacts (v5 add-on)' so PLAY RECAP
  doesn't misrepresent the v5 workflow.
anaeem closed this pull request 2026-05-23 22:37:44 +00:00
Some checks failed
APME static analysis / apme-check (pull_request) Has been cancelled
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
anaeem/ansible-collection-discovery!7
No description provided.