Add kernel Cmdline to data collection - #2
Conversation
Expose the ansible_cmdline fact, which is gathered by Ansible but was not surfaced anywhere in the output. Useful for auditing boot-time kernel parameters across a fleet (mitigations, IOMMU, hugepages, console settings). - html_fancy: add an optional (hidden by default) "Cmdline" overview column and a row in the software detail table. - markdown / markdown_split: add the matching detail rows. Ansible parses /proc/cmdline into a dict, so the overview column renders it back into a readable command line: keys sorted for stable output, and valueless parameters (represented as True) rendered as bare keys rather than 'quiet=True'. Non-dict values are passed through as-is for safety.
There was a problem hiding this comment.
🟡 Changes recommended
The new detail render paths call r_dict() on ansible_cmdline unconditionally, which will crash template rendering if that fact is not a dict.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR surfaces Ansible’s ansible_cmdline fact across the html_fancy, markdown, and markdown_split templates so kernel boot parameters are available for auditing and troubleshooting.
Changes:
- Adds a hidden
Cmdlineoverview column tohtml_fancy, reassembling the cmdline from the fact dict. - Adds a
Cmdlinerow to thehtml_fancyOperating System detail table. - Adds matching
Cmdlinedetail sections tomarkdownandmarkdown_split.
File summaries
| File | Description |
|---|---|
| src/ansiblecmdb/data/tpl/html_fancy_defs.html | Adds a hidden Cmdline column plus a new OS detail row for ansible_cmdline. |
| src/ansiblecmdb/data/tpl/markdown.tpl | Adds a Cmdline detail entry under Operating System. |
| src/ansiblecmdb/data/tpl/markdown_split_detail.tpl | Adds a Cmdline detail entry under Operating System in split-detail output. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
col_cmdline() already fell back to str() for a non-dict ansible_cmdline,
but the three detail rows passed the fact straight to r_dict(), so a
platform reporting it as a plain string crashed rendering:
html_fancy: AttributeError: 'str' object has no attribute 'keys'
markdown: AttributeError: 'str' object has no attribute 'items'
Guard all three on isinstance(dict) and render the scalar otherwise,
matching what the overview column already did.
Verified against a fact file with ansible_cmdline set to a string: both
templates crashed before, and now render the value. The normal dict case
is unchanged.
|
Merge note: conflicts with #1 in Both sides are additive so "keep both" is correct, but the naive concatenation silently breaks the template: the conflict region ends just before a shared After resolving, verify: grep -c '<%def ' src/ansiblecmdb/data/tpl/html_fancy_defs.html # must equal
grep -c '</%def>' src/ansiblecmdb/data/tpl/html_fancy_defs.htmlFull merge order and both conflict resolutions: #1 (comment) No other PR conflicts with this one. |
Surfaces
ansible_cmdline, a fact Ansible already gathers but that ansible-cmdb never displayed.Why
Boot-time kernel parameters are a real fleet-auditing need: confirming CPU mitigation flags are (or aren't) disabled, checking
intel_iommu/amd_iommubefore a passthrough rollout, verifying hugepage reservations, or finding hosts still pinned to an oldBOOT_IMAGEafter a kernel update.What changed
html_fancy: newCmdlineoverview column,visible: Falseso the default view is unchanged.html_fancy:Cmdlinerow in the software detail table, rendered via the existingr_dict(..., sort=True)helper for consistency with the SELinux row.markdownandmarkdown_split: matching detail rows.On the rendering
Ansible parses
/proc/cmdlineinto a dict, so dumping it directly gives you Pythonreproutput rather than a command line. The overview column reassembles it:True— render as bare keys, so you getquietrather thanquiet=True.Result for a host in
example/:Testing
Rendered
html_fancy,markdown, andmarkdown_splitagainst the bundledexample/inventory, and verified the reassembled command line matches the raw fact for hosts with mixed valueless and key=value parameters.Note: the vendored
lib/yaml*andlib/makodon't import on Python 3.10+ (collections.Hashablewas removed), so this was rendered against system PyYAML/Mako. Unrelated to this change.Unrelated bug spotted
While testing I hit a pre-existing crash in
markdown_split, reproducible on an unmodifiedmaster:That's
sorted(host['ansible_facts'].get('ansible_interfaces', []))— it fails when the interfaces fact is a list of dicts rather than strings.markdown.tplalready handles both shapes;markdown_split_detail.tpldoesn't. Not touched here — happy to send a separate PR.Note for the maintainer
I've opened a companion PR adding
ansible_board_name/ansible_bios_version. Both insert into the same column list, so whichever merges second will need a trivial one-line rebase. Happy to combine them into one PR if you'd prefer.