From 01e5c7df159662468198826aae5cc27ebf2968e7 Mon Sep 17 00:00:00 2001 From: Shaunak Kashyap Date: Mon, 4 May 2020 17:21:18 -0700 Subject: [PATCH] Give precedence to monitoring reporter hosts over output hosts (#17991) * Give precedence to monitoring reporter hosts over output hosts * Add CHANGELOG entry * Removing debugging statements * No delete * Helper function * Make new config if nil * Formatting code --- CHANGELOG.next.asciidoc | 1 + libbeat/monitoring/report/report.go | 58 +++++++++++++++++- libbeat/monitoring/report/report_test.go | 78 ++++++++++++++++++++++++ 3 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 libbeat/monitoring/report/report_test.go diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 3a6e740fb39..60efdced3d9 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -105,6 +105,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix Elasticsearch license endpoint URL referenced in error message. {issue}17880[17880] {pull}18030[18030] - Fix panic when assigning a key to a `nil` value in an event. {pull}18143[18143] - Change `decode_json_fields` processor, to merge parsed json objects with existing objects in the event instead of fully replacing them. {pull}17958[17958] +- Gives monitoring reporter hosts, if configured, total precedence over corresponding output hosts. {issue}17937[17937] {pull}17991[17991] *Auditbeat* diff --git a/libbeat/monitoring/report/report.go b/libbeat/monitoring/report/report.go index e6812515af9..0f79af4e874 100644 --- a/libbeat/monitoring/report/report.go +++ b/libbeat/monitoring/report/report.go @@ -21,6 +21,8 @@ import ( "errors" "fmt" + errw "github.com/pkg/errors" + "github.com/elastic/beats/v7/libbeat/beat" "github.com/elastic/beats/v7/libbeat/common" ) @@ -59,6 +61,10 @@ type Reporter interface { type ReporterFactory func(beat.Info, Settings, *common.Config) (Reporter, error) +type hostsCfg struct { + Hosts []string `config:"hosts"` +} + var ( defaultConfig = config{} @@ -111,9 +117,7 @@ func getReporterConfig( // merge reporter config with output config if both are present if outCfg := outputs.Config(); outputs.Name() == name && outCfg != nil { // require monitoring to not configure any hosts if output is configured: - hosts := struct { - Hosts []string `config:"hosts"` - }{} + hosts := hostsCfg{} rc.Unpack(&hosts) if settings.Format == FormatXPackMonitoringBulk && len(hosts.Hosts) > 0 { @@ -127,6 +131,13 @@ func getReporterConfig( if err != nil { return "", nil, err } + + // Make sure hosts from reporter configuration get precedence over hosts + // from output configuration + if err := mergeHosts(merged, outCfg, rc); err != nil { + return "", nil, err + } + rc = merged } @@ -155,3 +166,44 @@ func collectSubObject(cfg *common.Config) *common.Config { } return out } + +func mergeHosts(merged, outCfg, reporterCfg *common.Config) error { + if merged == nil { + merged = common.NewConfig() + } + + outputHosts := hostsCfg{} + if outCfg != nil { + if err := outCfg.Unpack(&outputHosts); err != nil { + return errw.Wrap(err, "unable to parse hosts from output config") + } + } + + reporterHosts := hostsCfg{} + if reporterCfg != nil { + if err := reporterCfg.Unpack(&reporterHosts); err != nil { + return errw.Wrap(err, "unable to parse hosts from reporter config") + } + } + + if len(outputHosts.Hosts) == 0 && len(reporterHosts.Hosts) == 0 { + return nil + } + + // Give precedence to reporter hosts over output hosts + var newHostsCfg *common.Config + var err error + if len(reporterHosts.Hosts) > 0 { + newHostsCfg, err = common.NewConfigFrom(reporterHosts.Hosts) + } else { + newHostsCfg, err = common.NewConfigFrom(outputHosts.Hosts) + } + if err != nil { + return errw.Wrap(err, "unable to make config from new hosts") + } + + if err := merged.SetChild("hosts", -1, newHostsCfg); err != nil { + return errw.Wrap(err, "unable to set new hosts into merged config") + } + return nil +} diff --git a/libbeat/monitoring/report/report_test.go b/libbeat/monitoring/report/report_test.go new file mode 100644 index 00000000000..45b0dadc83f --- /dev/null +++ b/libbeat/monitoring/report/report_test.go @@ -0,0 +1,78 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package report + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/elastic/beats/v7/libbeat/common" +) + +func TestMergeHosts(t *testing.T) { + tests := map[string]struct { + outCfg *common.Config + reporterCfg *common.Config + expectedCfg *common.Config + }{ + "no_hosts": { + expectedCfg: newConfigWithHosts(), + }, + "only_reporter_hosts": { + reporterCfg: newConfigWithHosts("r1", "r2"), + expectedCfg: newConfigWithHosts("r1", "r2"), + }, + "only_output_hosts": { + outCfg: newConfigWithHosts("o1", "o2"), + expectedCfg: newConfigWithHosts("o1", "o2"), + }, + "equal_hosts": { + outCfg: newConfigWithHosts("o1", "o2"), + reporterCfg: newConfigWithHosts("r1", "r2"), + expectedCfg: newConfigWithHosts("r1", "r2"), + }, + "more_output_hosts": { + outCfg: newConfigWithHosts("o1", "o2"), + reporterCfg: newConfigWithHosts("r1"), + expectedCfg: newConfigWithHosts("r1"), + }, + "more_reporter_hosts": { + outCfg: newConfigWithHosts("o1"), + reporterCfg: newConfigWithHosts("r1", "r2"), + expectedCfg: newConfigWithHosts("r1", "r2"), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + mergedCfg := common.MustNewConfigFrom(map[string]interface{}{}) + err := mergeHosts(mergedCfg, test.outCfg, test.reporterCfg) + require.NoError(t, err) + + require.Equal(t, test.expectedCfg, mergedCfg) + }) + } +} + +func newConfigWithHosts(hosts ...string) *common.Config { + if len(hosts) == 0 { + return common.MustNewConfigFrom(map[string][]string{}) + } + return common.MustNewConfigFrom(map[string][]string{"hosts": hosts}) +}