Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Metricbeat][Kubernetes] Remove mandatory permissions for namespace and node #38762

Merged
merged 10 commits into from
Apr 23, 2024
Merged

[Metricbeat][Kubernetes] Remove mandatory permissions for namespace and node #38762

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9f18a2e
remove mandatory permissions
constanca-m Apr 8, 2024
ed53fc6
update CHANGELOG-developer.next.asciidoc
constanca-m Apr 8, 2024
b9bab0d
Merge branch 'main' into namespace-permissions
constanca-m Apr 8, 2024
5665d43
Fix kubernetes tests
constanca-m Apr 8, 2024
da534ae
Merge remote-tracking branch 'origin/namespace-permissions' into name…
constanca-m Apr 8, 2024
227b92f
Add new test for defaults
constanca-m Apr 8, 2024
741a222
Merge branch 'main' into namespace-permissions
constanca-m Apr 15, 2024
6ebb6a2
Check for watcher creation on processor
constanca-m Apr 18, 2024
710ed63
Update CHANGELOG-developer.next.asciidoc
constanca-m Apr 23, 2024
8eaaca1
Merge branch 'main' into namespace-permissions
constanca-m Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG-developer.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ The list below covers the major changes between 7.0.0-rc2 and main only.

==== Bugfixes

- Do not start namespace and node watchers on metricbeat autodiscover if `add_resource_metadata` is disabled along with `hints`.{pull}38762[38762]
constanca-m marked this conversation as resolved.
Show resolved Hide resolved
- Fix multiple metricbeat instances reporting same metrics when using autodiscover with provider kubernetes, and ensure leader elector is always running in autodiscover mode.{pull}38471[38471]
- Fix how Prometheus histograms are calculated when percentiles are provide.{pull}36537[36537]
- Stop using `mage:import` in community beats. This was ignoring the vendorized beats directory for some mage targets, using the code available in GOPATH, this causes inconsistencies and compilation problems if the version of the code in the GOPATH is different to the vendored one. Use of `mage:import` will continue to be unsupported in custom beats till beats is migrated to go modules, or mage supports vendored dependencies. {issue}13998[13998] {pull}14162[14162]
Expand Down
35 changes: 20 additions & 15 deletions libbeat/autodiscover/providers/kubernetes/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type pod struct {
func NewPodEventer(uuid uuid.UUID, cfg *conf.C, client k8s.Interface, publish func(event []bus.Event)) (Eventer, error) {
logger := logp.NewLogger("autodiscover.pod")

var replicaSetWatcher, jobWatcher kubernetes.Watcher
var replicaSetWatcher, jobWatcher, namespaceWatcher, nodeWatcher kubernetes.Watcher

config := defaultConfig()
err := cfg.Unpack(&config)
Expand Down Expand Up @@ -96,22 +96,27 @@ func NewPodEventer(uuid uuid.UUID, cfg *conf.C, client k8s.Interface, publish fu
return nil, fmt.Errorf("couldn't create watcher for %T due to error %w", &kubernetes.Pod{}, err)
}

options := kubernetes.WatchOptions{
SyncTimeout: config.SyncPeriod,
Node: config.Node,
Namespace: config.Namespace,
}

metaConf := config.AddResourceMetadata
nodeWatcher, err := kubernetes.NewNamedWatcher("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() || config.Hints.Enabled() {
options := kubernetes.WatchOptions{
SyncTimeout: config.SyncPeriod,
Node: config.Node,
Namespace: config.Namespace,
}
nodeWatcher, err = kubernetes.NewNamedWatcher("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("namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
SyncTimeout: config.SyncPeriod,
}, nil)
if err != nil {
logger.Errorf("couldn't create watcher for %T due to error %+v", &kubernetes.Namespace{}, err)

if metaConf.Namespace.Enabled() || config.Hints.Enabled() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need config.Hints.Enabled()?
Especially that now metaConf.Namespace has default value true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not, but we had this before and I think changing it could be a breaking change, correct?

namespaceWatcher, err = kubernetes.NewNamedWatcher("namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
SyncTimeout: config.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 belongs to
Expand Down
107 changes: 107 additions & 0 deletions libbeat/autodiscover/providers/kubernetes/pod_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2108,6 +2108,113 @@ func TestNodePodUpdater(t *testing.T) {
}
}

func TestPodEventer_Namespace_Node_Watcher(t *testing.T) {
client := k8sfake.NewSimpleClientset()
uuid, err := uuid.NewV4()
if err != nil {
t.Fatal(err)
}

tests := []struct {
cfg mapstr.M
expectedNil bool
name string
msg string
}{
{
cfg: mapstr.M{
"resource": "pod",
"node": "node-1",
"add_resource_metadata": mapstr.M{
"namespace.enabled": false,
"node.enabled": false,
},
"hints.enabled": false,
"builders": []mapstr.M{
{
"mock": mapstr.M{},
},
},
},
expectedNil: true,
name: "add_resource_metadata.namespace and add_resource_metadata.node disabled and hints disabled.",
msg: "Watcher should be nil.",
},
{
cfg: mapstr.M{
"resource": "pod",
"node": "node-1",
"add_resource_metadata": mapstr.M{
"namespace.enabled": false,
"node.enabled": false,
},
"hints.enabled": true,
},
expectedNil: false,
name: "add_resource_metadata.namespace and add_resource_metadata.node disabled and hints enabled.",
msg: "Watcher should not be nil.",
},
{
cfg: mapstr.M{
"resource": "pod",
"node": "node-1",
"add_resource_metadata": mapstr.M{
constanca-m marked this conversation as resolved.
Show resolved Hide resolved
"namespace.enabled": true,
"node.enabled": true,
},
"hints.enabled": false,
"builders": []mapstr.M{
{
"mock": mapstr.M{},
},
},
},
expectedNil: false,
name: "add_resource_metadata.namespace and add_resource_metadata.node enabled and hints disabled.",
msg: "Watcher should not be nil.",
},
{
cfg: mapstr.M{
"resource": "pod",
"node": "node-1",
"builders": []mapstr.M{
{
"mock": mapstr.M{},
},
},
},
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) {
config := conf.MustNewConfigFrom(&test.cfg)
c := defaultConfig()
err = config.Unpack(&c)
assert.NoError(t, err)

eventer, err := NewPodEventer(uuid, config, client, nil)
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)
}
})
}
}

type mockUpdaterHandler struct {
objects []interface{}
}
Expand Down
20 changes: 11 additions & 9 deletions libbeat/autodiscover/providers/kubernetes/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,19 @@ func NewServiceEventer(uuid uuid.UUID, cfg *conf.C, client k8s.Interface, publis
var namespaceMeta metadata.MetaGen
var namespaceWatcher kubernetes.Watcher

metaConf := metadata.GetDefaultResourceMetadataConfig()
namespaceWatcher, err = kubernetes.NewNamedWatcher("namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
SyncTimeout: config.SyncPeriod,
Namespace: config.Namespace,
}, nil)
if err != nil {
return nil, fmt.Errorf("couldn't create watcher for %T due to error %w", &kubernetes.Namespace{}, err)
metaConf := config.AddResourceMetadata

if metaConf.Namespace.Enabled() || config.Hints.Enabled() {
namespaceWatcher, err = kubernetes.NewNamedWatcher("namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
SyncTimeout: config.SyncPeriod,
Namespace: config.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)

p := &service{
config: config,
uuid: uuid,
Expand Down
98 changes: 98 additions & 0 deletions libbeat/autodiscover/providers/kubernetes/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,104 @@ func TestEmitEvent_Service(t *testing.T) {
}
}

func TestServiceEventer_NamespaceWatcher(t *testing.T) {
client := k8sfake.NewSimpleClientset()
uuid, err := uuid.NewV4()
if err != nil {
t.Fatal(err)
}

tests := []struct {
cfg mapstr.M
expectedNil bool
name string
msg string
}{
{
cfg: mapstr.M{
"resource": "service",
"node": "node-1",
"add_resource_metadata": mapstr.M{
"namespace.enabled": false,
},
"hints.enabled": false,
"builders": []mapstr.M{
{
"mock": mapstr.M{},
},
},
},
expectedNil: true,
name: "add_resource_metadata.namespace disabled and hints disabled.",
msg: "Namespace watcher should be nil.",
},
{
cfg: mapstr.M{
"resource": "service",
"node": "node-1",
"add_resource_metadata": mapstr.M{
"namespace.enabled": false,
},
"hints.enabled": true,
},
expectedNil: false,
name: "add_resource_metadata.namespace disabled and hints enabled.",
msg: "Namespace watcher should not be nil.",
},
{
cfg: mapstr.M{
"resource": "service",
"node": "node-1",
"add_resource_metadata": mapstr.M{
"namespace.enabled": true,
},
"hints.enabled": false,
"builders": []mapstr.M{
{
"mock": mapstr.M{},
},
},
},
expectedNil: false,
name: "add_resource_metadata.namespace enabled and hints disabled.",
msg: "Namespace watcher should not be nil.",
},
{
cfg: mapstr.M{
"resource": "pod",
"node": "node-1",
"builders": []mapstr.M{
{
"mock": mapstr.M{},
},
},
},
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) {
config := conf.MustNewConfigFrom(&test.cfg)

eventer, err := NewServiceEventer(uuid, config, client, nil)
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)
}
})
}
}

func NewMockServiceEventerManager(svc *service) EventManager {
em := &eventerManager{}
em.eventer = svc
Expand Down
28 changes: 17 additions & 11 deletions libbeat/processors/add_kubernetes_metadata/kubernetes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@ import (

k8sclient "k8s.io/client-go/kubernetes"

"github.com/elastic/beats/v7/libbeat/beat"
"github.com/elastic/beats/v7/libbeat/processors"
"github.com/elastic/elastic-agent-autodiscover/kubernetes"
"github.com/elastic/elastic-agent-autodiscover/kubernetes/metadata"
"github.com/elastic/elastic-agent-libs/config"
"github.com/elastic/elastic-agent-libs/logp"
"github.com/elastic/elastic-agent-libs/mapstr"

"github.com/elastic/beats/v7/libbeat/beat"
"github.com/elastic/beats/v7/libbeat/processors"
)

const (
Expand Down Expand Up @@ -144,7 +145,7 @@ func newProcessorConfig(cfg *config.C, register *Register) (kubeAnnotatorConfig,

func (k *kubernetesAnnotator) init(config kubeAnnotatorConfig, cfg *config.C) {
k.initOnce.Do(func() {
var replicaSetWatcher, jobWatcher kubernetes.Watcher
var replicaSetWatcher, jobWatcher, namespaceWatcher, nodeWatcher kubernetes.Watcher

client, err := kubernetes.GetKubernetesClient(config.KubeConfig, config.KubeClientOptions)
if err != nil {
Expand Down Expand Up @@ -203,15 +204,20 @@ func (k *kubernetesAnnotator) init(config kubeAnnotatorConfig, cfg *config.C) {
Namespace: config.Namespace,
}

nodeWatcher, err := kubernetes.NewNamedWatcher("add_kubernetes_metadata_node", client, &kubernetes.Node{}, options, nil)
if err != nil {
k.log.Errorf("couldn't create watcher for %T due to error %+v", &kubernetes.Node{}, err)
if metaConf.Node.Enabled() {
nodeWatcher, err = kubernetes.NewNamedWatcher("add_kubernetes_metadata_node", client, &kubernetes.Node{}, options, nil)
if err != nil {
k.log.Errorf("couldn't create watcher for %T due to error %+v", &kubernetes.Node{}, err)
}
}
namespaceWatcher, err := kubernetes.NewNamedWatcher("add_kubernetes_metadata_namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
SyncTimeout: config.SyncPeriod,
}, nil)
if err != nil {
k.log.Errorf("couldn't create watcher for %T due to error %+v", &kubernetes.Namespace{}, err)

if metaConf.Namespace.Enabled() {
namespaceWatcher, err = kubernetes.NewNamedWatcher("add_kubernetes_metadata_namespace", client, &kubernetes.Namespace{}, kubernetes.WatchOptions{
SyncTimeout: config.SyncPeriod,
}, nil)
if err != nil {
k.log.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 belongs to
Expand Down
Loading
Loading