Abort the build if an OVAL is not included due to extend_definition #6402
Conversation
|
Changes identified: Show detailsOthers: Recommended tests to execute: |
|
test this please |
|
your pull requested has detected that some it should be: |
|
It was not so simple with OSP, as some OVALs are generated by templates, but it is resolved now. |
Great. But now there is some issue with OCP4 as it only build on fedora and when using certain version of openscap |
matejak
force-pushed
the
abort_missing_extending_defs
branch
2 times, most recently
from
38f9b96
to
e19f96d
Compare
ocp4/product.yml
Outdated
| additional_content_directories: | ||
| - "../linux_os/guide/system/software/integrity/endpoint_security_software/mcafee_security_software" | ||
| - "../linux_os/guide/system/network/network-ipv6/disabling_ipv6" | ||
| - "../linux_os/guide/system/selinux" |
There was a problem hiding this comment.
This doesn't sound right... why are these being included?
There was a problem hiding this comment.
That's a good catch. So we have some "shared" OVALs that don't belong to any rules. One would expect them to be included in the benchmark if they are needed by rule checks, but that's not the case - they are included unconditionally. Those incriminated definitions are the and ones.
The quick fix of these cases would be to set "affected platform" of those snippets to Linux OSs only, as they don't make sense for OCP, OSP and other application-level products.
In the longer run, we should do something about our messed-up system of platforms vs prodtypes vs products.
Finally, those shared checks should be included only on-demand, and there should be a distinction between an applicability of the check and whether the check is actually needed by some other check or by an existing rule.
Right, ocp4 content needs openscap 1.3.4 |
matejak
force-pushed
the
abort_missing_extending_defs
branch
2 times, most recently
from
fa1c104
to
93bd129
Compare
|
RHV has been affected as well - it is just a flavor of RHEL after all. |
|
/retest |
ssg/build_renumber.py
Outdated
| self.tree, self.oval_groups, indexed_oval_defs) | ||
| defs_miss = get_oval_checks_extending_non_existing_checks(self.tree, indexed_oval_defs) | ||
| if defs_miss: | ||
| if self.tolerate_missing_extending_defs: |
There was a problem hiding this comment.
This is never set which means this block is never executed. What are the plans?
There was a problem hiding this comment.
There is no plan - the code is just prepared for the possibility that missing extending definitions become tolerable.
There was a problem hiding this comment.
Aha, and can it ever happen that it will be tolerable? What is the scenario in which it would be used?
There was a problem hiding this comment.
Hard to say - you know how it is with assumptions. There must have been some reason why it is tolerated since it was introduced.
There was a problem hiding this comment.
But it's you who introduces it so you should be able to explain why you introduce it.
There was a problem hiding this comment.
I have removed the conserved code in a commit that can be reverted in case that there is an interest to build content that has "definition holes".
|
@matejak please refresh my mind about why this PR is hanging |
|
This PR can prevent products with inconsistent OVAL content from building :-( Perhaps we could start introducing it after the release. |
matejak
force-pushed
the
abort_missing_extending_defs
branch
from
4b38be8
to
62b6815
Compare
matejak
force-pushed
the
abort_missing_extending_defs
branch
from
be06a88
to
483d05b
Compare
|
The PR doesn't break any builds now, so please @jan-cerny give it a go. |
|
/retest |
If an extending definition is missing, then the OVAL won't be there as well. This indicates an incorrect setup of applicability - either extending definitions are not applicable as they should, or the compound definition should not be applicable.
Definitions used by other definitions need to have at least the same applicability as definitions that require them.
- The product name is specified more than once in the project, and all occurences have to be consistent.
matejak
force-pushed
the
abort_missing_extending_defs
branch
from
483d05b
to
c7b6094
Compare
|
/retest |
|
@matejak: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/retest CI failed gathering logs; it's unrelated to the CaC tests. |
If an extending definition is missing, then the OVAL won't be there as well.
This indicates an incorrect setup of applicability - either extending definitions are not applicable as they should, or the compound definition should not be applicable.
Fixes: #6185