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

feat: (recommend) Implement recommend functionality for Docker Client #461

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

feat: (recommend) Implement recommend functionality for Docker Client #461

wants to merge 16 commits into from

Conversation

tesla59
Copy link

@tesla59 tesla59 commented Sep 17, 2024 •
edited
Loading

@tesla59 tesla59 marked this pull request as draft September 17, 2024 10:34
@tesla59
recommend: refactor k8s policy generation
ddb361a 
use a common interface to support other clients as well
also create common Object{} to support different k8s obejcts such as Deployment, Daemonset etc

Signed-off-by: tesla59 <[email protected]>
@tesla59
recommend: use image name as Object's name while generating policy
2819a3f 
Signed-off-by: tesla59 <[email protected]>
@tesla59
recommend: remove DeploymentName Field from Object{}
5474f33 
Signed-off-by: tesla59 <[email protected]>
@tesla59
recommend: use ListOptions to select deployment by labels
a95b469 
Signed-off-by: tesla59 <[email protected]>
@tesla59
k8s: generate recommended policies for CronJob, DaemonSet and Jobs
82d6666 
Signed-off-by: tesla59 <[email protected]>
@tesla59
k8s: generate recommend policy for statefulset and unowned replicaset
993274e 
Signed-off-by: tesla59 <[email protected]>
@tesla59
k8s: remove K8sClientWrapper abstraction
2209139 
Signed-off-by: tesla59 <[email protected]>
@tesla59
recommend: initialize dockerClient and generate policies for containe…
c759cd7 
…rs as well

Signed-off-by: tesla59 <[email protected]>
@tesla59 tesla59 marked this pull request as ready for review September 29, 2024 10:00
@tesla59
recommend: add new flag k8s to specify which client to use
dd2788a 
this removes dependency of recommend command on kubearmor

Signed-off-by: tesla59 <[email protected]>
@tesla59
Copy link
Author

tesla59 commented Oct 2, 2024

@daemon1024 please review

@daemon1024
Copy link
Member

Can you include screenshots of how it's working?

@tesla59
Copy link
Author

tesla59 commented Oct 3, 2024

Case 1: systemd mode
image

  • karmor recommend fails due to --k8s=true by default and no cluster is running. K8s client is used
  • karmor recommend --k8s=false uses docker client but policy is not generated due to no containers running
  • karmor recommend --k8s=false generates policy after nginx container is run and policy is generated

image

Case 2: k8s mode
image
Works as before

There is an error ERRO[0010] Not a valid tar file file=/tmp/karmor867115595/blobs/sha256/0162fa012a5d588eb52b8edaef90c4aecf89021924a20a2ca62f8dad7b766bf7 but it is not related to this PR. Will push fix for that in another pr

rootxrishabh
rootxrishabh reviewed Nov 11, 2024
View reviewed changes
Copy link
Member

@rootxrishabh rootxrishabh left a comment

Choose a reason for hiding this comment

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

Initial review - Changes are working as expected.
Screenshot from 2024-11-11 20-49-43

@rootxrishabh
Copy link
Member

rootxrishabh commented Nov 11, 2024 •
edited
Loading

Output directory is -server-1 which gives an error if I try to enter it -
rootxrishabh@fedora:/tmp/out$ cd -server-1 bash: cd: -s: invalid option cd: usage: cd [-L|[-P [-e]] [-@]] [dir]

Is this intended?

@tesla59
Copy link
Author

tesla59 commented Nov 13, 2024

cd -server-1

shouldn't this be cd -- -server-1

ill check what is the issue with output directory name

@tesla59
Copy link
Author

tesla59 commented Nov 19, 2024

@rootxrishabh policy directory is fixed. it was due to img.namespace being null in VM mode.
image

@tesla59
Copy link
Author

tesla59 commented Nov 19, 2024

@rootxrishabh also added commit to trim new line character in final report generation. it fixes the weird number of blank lines after reports table
image
Old
image
New

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rootxrishabh rootxrishabh rootxrishabh left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tesla59 @daemon1024 @rootxrishabh