Merge pull request #2234 from xonvanetta/feature/add-wilcard-allowlist-annotations

feat: Support filtering annotations allowlist by "*"

Author: k8s-ci-robot

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

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.

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)
+		}
+	}
+}

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)
 	}

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

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