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

Add dashboard test loading for auditbeat #5938

Merged
merged 6 commits into from
Mar 15, 2018

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Dec 20, 2017

In combination with adding the dashboard loading a libbeat.yml.j2 was created so all beats can share the common config options and we do not have to add each option for each beat.

So far this is applied to metricbeat, auditbeat and filebeat. Other beats will follow in a follow up PR.

@ruflin
Copy link
Contributor Author

ruflin commented Dec 20, 2017

@elastic/apm-server Could be useful for apm-server too.

es = Elasticsearch([self.get_elasticsearch_url()])
self.render_config_template(
modules=[{
"name": "auditd",
Copy link
Member

@andrewkroh andrewkroh Dec 21, 2017

Choose a reason for hiding this comment

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

I don't think these test get executed because the config for the one above is out-of-date. You can enable them.

Let's use the file_integrity module instead of auditd because it can run on more platforms.

             modules=[{
                 "name": "file_integrity",
                 "extras": {
                     "paths": ["file.example"],
                 },

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to your suggested config. I assume the first paragraph was related to the second part with the new config to enable it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@exekias In general I wonder if we should require a module to run the setup command?

]

{% if geoip_paths is not none %}
geoip:
Copy link
Member

Choose a reason for hiding this comment

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

What's this for? AFAIK we don't have any builtin geoip enrichment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was copy paste from filebeat. Removed now as we don't use it anymore.

@@ -60,8 +60,10 @@ def test_dashboards(self):
es = Elasticsearch([self.get_elasticsearch_url()])
self.render_config_template(
modules=[{
"name": "auditd",
}],
"name": "file_integrity",
Copy link
Member

Choose a reason for hiding this comment

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

Can you fix the config for the one above too, please.

In auditbeat/Makefile I think SYSTEM_TESTS needs to be set to true for these to execute.

@ruflin
Copy link
Contributor Author

ruflin commented Dec 21, 2017

@andrewkroh I only realised now that the system tests we had were not running at all for auditbeat. So I enabled system tests + integration tests. I'm pushing a change now to see if everything works as expected but will squash and rebase again in case it goes green.

I also enabled the file_integrity module for all platforms, not only linux.

@ruflin
Copy link
Contributor Author

ruflin commented Dec 21, 2017

Opened #5945 This needs rebase as soon as the other PR is merged.

ruflin added 3 commits March 13, 2018 13:12
In combination with adding the dashboard loading a libbeat.yml.j2 was created so all beats can share the common config options and we do not have to add each option for each beat.

So far this is applied to metricbeat, auditbeat and filebeat. Other beats will follow in a follow up PR.
@ruflin ruflin force-pushed the add-dashboard-loading-tests branch from 9c80193 to 060f9a1 Compare March 13, 2018 12:12
]

{% if setup_template_name %}
setup.template.name: setup_template_name
Copy link
Member

Choose a reason for hiding this comment

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

Should these be a reference to the setup_template_name variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would say yes. It got copied from here: https://github.com/elastic/beats/pull/5938/files#diff-bfed2fbdbe1d791a97c6ca3a63e6a8deL152

Will fix it for both variables.

@ruflin
Copy link
Contributor Author

ruflin commented Mar 15, 2018

The failing test is a flaky test in k8s and should not be related.

@exekias ^

@andrewkroh andrewkroh merged commit 3f1e877 into elastic:master Mar 15, 2018
@ruflin ruflin deleted the add-dashboard-loading-tests branch March 15, 2018 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants