Skip to content

Fixes #38335 - Document Ansible Diff Mode template option #3758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bnerickson
Copy link
Contributor

@bnerickson bnerickson commented Mar 31, 2025

What changes are you introducing?

This PR adds a line of documentation explaining a new Enable Ansible Diff Mode option within Ansible Job Templates describing its basic functionality.

Why are you introducing these changes? (Explanation, links to references, issues, etc.)

Feature Request: https://projects.theforeman.org/issues/38335

This documentation explains the new functionality introduced in the following PRs:

Anything else to add? (Considerations, potential downsides, alternative solutions you have explored, etc.)

This PR is dependent on the following PRs being approved/merged first:

I added some security-conscious suggestions in the documentation to advise administrators when to disable diff mode. The maintainers may remove it if they feel it is unnecessary. I feel it is warranted for the following reason:

Administrators may deploy files with credentials using Ansible (whether or not this is advisable is not up for debate). If they enable diff mode for a job template (diff mode is disabled by default) and the module they used to deploy the credentials supports diff mode, then those deployed credentials may be displayed in the Foreman template_invocation window (and in proxy.log) in clear text. This is no different than an administrator manually setting diff: true on their Ansible playbook tasks, so I don't see it as an additional security risk but rather as something that administrators should be mindful of when using diff mode.

Checklists

  • I am okay with my commits getting squashed when you merge this PR.
  • I am familiar with the contributing guidelines.

Please cherry-pick my commits into:

  • Foreman 3.14/Katello 4.16
  • Foreman 3.13/Katello 4.15 (EL9 only)
  • Foreman 3.12/Katello 4.14 (Satellite 6.16)
  • Foreman 3.11/Katello 4.13 (orcharhino 6.11 on EL8 only; orcharhino 7.0 on EL8+EL9; orcharhino 7.1 with Leapp)
  • Foreman 3.10/Katello 4.12
  • Foreman 3.9/Katello 4.11 (Satellite 6.15; orcharhino 6.8/6.9/6.10)
  • Foreman 3.8/Katello 4.10
  • Foreman 3.7/Katello 4.9 (Satellite 6.14)
  • We do not accept PRs for Foreman older than 3.7.

@github-actions github-actions bot added Needs tech review Requires a review from the technical perspective Needs style review Requires a review from docs style/grammar perspective Needs testing Requires functional testing labels Mar 31, 2025
@bnerickson bnerickson force-pushed the ansible-diff-mode-doc branch from a6abd6c to 99ec6d6 Compare March 31, 2025 21:54
@bnerickson bnerickson marked this pull request as ready for review March 31, 2025 21:59
@bnerickson bnerickson marked this pull request as draft March 31, 2025 22:00
@bnerickson bnerickson marked this pull request as ready for review March 31, 2025 22:01
Copy link
Contributor

@maximiliankolb maximiliankolb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handing over to @aneta-petrova for additional style review regarding the emphasis.

@@ -32,6 +32,13 @@ endif::[]
ifdef::satellite[]
For more information on Ansible check mode, see {RHDocsBaseURL}red_hat_ansible_automation_platform/latest/html-single/using_automation_execution/index[Using automation execution] in _Red{nbsp}Hat Ansible Automation Platform documentation_.
endif::[]
.. Select *Enable Ansible Diff Mode* to run jobs based on the template in Ansible diff mode, which reports detailed changes Ansible tasks make _if_ the underlying Ansible modules supports Ansible diff mode. If a module does not support Ansible diff mode, then the task will only report that a change occurred and nothing else. When used in conjunction with Ansible check mode, the Ansible playbook tasks report changes they _would_ have made to the host, and no changes are made to the host. Please note that if a task is used to deploy credentials and the task supports Ansible diff mode, then changes to the credentials _may_ be reported in clear text. This can be mitigated by adding `diff: false` or `no_log: true` to the task in question which will prevent change reports for that task.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. Select *Enable Ansible Diff Mode* to run jobs based on the template in Ansible diff mode, which reports detailed changes Ansible tasks make _if_ the underlying Ansible modules supports Ansible diff mode. If a module does not support Ansible diff mode, then the task will only report that a change occurred and nothing else. When used in conjunction with Ansible check mode, the Ansible playbook tasks report changes they _would_ have made to the host, and no changes are made to the host. Please note that if a task is used to deploy credentials and the task supports Ansible diff mode, then changes to the credentials _may_ be reported in clear text. This can be mitigated by adding `diff: false` or `no_log: true` to the task in question which will prevent change reports for that task.
.. Select *Enable Ansible Diff Mode* to run jobs based on the template in Ansible diff mode, which reports detailed changes Ansible tasks make _if_ the underlying Ansible modules supports Ansible diff mode.
If a module does not support Ansible diff mode, then the task will only report that a change occurred and nothing else.
When used in conjunction with Ansible check mode, the Ansible playbook tasks report changes they _would_ have made to the host, and no changes are made to the host.
Note that if a task is used to deploy credentials and the task supports Ansible diff mode, then changes to the credentials _may_ be reported in plain text.
This can be mitigated by adding `diff: false` or `no_log: true` to the task in question which will prevent change reports for that task.

Please conform to one sentence per line. I have also applied "plain" in favor of "clear" and dropped "please". I am currently unsure about the italic emphasis on certain words. To me, it strikes odd compared to most other modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maximiliankolb I noticed the "plain" vs "clear" text lint error, but "clear-text" is used more commonly when describing the transmission of unencrypted credentials which is what the line in the document is referring to. Isn't "plain-text" more of a way of describing un-formatted text and wouldn't apply here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. Select *Enable Ansible Diff Mode* to run jobs based on the template in Ansible diff mode, which reports detailed changes Ansible tasks make _if_ the underlying Ansible modules supports Ansible diff mode. If a module does not support Ansible diff mode, then the task will only report that a change occurred and nothing else. When used in conjunction with Ansible check mode, the Ansible playbook tasks report changes they _would_ have made to the host, and no changes are made to the host. Please note that if a task is used to deploy credentials and the task supports Ansible diff mode, then changes to the credentials _may_ be reported in clear text. This can be mitigated by adding `diff: false` or `no_log: true` to the task in question which will prevent change reports for that task.
.. Select *Enable Ansible Diff Mode* to run jobs based on the template in Ansible diff mode.
This mode reports detailed changes that Ansible tasks make if the underlying Ansible module supports Ansible diff mode.
If a module does not support Ansible diff mode, the task only reports that a change occurred without providing details.
When used with Ansible check mode, Ansible playbook tasks report the changes they would make to the host, but no actual changes occur.
If a task deploys credentials and supports Ansible diff mode, the changes to the credentials may be reported in plain text.
To prevent change reports for a task, add `diff: false` or `no_log: true` to the task.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"clear-text" is used more commonly when describing the transmission of unencrypted credentials

You make a good point! I am OK either way. Your call unless other maintainers voice a strong preference.

@@ -32,6 +32,13 @@ endif::[]
ifdef::satellite[]
For more information on Ansible check mode, see {RHDocsBaseURL}red_hat_ansible_automation_platform/latest/html-single/using_automation_execution/index[Using automation execution] in _Red{nbsp}Hat Ansible Automation Platform documentation_.
endif::[]
.. Select *Enable Ansible Diff Mode* to run jobs based on the template in Ansible diff mode, which reports detailed changes Ansible tasks make _if_ the underlying Ansible modules supports Ansible diff mode. If a module does not support Ansible diff mode, then the task will only report that a change occurred and nothing else. When used in conjunction with Ansible check mode, the Ansible playbook tasks report changes they _would_ have made to the host, and no changes are made to the host. Please note that if a task is used to deploy credentials and the task supports Ansible diff mode, then changes to the credentials _may_ be reported in clear text. This can be mitigated by adding `diff: false` or `no_log: true` to the task in question which will prevent change reports for that task.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. Select *Enable Ansible Diff Mode* to run jobs based on the template in Ansible diff mode, which reports detailed changes Ansible tasks make _if_ the underlying Ansible modules supports Ansible diff mode. If a module does not support Ansible diff mode, then the task will only report that a change occurred and nothing else. When used in conjunction with Ansible check mode, the Ansible playbook tasks report changes they _would_ have made to the host, and no changes are made to the host. Please note that if a task is used to deploy credentials and the task supports Ansible diff mode, then changes to the credentials _may_ be reported in clear text. This can be mitigated by adding `diff: false` or `no_log: true` to the task in question which will prevent change reports for that task.
.. Select *Enable Ansible Diff Mode* to run jobs based on the template in Ansible diff mode.
This mode reports detailed changes that Ansible tasks make if the underlying Ansible module supports Ansible diff mode.
If a module does not support Ansible diff mode, the task only reports that a change occurred without providing details.
When used with Ansible check mode, Ansible playbook tasks report the changes they would make to the host, but no actual changes occur.
If a task deploys credentials and supports Ansible diff mode, the changes to the credentials may be reported in plain text.
To prevent change reports for a task, add `diff: false` or `no_log: true` to the task.

@pr-processor pr-processor bot added the Waiting on contributor Requires an action from the author label Apr 2, 2025
@bangelic
Copy link
Contributor

bangelic commented Apr 2, 2025

Took Maximilian's suggestion and eliminated the italics and passive voice where applicable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs style review Requires a review from docs style/grammar perspective Needs tech review Requires a review from the technical perspective Needs testing Requires functional testing Waiting on contributor Requires an action from the author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants