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

Error out on invalid Autodiscover template conditions settings #7200

Merged
merged 2 commits into from
May 29, 2018

Conversation

exekias
Copy link
Contributor

@exekias exekias commented May 29, 2018

This should fix #7183

@exekias exekias added in progress Pull request is currently in progress. libbeat labels May 29, 2018
@@ -54,7 +56,7 @@ func NewAutodiscover(name string, pipeline beat.Pipeline, adapter Adapter, confi
for _, providerCfg := range config.Providers {
provider, err := Registry.BuildProvider(bus, providerCfg)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "Error in autodiscover provider settings")
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Error/error

@exekias exekias force-pushed the autodiscover-error-bad-condition branch 5 times, most recently from f3ee3a6 to 280b257 Compare May 29, 2018 14:53
@exekias exekias force-pushed the autodiscover-error-bad-condition branch from 280b257 to c7c8e02 Compare May 29, 2018 14:56
@exekias exekias added review and removed in progress Pull request is currently in progress. labels May 29, 2018
@exekias
Copy link
Contributor Author

exekias commented May 29, 2018

This one should be ready for review

@ph
Copy link
Contributor

ph commented May 29, 2018

@exekias Is there a way to trigger the bug via a unit test?

@exekias exekias force-pushed the autodiscover-error-bad-condition branch from 8f2ce0b to 4b62b41 Compare May 29, 2018 17:47
@exekias
Copy link
Contributor Author

exekias commented May 29, 2018

👍 added some more tests

Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM waiting for the 💚

@ph ph merged commit 304a683 into elastic:master May 29, 2018
@tsg tsg added the v6.4.0 label Jul 26, 2018
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.

Empty condition in configuration crashes beats
3 participants