Add datastream to collect audit logs in kubernetes integration #2317

tetianakravchenko · 2021-12-06T14:56:39Z

Signed-off-by: Tetiana Kravchenko tetiana.kravchenko@elastic.co

What does this PR do?

This PR adds a new data stream in kubernetes integration to collect audit logs.

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that all data streams collect metrics or logs.
I have added an entry to my package's changelog.yml file.
I have verified that Kibana version constraints are current according to guidelines.

How to test this PR locally

cd  package/kubernetes
elastic-package build
elastic-package stack up -d -v --version 8.1.0-f479a40a-SNAPSHOT

Start kind with auditing enabled, as described here:

kind create cluster --config kind-config.yaml

connect networks:

docker network connect elastic-package-stack_default kind-control-plane

deploy elastic-agent:

kubectl apply -f elastic-agent-managed-kubernetes.yaml

Related issues

Closes Add audit logs datastream in K8s integration #2285

Screenshots

elasticmachine · 2021-12-06T15:07:02Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-12-15T11:20:17.386+0000
Duration: 29 min 15 sec
Commit: dca4219

Test stats 🧪

Test	Results
Failed	0
Passed	115
Skipped	0
Total	115

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

ChrsMark

Great addition!

Could we also add sample logs along with a pipeline test?
Also since I guess there is a specific pattern in these logs, will we be explicit in the fields we are storing here?

ChrsMark · 2021-12-06T16:29:06Z

packages/kubernetes/changelog.yml

@@ -1,4 +1,9 @@
 # newer versions go on top
+- version: "1.5.1"


Suggested change

- version: "1.5.1"

- version: "1.6.0"

New feature ;)

tetianakravchenko · 2021-12-06T16:58:26Z

@ChrsMark thank you for the feedback!

Could we also add sample logs along with a pipeline test?

I will have a look how to add it 👍

Also since I guess there is a specific pattern in these logs, will we be explicit in the fields we are storing here?

do you mean specifically kubernetes.audit.*?

ChrsMark · 2021-12-06T17:25:14Z

Also since I guess there is a specific pattern in these logs, will we be explicit in the fields we are storing here?

do you mean specifically kubernetes.audit.*?

Well if the possible fields are unknown then yes we can do something similar to what we do for kubernetes.labels.* or prometheus.labels.*. Otherwise if the fields are specific we can define them like kubernetes.audit.verb etc. Maybe https://kubernetes.io/docs/reference/config-api/apiserver-audit.v1/#audit-k8s-io-v1-Event can be of help here?

tetianakravchenko · 2021-12-13T12:28:02Z

packages/kubernetes/data_stream/audit_logs/manifest.yml

@@ -0,0 +1,15 @@
+title: "Kubernetes audit logs"
+type: logs
+release: experimental


@ChrsMark should I keep it as experimental ?

I think it should be experimental or beta at least until we have tests+dashboards. @akshay-saraswat could confirm this too.

tetianakravchenko · 2021-12-14T14:04:56Z

packages/kubernetes/data_stream/audit_logs/agent/stream/stream.yml.hbs

+parsers:
+- ndjson:
+    add_error_key: true
+    target: "kubernetes_audit"


without defined target - decoded json fields will be written under root
related issue - elastic/beats#29422

tetianakravchenko · 2021-12-14T14:06:08Z

packages/kubernetes/data_stream/audit_logs/agent/stream/stream.yml.hbs

+- rename:
+    fields:
+      - from: "kubernetes_audit"
+        to: "kubernetes.audit"


it is not possible to use kubernetes.audit as a target, related issue: elastic/beats#29395

MichaelKatsoulis · 2021-12-14T16:31:59Z

packages/kubernetes/data_stream/audit_logs/agent/stream/stream.yml.hbs

+      function process(event) {
+        var audit = event.Get("kubernetes.audit");
+        var annotation;
+        for (annotation in audit["annotations"]) {


You can remove declaration of annotation and change the iteration to this

for (var annotation in audit["annotations"]) {

done 6dad8d7

ChrsMark

Looks good! I left a few thoughts about the fields.

The way that we use the parser and the script processor is the best way possible at the moment I think so we are good here, good job!

Fyi we will need dashboard and tests for this at some point. Maybe a meta issue to keep track of this would help since it's not that much urgent to have them really soon :).

ChrsMark · 2021-12-15T07:50:53Z

packages/kubernetes/data_stream/audit_logs/fields/fields.yml

+      description: Time the request reached current audit stage
+    - name: annotations.*
+      type: object
+      object_type: text


Any reason for not using keyword here? In kubernetes it is defined as keyword: https://github.com/elastic/integrations/blob/master/packages/kubernetes/data_stream/container/fields/base-fields.yml#L57. In general we should use keyword when we want to make this field to be heavily searchable.

Also do you think we could move this field under kubernetes.annotations.* that is already defined for kubernetes? In this way we might be able to corelate events with resources, however I'm not quite sure what values these annotations of the events could take and if related with the annotations of a resource. Let me know what you think.

Any reason for not using keyword here? In kubernetes it is defined as keyword: https://github.com/elastic/integrations/blob/master/packages/kubernetes/data_stream/container/fields/base-fields.yml#L57. In general we should use keyword when we want to make this field to be heavily searchable.

I used text, because one of the fields authorization_k8s_io/reason seems to be rather an text according to the name and the example value below:

"annotations": { "authorization_k8s_io/decision": "allow", "authorization_k8s_io/reason": "RBAC: allowed by ClusterRoleBinding \"system:public-info-viewer\" of ClusterRole \"system:public-info-viewer\" to Group \"system:unauthenticated\"" },

Do you think it would be better to use keyword instead

Also do you think we could move this field under kubernetes.annotations.* that is already defined for kubernetes? In this way we might be able to corelate events with resources, however I'm not quite sure what values these annotations of the events could take and if related with the annotations of a resource. Let me know what you think.

I think those annotations.* are not related to the resource: from the doc:

Note that these annotations are for the audit event, and do not correspond to the metadata.annotations of the submitted object. Keys should uniquely identify the informing component to avoid name collisions (e.g. podsecuritypolicy.admission.k8s.io/policy). Values should be short. Annotations are included in the Metadata level.

Ok, I think we can leave them as is for now. In any case this is still beta so we will have the chance to iterate on this later if we have feedback.

ChrsMark · 2021-12-15T07:52:42Z

packages/kubernetes/data_stream/audit_logs/sample_event.json

+        "version": "8.0.0"
+    },
+    "host": {
+        "architecture": "x86_64",


For shake of simplicity we usually remove hosts's informations from sample events. A very baseline event with the actually interesting fields populated should be enough.

thanks for letting me know, I think I've checked few sample_event files in kubernetes package (node, pod) and they actually contain all fields, including host, so the idea is just to keep kubernetes.* here, right?

I would say that yes, the simple the better. I don't see any reason into having all these extra metadata for sample events that want to just show k8s relevant fields. Other data_streams might have been ported automatically and hence this is why they brought sample events from Beats repo that might container all those. I don't think it is documented somewhere however it's rather a "best-practice" convention :).

done 74b92dc

tetianakravchenko · 2021-12-15T09:16:16Z

Fyi we will need dashboard and tests for this at some point. Maybe a meta issue to keep track of this would help since it's not that much urgent to have them really soon :).

@ChrsMark follow up ticket for tests and dashboards: #2343

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

…ssor; add script processor to dedoc annotations Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

ChrsMark

🚢 it!

tetianakravchenko requested review from ChrsMark and MichaelKatsoulis December 6, 2021 15:07

ChrsMark reviewed Dec 6, 2021

View reviewed changes

tetianakravchenko force-pushed the audit-log-datastream branch from 991373d to f03d57a Compare December 6, 2021 16:57

tetianakravchenko commented Dec 13, 2021

View reviewed changes

tetianakravchenko commented Dec 14, 2021

View reviewed changes

MichaelKatsoulis reviewed Dec 14, 2021

View reviewed changes

ChrsMark self-requested a review December 14, 2021 16:39

ChrsMark reviewed Dec 15, 2021

View reviewed changes

tetianakravchenko mentioned this pull request Dec 15, 2021

Kubernetes - audit_logs datastream: add dashboard and tests #2343

Closed

tetianakravchenko added 7 commits December 15, 2021 10:45

add kubernetes audit logs

c39b77b

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

fix pr number; commit manifest changes

73a1eb2

add empty line in the end; remove data_stream.namespace; fix readme

9b7a40b

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

add sample_event; use json parser instead of decode_json_fields proce…

af9e646

…ssor; add script processor to dedoc annotations Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

fix formatting error

27ac42d

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

remove declaration of annotation

a54c84a

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

keep only kubernetes.* fields in sample_event

7dd1f6a

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

tetianakravchenko force-pushed the audit-log-datastream branch from 74b92dc to 7dd1f6a Compare December 15, 2021 09:49

tetianakravchenko added 4 commits December 15, 2021 11:14

remove only from sample_event

37c319b

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

add new sample_event

622d241

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

keep only one ip and mac

3d56152

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

run es-package format

dca4219

Signed-off-by: Tetiana Kravchenko <tetiana.kravchenko@elastic.co>

ChrsMark approved these changes Dec 15, 2021

View reviewed changes

MichaelKatsoulis approved these changes Dec 15, 2021

View reviewed changes

tetianakravchenko merged commit b45809a into elastic:master Dec 15, 2021

tetianakravchenko deleted the audit-log-datastream branch December 15, 2021 13:14

Add datastream to collect audit logs in kubernetes integration #2317

Add datastream to collect audit logs in kubernetes integration #2317

Uh oh!

Conversation

tetianakravchenko commented Dec 6, 2021

What does this PR do?

Checklist

How to test this PR locally

Related issues

Screenshots

Uh oh!

elasticmachine commented Dec 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

Uh oh!

ChrsMark left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tetianakravchenko commented Dec 6, 2021

Uh oh!

ChrsMark commented Dec 6, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tetianakravchenko Dec 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrsMark left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tetianakravchenko commented Dec 15, 2021

Uh oh!

ChrsMark left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

elasticmachine commented Dec 6, 2021 •

edited

Loading

tetianakravchenko Dec 14, 2021 •

edited

Loading