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

option names for karmor #76

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

Conversation

s1ntaxe770r
Copy link

This PR fixes some of the issues described in #66

@s1ntaxe770r s1ntaxe770r changed the title option names for karmor option names for karmor[Draft] May 27, 2022
@Ankurk99

This comment was marked as outdated.

@Ankurk99 Ankurk99 marked this pull request as draft May 27, 2022 10:12
@s1ntaxe770r s1ntaxe770r marked this pull request as ready for review May 27, 2022 11:56
@s1ntaxe770r s1ntaxe770r changed the title option names for karmor[Draft] option names for karmor May 27, 2022
@s1ntaxe770r
Copy link
Author

Ready for review

cmd/discover.go Outdated
discoverCmd.Flags().StringVarP(&discoverOptions.Policy, "policy", "p", "kubearmor", "Type of policies to be discovered: cilium or kubearmor")
discoverCmd.Flags().StringVar(&discoverOptions.Policy, "class", "", "Type of policies to be discovered: cilium or kubearmor")
Copy link
Member

Choose a reason for hiding this comment

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

here "kubearmor" option was provided as a default for the policy flag. You will also need to add a default flag here (application) and change the logic for the same.

cmd/insight.go Outdated Show resolved Hide resolved
@s1ntaxe770r s1ntaxe770r requested a review from Ankurk99 May 28, 2022 21:27
@Ankurk99
Copy link
Member

Please request a review once the PR is fully ready.

@s1ntaxe770r
Copy link
Author

@Ankurk99 review pls

cmd/discover.go Outdated
discoverCmd.Flags().StringVarP(&discoverOptions.Format, "format", "f", "json", "Format: json or yaml")
discoverCmd.Flags().StringVarP(&discoverOptions.Policy, "policy", "p", "kubearmor", "Type of policies to be discovered: cilium or kubearmor")
discoverCmd.Flags().StringVar(&discoverOptions.Policy, "class", "kubearmor", "Type of policies to be discovered: cilium or kubearmor")
Copy link
Contributor

Choose a reason for hiding this comment

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

The class should be same in discover/insight.

Copy link
Author

Choose a reason for hiding this comment

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

@nyrahul discover/insight or discover/discover ?

Copy link
Member

Choose a reason for hiding this comment

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

We will need to have a consistent naming options for discover and insight. Please update cmd/discover.go to have application and network as options and application as default.

Copy link
Author

Choose a reason for hiding this comment

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

@Ankurk99 I noticed this function . If i update cmd/discover as you requested, wouldn't this eliminate the need for this function?

Copy link
Member

Choose a reason for hiding this comment

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

Okay, so you will need to remove the cilium if condition and update kubearmor condition with o.Policy == "application" instead

Copy link
Author

Choose a reason for hiding this comment

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

@Ankurk99 re-review

@s1ntaxe770r s1ntaxe770r requested a review from nyrahul June 2, 2022 19:21
@s1ntaxe770r s1ntaxe770r linked an issue Jun 2, 2022 that may be closed by this pull request
4 tasks
discover/discover.go Outdated Show resolved Hide resolved
cmd/insight.go Outdated Show resolved Hide resolved
discover/discover.go Outdated Show resolved Hide resolved
@s1ntaxe770r s1ntaxe770r requested a review from Ankurk99 June 8, 2022 12:20
@nyrahul
Copy link
Contributor

nyrahul commented Jun 13, 2022

@s1ntaxe770r , please squash the commits into a single one and please re-request reviews if this is ready. Thanks

@s1ntaxe770r
Copy link
Author

squashed 👍🏽

@Ankurk99
Copy link
Member

Hey @s1ntaxe770r can you please address the ci-go warning and squash your commits into one?

@s1ntaxe770r
Copy link
Author

@Ankurk99 does this look good now?

@Ankurk99
Copy link
Member

Hey @s1ntaxe770r, the commits are not squashed yet, can you please do so?

initial sysdump utility

Collect required System Information to troubleshoot issues from the various k8s resources available

Signed-off-by: Barun Acharya <[email protected]>

collect logs from kubearmor pod

Signed-off-by: Barun Acharya <[email protected]>

Archive sysdump

Create dump files in the temp directory and then archive them into `karmor-sysdump.zip`

Signed-off-by: Barun Acharya <[email protected]>

dump more infromation using exec syscalls

added boot-config, ls,m, apparmor, dmesg to dump

Signed-off-by: Barun Acharya <[email protected]>

add timestamp to sysdump archive name

Signed-off-by: Barun Acharya <[email protected]>

copy from inside kubearmor pod

* Removed host side deps
* Used Remote Command executor for streaming file for sysdump ( inspired from kubectl cp )
* Fixed sec vuln for file permission bits

Signed-off-by: Barun Acharya <[email protected]>

volume mount apparmor.d if not minikube env

Signed-off-by: Barun Acharya <[email protected]>

Add description of annotated pods to sysdump

Signed-off-by: Barun Acharya <[email protected]>

concurrently dump resources

Fetch all the resources for sysdump concurrently, if there's an empty dump we return err, else if we have dump but there's an error, we create the partial sysdump and return error.

Signed-off-by: Barun Acharya <[email protected]>

Add pod events to sysdump

Signed-off-by: Barun Acharya <[email protected]>

Add sysdump usage to README

Signed-off-by: Barun Acharya <[email protected]>

update deps

updated direct dependencies
pinned archiver dep to latest commit to fix vulnerability in a transitive dep

Signed-off-by: Barun Acharya <[email protected]>

add codeql analysis workflow

Signed-off-by: Barun Acharya <[email protected]>

docs: updates README.md

Signed-off-by: Thiago Navarro <[email protected]>

add namespace flag to install and unistall

Increase timeout for lint action

Add troubleshoot information in log client

Failure to connect to log grpc server is mostly due to not port-forwarding, so added relevant commands in the error message for convenience

Signed-off-by: Barun Acharya <[email protected]>

Add eks environment detection

Configure daemonset options w.r.t. eks

Signed-off-by: Barun Acharya <[email protected]>

reconfigure daemonset

- added k3s support
- use maps to store environment specific configuration

Signed-off-by: Barun Acharya <[email protected]>

[VM] Added new command to download vm installation script from kvmsoperator

[VM Support]Added option for providing external IP as input

Support for VM command :
1. added option to provide namespace
2. option to provide port value

Karmor VM support -- Addressed review comments

using revive for go-lint

[VM Support] Modified code to identify the namespace of kvmservice instead of inputting the same

[VM Support] Modified code to identify the namespace of kvmservice instead of inputting the same

Fixed protobuf package definition to match the same as KVMService protobuf package

Fixed protobuf package definition to match the same as KVMService proto

added karmor install --image option

fixed lint warnings

add GH workflow for just code build

fixed gosec issue with kvm option

prepare for release 0.3

* cleanup duplicate protobuf
* add vm usage to README

Signed-off-by: Barun Acharya <[email protected]>

Modifying/Adding support for karmor to support non-k8s control plane

Signed-off-by: Eswar Rajan Subramanian <[email protected]>

refactor vm policy boilerplate code

added argument validation

Signed-off-by: daemon1024 <[email protected]>

Add policy handling mechanism

configure gRPC client in kArmor to send host policy event based on argument policy YAML

Signed-off-by: daemon1024 <[email protected]>

Prepare for release 0.4

- Update README with vm related commands
- Remove fork based KubeArmor dep
- Remove duplicate VM policy subcommand

Signed-off-by: daemon1024 <[email protected]>

fix: fix mounts for minikube

Karmor now works with minikube.

Signed-off-by: Gaurav Genani <[email protected]>

sync with deploygen

Signed-off-by: Jaehyun Nam <[email protected]>

add license headers

Signed-off-by: Jaehyun Nam <[email protected]>

fix golint issues

Signed-off-by: Jaehyun Nam <[email protected]>

clean up whitespaces

Signed-off-by: Jaehyun Nam <[email protected]>

add license headers

Signed-off-by: Jaehyun Nam <[email protected]>

update Makefile

Signed-off-by: Jaehyun Nam <[email protected]>

fix typo

Signed-off-by: Jaehyun Nam <[email protected]>

update log

Signed-off-by: Jaehyun Nam <[email protected]>

update logClient

Signed-off-by: Jaehyun Nam <[email protected]>

Fetch installation deployments from KubeArmor

Signed-off-by: daemon1024 <[email protected]>

new containerImage field added

Signed-off-by: Rahul Jadhav <[email protected]>

update karmor to use stable KubeArmor release instead of latest

Signed-off-by: Ankur Kothiwal <[email protected]>

update deployment package

Signed-off-by: daemon1024 <[email protected]>

Added new commands and modified existing vm commands for vm onboarding,
policy enforcement for nonk8s control plane

Signed-off-by: Eswar Rajan Subramanian <[email protected]>
Signed-off-by: s1ntaxe770r <[email protected]>
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@@ -23,10 +25,12 @@ install: build
clean:
cd $(CURDIR); rm -f karmor


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Makefile Outdated Show resolved Hide resolved
@@ -2,16 +2,25 @@

**karmor** is a client tool to help manage [KubeArmor](github.com/kubearmor/KubeArmor).


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Comment on lines +8 to +13
The following sections show how to install the kArmor. It can be installed either from source, or from pre-built binary releases.

### From Script

kArmor has an installer script that will automatically grab the latest version of kArmor and install it locally.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The following sections show how to install the kArmor. It can be installed either from source, or from pre-built binary releases.
### From Script
kArmor has an installer script that will automatically grab the latest version of kArmor and install it locally.

```

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

curl -sfL http://get.kubearmor.io/ | sudo sh -s -- -b /usr/local/bin
```

### Installing from Source

Build karmor from source if you want to test the latest (pre-release) karmor version.


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

cmd/policy.go Outdated Show resolved Hide resolved
Co-authored-by: Rahul Jadhav <[email protected]>
s1ntaxe770r and others added 3 commits October 3, 2022 00:17
Co-authored-by: Rahul Jadhav <[email protected]>
Co-authored-by: Rahul Jadhav <[email protected]>
Co-authored-by: Rahul Jadhav <[email protected]>
@yasin-cs-ko-ak
Copy link
Contributor

the naming changes are still necessary?
Issue: #66
cc: @Ankurk99 @nyrahul

@Ankurk99
Copy link
Member

@yasin-cs-ko-ak Yes, and we need to update the changes to reflect the currect version. For eg insight command is no more there.

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.

option names for karmor
5 participants