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

Cherry-pick to 7.12: [Heartbeat] Produce error rather than panic on missing source (#24404) #24563

Merged
merged 3 commits into from
Mar 18, 2021

Conversation

andrewvc
Copy link
Contributor

Backports the following commits to 7.12:

…c#24404)

Fixes elastic#24403.

With the changes to the heartbeat config syntax in 7.12 the `source`
field is now required. Our config validation code didn't actually check
for this field's presence, which caused an NPE.

This PR adds a validation checking for that config's presence. It also
adds tests for the validation code for config sub-fields. There were no
defects found in the validations for source.inline, or source.browser,
but a few tests were missing.

Instead of the panic seen in elastic#24403 users will now get the error seen
below.

```
2021-03-05T15:41:40.146-0600	ERROR	instance/beat.go:952	Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
```
@andrewvc andrewvc self-assigned this Mar 16, 2021
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 16, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 16, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #24563 updated

  • Start Time: 2021-03-18T00:52:11.692+0000

  • Duration: 34 min 17 sec

  • Commit: 5110079

Test stats 🧪

Test Results
Failed 0
Passed 49
Skipped 0
Total 49

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 49
Skipped 0
Total 49

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

lgtm

@andrewvc andrewvc added bug Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels Mar 18, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (Team:Uptime)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 18, 2021
@andrewvc andrewvc merged commit 08e2048 into elastic:7.12 Mar 18, 2021
@andrewvc andrewvc deleted the backport/7.12/pr-24404 branch March 18, 2021 01:29
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…c#24404) (elastic#24563)

Fixes elastic#24403.

With the changes to the heartbeat config syntax in 7.12 the `source`
field is now required. Our config validation code didn't actually check
for this field's presence, which caused an NPE.

This PR adds a validation checking for that config's presence. It also
adds tests for the validation code for config sub-fields. There were no
defects found in the validations for source.inline, or source.browser,
but a few tests were missing.

Instead of the panic seen in elastic#24403 users will now get the error seen
below.

```
2021-03-05T15:41:40.146-0600	ERROR	instance/beat.go:952	Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
Exiting: could not create monitor: job err could not parse suite config: config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs accessing 'heartbeat.monitors.0' (source:'sample-synthetics-config/heartbeat.yml')
```

Co-authored-by: Blake Rouse <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport bug Team:obs-ds-hosted-services Label for the Observability Hosted Services team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants