Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Clean up the "policy" field from ApparmorProfile CRD #2590

Merged
merged 12 commits into from
Dec 3, 2024
Merged

Clean up the "policy" field from ApparmorProfile CRD #2590

merged 12 commits into from
Dec 3, 2024

Conversation

ccojocar
Copy link
Contributor

@ccojocar ccojocar commented Dec 2, 2024 •
edited
Loading

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This pull request is a follow-up on #2388 and removes complete the policy field from ApparmorProfile CRD. This field is not required anymore since all the content is generated from the abstract.

Additional work was done to fix the apparmor e2e tests, and to cleanup the code, manifests and documentation.

This work is a prerequisite for #2559.

Which issue(s) this PR fixes:

follow-up on #2388

Does this PR have test?

Yes

Special notes for your reviewer:

Does this PR introduce a user-facing change?

API BREAKING CHANGES: policy field removed from ApparmorProfile CRD, use instead the abstract field which automatically generates the policy before installation.

cc @mhils

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 2, 2024
@ccojocar ccojocar requested review from saschagrunert and removed request for jhrozek and Vincent056 December 2, 2024 16:17
@ccojocar ccojocar changed the title Clean up the "policy" filed from ApparmorProfile CRD Clean up the "policy" field from ApparmorProfile CRD Dec 2, 2024
@ccojocar
Copy link
Contributor Author

ccojocar commented Dec 2, 2024

@saschagrunert I see also this issue locally, but the VM seem to work fine when I remove it completely:

    default: Loaded image: localhost/security-profiles-operator:latest
    default: ++ CNI_CONFIG=/etc/cni/net.d/10-crio-bridge.conflist
    default: ++ jq 'del(.plugins[0].ipam.routes[1], .plugins[0].ipam.ranges[1])' /etc/cni/net.d/10-crio-bridge.conflist
    default: jq: error: Could not open file /etc/cni/net.d/10-crio-bridge.conflist: No such file or directory

Should we remove it from all VMs definitions?

@saschagrunert
Copy link
Member

saschagrunert commented Dec 3, 2024 •
edited
Loading

@ccojocar I have a fix in mind, one second: #2594

@ccojocar
Copy link
Contributor Author

ccojocar commented Dec 3, 2024

I have a fix in mind, one second: #2594

@saschagrunert perfect, thanks a lot!

@saschagrunert
Copy link
Member

Please rebase to fix the CI

@ccojocar ccojocar force-pushed the apparmor_remove_policy branch 2 times, most recently from b7ea78b to b6f573d Compare December 3, 2024 10:45
ccojocar added 12 commits December 3, 2024 12:53
@ccojocar
Remove the Policy filed from Apparmor profile CRD
04d00fc 
Change-Id: I84637660b9034c37c78bcc4ec85a0e310cc7fa34
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Remove Policy support
878f96a 
Change-Id: I8aa8352cfdd78461fdf6f2dddd5564c9f2f3a694
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Update apparmorprofile CRD manifest
ec93d68 
Change-Id: I8505ae47e3be59961d1dc58e65391c048092f4b5
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Update apparmorprofile crd everywhere
a972d50 
Change-Id: Ibfe6ae4086a5c166694e3e6a2f2440cc0fa74572
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Remove unused unit test
5152360 
Change-Id: I2e1eafe7fb9f51f7be072c6bb504f2296289ab1a
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Update the apparmor profiles from examples to use the abstract
943b0fd 
Change-Id: I186ed93f5e4c0750577e92677bb4cee033f931cd
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Adapt and fix the apparmor e2e tests
c168953 
Change-Id: I855475b222fa8095a4ef52e02b9b784fb13dac8c
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Update the operator bundle
4908f57 
Change-Id: Ib2abcc5ba0fc35e25f7990acb7ea93b97ce544a4
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Update the apparmorprofile in apparmor tc base case
7e6a11f 
Change-Id: Id4d26a501d9bed996e5f59a5c7f6e5082008bce9
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Update the apparmor deny all profile
cbfbf69 
Change-Id: I262284bdecfbce1a2f1f23e95bb57d4d676c978b
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Update the bundle manifests
a4a5d27 
Change-Id: I419c8a172dfc41574c90a36e7346409c0e664936
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar
Remove the apparmor e2e test from test suite since is covered by hack…
88a40ea 
…/ci/e2e-apparmor test

Change-Id: I6e0df97860c205b97cd67a1545237e895c523b04
Signed-off-by: Cosmin Cojocar <[email protected]>
@ccojocar ccojocar force-pushed the apparmor_remove_policy branch from ce4f555 to 88a40ea Compare December 3, 2024 12:53
@ccojocar
Copy link
Contributor Author

ccojocar commented Dec 3, 2024

@saschagrunert All tests passed, please could you have a look? Thanks

saschagrunert
saschagrunert approved these changes Dec 3, 2024
View reviewed changes
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ccojocar, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [ccojocar,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit ce7b9ac into main Dec 3, 2024
28 checks passed
@k8s-ci-robot k8s-ci-robot deleted the apparmor_remove_policy branch December 3, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saschagrunert saschagrunert saschagrunert approved these changes

Assignees

@saschagrunert saschagrunert

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ccojocar @saschagrunert @k8s-ci-robot