-
Notifications
You must be signed in to change notification settings - Fork 696
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
Add al2023 product #12006
Add al2023 product #12006
Conversation
Hi @hipponix. Thanks for your PR. I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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-sigs/prow repository. |
@marcusburghardt should you need more work here from my end or even split it into multiple PRs to ease you review process, just let me know. |
Hi @hipponix , first thanks for the contribution. |
🤖 A k8s content image for this PR is available at: Click here to see how to deploy itIf you alread have Compliance Operator deployed: Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and: |
no intent to put some pressure here, thanks @marcusburghardt ! I noticed there are quite a few PRs to be processed so you maintainers might be very busy. |
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.
Thanks for the PR.
Please take a look at my comments. They boil down the following:
- Ensure fully qualified collection name (FCQN) for Ansible
- If you don't want rules in a profile backed by a control file, it might be bested to move them to the
related_rules
section on each control.
linux_os/guide/system/software/updating/ensure_amazon_gpgkey_installed/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/software/updating/ensure_amazon_gpgkey_installed/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/software/updating/ensure_amazon_gpgkey_installed/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/software/updating/ensure_amazon_gpgkey_installed/ansible/shared.yml
Outdated
Show resolved
Hide resolved
linux_os/guide/system/software/updating/ensure_amazon_gpgkey_installed/ansible/shared.yml
Outdated
Show resolved
Hide resolved
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 will need to rebase your branch as it has a bunch of commits from master on it.
@hipponix do not merge master into your feature branch, that creates unnecessary commits, please rebase your branch instead |
4d98497
to
49176c4
Compare
/packit build |
@hipponix a quick rebase on master to resolve the conflicts in |
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.
No extra comments, apart from already suggested changes it LGTM
49176c4
to
498ef66
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.
a few minor things to fix
build_product
Outdated
@@ -378,6 +378,7 @@ all_cmake_products=( | |||
MACOS1015 | |||
OPENEMBEDDED | |||
OPENEULER2203 | |||
AL2023 |
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.
indentation issue and also not alphabetically ordered.
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.
Done - Thanks!
CMakeLists.txt
Outdated
@@ -466,6 +468,9 @@ endif() | |||
if(SSG_PRODUCT_UOS20) | |||
add_subdirectory("products/uos20" "uos20") | |||
endif() | |||
if(SSG_PRODUCT_AL2023) |
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.
needs to be sorted alphabetically
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.
Done - Thanks!
.github/workflows/gate_fedora.yml
Outdated
ocp4 | ||
ocp4 \ | ||
uos20 \ | ||
al2023 |
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.
needs to be sorted alphabetically and is including twice the uos20
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.
Done - Thanks!
.github/workflows/gate.yaml
Outdated
@@ -170,7 +170,8 @@ jobs: | |||
rhel9 \ | |||
rhel10 \ | |||
uos20 \ | |||
ocp4 | |||
ocp4 \ |
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.
needs to be sorted alphabetically
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.
Done - Thanks!
…nstalled/ansible/shared.yml thanks Co-authored-by: Matthew Burket <[email protected]>
…nstalled/ansible/shared.yml thanks Co-authored-by: Matthew Burket <[email protected]>
…nstalled/ansible/shared.yml thanks Co-authored-by: Matthew Burket <[email protected]>
…nstalled/ansible/shared.yml thanks Co-authored-by: Matthew Burket <[email protected]>
…nstalled/ansible/shared.yml thanks Co-authored-by: Matthew Burket <[email protected]>
…inting to aws servers
…nf by pointing to aws servers" This reverts commit e745325.
… file - Add build/.gitkeep back - Remove AL2023 from building derivates
8e44b5c
to
1324b12
Compare
Code Climate has analyzed commit 1324b12 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 59.4% (0.0% change). View more on Code Climate. |
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.
Thanks for the hard work.
I am waving the following tests:
- Automatus Ubuntu 22.04 - not related to this PR
- Compare DS / Generate Diff - new product, nothing to compare
- Automatus CS8 - expected, not related to this PR
- Automatus CS9 - rule for amazon linux not found, expected
- Automatus Fedora - rule for amazon linux not found, expected
- Automatus SLE15 - rule for amazon linux not found, expected
- Build, Test on Fedora Rawhide - expected
@dodys You still have request for changes out. Everything look good to you? |
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.
Description:
Rationale:
Review Hints:
./build_product -j 8 al2023