Fix bug in Config.Update (#149)

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.

Author: yashtewari

config/config.go

@@ -63,11 +63,33 @@ func New(cfg *common.Config) (Config, error) {
 	return c, nil
 }
 
+// Update 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
 }
 

config/config_test.go

@@ -23,6 +23,7 @@ package config
 import (
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/elastic/beats/v7/libbeat/common"
 	"github.com/stretchr/testify/suite"
@@ -41,8 +42,8 @@ func (s *ConfigTestSuite) SetupTest() {
 
 func (s *ConfigTestSuite) TestNew() {
 	var tests = []struct {
-		config           string
-		expectedPatterns []string
+		config   string
+		expected []string
 	}{
 		{
 			`
@@ -54,8 +55,9 @@ func (s *ConfigTestSuite) TestNew() {
             - b
             - c
             - d
+            - e
 `,
-			[]string{"a", "b", "c", "d"},
+			[]string{"a", "b", "c", "d", "e"},
 		},
 	}
 
@@ -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 YAML 1.1 things
+                    - 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: 4h`,
+			expectedPeriod:     4 * time.Hour,
+			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:     4 * time.Hour,
+			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
 	}{
 		{
 			`
@@ -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))
 	}
 }