forked from elastic/beats
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Heartbeat] Produce error rather than panic on missing source
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') ```
- Loading branch information
Showing
6 changed files
with
148 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
// or more contributor license agreements. Licensed under the Elastic License; | ||
// you may not use this file except in compliance with the Elastic License. | ||
|
||
package browser | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/elastic/beats/v7/x-pack/heartbeat/monitors/browser/source" | ||
) | ||
|
||
func TestConfig_Validate(t *testing.T) { | ||
testSource := source.Source{Inline: &source.InlineSource{Script: "//something"}} | ||
|
||
tests := []struct { | ||
name string | ||
cfg *Config | ||
wantErr error | ||
}{ | ||
{ | ||
"no error", | ||
&Config{Id: "myid", Name: "myname", Source: &testSource}, | ||
nil, | ||
}, | ||
{ | ||
"no id", | ||
&Config{Name: "myname", Source: &testSource}, | ||
ErrIdRequired, | ||
}, | ||
{ | ||
"no name", | ||
&Config{Id: "myid", Source: &testSource}, | ||
ErrNameRequired, | ||
}, | ||
{ | ||
"no source", | ||
&Config{Id: "myid", Name: "myname"}, | ||
ErrSourceRequired, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
err := tt.cfg.Validate() | ||
|
||
if tt.wantErr != nil { | ||
require.Equal(t, tt.wantErr, err) | ||
} else { | ||
require.NoError(t, err) | ||
} | ||
}) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
// or more contributor license agreements. Licensed under the Elastic License; | ||
// you may not use this file except in compliance with the Elastic License. | ||
|
||
package source | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestInlineSourceValidation(t *testing.T) { | ||
type testCase struct { | ||
name string | ||
source *InlineSource | ||
wantErr error | ||
} | ||
testCases := []testCase{ | ||
{ | ||
"no error", | ||
&InlineSource{Script: "a script"}, | ||
nil, | ||
}, | ||
{ | ||
"no script", | ||
&InlineSource{}, | ||
ErrNoInlineScript, | ||
}, | ||
} | ||
|
||
for _, tt := range testCases { | ||
t.Run(tt.name, func(t *testing.T) { | ||
err := tt.source.Validate() | ||
if tt.wantErr != nil { | ||
require.Equal(t, tt.wantErr, err) | ||
} else { | ||
require.NoError(t, err) | ||
} | ||
}) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
// or more contributor license agreements. Licensed under the Elastic License; | ||
// you may not use this file except in compliance with the Elastic License. | ||
|
||
package source | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestSourceValidation(t *testing.T) { | ||
type testCase struct { | ||
name string | ||
source Source | ||
wantErr error | ||
} | ||
testCases := []testCase{ | ||
{ | ||
"no error", | ||
Source{Inline: &InlineSource{}}, | ||
nil, | ||
}, | ||
{ | ||
"no concrete source", | ||
Source{}, | ||
ErrInvalidSource, | ||
}, | ||
} | ||
|
||
for _, tt := range testCases { | ||
t.Run(tt.name, func(t *testing.T) { | ||
err := tt.source.Validate() | ||
if tt.wantErr != nil { | ||
require.Equal(t, tt.wantErr, err) | ||
} else { | ||
require.NoError(t, err) | ||
} | ||
}) | ||
} | ||
} |