Skip to content

Commit

Permalink
Fix bug in Config.Update
Browse files Browse the repository at this point in the history
Incoming config updates with slices smaller than the current slice
didn't resize the current slice, resulting in undesirable values
in the tail of the slice.
  • Loading branch information
yashtewari committed May 16, 2022
1 parent e061084 commit 8ab2289
Show file tree
Hide file tree
Showing 2 changed files with 220 additions and 8 deletions.
22 changes: 22 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,33 @@ func New(cfg *common.Config) (Config, error) {
return c, nil
}

// Update replaces replaces values of those keys in the current config which are
// present in the incoming config.
//
// NOTE(yashtewari): This will be removed with the planned update to restart the
// beat with the new config.
func (c *Config) Update(cfg *common.Config) error {
if err := cfg.Unpack(&c); err != nil {
return err
}

// Check if the incoming config has streams.
m := make(map[string]interface{})
if err := cfg.Unpack(&m); err != nil {
return err
}

if _, ok := m["streams"]; !ok {
return nil
}

uc, err := New(cfg)
if err != nil {
return err
}

c.Streams = uc.Streams

return nil
}

Expand Down
206 changes: 198 additions & 8 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package config
import (
"strings"
"testing"
"time"

"github.com/elastic/beats/v7/libbeat/common"
"github.com/stretchr/testify/suite"
Expand All @@ -41,8 +42,8 @@ func (s *ConfigTestSuite) SetupTest() {

func (s *ConfigTestSuite) TestNew() {
var tests = []struct {
config string
expectedPatterns []string
config string
expected []string
}{
{
`
Expand All @@ -54,8 +55,9 @@ func (s *ConfigTestSuite) TestNew() {
- b
- c
- d
- e
`,
[]string{"a", "b", "c", "d"},
[]string{"a", "b", "c", "d", "e"},
},
}

Expand All @@ -66,14 +68,202 @@ func (s *ConfigTestSuite) TestNew() {
c, err := New(cfg)
s.NoError(err)

s.Equal(test.expectedPatterns, c.Streams[0].DataYaml.ActivatedRules.CISK8S)
s.Equal(test.expected, c.Streams[0].DataYaml.ActivatedRules.CISK8S)
}
}

func (s *ConfigTestSuite) TestConfigUpdate() {
config := `
streams:
- data_yaml:
activated_rules:
cis_k8s:
- a
- b
- c
- d
- e`

var tests = []struct {
update string
expected []string
}{
{
`
streams:
- data_yaml:
activated_rules:
cis_k8s:
- a
- b
- c
- d
`,
[]string{"a", "b", "c", "d"},
},
{
`
streams:
- data_yaml:
activated_rules:
cis_k8s:
- b
- c
- d
- e
- f
`,
[]string{"b", "c", "d", "e", "f"},
},
{
`
streams:
- data_yaml:
activated_rules:
cis_k8s: []
`,
[]string{},
},
{
`
streams:
- data_yaml:
activated_rules:
cis_k8s:
- a
- b
- c
- d
- e
`,
[]string{"a", "b", "c", "d", "e"},
},
// We're not currently aware of a scenario where we receive
// multiple input streams, but still to make sure that it doesn't
// break for this scenario.
{
`
streams:
- data_yaml:
activated_rules:
cis_k8s:
- x
- "y" # Just 'y' in YAML is unmarshaled to the boolean true????????
- z
- data_yaml:
activated_rules:
- f
- g
- h
- i
- j
- k
`,
[]string{"x", "y", "z"},
},
}

cfg, err := common.NewConfigFrom(config)
s.NoError(err)

c, err := New(cfg)
s.NoError(err)

for _, test := range tests {
cfg, err := common.NewConfigFrom(test.update)
s.NoError(err)

err = c.Update(cfg)
s.NoError(err)

s.Equal(test.expected, c.Streams[0].DataYaml.ActivatedRules.CISK8S)
}
}

// TestConfigUpdateIsolated tests whether updates made to a config from
// are isolated; only those parts of the config specified in the incoming
// config should get updated.
func (s *ConfigTestSuite) TestConfigUpdateIsolated() {
config := `
period: 10s
kube_config: some_path
streams:
- data_yaml:
activated_rules:
cis_k8s:
- a
- b
- c
- d
- e`

var tests = []struct {
update string
expectedPeriod time.Duration
expectedKubeConfig string
expectedCISK8S []string
}{
{
update: `
streams:
- data_yaml:
activated_rules:
cis_k8s:
- a
- b
- c
- d`,
expectedPeriod: 10 * time.Second,
expectedKubeConfig: "some_path",
expectedCISK8S: []string{"a", "b", "c", "d"},
},
{
update: `period: 30s`,
expectedPeriod: 30 * time.Second,
expectedKubeConfig: "some_path",
expectedCISK8S: []string{"a", "b", "c", "d"},
},
{
update: `
kube_config: some_other_path
streams:
- data_yaml:
activated_rules:
cis_k8s:
- a
- b
- c
- d
- e`,
expectedPeriod: 30 * time.Second,
expectedKubeConfig: "some_other_path",
expectedCISK8S: []string{"a", "b", "c", "d", "e"},
},
}

cfg, err := common.NewConfigFrom(config)
s.NoError(err)

c, err := New(cfg)
s.NoError(err)

for _, test := range tests {
cfg, err := common.NewConfigFrom(test.update)
s.NoError(err)

err = c.Update(cfg)
s.NoError(err)

s.Equal(test.expectedPeriod, c.Period)
s.Equal(test.expectedKubeConfig, c.KubeConfig)
s.Equal(test.expectedCISK8S, c.Streams[0].DataYaml.ActivatedRules.CISK8S)
}
}

func (s *ConfigTestSuite) TestDataYaml() {
func (s *ConfigTestSuite) TestConfigDataYaml() {
var tests = []struct {
config string
expectedYaml string
config string
expected string
}{
{
`
Expand Down Expand Up @@ -107,6 +297,6 @@ activated_rules:
dy, err := c.DataYaml()
s.NoError(err)

s.Equal(strings.TrimSpace(test.expectedYaml), strings.TrimSpace(dy))
s.Equal(strings.TrimSpace(test.expected), strings.TrimSpace(dy))
}
}

0 comments on commit 8ab2289

Please sign in to comment.