Add KibanaRef to Beats and support setup.kibana #3211

pebrc · 2020-06-10T08:21:21Z

Adds a kibanaRef to the Beats CRD

wires up the CAs mostly fashioned after the way we do it for APM
creates a separate user for the Kibana ref for cases where Beats are not directly connected to Elasticsearch
uses kibana_admin role to talk to Kibana

Adds support for setup.kibana (sorry for folding this into the same PR but testing was easier this way)

only Metricbeat, Filebeat and Auditbeat support the feature afaik, so I added the config only to Metricbeat and Filebeat
I did not introduce a validation that prevents the usage of kibanaRef for generic Beats (needs documentation how to use the credentials)

TODO:

e2e test would be nice, but maybe can be done in a follow up to keep the scope small for this PR

Fixes #3191 and #3192

david-kow

Nice! Did a first pass, just some nits. I'd do consider moving adding the config to the common part. I gave it a spin and I have two observations:

unfortunately, Beat crashes if it can't reach Kibana (which is a common case since it starts up much faster)
Filbebeat doesn't seem to have any useful dashboards for just plain logging

pkg/apis/beat/v1beta1/beat_types.go

pkg/controller/association/controller/apm_kibana.go

pkg/controller/beat/common/health.go

pkg/controller/association/controller/beat_kibana.go

pkg/controller/beat/common/config.go

pkg/controller/beat/filebeat/config.go

david-kow · 2020-06-10T10:32:01Z

pkg/controller/beat/filebeat/config.go

@@ -24,3 +28,14 @@ processors:
 - add_host_metadata: {}
 `))
 )
+
+func (d *Driver) configParams() (common.DefaultConfigs, error) {
+ kibanaConfig, err := common.BuildKibanaConfig(d.Client, v1beta1.BeatKibanaAssociation{Beat: &d.Beat})


I think all Beats support that, that's at least what's there in their reference yamls (heartbeat, auditbeat, journalbeat, packetbeat). I think I'd assume we can just add it to the config for any other beat too. If it's supported it will likely follow the established pattern, if not, it will be ignored.

That was my initial assumption as well also my initial implementation. I tried it with Heartbeat and it did not go well, meaning Heartbeat terminated when the Kibana config was in place. I did not look at the reference.yaml but the written documentation and saw that it is only mentioned in the Metricbeat, Filebeat and Auditbeat docs. The fact that it is actually also in all the other reference YAMLs means I will repeat the test once more to double check that my initial observation is correct.

So take Heartbeat as an example: while it supports the settings in general, it does not come with dashboards to install and fails subsequently when not finding any stored objects to install into Kibana:

2020-06-10T12:46:32.632Z ERROR instance/beat.go:932 Exiting: Error importing Kibana dashboards: fail to import the dashboards in Kibana: Error importing directory /usr/share/heartbeat/kibana: No directory /usr/share/heartbeat/kibana/7

Seems like (see below):

filebeat, metricbeat, auditbeat, packetbeat have dashboards

journalbeat has index patterns

heartbeat doesn't have anything

$ cat beats | while read BEAT; do docker run --rm --entrypoint bash "docker.elastic.co/beats/$BEAT:7.7.0" -c "ls -laR /usr/share/$BEAT/kibana"; done ... /usr/share/filebeat/kibana/7/dashboard: total 1204 drwxr-x--- 1 root filebeat 4096 May 12 00:56 . drwxr-x--- 1 root filebeat 4096 May 12 00:56 .. -rw-r----- 1 root filebeat 18520 May 12 00:56 749203a0-67b1-11ea-a76f-bf44814e437d.json -rw-r----- 1 root filebeat 10270 May 12 00:56 Coredns-Overview-Dashboard.json -rw-r----- 1 root filebeat 23030 May 12 00:56 Filebeat-Cisco-ASA.json ... /usr/share/metricbeat/kibana/7/dashboard: total 1888 drwxr-x--- 1 root metricbeat 4096 May 12 01:02 . drwxr-x--- 1 root metricbeat 4096 May 12 01:02 .. -rw-r----- 1 root metricbeat 19152 May 12 01:02 Metricbeat-Host-Services-overview.json -rw-r----- 1 root metricbeat 35650 May 12 01:02 Metricbeat-Oracle-overview.json... ... /usr/share/journalbeat/kibana/default/index-pattern: total 16 drwxr-x--- 1 root journalbeat 4096 May 12 00:30 . drwxr-x--- 1 root journalbeat 4096 May 12 00:30 .. -rw-r----- 1 root journalbeat 6616 May 12 00:30 journalbeat.json /usr/share/heartbeat/kibana: total 8 drwxr-x--- 1 root heartbeat 4096 May 12 00:26 . drwxr-x--- 1 root heartbeat 4096 May 12 00:26 .. ... /usr/share/auditbeat/kibana/7/dashboard: total 232 drwxr-x--- 1 root auditbeat 4096 May 12 00:51 . drwxr-x--- 1 root auditbeat 4096 May 12 00:51 .. -rw-r----- 1 root auditbeat 22501 May 12 00:51 auditbeat-file-integrity.json -rw-r----- 1 root auditbeat 6945 May 12 00:51 auditbeat-kernel-executions.json ... /usr/share/packetbeat/kibana/7/dashboard: total 300 drwxr-x--- 1 root packetbeat 4096 May 12 00:41 . drwxr-x--- 1 root packetbeat 4096 May 12 00:41 .. -rw-r----- 1 root packetbeat 22551 May 12 00:41 Packetbeat-cassandra.json -rw-r----- 1 root packetbeat 14045 May 12 00:41 Packetbeat-dhcpv4.json ...

Very nice shell magic there David :-) But for the purposes of this PR unless we want to enumerate the supported Beats we need to stick with Beat specific 'managed' config as suggested by me. Agreed?

Yes, that's fine. My comment was more of a FYI :)

The heartbeat error you encountered seems like a bug that should be fixed in the reference yaml, correct?

I am not sure. I read it as "you asked me to import dashboards but there are no dashboards to import so I give up"

david-kow · 2020-06-10T10:36:05Z

pkg/controller/elasticsearch/user/roles.go

@@ -28,6 +28,9 @@ const (

 // ApmAgentUserRole is the name of the role used by APMServer instances to connect to Kibana
 ApmAgentUserRole = "eck_apm_agent_user_role"
+
+ // KibanaUserBuiltinRole is the name of the built-in Kibana user role
+ KibanaUserBuiltinRole = "kibana_user"


Remove if not used?

I changed it to kibana_admin and started using it.

pkg/controller/beat/common/config.go

pkg/controller/beat/common/pod.go

anyasabo · 2020-06-10T21:17:22Z

pkg/apis/beat/v1beta1/beat_types.go

@@ -25,6 +25,10 @@ type BeatSpec struct {
 // +kubebuilder:validation:Optional
 ElasticsearchRef commonv1.ObjectSelector `json:"elasticsearchRef,omitempty"`

+ // KibanaRef is a reference to a Kibana instance running in the same Kubernetes cluster.
+ // It allows automatic setup of dashboards and visualizations.
+ KibanaRef commonv1.ObjectSelector `json:"kibanaRef,omitempty"`


Just a note for other reviewers: I was wondering if we needed the Optional tag here since all the other ones have it, it looks like we only need it if we also set kubebuilder:validation:Required on the package which marks them all as required by default

anyasabo · 2020-06-10T22:50:28Z

pkg/controller/beat/common/config.go

-
- if err := cfg.MergeWith(esOutput); err != nil {
- return err
+ esOutput := map[string]interface{}{


Doesn't need to be resolved in your change, but this made me realize we need to add documentation on how beats and ssl verification interacts with custom certs. Opened this issue:
#3216

The only way I can think of being smarter here is if we check that the current associated certificate is valid for the host name (and everything else?) the beat will use, and if not, setting ssl verification mode to none. I'm not sure we want to do that silently though, and probably want the users to know they are disabling ssl.

I do wonder if we want to fail the association if we know that the beat cannot actually associate successfully with the current certificate.

anyasabo · 2020-06-10T23:40:18Z

LGTM on first pass, will take another look tomorrow for 👍 unless someone gets to it first. I'm good with waiting for the e2e test until a future PR, but can you open an issue so we don't forget?

anyasabo · 2020-06-11T22:08:55Z

pkg/controller/beat/common/config.go

+ return settings.NewCanonicalConfigFrom(esOutput)
+}
+
+// BuildKibanaConfig builds on optional Kibana configuration for dashboard setup and visualizations.


Suggested change

// BuildKibanaConfig builds on optional Kibana configuration for dashboard setup and visualizations.

// BuildKibanaConfig builds an optional Kibana configuration for dashboard setup and visualizations.

anyasabo · 2020-06-11T22:21:19Z

pkg/controller/beat/common/config.go

 // use only the default config or only the provided config - no overriding, no merging
 userConfig := beat.Spec.Config
 if userConfig == nil {
- if err := cfg.MergeWith(defaultConfig); err != nil {
+ if err := cfg.MergeWith(defaultConfig.Unmanaged); err != nil {


nit: it's not in your change but I just realized that in the debug log below we probably want to include the beat name/ns

anyasabo

LGTM, some questions about future work that we should discuss but do not need to be resolved in this PR

david-kow · 2020-06-12T05:20:28Z

pkg/controller/association/controller/beat_kibana.go

+ UserSecretSuffix: "beat-kb-user",
+ CASecretLabelName: kibana.KibanaNameLabelName,
+ ESUserRole: func(commonv1.Associated) (string, error) {
+ return user.KibanaAdminBuiltinRole, nil


Will this work with <7.5 version of Kibana? I don't see it in the docs.

Good point. I forgot about that.

david-kow · 2020-06-12T05:22:24Z

pkg/controller/beat/common/config.go

+ "username": username,
+ "password": password,
+ },
+ }


Should we consider addingsetup.dashboards.retry.enabled (docs) to avoid crashing?

Not sure: it would hide this unpleasant startup behaviour but it could hide an actual misconfiguration by never failing. Having said that given the low priority of the auto-install dashboards feature I am happy to include it.

That's true, but with this change it will be a common behavior between ES and Kibana - if someone configures output incorrectly Beat won't fail either. We can try to come up with a common solution as a part of #3197.

Turns out there is a bug in libbeat that prevents the retry from actually working elastic/beats#12283 because a Kibana Client is created before the retry logic starts and does a first request that fails.

https://github.com/elastic/beats/blob/0c15394a85134e1797d30592cd08789c4c1a8e1d/libbeat/cmd/instance/beat.go#L782

david-kow

LGTM!

pebrc added 2 commits June 10, 2020 09:29

Add KibanaRef to Beats

6160a3b

Only add Kibana config for Beats that support it

b526853

botelastic bot added the triage label Jun 10, 2020

regenerate api docs

b266156

pebrc force-pushed the add-kibana-ref-beats branch from ce20b9a to b266156 Compare June 10, 2020 08:23

pebrc added 2 commits June 10, 2020 10:25

fix typo, reorder

dd13b70

fix godoc

81a19e6

david-kow reviewed Jun 10, 2020

View reviewed changes

pebrc added the >enhancement Enhancement of existing functionality label Jun 10, 2020

botelastic bot removed the triage label Jun 10, 2020

pebrc added the v1.2.0 label Jun 10, 2020

david-kow reviewed Jun 10, 2020

View reviewed changes

pkg/controller/beat/common/pod.go Outdated Show resolved Hide resolved

pebrc added 4 commits June 10, 2020 14:43

review input

8768dee

inline constant

ccf960a

use Kibana admin role

aeff078

add unit test for BuildKibanaConfig

3565513

pebrc marked this pull request as ready for review June 10, 2020 16:21

pebrc added the :beats label Jun 10, 2020

anyasabo reviewed Jun 10, 2020

View reviewed changes

anyasabo reviewed Jun 11, 2020

View reviewed changes

anyasabo mentioned this pull request Jun 11, 2020

Remove config defaults and Beat RBAC management #3214

Merged

anyasabo approved these changes Jun 11, 2020

View reviewed changes

david-kow reviewed Jun 12, 2020

View reviewed changes

make kibana user version specific

23ee22d

pebrc requested a review from david-kow June 12, 2020 07:14

david-kow approved these changes Jun 12, 2020

View reviewed changes

pebrc mentioned this pull request Jun 12, 2020

E2E test for Beats Kibana Dashboard Setup #3230

Closed

contextualize log statement

0e07422

pebrc merged commit b5dc516 into elastic:master Jun 12, 2020

david-kow mentioned this pull request Jun 17, 2020

Auto-create dashboards in the default metricbeat/filebeat config #3192

Closed

charith-elastic added the exclude-from-release-notes Exclude this PR from appearing in the release notes label Jun 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add KibanaRef to Beats and support setup.kibana #3211

Add KibanaRef to Beats and support setup.kibana #3211

pebrc commented Jun 10, 2020 •

edited

Loading

david-kow left a comment

david-kow Jun 10, 2020

pebrc Jun 10, 2020

pebrc Jun 10, 2020 •

edited

Loading

david-kow Jun 10, 2020

pebrc Jun 10, 2020

david-kow Jun 10, 2020

anyasabo Jun 10, 2020

pebrc Jun 12, 2020

david-kow Jun 10, 2020

pebrc Jun 10, 2020

anyasabo Jun 10, 2020

anyasabo Jun 10, 2020 •

edited

Loading

anyasabo commented Jun 10, 2020

anyasabo Jun 11, 2020

anyasabo Jun 11, 2020

anyasabo left a comment

david-kow Jun 12, 2020

pebrc Jun 12, 2020

david-kow Jun 12, 2020

pebrc Jun 12, 2020

david-kow Jun 12, 2020

pebrc Jun 12, 2020 •

edited

Loading

david-kow left a comment

	// BuildKibanaConfig builds on optional Kibana configuration for dashboard setup and visualizations.
	// BuildKibanaConfig builds an optional Kibana configuration for dashboard setup and visualizations.

Add KibanaRef to Beats and support setup.kibana #3211

Add KibanaRef to Beats and support setup.kibana #3211

Conversation

pebrc commented Jun 10, 2020 • edited Loading

david-kow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pebrc Jun 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anyasabo Jun 10, 2020 • edited Loading

Choose a reason for hiding this comment

anyasabo commented Jun 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anyasabo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pebrc Jun 12, 2020 • edited Loading

Choose a reason for hiding this comment

david-kow left a comment

Choose a reason for hiding this comment

pebrc commented Jun 10, 2020 •

edited

Loading

pebrc Jun 10, 2020 •

edited

Loading

anyasabo Jun 10, 2020 •

edited

Loading

pebrc Jun 12, 2020 •

edited

Loading