Fix merging Kubernetes configuration (#1253)

(cherry picked from commit 16b5f04)

internal/source/kubernetes/registration.go

@@ -141,19 +141,19 @@ func (r registration) matchEvent(routes []route, event event.Event) (bool, error
 	errs := multierror.New()
 	for _, rt := range routes {
 		// event reason
-		if rt.event != nil && rt.event.Reason.AreConstraintsDefined() {
-			match, err := rt.event.Reason.IsAllowed(event.Reason)
+		if rt.Event != nil && rt.Event.Reason.AreConstraintsDefined() {
+			match, err := rt.Event.Reason.IsAllowed(event.Reason)
 			if err != nil {
 				return false, err
 			}
 			if !match {
-				r.log.Debugf("Ignoring as reason %q doesn't match constraints %+v", event.Reason, rt.event.Reason)
+				r.log.Debugf("Ignoring as reason %q doesn't match constraints %+v", event.Reason, rt.Event.Reason)
 				return false, nil
 			}
 		}
 
 		// event message
-		if rt.event != nil && rt.event.Message.AreConstraintsDefined() {
+		if rt.Event != nil && rt.Event.Message.AreConstraintsDefined() {
 			var anyMsgMatches bool
 
 			eventMsgs := event.Messages
@@ -163,7 +163,7 @@ func (r registration) matchEvent(routes []route, event event.Event) (bool, error
 			}
 
 			for _, msg := range eventMsgs {
-				match, err := rt.event.Message.IsAllowed(msg)
+				match, err := rt.Event.Message.IsAllowed(msg)
 				if err != nil {
 					return false, err
 				}
@@ -173,42 +173,42 @@ func (r registration) matchEvent(routes []route, event event.Event) (bool, error
 				}
 			}
 			if !anyMsgMatches {
-				r.log.Debugf("Ignoring as any event message from %q doesn't match constraints %+v", strings.Join(event.Messages, ";"), rt.event.Message)
+				r.log.Debugf("Ignoring as any event message from %q doesn't match constraints %+v", strings.Join(event.Messages, ";"), rt.Event.Message)
 				return false, nil
 			}
 		}
 
 		// resource name
-		if rt.resourceName.AreConstraintsDefined() {
-			allowed, err := rt.resourceName.IsAllowed(event.Name)
+		if rt.ResourceName.AreConstraintsDefined() {
+			allowed, err := rt.ResourceName.IsAllowed(event.Name)
 			if err != nil {
 				return false, err
 			}
 			if !allowed {
-				r.log.Debugf("Ignoring as resource name %q doesn't match constraints %+v", event.Name, rt.resourceName)
+				r.log.Debugf("Ignoring as resource name %q doesn't match constraints %+v", event.Name, rt.ResourceName)
 				return false, nil
 			}
 		}
 
 		// namespace
-		if rt.namespaces != nil && rt.namespaces.AreConstraintsDefined() {
-			match, err := rt.namespaces.IsAllowed(event.Namespace)
+		if rt.Namespaces != nil && rt.Namespaces.AreConstraintsDefined() {
+			match, err := rt.Namespaces.IsAllowed(event.Namespace)
 			if err != nil {
 				return false, err
 			}
 			if !match {
-				r.log.Debugf("Ignoring as namespace %q doesn't match constraints %+v", event.Namespace, rt.namespaces)
+				r.log.Debugf("Ignoring as namespace %q doesn't match constraints %+v", event.Namespace, rt.Namespaces)
 				return false, nil
 			}
 		}
 
 		// annotations
-		if !kvsSatisfiedForMap(rt.annotations, event.ObjectMeta.Annotations) {
+		if !kvsSatisfiedForMap(rt.Annotations, event.ObjectMeta.Annotations) {
 			continue
 		}
 
 		// labels
-		if !kvsSatisfiedForMap(rt.labels, event.ObjectMeta.Labels) {
+		if !kvsSatisfiedForMap(rt.Labels, event.ObjectMeta.Labels) {
 			continue
 		}
 		return true, nil
@@ -309,26 +309,26 @@ func (r registration) qualifyEventForUpdate(
 			continue
 		}
 
-		if route.updateSetting == nil {
+		if route.UpdateSetting == nil {
 			// in theory this should never happen, as we check if it is not nil in `route.hasActionableUpdateSetting()`, but just in case
 			r.log.Debugf("Nil updateSetting but hasActionableUpdateSetting returned true for route: %v. This looks like a bug...", route)
 			continue
 		}
 
 		r.log.WithFields(logrus.Fields{"old": oldUnstruct.Object, "new": newUnstruct.Object}).Debug("Getting diff for objects...")
-		diff, err := k8sutil.Diff(oldUnstruct.Object, newUnstruct.Object, *route.updateSetting)
+		diff, err := k8sutil.Diff(oldUnstruct.Object, newUnstruct.Object, *route.UpdateSetting)
 		if err != nil {
 			r.log.Errorf("while getting diff: %w", err)
 		}
-		r.log.Debugf("About to qualify event for route: %v for update, diff: %s, updateSetting: %+v", route, diff, route.updateSetting)
+		r.log.Debugf("About to qualify event for route: %v for update, diff: %s, updateSetting: %+v", route, diff, route.UpdateSetting)
 
-		if route.updateSetting.IncludeDiff {
+		if route.UpdateSetting.IncludeDiff {
 			diffs = append(diffs, diff)
 		}
 
 		if len(diff) > 0 {
 			result = true
-			r.log.Debugf("Qualified for update: route: %v for update, diff: %s, updateSetting: %+v", route, diff, route.updateSetting)
+			r.log.Debugf("Qualified for update: route: %v for update, diff: %s, updateSetting: %+v", route, diff, route.UpdateSetting)
 		}
 	}
 

internal/source/kubernetes/router.go

@@ -20,21 +20,21 @@ type registrationHandler func(resource string) (cache.SharedIndexInformer, error
 type eventHandler func(ctx context.Context, source Source, event event.Event, updateDiffs []string)
 
 type route struct {
-	resourceName  config.RegexConstraints
-	labels        *map[string]string
-	annotations   *map[string]string
-	namespaces    *config.RegexConstraints
-	updateSetting *config.UpdateSetting
-	event         *config.KubernetesEvent
+	ResourceName  config.RegexConstraints
+	Labels        *map[string]string
+	Annotations   *map[string]string
+	Namespaces    *config.RegexConstraints
+	UpdateSetting *config.UpdateSetting
+	Event         *config.KubernetesEvent
 }
 
 func (r route) hasActionableUpdateSetting() bool {
-	return r.updateSetting != nil && len(r.updateSetting.Fields) > 0
+	return r.UpdateSetting != nil && len(r.UpdateSetting.Fields) > 0
 }
 
 type entry struct {
-	event  config.EventType
-	routes []route
+	Event  config.EventType
+	Routes []route
 }
 
 // Router maintains handled event types from registered
@@ -66,7 +66,7 @@ func (r *Router) BuildTable(cfg *config.Config) *Router {
 	for resource, resourceEvents := range mergedEvents {
 		eventRoutes := r.mergeEventRoutes(resource, cfg)
 		for evt := range resourceEvents {
-			r.table[resource] = append(r.table[resource], entry{event: evt, routes: eventRoutes[evt]})
+			r.table[resource] = append(r.table[resource], entry{Event: evt, Routes: eventRoutes[evt]})
 		}
 	}
 	r.log.Debugf("routing table: %+v", r.table)
@@ -169,25 +169,26 @@ func mergeResourceEvents(cfg *config.Config) mergedEvents {
 
 func (r *Router) mergeEventRoutes(resource string, cfg *config.Config) map[config.EventType][]route {
 	out := make(map[config.EventType][]route)
-	for _, r := range cfg.Resources {
+	for idx := range cfg.Resources {
+		r := cfg.Resources[idx] // make sure that we work on a copy
 		for _, e := range flattenEventTypes(cfg.Event.Types, r.Event.Types) {
 			if resource != r.Type {
 				continue
 			}
-
 			route := route{
-				namespaces:   resourceNamespaces(cfg.Namespaces, &r.Namespaces),
-				annotations:  resourceStringMap(cfg.Annotations, r.Annotations),
-				labels:       resourceStringMap(cfg.Labels, r.Labels),
-				resourceName: r.Name,
-				event:        resourceEvent(*cfg.Event, r.Event),
+				Namespaces:   resourceNamespaces(cfg.Namespaces, &r.Namespaces),
+				Annotations:  resourceStringMap(cfg.Annotations, r.Annotations),
+				Labels:       resourceStringMap(cfg.Labels, r.Labels),
+				ResourceName: r.Name,
+				Event:        resourceEvent(*cfg.Event, r.Event),
 			}
 			if e == config.UpdateEvent {
-				route.updateSetting = &config.UpdateSetting{
+				route.UpdateSetting = &config.UpdateSetting{
 					Fields:      r.UpdateSetting.Fields,
 					IncludeDiff: r.UpdateSetting.IncludeDiff,
 				}
 			}
+
 			out[e] = append(out[e], route)
 		}
 	}
@@ -211,8 +212,8 @@ func (r *Router) setEventRouteForRecommendationsIfShould(routeMap *map[config.Ev
 	}
 
 	recommRoute := route{
-		namespaces: cfg.Namespaces,
-		event: &config.KubernetesEvent{
+		Namespaces: cfg.Namespaces,
+		Event: &config.KubernetesEvent{
 			Reason:  config.RegexConstraints{},
 			Message: config.RegexConstraints{},
 			Types:   nil,
@@ -222,7 +223,7 @@ func (r *Router) setEventRouteForRecommendationsIfShould(routeMap *map[config.Ev
 	// Override route and get all these events for all namespaces.
 	// The events without recommendations will be filtered out when sending the event.
 	for i, r := range (*routeMap)[eventType] {
-		recommRoute.namespaces = resourceNamespaces(cfg.Namespaces, r.namespaces)
+		recommRoute.Namespaces = resourceNamespaces(cfg.Namespaces, r.Namespaces)
 		(*routeMap)[eventType][i] = recommRoute
 		return
 	}
@@ -234,8 +235,8 @@ func (r *Router) setEventRouteForRecommendationsIfShould(routeMap *map[config.Ev
 func eventRoutes(routeTable map[string][]entry, targetResource string, targetEvent config.EventType) []route {
 	var out []route
 	for _, routedEvent := range routeTable[targetResource] {
-		if routedEvent.event == targetEvent {
-			out = append(out, routedEvent.routes...)
+		if routedEvent.Event == targetEvent {
+			out = append(out, routedEvent.Routes...)
 		}
 	}
 	return out
@@ -244,7 +245,7 @@ func eventRoutes(routeTable map[string][]entry, targetResource string, targetEve
 func (r *Router) resourceEvents(resource string) []config.EventType {
 	var out []config.EventType
 	for _, routedEvent := range r.table[resource] {
-		out = append(out, routedEvent.event)
+		out = append(out, routedEvent.Event)
 	}
 	return out
 }
@@ -254,7 +255,7 @@ func (r *Router) resourcesForEvents(targets []config.EventType) []string {
 	for _, target := range targets {
 		for resource, routedEvents := range r.table {
 			for _, routedEvent := range routedEvents {
-				if routedEvent.event == target {
+				if routedEvent.Event == target {
 					out = append(out, resource)
 					break
 				}

internal/source/kubernetes/router_test.go

@@ -1,9 +1,16 @@
 package kubernetes
 
 import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
+	"gotest.tools/v3/golden"
 
 	"github.com/kubeshop/botkube/internal/loggerx"
 	"github.com/kubeshop/botkube/internal/source/kubernetes/config"
@@ -61,3 +68,26 @@ func TestRouter_BuildTable_CreatesRoutesWithProperEventsList(t *testing.T) {
 		})
 	}
 }
+
+func TestRouterListMergingNestedFields(t *testing.T) {
+	// given
+	router := NewRouter(nil, nil, loggerx.NewNoop())
+
+	var cfg config.Config
+	fixConfig, err := os.ReadFile(filepath.Join("testdata", t.Name(), "override-fields-config.yaml"))
+	require.NoError(t, err)
+
+	err = yaml.Unmarshal(fixConfig, &cfg)
+	require.NoError(t, err)
+
+	// when
+	router = router.BuildTable(&cfg)
+
+	// then
+	for key := range router.table {
+		out, err := yaml.Marshal(router.table[key])
+		require.NoError(t, err)
+		filename := fmt.Sprintf("route-%s.golden.yaml", strings.ReplaceAll(key, "/", "."))
+		golden.Assert(t, string(out), filepath.Join(t.Name(), filename))
+	}
+}

internal/source/kubernetes/testdata/TestRouterListMergingNestedFields/override-fields-config.yaml

@@ -0,0 +1,44 @@
+event:
+  types:
+    - delete
+  reason:
+    include: ["reason-include-1-level"]
+    exclude: ["reason-exclude-1-level"]
+  message:
+    include: ["message-include-1-level"]
+    exclude: ["message-exclude-1-level"]
+
+annotations:
+  test: "annotation-1-level"
+
+labels:
+  test: "label-1-level"
+
+namespaces:
+  include: ["namespace-include-1-level"]
+  exclude: ["namespace-exclude-1-level"]
+
+resources:
+  - type: v1/configmaps # change all nested properties
+    namespaces:
+      include: ["namespace-include-2-level"]
+      exclude: ["namespace-exclude-2-level"]
+    event:
+      types:
+        - create
+      reason:
+        include: ["reason-include-2-level"]
+        exclude: ["reason-exclude-2-level"]
+      message:
+        include: ["message-include-2-level"]
+        exclude: ["message-exclude-2-level"]
+    annotations:
+      test: "annotation-2-level"
+    labels:
+      test: "label-2-level"
+
+  - type: apps/v1/deployments # change only ns
+    namespaces:
+      exclude:
+        - "other"
+  - type: v1/pod # use top level properties

internal/source/kubernetes/testdata/TestRouterListMergingNestedFields/route-apps.v1.deployments.golden.yaml

@@ -0,0 +1,26 @@
+- event: delete
+  routes:
+    - resourcename:
+        include: []
+      labels:
+        test: label-1-level
+      annotations:
+        test: annotation-1-level
+      namespaces:
+        include: []
+        exclude:
+            - other
+      updatesetting: null
+      event:
+        reason:
+            include:
+                - reason-include-1-level
+            exclude:
+                - reason-exclude-1-level
+        message:
+            include:
+                - message-include-1-level
+            exclude:
+                - message-exclude-1-level
+        types:
+            - delete

internal/source/kubernetes/testdata/TestRouterListMergingNestedFields/route-v1.configmaps.golden.yaml

@@ -0,0 +1,27 @@
+- event: create
+  routes:
+    - resourcename:
+        include: []
+      labels:
+        test: label-2-level
+      annotations:
+        test: annotation-2-level
+      namespaces:
+        include:
+            - namespace-include-2-level
+        exclude:
+            - namespace-exclude-2-level
+      updatesetting: null
+      event:
+        reason:
+            include:
+                - reason-include-2-level
+            exclude:
+                - reason-exclude-2-level
+        message:
+            include:
+                - message-include-2-level
+            exclude:
+                - message-exclude-2-level
+        types:
+            - create