Skip to content

Commit

Permalink
Give precedence to monitoring reporter hosts over output hosts (#17991)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ycombinator committed May 6, 2020
1 parent b1fdd94 commit 34febb9
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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*

Expand Down
58 changes: 55 additions & 3 deletions libbeat/monitoring/report/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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{}

Expand Down Expand Up @@ -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 {
Expand All @@ -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
}

Expand Down Expand Up @@ -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
}
78 changes: 78 additions & 0 deletions libbeat/monitoring/report/report_test.go
Original file line number Diff line number Diff line change
@@ -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})
}

0 comments on commit 34febb9

Please sign in to comment.