[Kubernetes] Check AddResourceMetadata and hints before starting name…

…space and node watchers (#4618) * Check addresourcemetadata and hints before starting ns and node watchers * Add changelog. * Update changelog/fragments/1713946896-remove-mandatory-ns-node-permissions.yaml Co-authored-by: Blake Rouse <blake.rouse@elastic.co> --------- Co-authored-by: Blake Rouse <blake.rouse@elastic.co>
elastic · Apr 25, 2024 · 5503e4c · 5503e4c
1 parent 6cdb8af
commit 5503e4c
Show file tree

Hide file tree

Showing 5 changed files with 235 additions and 17 deletions.
diff --git a/changelog/fragments/1713946896-remove-mandatory-ns-node-permissions.yaml b/changelog/fragments/1713946896-remove-mandatory-ns-node-permissions.yaml
@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: enhancement
+
+# Change summary; a 80ish characters long description of the change.
+summary: Handle the starting of namespace and node watchers for metadata enrichment according to `add_resource_metadata` and hints configuration.
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+#description:
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: elastic-agent
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+pr: https://github.com/elastic/elastic-agent/pull/4618
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+#issue: https://github.com/owner/repo/1234
diff --git a/internal/pkg/composable/providers/kubernetes/pod.go b/internal/pkg/composable/providers/kubernetes/pod.go
@@ -74,23 +74,28 @@ func NewPodEventer(
 		return nil, errors.New(err, "couldn't create kubernetes watcher")
 	}
 
-	var replicaSetWatcher, jobWatcher kubernetes.Watcher
+	var replicaSetWatcher, jobWatcher, namespaceWatcher, nodeWatcher kubernetes.Watcher
 
 	options := kubernetes.WatchOptions{
 		SyncTimeout: cfg.SyncPeriod,
 		Node:        cfg.Node,
 	}
 	metaConf := cfg.AddResourceMetadata
 
-	nodeWatcher, err := kubernetes.NewNamedWatcher("agent-node", client, &kubernetes.Node{}, options, nil)
-	if err != nil {
-		logger.Errorf("couldn't create watcher for %T due to error %+v", &kubernetes.Node{}, err)
+	if metaConf.Node.Enabled() || cfg.Hints.Enabled {
+		nodeWatcher, err = kubernetes.NewNamedWatcher("agent-node", client, &kubernetes.Node{}, options, nil)
+		if err != nil {
+			logger.Errorf("couldn't create watcher for %T due to error %+v", &kubernetes.Node{}, err)
+		}
 	}
-	namespaceWatcher, err := kubernetes.NewNamedWatcher("agent-namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
-		SyncTimeout: cfg.SyncPeriod,
-	}, nil)
-	if err != nil {
-		logger.Errorf("couldn't create watcher for %T due to error %+v", &kubernetes.Namespace{}, err)
+
+	if metaConf.Namespace.Enabled() || cfg.Hints.Enabled {
+		namespaceWatcher, err = kubernetes.NewNamedWatcher("agent-namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
+			SyncTimeout: cfg.SyncPeriod,
+		}, nil)
+		if err != nil {
+			logger.Errorf("couldn't create watcher for %T due to error %+v", &kubernetes.Namespace{}, err)
+		}
 	}
 
 	// Resource is Pod so we need to create watchers for Replicasets and Jobs that it might belong to

diff --git a/internal/pkg/composable/providers/kubernetes/pod_test.go b/internal/pkg/composable/providers/kubernetes/pod_test.go
@@ -12,15 +12,20 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	k8sfake "k8s.io/client-go/kubernetes/fake"
 
 	"github.com/stretchr/testify/assert"
 
 	"github.com/elastic/elastic-agent-libs/logp"
+
 	"github.com/elastic/elastic-agent/pkg/core/logger"
 
 	"github.com/elastic/elastic-agent-autodiscover/kubernetes"
 	"github.com/elastic/elastic-agent-autodiscover/kubernetes/metadata"
 	"github.com/elastic/elastic-agent-libs/mapstr"
+
+	c "github.com/elastic/elastic-agent-libs/config"
+
 	"github.com/elastic/elastic-agent/internal/pkg/config"
 )
 
@@ -370,6 +375,90 @@ func TestEphemeralContainers(t *testing.T) {
 
 }
 
+func TestPodEventer_Namespace_Node_Watcher(t *testing.T) {
+	client := k8sfake.NewSimpleClientset()
+
+	log, err := logger.New("service-eventer-test", true)
+	assert.NoError(t, err)
+
+	providerDataChan := make(chan providerData, 1)
+
+	comm := MockDynamicComm{
+		context.TODO(),
+		providerDataChan,
+	}
+
+	tests := []struct {
+		namespaceEnabled bool
+		nodeEnabled      bool
+		hintsEnabled     bool
+		expectedNil      bool
+		name             string
+		msg              string
+	}{
+		{
+			namespaceEnabled: false,
+			nodeEnabled:      false,
+			hintsEnabled:     false,
+			expectedNil:      true,
+			name:             "add_resource_metadata.namespace and add_resource_metadata.node disabled and hints disabled.",
+			msg:              "Watcher should be nil.",
+		},
+		{
+			namespaceEnabled: false,
+			nodeEnabled:      false,
+			hintsEnabled:     true,
+			expectedNil:      false,
+			name:             "add_resource_metadata.namespace and add_resource_metadata.node disabled and hints enabled.",
+			msg:              "Watcher should not be nil.",
+		},
+		{
+			namespaceEnabled: true,
+			nodeEnabled:      true,
+			hintsEnabled:     false,
+			expectedNil:      false,
+			name:             "add_resource_metadata.namespace and add_resource_metadata.node enabled and hints disabled.",
+			msg:              "Watcher should not be nil.",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var cfg Config
+			cfg.InitDefaults()
+
+			nsCfg, err := c.NewConfigFrom(map[string]interface{}{
+				"enabled": test.namespaceEnabled,
+			})
+			assert.NoError(t, err)
+			nodeCfg, err := c.NewConfigFrom(map[string]interface{}{
+				"enabled": test.nodeEnabled,
+			})
+			assert.NoError(t, err)
+
+			cfg.AddResourceMetadata.Namespace = nsCfg
+			cfg.AddResourceMetadata.Node = nodeCfg
+			cfg.Hints.Enabled = test.hintsEnabled
+
+			eventer, err := NewPodEventer(&comm, &cfg, log, client, "cluster", false)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			namespaceWatcher := eventer.(*pod).namespaceWatcher
+			nodeWatcher := eventer.(*pod).nodeWatcher
+
+			if test.expectedNil {
+				assert.Equalf(t, nil, namespaceWatcher, "Namespace "+test.msg)
+				assert.Equalf(t, nil, nodeWatcher, "Node "+test.msg)
+			} else {
+				assert.NotEqualf(t, nil, namespaceWatcher, "Namespace "+test.msg)
+				assert.NotEqualf(t, nil, nodeWatcher, "Node "+test.msg)
+			}
+		})
+	}
+}
+
 // MockDynamicComm is used in tests.
 type MockDynamicComm struct {
 	context.Context

diff --git a/internal/pkg/composable/providers/kubernetes/service.go b/internal/pkg/composable/providers/kubernetes/service.go
@@ -16,6 +16,7 @@ import (
 	"github.com/elastic/elastic-agent-libs/logp"
 	"github.com/elastic/elastic-agent-libs/mapstr"
 	"github.com/elastic/elastic-agent-libs/safemapstr"
+
 	"github.com/elastic/elastic-agent/internal/pkg/agent/errors"
 	"github.com/elastic/elastic-agent/internal/pkg/composable"
 )
@@ -54,15 +55,21 @@ func NewServiceEventer(
 		return nil, errors.New(err, "couldn't create kubernetes watcher")
 	}
 
-	metaConf := metadata.GetDefaultResourceMetadataConfig()
-	namespaceWatcher, err := kubernetes.NewNamedWatcher("agent-namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
-		SyncTimeout: cfg.SyncPeriod,
-		Namespace:   cfg.Namespace,
-	}, nil)
-	if err != nil {
-		return nil, fmt.Errorf("couldn't create watcher for %T due to error %w", &kubernetes.Namespace{}, err)
+	metaConf := cfg.AddResourceMetadata
+
+	var namespaceMeta metadata.MetaGen
+	var namespaceWatcher kubernetes.Watcher
+
+	if metaConf.Namespace.Enabled() || cfg.Hints.Enabled {
+		namespaceWatcher, err = kubernetes.NewNamedWatcher("agent-namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
+			SyncTimeout: cfg.SyncPeriod,
+			Namespace:   cfg.Namespace,
+		}, nil)
+		if err != nil {
+			return nil, fmt.Errorf("couldn't create watcher for %T due to error %w", &kubernetes.Namespace{}, err)
+		}
+		namespaceMeta = metadata.NewNamespaceMetadataGenerator(metaConf.Namespace, namespaceWatcher.Store(), client)
 	}
-	namespaceMeta := metadata.NewNamespaceMetadataGenerator(metaConf.Namespace, namespaceWatcher.Store(), client)
 
 	rawConfig, err := config.NewConfigFrom(cfg)
 	if err != nil {

diff --git a/internal/pkg/composable/providers/kubernetes/service_test.go b/internal/pkg/composable/providers/kubernetes/service_test.go
@@ -5,9 +5,13 @@
 package kubernetes
 
 import (
+	"context"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	k8sfake "k8s.io/client-go/kubernetes/fake"
+
+	"github.com/elastic/elastic-agent-libs/config"
 
 	"github.com/elastic/elastic-agent-autodiscover/kubernetes"
 	"github.com/elastic/elastic-agent-autodiscover/kubernetes/metadata"
@@ -16,6 +20,8 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+
+	"github.com/elastic/elastic-agent/pkg/core/logger"
 )
 
 func TestGenerateServiceData(t *testing.T) {
@@ -104,6 +110,85 @@ func TestGenerateServiceData(t *testing.T) {
 	}
 }
 
+func TestServiceEventer_NamespaceWatcher(t *testing.T) {
+	client := k8sfake.NewSimpleClientset()
+
+	log, err := logger.New("service-eventer-test", true)
+	assert.NoError(t, err)
+
+	providerDataChan := make(chan providerData, 1)
+
+	comm := MockDynamicComm{
+		context.TODO(),
+		providerDataChan,
+	}
+
+	tests := []struct {
+		namespaceEnabled bool
+		hintsEnabled     bool
+		expectedNil      bool
+		name             string
+		msg              string
+	}{
+		{
+			namespaceEnabled: false,
+			hintsEnabled:     false,
+			expectedNil:      true,
+			name:             "add_resource_metadata.namespace disabled and hints disabled.",
+			msg:              "Namespace watcher should be nil.",
+		},
+		{
+			namespaceEnabled: false,
+			hintsEnabled:     true,
+			expectedNil:      false,
+			name:             "add_resource_metadata.namespace disabled and hints enabled.",
+			msg:              "Namespace watcher should not be nil.",
+		},
+		{
+			namespaceEnabled: true,
+			hintsEnabled:     false,
+			expectedNil:      false,
+			name:             "add_resource_metadata.namespace enabled and hints disabled.",
+			msg:              "Namespace watcher should not be nil.",
+		},
+		{
+			namespaceEnabled: true,
+			hintsEnabled:     false,
+			expectedNil:      false,
+			name:             "add_resource_metadata default and hints default.",
+			msg:              "Watcher should not be nil.",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var cfg Config
+			cfg.InitDefaults()
+
+			nsCfg, err := config.NewConfigFrom(map[string]interface{}{
+				"enabled": test.namespaceEnabled,
+			})
+			assert.NoError(t, err)
+
+			cfg.AddResourceMetadata.Namespace = nsCfg
+			cfg.Hints.Enabled = test.hintsEnabled
+
+			eventer, err := NewServiceEventer(&comm, &cfg, log, client, "cluster", false)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			namespaceWatcher := eventer.(*service).namespaceWatcher
+
+			if test.expectedNil {
+				assert.Equalf(t, nil, namespaceWatcher, test.msg)
+			} else {
+				assert.NotEqualf(t, nil, namespaceWatcher, test.msg)
+			}
+		})
+	}
+}
+
 type svcMeta struct{}
 
 // Generate generates svc metadata from a resource object