kubearmor / kubearmor-client

KubeArmor cli tool aka kArmor :robot:
Apache License 2.0
34 stars 82 forks source link

Enabled operator installation for karmor #402

Closed rootxrishabh closed 5 months ago

rootxrishabh commented 7 months ago

Purpose: This PR shifts karmor install and karmor uninstall to use kubearmor operator using helm.

Does this PR introduce a breaking change? Yes

Aryan-sharma11 commented 6 months ago

Snyk issues remediation:-

There are still some pkgs whose fixes are pushed to the main branch but not published yet.

daemon1024 commented 6 months ago

Fixes https://github.com/kubearmor/KubeArmor/issues/1256

daemon1024 commented 6 months ago

karmor install --save failing if no kubernetes cluster is configured We do not mandate K8s cluster if we only need to save

image

daemon1024 commented 6 months ago

Registry flag is not working

./karmor install -r "ttl.sh" --save

Expected - config images and operator image to be prepended with registry info Actual - No change in config

daemon1024 commented 6 months ago
./karmor install --tag=v1.2.1 --save

Not working as expected

Expected - config images and operator image to have their tags changed Actual - No change in config

daemon1024 commented 6 months ago

I couldn't help but notice, even if I am just using save. It takes a lot of time to generate things. Is it a drawback of using helm client?

rootxrishabh commented 6 months ago

I couldn't help but notice, even if I am just using save. It takes a lot of time to generate things. Is it a drawback of using helm client?

I tried printing manifests without running the client and it returns a null pointer. So, the install/upgrade client had to dryrun in order to get the manifests. Taking a look at other points :eyes:

rootxrishabh commented 6 months ago

Have we verified this behaviour with a legacy karmor install? Please include screenshots

`rootxrishabh@rootxrishabh:~/kubearmor-client$ ./karmor install --legacy=true ๐Ÿ˜„ Environment : k3s
๐Ÿ”ฅ CRD kubearmorpolicies.security.kubearmor.com
โ„น๏ธ CRD kubearmorpolicies.security.kubearmor.com already exists
๐Ÿ”ฅ CRD kubearmorhostpolicies.security.kubearmor.com
โ„น๏ธ CRD kubearmorhostpolicies.security.kubearmor.com already exists
๐Ÿ’ซ Service Account
๐Ÿ’ซ Service Account
โš™๏ธ Cluster Role
โ„น๏ธ Cluster Role already exists
โš™๏ธ Cluster Role Bindings
โ„น๏ธ Cluster Role Bindings already exists
โš™๏ธ KubeArmor Relay Roles
๐Ÿ›ก KubeArmor Relay Service
๐Ÿ›ฐ KubeArmor Relay Deployment
๐Ÿ›ก KubeArmor DaemonSet - Init kubearmor/kubearmor-init:stable, Container kubearmor/kubearmor:stable -gRPC=32767
โ„น๏ธ KubeArmor DaemonSet already exists
๐Ÿ›ก KubeArmor Controller TLS certificates
๐Ÿ’ซ KubeArmor Controller Service Account
โš™๏ธ KubeArmor Controller Roles
๐Ÿš€ KubeArmor Controller Deployment
๐Ÿš€ KubeArmor Controller Metrics Service
๐Ÿš€ KubeArmor Controller Webhook Service
๐Ÿฅณ Done Installing KubeArmor
๐Ÿคฉ KubeArmor Controller Mutation Admission Registration
โ„น๏ธ KubeArmor Controller Mutation Admission Registration already exists 88% ๐Ÿš€ KubeArmor ConfigMap Creation 88% ๐Ÿ˜‹ Checking if KubeArmor pods are running...โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡โ–‡] 111.76% ๐Ÿฅณ Done Checking , ALL Services are running!
โŒš๏ธ Execution Time : 13.858858021s

๐Ÿ”ง Verifying KubeArmor functionality (this may take upto a minute)...

    ๐Ÿ›ก๏ธ       Your Cluster is Armored Up! 

rootxrishabh@rootxrishabh:~/kubearmor-client$ ./karmor uninstall โŒ KubeArmor is either not installed, or the specified namespace is incorrect. โ„น๏ธ Please ensure you have installed KubeArmor, and check that you are specifying the correct namespace. โ„น๏ธ Attempting legacy uninstallation. ๐Ÿ—‘๏ธ Mutation Admission Registration ๐Ÿ—‘๏ธ KubeArmor Services โŒ Service: kubearmor removed โŒ Service: kubearmor-controller-metrics-service removed ๐Ÿ’จ Service Accounts โŒ ServiceAccount kubearmor removed โŒ ServiceAccount kubearmor-relay removed โŒ ServiceAccount kubearmor-controller removed ๐Ÿ’จ Cluster Roles โŒ ClusterRole kubearmor-snitch removed โŒ ClusterRole kubearmor-clusterrole removed โŒ ClusterRole kubearmor-relay-clusterrole removed โŒ ClusterRole kubearmor-controller-proxy-role removed โŒ ClusterRole kubearmor-controller-clusterrole removed ๐Ÿ’จ Cluster Role Bindings โŒ ClusterRoleBinding kubearmor-clusterrolebinding removed โŒ ClusterRoleBinding kubearmor-relay-clusterrolebinding removed โŒ ClusterRoleBinding kubearmor-controller-clusterrolebinding removed โŒ ClusterRoleBinding kubearmor-controller-proxy-rolebinding removed โŒ ClusterRoleBinding kubearmor-snitch-binding removed ๐Ÿงน Roles โŒ Role kubearmor-controller-leader-election-role removed ๐Ÿงน RoleBindings โŒ RoleBinding kubearmor-controller-leader-election-rolebinding removed ๐Ÿ‘ป KubeArmor Controller TLS certificates โŒ KubeArmor Controller TLS certificate kubearmor-controller-webhook-server-cert removed ๐Ÿ‘ป KubeArmor ConfigMap โŒ ConfigMap kubearmor-config removed ๐Ÿ‘ป KubeArmor DaemonSet โŒ KubeArmor DaemonSet kubearmor removed ๐Ÿ‘ป KubeArmor Deployments โŒ KubeArmor Deployment kubearmor-relay removed โŒ KubeArmor Deployment kubearmor-controller removed ๐Ÿ”„ Checking if KubeArmor pods are stopped... ๐Ÿ”ด Done Checking; all services are stopped!
โŒš๏ธ Termination Time: 9.787902842s ` Yes

daemon1024 commented 6 months ago

image

Installation Process in general is smooooth. Great Work.

But this does not look cohesive.

daemon1024 commented 6 months ago
karmor uninstall

This is not working as expected I did the standard helm install But it is saying it cannot detect it image It fellback to legacy uninstallation, but my helm package still exists

rootxrishabh commented 6 months ago

image

Installation Process in general is smooooth. Great Work.

But this does not look cohesive.

* the helm logs and emoji logs are not streamlines, We should suppress helm logs and manually log based on the output maybe. Not sure what's best here.

* We are spamming `Deployment is not ready` messages

* We need to type something when KubeArmor Daemonset is deploying, Because it takes a while, including messaging around expected time, inform that snitch is running

I think we should handle logs ourselves and suppress helm logs. We could manually output logs for deployments etc. I will add deployment message for snitch and include an expected time logs.

rootxrishabh commented 6 months ago
karmor uninstall

This is not working as expected I did the standard helm install But it is saying it cannot detect it image It fellback to legacy uninstallation, but my helm package still exists

Yes, I was thinking of discussing this earlier. If we remove the default namespace in uninstall and don't specify namespace for example "kubearmor" then our current helm implementation doesn't automatically detect the installed namespace. I guess implementing auto detection of namespace is required?

daemon1024 commented 6 months ago

I guess implementing auto detection of namespace is required?

Yup. We can list packages and go through that ig.

rootxrishabh commented 6 months ago

I don't think the first log is looking nice, is it something we need to keep?

image

It is not necessary, just denoting the initiation of helm client. Should we suppress logs for save flag?

daemon1024 commented 6 months ago

I think it will automatically be handled when we have the entire logging figured out for helm.

rootxrishabh commented 6 months ago
./karmor install --tag=v1.2.1 --save

Not working as expected

Expected - config images and operator image to have their tags changed Actual - No change in config

We currently dont have --save watching tag and registry flags. I will do it :+1:

daemon1024 commented 6 months ago

We currently dont have --save watching tag and registry flags. I will do it ๐Ÿ‘

That's concerning. Ideal case it should be not be 2 different approaches. We finalise on the config then decide whether to save it or install it.

rootxrishabh commented 6 months ago

We currently dont have --save watching tag and registry flags. I will do it ๐Ÿ‘

That's concerning. Ideal case it should be not be 2 different approaches. We finalise on the config then decide whether to save it or install it.

Got it. I will make sure we are handling all flags first.

rootxrishabh commented 6 months ago

I couldn't help but notice, even if I am just using save. It takes a lot of time to generate things. Is it a drawback of using helm client?

Update: I fixed --save not running for clusterless environments and now the process is running client side only, executes without helm logs and is quick.

rootxrishabh commented 6 months ago

Here's a demo of the functionality - kubearmorgif

daemon1024 commented 6 months ago

We should be updating the operator image as well when we use tag/flags/registry image

operator image is not changing with any flags. We might need to impement a new flag to update operator image. I believe the helm chart accepts updates to operator image as well

rootxrishabh commented 6 months ago

We can now change imagePullPolicy, tag, image, registry. image

DelusionalOptimist commented 6 months ago

Also the uninstall command stays stuck right now because of - https://github.com/kubearmor/KubeArmor/issues/1638

rootxrishabh commented 6 months ago

@DelusionalOptimist can we address the review changes first and then take a look at the controller issue?

DelusionalOptimist commented 6 months ago

@DelusionalOptimist can we address the review changes first and then take a look at the controller issue?

Yeah yeah it's cool. Just mentioned it here as an FYI for reviewers. We can look into it later. This PR is priority.

PrimalPimmy commented 6 months ago

@rootxrishabh the defaultposture flags are not working for me image

image

I used block capabilities as flag but it is still in audit mode

rootxrishabh commented 6 months ago

@PrimalPimmy can you please check the kubearmor-config configmap. Currently posture and visibility settings are being incorrectly displayed by karmor probe for operator installation. Ref

PrimalPimmy commented 6 months ago

@PrimalPimmy can you please check the kubearmor-config configmap. Currently posture and visibility settings are being incorrectly displayed by karmor probe for operator installation.

Still not working image

file is not going to audit mode when set?

rootxrishabh commented 6 months ago

I see that the kubearmorconfig is not being recreated. You need to do karmor uninstall --force to remove the CR and then reinstall with the posture settings.

Aryan-sharma11 commented 6 months ago

Snyk issues remediation:-

PrimalPimmy commented 6 months ago

I see that the kubearmorconfig is not being recreated. You need to do karmor uninstall --force to remove the CR and then reinstall with the posture settings.

Is there any way to get the configmap recreated, imo it's a better way. Maybe remove the configmap after uninstalling

rootxrishabh commented 6 months ago

I see that the kubearmorconfig is not being recreated. You need to do karmor uninstall --force to remove the CR and then reinstall with the posture settings.

Is there any way to get the configmap recreated, imo it's a better way. Maybe remove the configmap after uninstalling

The configmap kubearmor-config does get deleted but the operator CR kubearmorconfig-default does not and so for every new installation after this, the CR does not get updated and thus we don't see posture settings supplied through CLI. Inorder to remove the CR(kubearmorconfig-default), we must do karmor uninstall --force and do a fresh installation. Ref

rootxrishabh commented 6 months ago

I see that the kubearmorconfig is not being recreated. You need to do karmor uninstall --force to remove the CR and then reinstall with the posture settings.

Is there any way to get the configmap recreated, imo it's a better way. Maybe remove the configmap after uninstalling

The configmap kubearmor-config does get deleted but the operator CR kubearmorconfig-default does not and so for every new installation after this, the CR does not get updated and thus we don't see posture settings supplied through CLI. Inorder to remove the CR(kubearmorconfig-default), we must do karmor uninstall --force and do a fresh installation. Ref

We can notice it outputs KubeArmorConfig created in a fresh installation. image

daemon1024 commented 6 months ago

If I do a kubectl apply -f with the update config, it does update existing config if there are changes. I believe the same behaviour should translate here

image

daemon1024 commented 6 months ago

We should switch to doing

err:= Resource.Create()
if !strings.Contains(err.Error(), "already exists") {
        err:= Resource.Update()
        return err/nil
}
rootxrishabh commented 6 months ago

If I do a kubectl apply -f with the update config, it does update existing config if there are changes. I believe the same behaviour should translate here

image

I remember us discussing the idea of karmor update. I think we discussed that we would update all operator CR options separately using the update command, it can be handled in a separate issue. Wdyt?

daemon1024 commented 6 months ago

I remember us discussing the idea of karmor update. I think we discussed that we would update all operator CR options separately using the update command, it can be handled in a separate issue. Wdyt?

This is a different issue. People won't realise that CR is not updated. the update command was meant to patch things as well patch the helm release if needed.

We cannot ask people to run uninstall with force just so that they have a fresh installation.

It's a 2 liner change here so I believe it's worth it to handle here. Wdyt?

rootxrishabh commented 6 months ago

I remember us discussing the idea of karmor update. I think we discussed that we would update all operator CR options separately using the update command, it can be handled in a separate issue. Wdyt?

This is a different issue. People won't realise that CR is not updated. the update command was meant to patch things as well patch the helm release if needed.

We cannot ask people to run uninstall with force just so that they have a fresh installation.

It's a 2 liner change here so I believe it's worth it to handle here. Wdyt?

Agreed. --force uninstallation shouldn't be an intermediatory to update the CR. I will implement the CR updation here.