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

falco-addon #1922

Closed
wants to merge 9 commits into from
Closed

falco-addon #1922

wants to merge 9 commits into from

Conversation

balasu
Copy link
Contributor

@balasu balasu commented Jan 21, 2021

Thank you for making MicroK8s better

Please reference the issue this PR is fixing, or provide a description of the problem addressed.

Also verify you have:

@balasu balasu marked this pull request as ready for review January 26, 2021 05:42
refresh_opt_in_config "audit-webhook-config-file" "${SNAP_DATA}/args/auditlogging/webhook-config-falco.yaml" kube-apiserver
refresh_opt_in_config "audit-webhook-batch-max-wait" "5s" kube-apiserver
AGENT_SERVICE_CLUSTERIP="$($KUBECTL get svc sysdig-agent -o=jsonpath={.spec.clusterIP} -n sysdig-agent)" envsubst < "${SNAP_DATA}/args/auditlogging/webhook-config.yaml" > "${SNAP_DATA}/args/auditlogging/webhook-config-falco.yaml"
run_with_sudo preserve_env snapctl restart "${SNAP_NAME}.daemon-apiserver"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may not work on multi nodes. It will only restart the apiserver on the node where the addon is enabled.
Need to use the helper function restart_service apiserver

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes changed to helper function

@@ -0,0 +1,69 @@
apiVersion: audit.k8s.io/v1 # This is required.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we rename this file to have yaml extension?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, included as yaml

print("Enabling falco")
microk8s_enable("falco")
print("Validating falco")
validate_keda()
Copy link
Collaborator

@balchua balchua Jan 26, 2021

Choose a reason for hiding this comment

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

There's some mistake on what is validating here. I guess its should be validate_falco
This can be reason why build is failing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, got it, changed the same

@balchua
Copy link
Collaborator

balchua commented Jan 26, 2021

Hi @balasu
Can yoi check why the falco test is failing? Can you try running the test-addon pytests on yoir environment?

This shows how you can run your test in your environment. https://github.com/ubuntu/microk8s/blob/master/docs/build.md#running-the-tests-locally

@@ -18,7 +18,7 @@ spec:
app: microbot
spec:
containers:
- image: cdkbot/microbot-$ARCH
- image: cdkbot/microbot-amd64
Copy link
Collaborator

Choose a reason for hiding this comment

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

You accidentally committed the amd64

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes its from the test template,did a correction for it

@ktsakalozos
Copy link
Member

@balasu is this still a WIP? Let me know when you want a review.

@balasu balasu changed the title Wip falco falco-addon Feb 25, 2021
@balasu
Copy link
Contributor Author

balasu commented Feb 25, 2021

@balasu is this still a WIP? Let me know when you want a review.

Hi please review:)

@@ -0,0 +1,69 @@
apiVersion: audit.k8s.io/v1 # This is required.
Copy link
Member

@ktsakalozos ktsakalozos Apr 2, 2021

Choose a reason for hiding this comment

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

Do not place yaml files under actions because they are considered addons. In this case for example you have created a microk8s enable kube-api-audit addon.

What you should do is to create a falco directory under actions and put all your yamls inside there.

@@ -0,0 +1,344 @@
# Default values for Falco.
Copy link
Member

Choose a reason for hiding this comment

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

Same here, create a directory named falco under actions and place the manifest there.


containerd:
enabled: true
socket: /run/containerd/containerd.sock
Copy link
Member

Choose a reason for hiding this comment

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

How is this used? I am asking because the containerd socket in MicroK8s is not in this path.

refresh_opt_in_config "audit-log-maxbackup" "3" kube-apiserver
refresh_opt_in_config "audit-log-maxsize" "1024" kube-apiserver
refresh_opt_in_config "audit-log-maxage" "30" kube-apiserver
refresh_opt_in_config "audit-log-path" "/var/log/kube-apiserver-audit.log" kube-apiserver
Copy link
Member

Choose a reason for hiding this comment

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

Let's not polute the host filesystem. Should we keep the MicroK8s logs under SNAP_COMON.


NAMESPACE_PTR="sysdig-agent"

MANIFEST_PTR="https://raw.githubusercontent.com/draios/sysdig-cloud-scripts/master/agent_deploy/kubernetes/sysdig-agent-service.yaml"
Copy link
Member

Choose a reason for hiding this comment

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

I think getting the manifest from the master will be a problem.

  • Are you certain the master will always work? It is not a release so there might be a commit that temporarily breaks the master causing microk8s.enable falco to fail.
  • It makes it really hard for us to have reproducible deployments. Let's say someone opens a issue saying 'falco did not work for me', how would you know what did the user deploy?
  • You pair a manifest with the default values that lives in the snap with the manifests in the master branch. How can you be sure that the default values manifest will stay exactly the same as falco evolves?
  • Are you sure that the falco master branch will deploy correctly on kubernetes v1.21 after 2 year? I am not.

Please select a specific release to deploy from https://github.com/falcosecurity/falco/releases

pullPolicy: IfNotPresent
pullSecrets: []

docker:
Copy link
Member

Choose a reason for hiding this comment

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

Not sure how this variable is used, but in MicroK8s there is no docker.

version: "0.26.2"
check_status: "pod/falco"
supported_architectures:
- arm64
Copy link
Member

Choose a reason for hiding this comment

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

Is deployments on ARM64 tested enough? Looking at [1, 2,3] it is not clear to me what the state of ARM64 builds/configurations is.

Maybe we should have this addon for amd64 for now?

[1] falcosecurity/falco#520
[2] falcosecurity/falco#1442
[3] xunholy/k8s-gitops#152

Copy link
Member

@ktsakalozos ktsakalozos left a comment

Choose a reason for hiding this comment

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

Please see the comments in the PR. Also I do not think tests/backupfile.tar.gz is needed.

@balasu balasu closed this Jan 14, 2022
@balasu balasu deleted the wip-falco branch January 14, 2022 13:23
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.

4 participants