-
Notifications
You must be signed in to change notification settings - Fork 684
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
Introduced macros for working with XCCDF values into the wide content #6048
Conversation
Skipping CI for Draft Pull Request. |
a927ce7
to
dd3b43a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
dd3b43a
to
0321ef9
Compare
0321ef9
to
bd9a288
Compare
/retest |
bd9a288
to
6460008
Compare
/retest |
@@ -46,6 +46,6 @@ ocil: |- | |||
configured for all users on the system: | |||
<pre># grep "maxlogins" /etc/security/limits.conf</pre> | |||
You should receive output similar to the following: | |||
<pre>*\t\thard\tmaxlogins\t<sub idref="var_accounts_max_concurrent_login_sessions" /></pre> | |||
<pre>*\t\thard\tmaxlogins\tsub_var_value("var_accounts_max_concurrent_login_sessions")</pre> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great catch!
The macro hides the actual implementation of the substitution, it "just works", and it opens ways how to support variables even outside of the SCAP content, where there is no scanner to do the acutal substitution. Renamed the macro to xccdf_value, kept the old one for backward compatibility.
The former populate ... mechanism is not Bash, it is a special trick perforemd by our build system. This trick is confusing, its support in the build system is implemented as a complex code, and it doesnt support multiple values per remediation intuitively. This makes the build system involvement explicit, and it opens possibilities to perform implementation changes without breaking backward compatibility.
The former - (xccdf-var ...) mechanism is not Ansible, and jinja is well-established in our project as an interface between user input and final content.
6460008
to
359c54f
Compare
Changes identified: Recommended tests to execute: |
/test e2e-aws-rhcos4-e8 |
Thank you, this is a big improvement. |
Those macros always were there, and this PR makes sure that they are used, so they make the content less cryptic, and they navigate content authors that create new content based on the existing one.
xccdf_value
macro: This macro hides the actual implementation of the substitution, it "just works", and it opens ways how to support variables even outside of the SCAP content, where there is no scanner to do the acutal substitution.populate ...
mechanism is not Bash, it is a special trick perforemd by our build system.This trick is confusing, its support in the build system is implemented as a complex code, and it doesnt support multiple values per remediation intuitively.
This makes the build system involvement explicit, and it opens possibilities to perform implementation changes without breaking backward compatibility.
- (xccdf-var ...)
mechanism is not Ansible, and jinja is well-established in our project as an interface between user input and final content.There is no OVAL macro for
external_variable
. On one hand, it's quite a lot of typing that could be performed automatically, but at the same time, unlike the previous cases, that construct is a legitimate way of using external variables, so I have decided to keep it that way.