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.

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

mutation webhook #655

Merged
merged 1 commit into from
May 4, 2022
Merged

Conversation

achrefbensaad
Copy link
Member

Work related to this issue

@achrefbensaad
Copy link
Member Author

// TODO: reset all versions to v1 before releasing.

@codecov-commenter
Copy link

codecov-commenter commented Apr 4, 2022

Codecov Report

Merging #655 (ef9a5bf) into main (93794fe) will decrease coverage by 0.91%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #655      +/-   ##
==========================================
- Coverage   35.93%   35.02%   -0.92%     
==========================================
  Files          25       25              
  Lines        9018     8888     -130     
==========================================
- Hits         3241     3113     -128     
- Misses       5327     5331       +4     
+ Partials      450      444       -6     
Impacted Files Coverage Δ
KubeArmor/core/kubeUpdate.go 43.07% <100.00%> (-0.62%) ⬇️
KubeArmor/feeder/feeder.go 46.19% <0.00%> (-9.05%) ⬇️
KubeArmor/core/k8sHandler.go 31.69% <0.00%> (-3.93%) ⬇️
KubeArmor/feeder/policyMatcher.go 36.33% <0.00%> (-1.03%) ⬇️
KubeArmor/enforcer/appArmorEnforcer.go 47.56% <0.00%> (-0.74%) ⬇️
KubeArmor/monitor/systemMonitor.go 47.51% <0.00%> (-0.56%) ⬇️
KubeArmor/enforcer/runtimeEnforcer.go 37.93% <0.00%> (+1.72%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93794fe...ef9a5bf. Read the comment docs.

@achrefbensaad achrefbensaad force-pushed the annotations-webhook branch 3 times, most recently from 595de4b to 176e6bf Compare April 12, 2022 17:34
@achrefbensaad achrefbensaad marked this pull request as ready for review April 12, 2022 22:26
@nam-jaehyun
Copy link
Collaborator

Here is my first feedback.

  1. please clean up your update in ci-test.yml and latest-release.yml

  2. please keep the logic in kubeUpdate.go

The overall behavior should be something like this.
KubeArmor itself does not add annotations if the admission controller already adds them.
However, in the case that the admission controller is not running (required annotations are not added), 
we need to have a backup plan to handle annotations.

Besides, I'll give you some feedback for KubeArmorAdmission soon.

1 similar comment
@nam-jaehyun
Copy link
Collaborator

Here is my first feedback.

  1. please clean up your update in ci-test.yml and latest-release.yml

  2. please keep the logic in kubeUpdate.go

The overall behavior should be something like this.
KubeArmor itself does not add annotations if the admission controller already adds them.
However, in the case that the admission controller is not running (required annotations are not added), 
we need to have a backup plan to handle annotations.

Besides, I'll give you some feedback for KubeArmorAdmission soon.

@daemon1024 daemon1024 self-requested a review April 20, 2022 10:42
@achrefbensaad achrefbensaad marked this pull request as draft April 20, 2022 10:42
@daemon1024 daemon1024 requested a review from nyrahul April 20, 2022 10:44
@nam-jaehyun
Copy link
Collaborator

nam-jaehyun commented May 2, 2022

@achrefbensaad can you (1) rebase the code, (2) exclude the updates for CI and kubeUpdate.go, and (3) make it a single commit?

@achrefbensaad achrefbensaad force-pushed the annotations-webhook branch from ef9a5bf to 7873221 Compare May 2, 2022 12:26
@achrefbensaad
Copy link
Member Author

achrefbensaad commented May 2, 2022

Hey @nam-jaehyun, I rebased(1) and squashed (3) the commits on top of main.

@achrefbensaad achrefbensaad force-pushed the annotations-webhook branch from 7873221 to c4623e6 Compare May 2, 2022 12:44
@achrefbensaad achrefbensaad marked this pull request as ready for review May 2, 2022 12:47
Signed-off-by: Achref Ben Saadd <[email protected]>
@achrefbensaad achrefbensaad force-pushed the annotations-webhook branch from c4623e6 to ce04875 Compare May 2, 2022 12:54
Copy link
Collaborator

@nam-jaehyun nam-jaehyun left a comment

Choose a reason for hiding this comment

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

Good to merge it

@nam-jaehyun nam-jaehyun merged commit cf7d732 into kubearmor:main May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants