From 65388b10121d540c760eb0db272054c043bdbfaa Mon Sep 17 00:00:00 2001 From: Fabian Heib <11271952+xonvanetta@users.noreply.github.com> Date: Thu, 2 Nov 2023 17:22:02 +0100 Subject: [PATCH] Support filtering annotations allowlist by "*" Resolves: add-wilcard-allowlist-annotations --- docs/cli-arguments.md | 2 +- internal/store/builder.go | 53 ++++++++++++--------- internal/store/builder_test.go | 83 +++++++++++++++++++++++++++++++++ pkg/app/server.go | 4 +- pkg/builder/builder.go | 4 +- pkg/builder/types/interfaces.go | 2 +- 6 files changed, 122 insertions(+), 26 deletions(-) diff --git a/docs/cli-arguments.md b/docs/cli-arguments.md index 962798d0f2..51c7e000eb 100644 --- a/docs/cli-arguments.md +++ b/docs/cli-arguments.md @@ -52,7 +52,7 @@ Flags: --log_file_max_size uint Defines the maximum size a log file can grow to (no effect when -logtostderr=true). Unit is megabytes. If the value is 0, the maximum file size is unlimited. (default 1800) --logtostderr log to standard error instead of files (default true) --metric-allowlist string Comma-separated list of metrics to be exposed. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive. - --metric-annotations-allowlist string Comma-separated list of Kubernetes annotations keys that will be used in the resource' labels metric. By default the annotations metrics are not exposed. To include them, provide a list of resource names in their plural form and Kubernetes annotation keys you would like to allow for them (Example: '=namespaces=[kubernetes.io/team,...],pods=[kubernetes.io/team],...)'. A single '*' can be provided per resource instead to allow any annotations, but that has severe performance implications (Example: '=pods=[*]'). + --metric-annotations-allowlist string Comma-separated list of Kubernetes annotations keys that will be used in the resource' labels metric. By default the annotations metrics are not exposed. To include them, provide a list of resource names in their plural form and Kubernetes annotation keys you would like to allow for them (Example: '=namespaces=[kubernetes.io/team,...],pods=[kubernetes.io/team],...)'. A single '*' can be provided per resource instead to allow any annotations, but that has severe performance implications (Example: '=pods=[*]'). Additionally, an asterisk (*) can be provided as a key, which will resolve to all resources, i.e., assuming '--resources=deployments,pods', '=*=[*]' will resolve to '=deployments=[*],pods=[*]'. --metric-denylist string Comma-separated list of metrics not to be enabled. This list comprises of exact metric names and/or regex patterns. The allowlist and denylist are mutually exclusive. --metric-labels-allowlist string Comma-separated list of additional Kubernetes label keys that will be used in the resource' labels metric. By default the labels metrics are not exposed. To include them, provide a list of resource names in their plural form and Kubernetes label keys you would like to allow for them (Example: '=namespaces=[k8s-label-1,k8s-label-n,...],pods=[app],...)'. A single '*' can be provided per resource instead to allow any labels, but that has severe performance implications (Example: '=pods=[*]'). Additionally, an asterisk (*) can be provided as a key, which will resolve to all resources, i.e., assuming '--resources=deployments,pods', '=*=[*]' will resolve to '=deployments=[*],pods=[*]'. --metric-opt-in-list string Comma-separated list of metrics which are opt-in and not enabled by default. This is in addition to the metric allow- and denylists diff --git a/internal/store/builder.go b/internal/store/builder.go index 837f8bc94d..5c38857d7b 100644 --- a/internal/store/builder.go +++ b/internal/store/builder.go @@ -219,32 +219,43 @@ func (b *Builder) WithCustomResourceStoreFactories(fs ...customresource.Registry } } -// WithAllowAnnotations configures which annotations can be returned for metrics -func (b *Builder) WithAllowAnnotations(annotations map[string][]string) { - if len(annotations) > 0 { - b.allowAnnotationsList = annotations +// allowList validates the given map and checks if the resources exists. +// If there is a '*' as key, return new map with all enabled resources. +func (b *Builder) allowList(list map[string][]string) (map[string][]string, error) { + if len(list) == 0 { + return nil, nil + } + + for l := range list { + if !resourceExists(l) && l != "*" { + return nil, fmt.Errorf("resource %s does not exist. Available resources: %s", l, strings.Join(availableResources(), ",")) + } + } + + // "*" takes precedence over other specifications + allowedList, ok := list["*"] + if !ok { + return list, nil + } + m := make(map[string][]string) + for _, resource := range b.enabledResources { + m[resource] = allowedList } + return m, nil +} + +// WithAllowAnnotations configures which annotations can be returned for metrics +func (b *Builder) WithAllowAnnotations(annotations map[string][]string) error { + var err error + b.allowAnnotationsList, err = b.allowList(annotations) + return err } // WithAllowLabels configures which labels can be returned for metrics func (b *Builder) WithAllowLabels(labels map[string][]string) error { - if len(labels) > 0 { - for label := range labels { - if !resourceExists(label) && label != "*" { - return fmt.Errorf("resource %s does not exist. Available resources: %s", label, strings.Join(availableResources(), ",")) - } - } - b.allowLabelsList = labels - // "*" takes precedence over other specifications - if allowedLabels, ok := labels["*"]; ok { - m := make(map[string][]string) - for _, resource := range b.enabledResources { - m[resource] = allowedLabels - } - b.allowLabelsList = m - } - } - return nil + var err error + b.allowLabelsList, err = b.allowList(labels) + return err } // Build initializes and registers all enabled stores. diff --git a/internal/store/builder_test.go b/internal/store/builder_test.go index 758781d6f7..3bb5dea3aa 100644 --- a/internal/store/builder_test.go +++ b/internal/store/builder_test.go @@ -113,3 +113,86 @@ func TestWithAllowLabels(t *testing.T) { } } } + +func TestWithAllowAnnotations(t *testing.T) { + tests := []struct { + Desc string + AnnotationsAllowlist map[string][]string + EnabledResources []string + Wanted LabelsAllowList + err expectedError + }{ + { + Desc: "wildcard key-value as the only element", + AnnotationsAllowlist: map[string][]string{"*": {"*"}}, + EnabledResources: []string{"cronjobs", "pods", "deployments"}, + Wanted: LabelsAllowList(map[string][]string{ + "deployments": {"*"}, + "pods": {"*"}, + "cronjobs": {"*"}, + }), + }, + { + Desc: "wildcard key-value as not the only element", + AnnotationsAllowlist: map[string][]string{"*": {"*"}, "pods": {"*"}, "cronjobs": {"*"}}, + EnabledResources: []string{"cronjobs", "pods", "deployments"}, + Wanted: LabelsAllowList(map[string][]string{ + "deployments": {"*"}, + "pods": {"*"}, + "cronjobs": {"*"}, + }), + }, + { + Desc: "wildcard key-value as not the only element, with resource mismatch", + AnnotationsAllowlist: map[string][]string{"*": {"*"}, "pods": {"*"}, "cronjobs": {"*"}, "configmaps": {"*"}}, + EnabledResources: []string{"cronjobs", "pods", "deployments"}, + Wanted: LabelsAllowList{}, + err: expectedError{ + expectedNotEqual: true, + }, + }, + { + Desc: "wildcard key-value as not the only element, with other mutually-exclusive keys", + AnnotationsAllowlist: map[string][]string{"*": {"*"}, "foo": {"*"}, "bar": {"*"}, "cronjobs": {"*"}}, + EnabledResources: []string{"cronjobs", "pods", "deployments"}, + Wanted: LabelsAllowList(nil), + err: expectedError{ + expectedLabelError: true, + }, + }, + { + Desc: "wildcard key-value as not the only element, with other resources that do not exist", + AnnotationsAllowlist: map[string][]string{"*": {"*"}, "cronjobs": {"*"}}, + EnabledResources: []string{"cronjobs", "pods", "deployments", "foo", "bar"}, + Wanted: LabelsAllowList{}, + err: expectedError{ + expectedResourceError: true, + }, + }, + } + + for _, test := range tests { + b := NewBuilder() + + // Set the enabled resources. + err := b.WithEnabledResources(test.EnabledResources) + if err != nil && !test.err.expectedResourceError { + t.Log("Did not expect error while setting resources (--resources).") + t.Errorf("Test error for Desc: %s. Got Error: %v", test.Desc, err) + } + + // Resolve the allow list. + err = b.WithAllowAnnotations(test.AnnotationsAllowlist) + if err != nil && !test.err.expectedLabelError { + t.Log("Did not expect error while parsing allow list annotations (--metric-annotations-allowlist).") + t.Errorf("Test error for Desc: %s. Got Error: %v", test.Desc, err) + } + resolvedAllowAnnotations := LabelsAllowList(b.allowAnnotationsList) + + // Evaluate. + if !reflect.DeepEqual(resolvedAllowAnnotations, test.Wanted) && !test.err.expectedNotEqual { + t.Log("Expected maps to be equal.") + t.Errorf("Test error for Desc: %s\n Want: \n%+v\n Got: \n%#+v", test.Desc, test.Wanted, resolvedAllowAnnotations) + } + } +} diff --git a/pkg/app/server.go b/pkg/app/server.go index 09ce7cad82..7da8ddcf20 100644 --- a/pkg/app/server.go +++ b/pkg/app/server.go @@ -250,7 +250,9 @@ func RunKubeStateMetrics(ctx context.Context, opts *options.Options) error { storeBuilder.WithKubeClient(kubeClient) storeBuilder.WithSharding(opts.Shard, opts.TotalShards) - storeBuilder.WithAllowAnnotations(opts.AnnotationsAllowList) + if err := storeBuilder.WithAllowAnnotations(opts.AnnotationsAllowList); err != nil { + return fmt.Errorf("failed to set up annotations allowlist: %v", err) + } if err := storeBuilder.WithAllowLabels(opts.LabelsAllowList); err != nil { return fmt.Errorf("failed to set up labels allowlist: %v", err) } diff --git a/pkg/builder/builder.go b/pkg/builder/builder.go index 25b4d48c0d..43a09b57e0 100644 --- a/pkg/builder/builder.go +++ b/pkg/builder/builder.go @@ -101,8 +101,8 @@ func (b *Builder) WithFamilyGeneratorFilter(l generator.FamilyGeneratorFilter) { } // WithAllowAnnotations configures which annotations can be returned for metrics -func (b *Builder) WithAllowAnnotations(annotations map[string][]string) { - b.internal.WithAllowAnnotations(annotations) +func (b *Builder) WithAllowAnnotations(annotations map[string][]string) error { + return b.internal.WithAllowAnnotations(annotations) } // WithAllowLabels configures which labels can be returned for metrics diff --git a/pkg/builder/types/interfaces.go b/pkg/builder/types/interfaces.go index 6d3cbfc6b7..b7ba595da1 100644 --- a/pkg/builder/types/interfaces.go +++ b/pkg/builder/types/interfaces.go @@ -42,7 +42,7 @@ type BuilderInterface interface { WithCustomResourceClients(cs map[string]interface{}) WithUsingAPIServerCache(u bool) WithFamilyGeneratorFilter(l generator.FamilyGeneratorFilter) - WithAllowAnnotations(a map[string][]string) + WithAllowAnnotations(a map[string][]string) error WithAllowLabels(l map[string][]string) error WithGenerateStoresFunc(f BuildStoresFunc) DefaultGenerateStoresFunc() BuildStoresFunc