SUSE contributions and future steps

[ggbecker]

@brett060102 @guangyee

Hello, I would like to start a discussion topic to talk about contributions that recently happened to sle12 product.

First of all, the contributions were great. They are really valuable for the project and I'm sure it will benefit the community a lot. New content is always appreciated. So far, most of the content is enabled only to Suse12 but they will eventually be enabled for other products as well.

But there were some obstacles that made the pull request review more difficult. For example: lack of tests, pull request granularity, the duplicity of changes among pull requests, etc and I would like to continue discussing how we can improve in the future to avoid them so new contributions can go through more easily.

I'll try to summarize the problems and their solutions or at least what the project's community sees as best practice:

1. Pull requests didn't have any tests.
   • For content related tests, we use the SSG Test Suite. It can run scans, remediations, and everything we need to tests OVAL checks and remediations. You can use virtual machines and containers as backends. These tests can help us preventing regressions and can also be used to check the correctness of checks and remediations.
2. Bulk pull requests
   • It's harder to review pull requests that contain many things at once. So more granular pull requests are expected in this case. Even better if they can come at a slow pace, so we have time to react properly. If that's not possible, then we can try to think of a better solution for both sides.
3. Duplicated changes among different pull requests
   • Few pull requests had duplicated changes across different pull requests. This pretty much aligns with the problem above. I guess it was because you had to manually craft the pull requests out of one single commit.
4. Content in some cases was not generic enough
   • This happens because the content was targetting mainly DISA STIG so many of the rules were aligned exclusively with STIG by having hardcoded coded variables, etc. The problems were fixed when requested, but I think it's a good opportunity to better understand the project principles and align better in the future. The main idea here is to create customizable content by using XCCDF variables, so other products/profiles can make use of this shared content. I think you guys already know this, but it's good to reinforce.
5. (update) Force push
   • Although it's tempting to use force push all the time, it can make the reviewer's life much more difficult when he's trying to identify what has changed after he requested changes to the pull request. So in this case, after the initial pull request is created, every new change requested should be included in a separate commit.

If you have any questions or doubts, please do not hesitate and ask them. I'll be more than happy to help.

[brett060102]

@ggbecker I have added @abergmann who is also part of our team so that we are on the same page.

Thank you very much for this and for your patience and support on getting these in. We had a steep learning curve on this. I am sorry about the duplicates in the last set of reviews. Our local development branch was about 2 months a head of what we had managed to get pushed upstream and we wanted to get upstream fully in sync with what we had locally and what we had locally in sync with upstream master. The initial try was #6629 which was really huge and represented about 30 internal PRs. I could not break it up on a change by change basis because underlying code had changed as we tracked changes in upstream. So, while not ideal, the method I used to break up 6229 was to quash merge the changes in it into another branch and then restore files that I did not want to include in the PR. Which meant storing rough 75% of the files changed by the squash. The process presented a huge opportunity for error and is not something I want to or plan to do again and is really just the result of much we needed to get done in a short period of time.

The process for SL15, will be much more orderly:

1: Final SLE-12 cleanup #6665 Thank you for that review.

2: Initial SL15 support structure, #6666

3: PR to add SLE-15 into existing rules that reference SLE-12, but no new content, just adding SLE-15 STIGS, CCE ID's, updating prodtype, and in few places updating checks like {{% if product == "sle12" %}} to {% if product in ['sle12', 'sle15'] %}}.
The PR will change about 190 files. But that seems in line with what was done for RHEL-8.
There are a few places where we currently have sle12.yml and sl12.xml files where I needed to create sle15 versions that are just a copy of the sle12 contents. Is there a better way to do that? Create a SUSE specific rule that would get pulled on for both SLE12 and SLE15? The only other use of OS specific files I have seen are for JRE, so not a lot of prior art here. Another option is to merge that into the shared. files. Which I would be OK with, but would like to cover as separate effort from enabling existing SLE-12 rules for SLE-15.

4: SLE-15 specific content will be done as separate PR's. So normally one rule to a PR. The only exception would be if a STIG spawned multiple rules, which I hope won't be the case.

I still need to digest everything in the testing document, but we will provide tests for all new SLE-15 content and once the SLE-15 is complete go back and add tests in for the new content we created for SLE-12.

I will stop the forced pushes. We do that internally just to keep the review history manageable and make make sure that the SHA1 for changed was complete and not missing some change that was done later.

Again, thank you for all of your time and guidance on this.

[ggbecker]

Thank you very much for the answer.

3: PR to add SLE-15 into existing rules that reference SLE-12, but no new content, just adding SLE-15 STIGS, CCE ID's, updating prodtype, and in few places updating checks like {{% if product == "sle12" %}} to {% if product in ['sle12', 'sle15'] %}}.
The PR will change about 190 files. But that seems in line with what was done for RHEL-8.
There are a few places where we currently have sle12.yml and sl12.xml files where I needed to create sle15 versions that are just a copy of the sle12 contents. Is there a better way to do that? Create a SUSE specific rule that would get pulled on for both SLE12 and SLE15? The only other use of OS specific files I have seen are for JRE, so not a lot of prior art here. Another option is to merge that into the shared. files. Which I would be OK with, but would like to cover as separate effort from enabling existing SLE-12 rules for SLE-15.

Unfortunately, there is no easy way how to achieve this multiple OS specific check at this moment. You can create a new file which would be a duplicate (not ideal) or embed the content into the shared file but use Jinja macros to keep the code separate. Ideally the best would be to merge everything together and have the jinja macros only when the content actually differs.

[guangyee]

@ggbecker Thanks for information! We really appreciate the guidance. Regarding SSG Test Suite, will we be required to expose a SUSE-based VM or container upstream? If so, how does upstream handle product licensing and activation? So far we have been running our tests locally using a SLES12 vagrant box. And license activation is part of provisioning, with an internal developer license code which unfortunately cannot be shared outside of the organization.

While reading through the documentation, I came across this disclaimer.

"SSG Test Suite currently does not provide automated provisioning of VMs or containers."

Does this mean running the tests will be on-demand by the platform-specific developers? And license activation will be done out-of-band? If so, how does upstream verify the test code? Or is that process also expected to be out-of-band as well?

[brett060102]

@guangyee @ggbecker Thank you both. I can now build the overlays for both sle-12 and sle-15 on my system with python 3.8.5 using:
sampsone@bigubuntu:~/github/STIGS/merge/content$ PYTHONPATH=$(./.pyenv.sh) ./utils/create-stig-overlay.py --disa-xccdf=/tmp/U_SLES_15_STIG_V1R1_Manual-xccdf.xml --ssg-xccdf=build/ssg-sle15-xccdf.xml -o sle15/overlays/stig_overlay.xml

Generated the new STIG overlay file: sle15/overlays/stig_overlay.xml
sampsone@bigubuntu:~/github/STIGS/merge/content$ PYTHONPATH=$(./.pyenv.sh) ./utils/create-stig-overlay.py --disa-xccdf=/tmp/U_SLES_12_STIG_V2R1_Manual-xccdf.xml --ssg-xccdf=build/ssg-sle12-xccdf.xml -o sle12/overlays/stig_overlay.xml

Generated the new STIG overlay file: sle12/overlays/stig_overlay.xml

Did need to change PYTHONPATH=./.pyenv.sh to PYTHONPATH=$(./.pyenv.sh)

From what was in https://complianceascode.readthedocs.io/en/latest/manual/developer/04_updating_reference_and_overlay.html?highlight=create-stig-overlay#stig-overlay-content
Thank you both.

[ggbecker]

@brett060102 Please take a look at: #6666 (comment)

I took the liberty and pushed some commits to the pull request. They add the initial stig_overlay.xml files and fix an issue in SLE15.

That's great you were able to make it work.

[brett060102]

@ggbecker thank you for all your work on this. I have https://github.com/SUSE/ComplianceAsCodeContent/pull/251 up which is a bulk enable of existing SLE-12 rules to tag them as SLE-15. No "new" content was created. Please tell me if we are on the right track PR wise.

[matejak]

@guangyee wrote:

The decision to host a VPS node is beyond my pay grade at this point. But I'll convey the sentiment higher up.

To put this in a context, our expenses for a VPS is about 150 € / year (including tax), so please do mention this higher up as well.

Nice! Is the recommendation algorithm in the same repo?

Not yet, but you can check it out at https://github.com/mildas/content-test-filtering

What was the reason to standardized on using Bash instead of Ansible for the test setup? Look forward to reading the blog.

That's an interesting question - when I think of it, I would even say that Ansible could do the same job with objectively simpler setup code.
Regarding the answer, it's probably an MVP that was good enough, so it stayed that way. The test env setup is handled in one method, so expanding possible formats is certainly doable.
The blog post is proposed as a PR ComplianceAsCode/ComplianceAsCode.github.io#19 so you can check it out and provide any feedback that comes to your mind.

[ggbecker]

@guangyee @brett060102 @abergmann

FYI, the blogpost about testing that @matejak has mentioned is now live and you can read it here: https://complianceascode.github.io/template/2021/03/25/tests_howto.html