-
Notifications
You must be signed in to change notification settings - Fork 743
OCP4 content cleanup #4970
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
OCP4 content cleanup #4970
Conversation
|
Unfortunately this still does not work for me. I pushed the content into this repo/branch: I used this pod to test: Then you can create the pod with: This would first copy the content from the init container and then the scanner container would just sleep. So you can rsh into it: And then you can just do whatever you like, inspect the content: Or run the scan: |
|
feel free to ping me if you need access to an openshift cluster. |
|
If there isn't any RPM database in the scanned container we will have to invent a different way of determining platform applicability, eg. based on reading /etc/os-release. |
|
@jhrozek on the node itself You could also look at |
|
Yeah, I think this is good, we're not trying to write content for RHCOS in any other context than OCP node (edited: earlier I said worker which is ambiguous), so at least for now we can assume that RHCOS == OCP. If the That said, we're likely need to go a step further (not needed /right now/, but still..) because AFAIK it's possible to use RHEL for the OCP worker nodes. So we'll also need to allow the OCP content to run on RHEL nodes. But, that's not needed for this first iteration, we should just keep that in mind to not shoot ourselves in the foot now. |
|
@jhrozek 👍. The fields should be stable enough. You could also look at |
| @@ -0,0 +1,17 @@ | |||
| documentation_complete: true | |||
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 profile should not be added at this point in time. While Fedramp is the starting target, there are other profiles that are named differently that are essentially the same profile.
|
The rpm database definitely exists on CoreOS and the |
| </ind:textfilecontent54_test> | ||
| <ind:textfilecontent54_object id="obj_rhel8_coreos" version="1"> | ||
| <ind:filepath>/etc/os-release</ind:filepath> | ||
| <ind:pattern operation="pattern match">^PRETTY_NAME="Red Hat Enterprise Linux CoreOS \d+\.(\d+)\.\d+\.\d+ \([\w\s]+\)"$</ind:pattern> |
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.
@ashcrow what do you think of this, to check if a node is RHCOS?
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.
NAME would be a simpler check if all that matters is if it's RHCOS or not ... but I think this would work. NOTE: I just eyeballed the regex 🙂
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.
I hope that it did not put too much strain to your eyes. I'm open to suggestions for improvements of the regex used in the check :)
|
So, first of all, the RPM-based check is no good at this moment because, while there is an RPM database in the CoreOS edition of RHEL (as a backward compatibility) it is at least located in a different place and current OVAL probe of the oscap scanner is unable to work with it. We will most likely be fixing the probe as there are a lot of RPM checks in the rules and they might be needed in the future. But, to continue with the OCP4 content evolution, I suggest to stick with the Next, the version. Original check, submited by @redhatrises, was checking for package version of |
|
On 11/7/19 9:22 AM, Evgeny Kolesnikov wrote:
Next, the version. Original check, submited by @redhatrises
<https://github.com/redhatrises>, was checking for package version of
|redhat-release-coreos| to be 8. But the CoreOS edition actually bears
two versions mixed together, the CoreOS itself and RHEL (e.g. 4.3 and
8.1) forming a very peculiar string |VERSION="42.80.20191022.0"|.
It sounds to me like|the 42 in||VERSION="42.80.20191022.0" |is
actually|the OCP version and 8.0 is the base RHEL version. My OCP 4.3
cluster has 43.81....
|
I used the |(\d)\d+| part out of it, in coherence with previous
RPM-based check, but I'm not sure if it's a good idea. This check is a
complimentary check to the base RHEL's, and maybe we'd like to check
here for CoreOS version instead.
I'm not sure how easy it is, but checking if the OCP version is 4[digit]
and the RHEL version is 8[digit] sounds the safest to me in the sense
that it checks both version substrings.
But maybe we're overdoing this and as was said already earlier in the
PR, just checking the RHCOS is OK. Let me test your latest PR and if it
works, then I guess it could be good enough.
|
Add initial fedramp profile for CoreOS; simplified OCP4 environment check. The "test_rhel8_coreos" check is for now based on /etc/os-release file contents.
|
Looks good to me, I agree with you that we need to check the version == 8.*, because the check is a part of a check that checks if the operating system is RHEL 8. I think a check that would check only CoreOS without version might match future versions that will not be based on RHEL8. |
|
I tested the latest version and can confirm that it works: |
| @@ -10,19 +10,6 @@ | |||
| </metadata> | |||
| <criteria> | |||
| <extend_definition comment="RHEL8 OS installed" definition_ref="installed_OS_is_rhel8" /> | |||
| <criterion comment="OpenShift Node is installed" test_ref="test_ocp4_node" /> | |||
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.
You still need to check if OCP4 is installed. Checking just the OS install is not enough.
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.
@redhatrises we don't support running RHCOS standalone. So OCP4 has to be there.
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.
@JAORMX agreed. That is my original statement in this part of the review.
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.
If you tell me how exactly this node can be detected, I'd happily add this check.
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.
I mean beside the obvious way of copying the checks from ocp3 and replacing the number, because RPM-based checks would not work.
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.
In fact while we are having this discussion, there is going to have to be a way to validate with a gpg check not only the software installed in CoreOS, but also a container that is installed on coreos.
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.
In fact while we are having this discussion, there is going to have to be a way to validate with a gpg check not only the software installed in CoreOS, but also a container that is installed on coreos.
Nothing above or beyond RHEL itself (same packages) for installed software, or OCP itself for containers.
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.
Still have to validate regardless of source against a key to meet compliance requirements.
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.
I say we merge this for now so we can start using this to build content already. And we start iterating on this as we go. I'm fine with merging this as it is.
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.
I understand where you're coming from and especially the points about RHEL nodes are valid. But we need to start working on the content and even just RHCOS-based is good enough.
On the RHCOS nodes: just knowing that "a node is RHCOS" is good enough at least for now because 1) RHCOS only ever exists as an OCP node 2) The content is supposed to be used from an operator anyway which can only run in OCP.
Again, I agree with your points. But if possible, I would prefer to merge the stub of the content ASAP and then iterate on it to provide a technically more sound solution.
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.
I'm fine with merging this. We can iterate on this later. Let's unblock folks so we can all continue going forward. Great work here!
|
The support for RHEL nodes can be implemented later in separate PR. This is good enough for this moment. |
Make OCP4 content usable