[sabre1041]

What

Replaces the openssl x509 shell command in the mtv_provider role with the community.crypto.x509_certificate_convert module and cleans up temporary certificate files after use.

Why

The mtv_provider role downloads the vCenter VMCA CA certificate in DER format and converts it to PEM. The conversion previously shelled out to openssl via ansible.builtin.command, which depends on the openssl binary being present on the execution environment and bypasses Ansible's module behaviors (idempotency, structured results, native change reporting). Using the dedicated community.crypto.x509_certificate_convert module aligns with Ansible best practices and is already satisfied by the collection's existing community.crypto >= 2.26.0 dependency. Additionally, the temporary DER and PEM files were left behind in /tmp on the execution environment; they are now removed when the block completes.

Changes

• roles/mtv_provider/tasks/validate.yml
  • Replaced the ansible.builtin.command task running openssl x509 -inform DER -in ... -outform PEM with the community.crypto.x509_certificate_convert module (format: pem), removing the changed_when: false workaround since the module reports change state natively and is idempotent
  • Added the _mtv_provider_vmca_pem_path variable for the converted PEM output, mirroring the existing DER path variable
  • The converted PEM is now read from the output file via lookup('ansible.builtin.file', ...) instead of capturing command stdout, preserving the trailing newline behavior expected by the provider secret template
  • Added an always: section to the VMCA block that removes both temporary certificate files (/tmp/.mtv_provider_vmca_ca.der and /tmp/.mtv_provider_vmca_ca.pem) on both success and rescue paths
• roles/mtv_provider/README.md
  • Updated the auto-generated task table to reflect the new module for the conversion task

[tech2734]

Approved — tested against homelab vCenter (AAP job #320 successful).

Clean, well-scoped improvement:

• community.crypto.x509_certificate_convert replaces the openssl x509 shell-out, removing the binary dependency and the changed_when: false workaround
• The always: block for temp file cleanup is a nice addition — previously the DER file lingered in /tmp
• community.crypto >= 2.26.0 is already a collection dependency so the module is guaranteed available

VMCA DER→PEM conversion and provider creation both passed on a live vCenter environment.

[vvaldez]

Thanks for the clean, well-scoped PR — CI green, and the temp-file cleanup is a nice addition. Findings from a dual (devops + security) review of the delta:

Direction note. The maintainer side is aligned with the module direction — PR #120 (a justification comment for the old command) will be closed as superseded by this PR, and the workspace conventions are being updated to match. No action needed from you on that.

TRACK: trailing-newline claim. The old code stored stdout ~ '\n' — openssl PEM output already ends with a newline, so the old value ended with two; the new file lookup returns one. The consumer (provider secret ca.crt) tolerates either, impact nil — but the body should say "normalized to a single trailing newline", not "preserved".

TRACK: bare item loop. The conventions require loop_control.loop_var on all loops — rename the cleanup loop var (e.g. _mtv_provider_tmp_cert_path).

TRACK: changelog fragment missing (required per CONTRIBUTING).

NOTE: per-run change reporting — since always: deletes dest_path each run, the conversion now reports changed on every run (the idempotency framing is per-task only). Nothing keys on the result — cosmetic.

NOTE: unused register — register: _mtv_provider_vmca_pem is no longer read; consider dropping it.

Security: clean — no secrets in the delta; the cleanup is a net improvement. Two pre-existing items for separate awareness: fixed /tmp paths (hardening option: mktemp-style names) and validate_certs: false on the VMCA fetch (the trust anchor is MITM-substitutable) — both pre-date this PR.

[sabre1041]

Thanks for the clean, well-scoped PR — CI green, and the temp-file cleanup is a nice addition. Findings from a dual (devops + security) review of the delta:

Direction note. The maintainer side is aligned with the module direction — PR #120 (a justification comment for the old command) will be closed as superseded by this PR, and the workspace conventions are being updated to match. No action needed from you on that.

TRACK: trailing-newline claim. The old code stored stdout ~ '\n' — openssl PEM output already ends with a newline, so the old value ended with two; the new file lookup returns one. The consumer (provider secret ca.crt) tolerates either, impact nil — but the body should say "normalized to a single trailing newline", not "preserved".

TRACK: bare item loop. The conventions require loop_control.loop_var on all loops — rename the cleanup loop var (e.g. _mtv_provider_tmp_cert_path).

TRACK: changelog fragment missing (required per CONTRIBUTING).

NOTE: per-run change reporting — since always: deletes dest_path each run, the conversion now reports changed on every run (the idempotency framing is per-task only). Nothing keys on the result — cosmetic.

NOTE: unused register — register: _mtv_provider_vmca_pem is no longer read; consider dropping it.

Security: clean — no secrets in the delta; the cleanup is a net improvement. Two pre-existing items for separate awareness: fixed /tmp paths (hardening option: mktemp-style names) and validate_certs: false on the VMCA fetch (the trust anchor is MITM-substitutable) — both pre-date this PR.

Addressed most of your concerns

[vvaldez]

Re-reviewed the squashed head (6aec099):

• Bare item loop renamed via loop_control.loop_var (conventions compliant)
• Changelog fragment added; changelog-lint green
• Unused register dropped

Live test against pool1 passed end to end: the new community.crypto.x509_certificate_convert path ran green and the provider CR reached ConnectionTestSucceeded, Validated, InventoryCreated. The trailing-newline wording in the body has nil impact (the consumer tolerates either); fine to leave as-is.

Approving.

[sabre1041]

Thanks @vvaldez for approving. @tech2734 do you want to take a look and either one of you guys merge?