Add warning about dependent rule to dconf rules. #3755
Conversation
|
Seems reasonable workaround for now. Where can we follow the work to create the dconf DB OVAL probe? |
|
The only comment I have is to please, please do not use jinja templates to encapsulate yaml keys like it is done here... it not only makes it extremely difficult to merge this content using tools that don't use jinja including future GUI work, but it also means that there is rework to fix this later. |
matejak
force-pushed
the
dconf_text_all
branch
from
215126f
to
4af891d
Compare
|
@redhatrises Thank you for your feedback, I have changed the implementation, so it now substitutes only the text of the warning. |
shared/macros.jinja
Outdated
| {{% macro body_of_warning_about_dependent_rule(rule_id, why) -%}} | ||
| When selecting this rule in a profile, | ||
| {{%- if why %}} | ||
| make sure that rule with ID {{{ weblink(link="#xccdf_org.ssgproject.content_rule_" + rule_id, text=rule_id) }}} is selected as well: {{{ why }}} |
There was a problem hiding this comment.
@matejak does {{{ weblink(link="#xccdf_org.ssgproject.content_rule_" + rule_id, text=rule_id) }}} work for both Datastream and XCCDF?
There was a problem hiding this comment.
I suppose so, what would be the catch? It works for datastream-powered report for sure, and it is present as a HTML a tag in the xccdf XML as well.
There was a problem hiding this comment.
I think the catch is that in a pure XCCDF Benchmark, the rules don't have the prefix xccdf_org.ssgproject.content_rule_, they are just rule_id.
There was a problem hiding this comment.
So what is better?
- Alink that does nothing, but that is correct in the recommended use case, or
- no link at all, only text short rule ID?
There was a problem hiding this comment.
I would probably say a short rule ID because you would have a bug in the XCCDF-only content if you use a link.
There was a problem hiding this comment.
Yeah, in the past we have gone with short rule ID, see #3113.
There was a problem hiding this comment.
OK then - fixed by rebase.
matejak
force-pushed
the
dconf_text_all
branch
from
4af891d
to
357bdb8
Compare
|
The inspection completed: No new issues |
|
LGTM. Thanks for your PR @matejak! |
At the moment, dconf rules are about text config files, while dconf uses binary database as the backend by default. Extending the OVAL of other rules with the backend check is not a good solution, as it complicates tests, remediations and debugging of what went wrong.
Therefore, we thought of a compromise - let's put a warning to the rule description, which this PR does.