From ab2713caf792e4ff8eac5cf63f507f590c55f0a2 Mon Sep 17 00:00:00 2001 From: Andrew Cholakian Date: Tue, 16 Mar 2021 07:58:36 -0500 Subject: [PATCH] [Heartbeat] Produce error rather than panic on missing source (#24404) Fixes #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 #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') ``` --- x-pack/heartbeat/monitors/browser/config.go | 5 ++ .../heartbeat/monitors/browser/config_test.go | 55 +++++++++++++++++++ .../monitors/browser/source/inline.go | 4 +- .../monitors/browser/source/inline_test.go | 42 ++++++++++++++ .../monitors/browser/source/source.go | 2 +- .../monitors/browser/source/source_test.go | 42 ++++++++++++++ 6 files changed, 148 insertions(+), 2 deletions(-) create mode 100644 x-pack/heartbeat/monitors/browser/config_test.go create mode 100644 x-pack/heartbeat/monitors/browser/source/inline_test.go create mode 100644 x-pack/heartbeat/monitors/browser/source/source_test.go diff --git a/x-pack/heartbeat/monitors/browser/config.go b/x-pack/heartbeat/monitors/browser/config.go index 1615db1a331..de226f8ca79 100644 --- a/x-pack/heartbeat/monitors/browser/config.go +++ b/x-pack/heartbeat/monitors/browser/config.go @@ -31,6 +31,7 @@ type Config struct { var ErrNameRequired = fmt.Errorf("config 'name' must be specified for this monitor") var ErrIdRequired = fmt.Errorf("config 'id' must be specified for this monitor") +var ErrSourceRequired = fmt.Errorf("config 'source' must be specified for this monitor, if upgrading from a previous experimental version please see our new config docs") func (c *Config) Validate() error { if c.Name == "" { @@ -40,5 +41,9 @@ func (c *Config) Validate() error { return ErrIdRequired } + if c.Source == nil { + return ErrSourceRequired + } + return nil } diff --git a/x-pack/heartbeat/monitors/browser/config_test.go b/x-pack/heartbeat/monitors/browser/config_test.go new file mode 100644 index 00000000000..f1a8cab8565 --- /dev/null +++ b/x-pack/heartbeat/monitors/browser/config_test.go @@ -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) + } + }) + } +} diff --git a/x-pack/heartbeat/monitors/browser/source/inline.go b/x-pack/heartbeat/monitors/browser/source/inline.go index d3ef2c452e5..abdce2a87e9 100644 --- a/x-pack/heartbeat/monitors/browser/source/inline.go +++ b/x-pack/heartbeat/monitors/browser/source/inline.go @@ -14,9 +14,11 @@ type InlineSource struct { BaseSource } +var ErrNoInlineScript = fmt.Errorf("no 'script' value specified for inline source") + func (s *InlineSource) Validate() error { if !regexp.MustCompile("\\S").MatchString(s.Script) { - return fmt.Errorf("no 'script' value specified for inline source") + return ErrNoInlineScript } return nil diff --git a/x-pack/heartbeat/monitors/browser/source/inline_test.go b/x-pack/heartbeat/monitors/browser/source/inline_test.go new file mode 100644 index 00000000000..d97dbd8a120 --- /dev/null +++ b/x-pack/heartbeat/monitors/browser/source/inline_test.go @@ -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) + } + }) + } +} diff --git a/x-pack/heartbeat/monitors/browser/source/source.go b/x-pack/heartbeat/monitors/browser/source/source.go index 62c7ce03e9e..c92b0c3792b 100644 --- a/x-pack/heartbeat/monitors/browser/source/source.go +++ b/x-pack/heartbeat/monitors/browser/source/source.go @@ -28,7 +28,7 @@ func (s *Source) Active() ISource { return s.ActiveMemo } -var ErrInvalidSource = fmt.Errorf("no or unknown source type specified for synthetic monitor") +var ErrInvalidSource = fmt.Errorf("no or unknown source type specified for synthetic monitor.") func (s *Source) Validate() error { if s.Active() == nil { diff --git a/x-pack/heartbeat/monitors/browser/source/source_test.go b/x-pack/heartbeat/monitors/browser/source/source_test.go new file mode 100644 index 00000000000..f8e9e687915 --- /dev/null +++ b/x-pack/heartbeat/monitors/browser/source/source_test.go @@ -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) + } + }) + } +}