From 7b53851ef0f0148732b70efb133acb6115de2aba Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Mon, 8 Apr 2024 13:30:39 +0200 Subject: [PATCH 1/7] Fixed labels and annotations filter Signed-off-by: Yuri Sa --- .chloggen/fix-filter-labels-annotations.yaml | 17 +++++++++++ .github/workflows/e2e.yaml | 2 +- .../manifests/collector/daemonset_test.go | 4 +-- .../manifests/collector/deployment_test.go | 4 +-- .../manifests/collector/statefulset_test.go | 4 +-- .../manifestutils/annotations_test.go | 28 ++++++++++++++++++- internal/manifests/manifestutils/labels.go | 6 ++-- .../manifests/manifestutils/labels_test.go | 2 +- .../manifests/opampbridge/deployment_test.go | 4 +-- main.go | 4 +-- 10 files changed, 59 insertions(+), 16 deletions(-) create mode 100755 .chloggen/fix-filter-labels-annotations.yaml diff --git a/.chloggen/fix-filter-labels-annotations.yaml b/.chloggen/fix-filter-labels-annotations.yaml new file mode 100755 index 0000000000..328539a796 --- /dev/null +++ b/.chloggen/fix-filter-labels-annotations.yaml @@ -0,0 +1,17 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: 'bug_fix' + +# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action) +component: operator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix of Labels and Annotations filter + +# One or more tracking issues related to the change +issues: [2770] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: Changed the approach from regexp.MatchString, which was representing a high resource cost and | +confusing the users to strings.Contains, which turned out to be simpler and more efficient. diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 7ee5c89d79..837484abaa 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -37,7 +37,7 @@ jobs: - group: e2e-multi-instrumentation setup: "add-operator-arg OPERATOR_ARG=--enable-multi-instrumentation prepare-e2e" - group: e2e-metadata-filters - setup: "add-operator-arg OPERATOR_ARG='--annotations-filter=*filter.out --labels=*filter.out' prepare-e2e" + setup: "add-operator-arg OPERATOR_ARG='--annotations-filter=filter.out --labels=filter.out' prepare-e2e" steps: - name: Check out code into the Go module directory diff --git a/internal/manifests/collector/daemonset_test.go b/internal/manifests/collector/daemonset_test.go index 778c3791f6..bdcd52a13b 100644 --- a/internal/manifests/collector/daemonset_test.go +++ b/internal/manifests/collector/daemonset_test.go @@ -227,7 +227,7 @@ func TestDaemonsetFilterLabels(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithLabelFilters([]string{"foo*", "app.*.bar"})) + cfg := config.New(config.WithLabelFilters([]string{"foo", ".bar"})) params := manifests.Params{ Config: cfg, @@ -258,7 +258,7 @@ func TestDaemonsetFilterAnnotations(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithAnnotationFilters([]string{"foo*", "app.*.bar"})) + cfg := config.New(config.WithAnnotationFilters([]string{"foo", ".bar"})) params := manifests.Params{ Config: cfg, diff --git a/internal/manifests/collector/deployment_test.go b/internal/manifests/collector/deployment_test.go index 0523a214b6..7ddc3fda05 100644 --- a/internal/manifests/collector/deployment_test.go +++ b/internal/manifests/collector/deployment_test.go @@ -309,7 +309,7 @@ func TestDeploymentFilterLabels(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithLabelFilters([]string{"foo*", "app.*.bar"})) + cfg := config.New(config.WithLabelFilters([]string{"foo", ".bar"})) params := manifests.Params{ Config: cfg, @@ -340,7 +340,7 @@ func TestDeploymentFilterAnnotations(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithAnnotationFilters([]string{"foo*", "app.*.bar"})) + cfg := config.New(config.WithAnnotationFilters([]string{"foo", ".bar"})) params := manifests.Params{ Config: cfg, diff --git a/internal/manifests/collector/statefulset_test.go b/internal/manifests/collector/statefulset_test.go index 5e0af9d110..d908a9f0ad 100644 --- a/internal/manifests/collector/statefulset_test.go +++ b/internal/manifests/collector/statefulset_test.go @@ -319,7 +319,7 @@ func TestStatefulSetFilterLabels(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithLabelFilters([]string{"foo*", "app.*.bar"})) + cfg := config.New(config.WithLabelFilters([]string{"foo", ".bar"})) params := manifests.Params{ OtelCol: otelcol, @@ -350,7 +350,7 @@ func TestStatefulSetFilterAnnotations(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithAnnotationFilters([]string{"foo*", "app.*.bar"})) + cfg := config.New(config.WithAnnotationFilters([]string{"foo", ".bar"})) params := manifests.Params{ OtelCol: otelcol, diff --git a/internal/manifests/manifestutils/annotations_test.go b/internal/manifests/manifestutils/annotations_test.go index a09831bbf9..1b6451b48b 100644 --- a/internal/manifests/manifestutils/annotations_test.go +++ b/internal/manifests/manifestutils/annotations_test.go @@ -160,11 +160,36 @@ func TestAnnotationsPropagateDown(t *testing.T) { assert.Equal(t, "pod_annotation_value", podAnnotations["pod_annotation"]) } +func TestAnnotationsSingleFilter(t *testing.T) { + otelcol := v1beta1.OpenTelemetryCollector{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "test.bar.io": "foo", + "test.io/port": "1234", + "test.io/path": "/test", + }, + }, + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Mode: "deployment", + }, + } + + // This requires the filter to be in regex match form and not the other simpler wildcard one. + annotations, err := Annotations(otelcol, []string{".bar.io"}) + + // verify + require.NoError(t, err) + assert.Len(t, annotations, 6) + assert.NotContains(t, annotations, "test.bar.io") + assert.Equal(t, "1234", annotations["test.io/port"]) +} + func TestAnnotationsFilter(t *testing.T) { otelcol := v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ "test.bar.io": "foo", + "test.otel.io": "true", "test.io/port": "1234", "test.io/path": "/test", }, @@ -175,11 +200,12 @@ func TestAnnotationsFilter(t *testing.T) { } // This requires the filter to be in regex match form and not the other simpler wildcard one. - annotations, err := Annotations(otelcol, []string{".*\\.bar\\.io"}) + annotations, err := Annotations(otelcol, []string{".bar.io", "otel.io"}) // verify require.NoError(t, err) assert.Len(t, annotations, 6) assert.NotContains(t, annotations, "test.bar.io") + assert.NotContains(t, annotations, "test.otel.io") assert.Equal(t, "1234", annotations["test.io/port"]) } diff --git a/internal/manifests/manifestutils/labels.go b/internal/manifests/manifestutils/labels.go index 76fd5a4e58..dc2fc90514 100644 --- a/internal/manifests/manifestutils/labels.go +++ b/internal/manifests/manifestutils/labels.go @@ -15,7 +15,6 @@ package manifestutils import ( - "regexp" "strings" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" @@ -26,8 +25,9 @@ import ( func IsFilteredSet(sourceSet string, filterSet []string) bool { for _, pattern := range filterSet { - match, _ := regexp.MatchString(pattern, sourceSet) - return match + if strings.Contains(sourceSet, pattern) { + return true + } } return false } diff --git a/internal/manifests/manifestutils/labels_test.go b/internal/manifests/manifestutils/labels_test.go index e76e9fc958..5db392540e 100644 --- a/internal/manifests/manifestutils/labels_test.go +++ b/internal/manifests/manifestutils/labels_test.go @@ -147,7 +147,7 @@ func TestLabelsFilter(t *testing.T) { } // This requires the filter to be in regex match form and not the other simpler wildcard one. - labels := Labels(otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{".*.bar.io"}) + labels := Labels(otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{".bar.io"}) // verify assert.Len(t, labels, 7) diff --git a/internal/manifests/opampbridge/deployment_test.go b/internal/manifests/opampbridge/deployment_test.go index 77af7843b0..cc35ed9f21 100644 --- a/internal/manifests/opampbridge/deployment_test.go +++ b/internal/manifests/opampbridge/deployment_test.go @@ -237,7 +237,7 @@ func TestDeploymentFilterLabels(t *testing.T) { Spec: v1alpha1.OpAMPBridgeSpec{}, } - cfg := config.New(config.WithLabelFilters([]string{"foo*", "app.*.bar"})) + cfg := config.New(config.WithLabelFilters([]string{"foo", ".bar"})) params := manifests.Params{ Config: cfg, @@ -268,7 +268,7 @@ func TestDeploymentFilterAnnotations(t *testing.T) { Spec: v1alpha1.OpAMPBridgeSpec{}, } - cfg := config.New(config.WithAnnotationFilters([]string{"foo*", "app.*.bar"})) + cfg := config.New(config.WithAnnotationFilters([]string{"foo", ".bar"})) params := manifests.Params{ Config: cfg, diff --git a/main.go b/main.go index fca734ce3b..6a0ede7500 100644 --- a/main.go +++ b/main.go @@ -150,8 +150,8 @@ func main() { stringFlagOrEnv(&autoInstrumentationGo, "auto-instrumentation-go-image", "RELATED_IMAGE_AUTO_INSTRUMENTATION_GO", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-go-instrumentation/autoinstrumentation-go:%s", v.AutoInstrumentationGo), "The default OpenTelemetry Go instrumentation image. This image is used when no image is specified in the CustomResource.") stringFlagOrEnv(&autoInstrumentationApacheHttpd, "auto-instrumentation-apache-httpd-image", "RELATED_IMAGE_AUTO_INSTRUMENTATION_APACHE_HTTPD", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-apache-httpd:%s", v.AutoInstrumentationApacheHttpd), "The default OpenTelemetry Apache HTTPD instrumentation image. This image is used when no image is specified in the CustomResource.") stringFlagOrEnv(&autoInstrumentationNginx, "auto-instrumentation-nginx-image", "RELATED_IMAGE_AUTO_INSTRUMENTATION_NGINX", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-apache-httpd:%s", v.AutoInstrumentationNginx), "The default OpenTelemetry Nginx instrumentation image. This image is used when no image is specified in the CustomResource.") - pflag.StringArrayVar(&labelsFilter, "label", []string{}, "Labels to filter away from propagating onto deploys") - pflag.StringArrayVar(&annotationsFilter, "annotations-filter", []string{}, "Annotations to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings optionally containing a * wildcard character. Example: --annotations-filter=*filter.out will filter out annotations that looks like: annotation.filter.out: true") + pflag.StringArrayVar(&labelsFilter, "label", []string{}, "Labels to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings. Example: --labels-filter=filter.out will filter out labels that looks like: label.filter.out: true") + pflag.StringArrayVar(&annotationsFilter, "annotations-filter", []string{}, "Annotations to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings. Example: --annotations-filter=filter.out will filter out annotations that looks like: annotation.filter.out: true") pflag.IntVar(&webhookPort, "webhook-port", 9443, "The port the webhook endpoint binds to.") pflag.StringVar(&tlsOpt.minVersion, "tls-min-version", "VersionTLS12", "Minimum TLS version supported. Value must match version names from https://golang.org/pkg/crypto/tls/#pkg-constants.") pflag.StringSliceVar(&tlsOpt.cipherSuites, "tls-cipher-suites", nil, "Comma-separated list of cipher suites for the server. Values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants). If omitted, the default Go cipher suites will be used") From 695f903dca1a44cfedf9aea9404c25b9f6d96ab4 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Mon, 8 Apr 2024 13:38:00 +0200 Subject: [PATCH 2/7] Fixed labels and annotations filter Signed-off-by: Yuri Sa --- .chloggen/fix-filter-labels-annotations.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/fix-filter-labels-annotations.yaml b/.chloggen/fix-filter-labels-annotations.yaml index 328539a796..bb3d063374 100755 --- a/.chloggen/fix-filter-labels-annotations.yaml +++ b/.chloggen/fix-filter-labels-annotations.yaml @@ -13,5 +13,5 @@ issues: [2770] # (Optional) One or more lines of additional information to render under the primary note. # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. -subtext: Changed the approach from regexp.MatchString, which was representing a high resource cost and | +subtext: | Changed the approach from regexp.MatchString, which was representing a high resource cost and confusing the users to strings.Contains, which turned out to be simpler and more efficient. From 5e3b198120abd9a043a6efff87e71ae9fc9a8c81 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Mon, 8 Apr 2024 14:01:55 +0200 Subject: [PATCH 3/7] Fixed labels and annotations filter Signed-off-by: Yuri Sa --- .chloggen/fix-filter-labels-annotations.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.chloggen/fix-filter-labels-annotations.yaml b/.chloggen/fix-filter-labels-annotations.yaml index bb3d063374..aae1ae2043 100755 --- a/.chloggen/fix-filter-labels-annotations.yaml +++ b/.chloggen/fix-filter-labels-annotations.yaml @@ -13,5 +13,6 @@ issues: [2770] # (Optional) One or more lines of additional information to render under the primary note. # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. -subtext: | Changed the approach from regexp.MatchString, which was representing a high resource cost and -confusing the users to strings.Contains, which turned out to be simpler and more efficient. +subtext: | + Changed the approach from regexp.MatchString, which was representing a high resource cost + and confusing the users to strings.Contains, which turned out to be simpler and more efficient. From 5baf4c87f2b669196549b659a24fe36fde98fc9a Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Tue, 9 Apr 2024 11:41:01 +0200 Subject: [PATCH 4/7] Fixed labels and annotations filter Signed-off-by: Yuri Sa --- .chloggen/fix-filter-labels-annotations.yaml | 4 +--- .github/workflows/e2e.yaml | 2 +- internal/manifests/collector/daemonset_test.go | 4 ++-- internal/manifests/collector/deployment_test.go | 4 ++-- internal/manifests/collector/statefulset_test.go | 4 ++-- .../manifests/manifestutils/annotations_test.go | 15 ++++++++------- internal/manifests/manifestutils/labels.go | 14 ++++++++++++-- internal/manifests/manifestutils/labels_test.go | 2 +- internal/manifests/opampbridge/deployment_test.go | 4 ++-- 9 files changed, 31 insertions(+), 22 deletions(-) diff --git a/.chloggen/fix-filter-labels-annotations.yaml b/.chloggen/fix-filter-labels-annotations.yaml index aae1ae2043..3a65b2c688 100755 --- a/.chloggen/fix-filter-labels-annotations.yaml +++ b/.chloggen/fix-filter-labels-annotations.yaml @@ -13,6 +13,4 @@ issues: [2770] # (Optional) One or more lines of additional information to render under the primary note. # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. -subtext: | - Changed the approach from regexp.MatchString, which was representing a high resource cost - and confusing the users to strings.Contains, which turned out to be simpler and more efficient. +subtext: diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 837484abaa..c89ebb320b 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -37,7 +37,7 @@ jobs: - group: e2e-multi-instrumentation setup: "add-operator-arg OPERATOR_ARG=--enable-multi-instrumentation prepare-e2e" - group: e2e-metadata-filters - setup: "add-operator-arg OPERATOR_ARG='--annotations-filter=filter.out --labels=filter.out' prepare-e2e" + setup: "add-operator-arg OPERATOR_ARG='--annotations-.*filter=filter.out --labels=.*filter.out' prepare-e2e" steps: - name: Check out code into the Go module directory diff --git a/internal/manifests/collector/daemonset_test.go b/internal/manifests/collector/daemonset_test.go index bdcd52a13b..847a5c68fd 100644 --- a/internal/manifests/collector/daemonset_test.go +++ b/internal/manifests/collector/daemonset_test.go @@ -227,7 +227,7 @@ func TestDaemonsetFilterLabels(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithLabelFilters([]string{"foo", ".bar"})) + cfg := config.New(config.WithLabelFilters([]string{"foo*", ".*bar"})) params := manifests.Params{ Config: cfg, @@ -258,7 +258,7 @@ func TestDaemonsetFilterAnnotations(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithAnnotationFilters([]string{"foo", ".bar"})) + cfg := config.New(config.WithAnnotationFilters([]string{"foo*", ".*bar"})) params := manifests.Params{ Config: cfg, diff --git a/internal/manifests/collector/deployment_test.go b/internal/manifests/collector/deployment_test.go index 7ddc3fda05..0523a214b6 100644 --- a/internal/manifests/collector/deployment_test.go +++ b/internal/manifests/collector/deployment_test.go @@ -309,7 +309,7 @@ func TestDeploymentFilterLabels(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithLabelFilters([]string{"foo", ".bar"})) + cfg := config.New(config.WithLabelFilters([]string{"foo*", "app.*.bar"})) params := manifests.Params{ Config: cfg, @@ -340,7 +340,7 @@ func TestDeploymentFilterAnnotations(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithAnnotationFilters([]string{"foo", ".bar"})) + cfg := config.New(config.WithAnnotationFilters([]string{"foo*", "app.*.bar"})) params := manifests.Params{ Config: cfg, diff --git a/internal/manifests/collector/statefulset_test.go b/internal/manifests/collector/statefulset_test.go index d908a9f0ad..5e0af9d110 100644 --- a/internal/manifests/collector/statefulset_test.go +++ b/internal/manifests/collector/statefulset_test.go @@ -319,7 +319,7 @@ func TestStatefulSetFilterLabels(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithLabelFilters([]string{"foo", ".bar"})) + cfg := config.New(config.WithLabelFilters([]string{"foo*", "app.*.bar"})) params := manifests.Params{ OtelCol: otelcol, @@ -350,7 +350,7 @@ func TestStatefulSetFilterAnnotations(t *testing.T) { Spec: v1beta1.OpenTelemetryCollectorSpec{}, } - cfg := config.New(config.WithAnnotationFilters([]string{"foo", ".bar"})) + cfg := config.New(config.WithAnnotationFilters([]string{"foo*", "app.*.bar"})) params := manifests.Params{ OtelCol: otelcol, diff --git a/internal/manifests/manifestutils/annotations_test.go b/internal/manifests/manifestutils/annotations_test.go index 1b6451b48b..82d97eefff 100644 --- a/internal/manifests/manifestutils/annotations_test.go +++ b/internal/manifests/manifestutils/annotations_test.go @@ -175,7 +175,7 @@ func TestAnnotationsSingleFilter(t *testing.T) { } // This requires the filter to be in regex match form and not the other simpler wildcard one. - annotations, err := Annotations(otelcol, []string{".bar.io"}) + annotations, err := Annotations(otelcol, []string{".*bar.io"}) // verify require.NoError(t, err) @@ -188,10 +188,10 @@ func TestAnnotationsFilter(t *testing.T) { otelcol := v1beta1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "test.bar.io": "foo", - "test.otel.io": "true", - "test.io/port": "1234", - "test.io/path": "/test", + "test.bar.io": "foo", + "test.io/port": "1234", + "test.io/path": "/test", + "config.otel.test/filter": "true", }, }, Spec: v1beta1.OpenTelemetryCollectorSpec{ @@ -200,12 +200,13 @@ func TestAnnotationsFilter(t *testing.T) { } // This requires the filter to be in regex match form and not the other simpler wildcard one. - annotations, err := Annotations(otelcol, []string{".bar.io", "otel.io"}) + annotations, err := Annotations(otelcol, []string{".*bar.io", ".*otel/io", "config.*otel.test.*"}) // verify require.NoError(t, err) assert.Len(t, annotations, 6) assert.NotContains(t, annotations, "test.bar.io") - assert.NotContains(t, annotations, "test.otel.io") + assert.NotContains(t, annotations, "test.otel/io") + assert.NotContains(t, annotations, "config.otel.test/filter") assert.Equal(t, "1234", annotations["test.io/port"]) } diff --git a/internal/manifests/manifestutils/labels.go b/internal/manifests/manifestutils/labels.go index dc2fc90514..6438191854 100644 --- a/internal/manifests/manifestutils/labels.go +++ b/internal/manifests/manifestutils/labels.go @@ -15,17 +15,27 @@ package manifestutils import ( + "regexp" "strings" + "github.com/go-logr/logr" + "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" "github.com/open-telemetry/opentelemetry-operator/internal/naming" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +var logger logr.Logger + func IsFilteredSet(sourceSet string, filterSet []string) bool { - for _, pattern := range filterSet { - if strings.Contains(sourceSet, pattern) { + for _, basePattern := range filterSet { + pattern, compileErr := regexp.Compile(basePattern) + match := pattern.MatchString(sourceSet) + if compileErr != nil { + logger.Error(compileErr, "could not compile the regexp pattern") + } + if match { return true } } diff --git a/internal/manifests/manifestutils/labels_test.go b/internal/manifests/manifestutils/labels_test.go index 5db392540e..3333c541be 100644 --- a/internal/manifests/manifestutils/labels_test.go +++ b/internal/manifests/manifestutils/labels_test.go @@ -147,7 +147,7 @@ func TestLabelsFilter(t *testing.T) { } // This requires the filter to be in regex match form and not the other simpler wildcard one. - labels := Labels(otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{".bar.io"}) + labels := Labels(otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{".*bar.io"}) // verify assert.Len(t, labels, 7) diff --git a/internal/manifests/opampbridge/deployment_test.go b/internal/manifests/opampbridge/deployment_test.go index cc35ed9f21..77af7843b0 100644 --- a/internal/manifests/opampbridge/deployment_test.go +++ b/internal/manifests/opampbridge/deployment_test.go @@ -237,7 +237,7 @@ func TestDeploymentFilterLabels(t *testing.T) { Spec: v1alpha1.OpAMPBridgeSpec{}, } - cfg := config.New(config.WithLabelFilters([]string{"foo", ".bar"})) + cfg := config.New(config.WithLabelFilters([]string{"foo*", "app.*.bar"})) params := manifests.Params{ Config: cfg, @@ -268,7 +268,7 @@ func TestDeploymentFilterAnnotations(t *testing.T) { Spec: v1alpha1.OpAMPBridgeSpec{}, } - cfg := config.New(config.WithAnnotationFilters([]string{"foo", ".bar"})) + cfg := config.New(config.WithAnnotationFilters([]string{"foo*", "app.*.bar"})) params := manifests.Params{ Config: cfg, From 533e935f3faf31865353080acb25a18525f7aa6c Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Tue, 9 Apr 2024 11:43:06 +0200 Subject: [PATCH 5/7] Fixed labels and annotations filter Signed-off-by: Yuri Sa --- main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.go b/main.go index 6a0ede7500..5f20e2930d 100644 --- a/main.go +++ b/main.go @@ -150,8 +150,8 @@ func main() { stringFlagOrEnv(&autoInstrumentationGo, "auto-instrumentation-go-image", "RELATED_IMAGE_AUTO_INSTRUMENTATION_GO", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-go-instrumentation/autoinstrumentation-go:%s", v.AutoInstrumentationGo), "The default OpenTelemetry Go instrumentation image. This image is used when no image is specified in the CustomResource.") stringFlagOrEnv(&autoInstrumentationApacheHttpd, "auto-instrumentation-apache-httpd-image", "RELATED_IMAGE_AUTO_INSTRUMENTATION_APACHE_HTTPD", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-apache-httpd:%s", v.AutoInstrumentationApacheHttpd), "The default OpenTelemetry Apache HTTPD instrumentation image. This image is used when no image is specified in the CustomResource.") stringFlagOrEnv(&autoInstrumentationNginx, "auto-instrumentation-nginx-image", "RELATED_IMAGE_AUTO_INSTRUMENTATION_NGINX", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/autoinstrumentation-apache-httpd:%s", v.AutoInstrumentationNginx), "The default OpenTelemetry Nginx instrumentation image. This image is used when no image is specified in the CustomResource.") - pflag.StringArrayVar(&labelsFilter, "label", []string{}, "Labels to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings. Example: --labels-filter=filter.out will filter out labels that looks like: label.filter.out: true") - pflag.StringArrayVar(&annotationsFilter, "annotations-filter", []string{}, "Annotations to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings. Example: --annotations-filter=filter.out will filter out annotations that looks like: annotation.filter.out: true") + pflag.StringArrayVar(&labelsFilter, "label", []string{}, "Labels to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings optionally containing a * wildcard character. Example: --labels-filter=.*filter.out will filter out labels that looks like: label.filter.out: true") + pflag.StringArrayVar(&annotationsFilter, "annotations-filter", []string{}, "Annotations to filter away from propagating onto deploys. It should be a string array containing patterns, which are literal strings optionally containing a * wildcard character. Example: --annotations-filter=.*filter.out will filter out annotations that looks like: annotation.filter.out: true") pflag.IntVar(&webhookPort, "webhook-port", 9443, "The port the webhook endpoint binds to.") pflag.StringVar(&tlsOpt.minVersion, "tls-min-version", "VersionTLS12", "Minimum TLS version supported. Value must match version names from https://golang.org/pkg/crypto/tls/#pkg-constants.") pflag.StringSliceVar(&tlsOpt.cipherSuites, "tls-cipher-suites", nil, "Comma-separated list of cipher suites for the server. Values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants). If omitted, the default Go cipher suites will be used") From fa4bcc4d0c7c30ed21492fd5dc64ac6aa3ce5fe8 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Tue, 9 Apr 2024 12:18:43 +0200 Subject: [PATCH 6/7] Fixed labels and annotations filter Signed-off-by: Yuri Sa --- .github/workflows/e2e.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index c89ebb320b..d15fada60e 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -37,7 +37,7 @@ jobs: - group: e2e-multi-instrumentation setup: "add-operator-arg OPERATOR_ARG=--enable-multi-instrumentation prepare-e2e" - group: e2e-metadata-filters - setup: "add-operator-arg OPERATOR_ARG='--annotations-.*filter=filter.out --labels=.*filter.out' prepare-e2e" + setup: "add-operator-arg OPERATOR_ARG='--annotations-filter=.*filter.out --labels=.*filter.out' prepare-e2e" steps: - name: Check out code into the Go module directory From 7d2a9ce5484c5f5e83235e78e27c1878e7c423a8 Mon Sep 17 00:00:00 2001 From: Yuri Sa Date: Wed, 10 Apr 2024 10:54:12 +0200 Subject: [PATCH 7/7] Changed function signature Signed-off-by: Yuri Sa --- internal/manifests/collector/configmap.go | 2 +- internal/manifests/collector/daemonset.go | 6 ++--- internal/manifests/collector/deployment.go | 6 ++--- .../collector/horizontalpodautoscaler.go | 4 ++-- internal/manifests/collector/ingress.go | 2 +- .../collector/poddisruptionbudget.go | 4 ++-- internal/manifests/collector/podmonitor.go | 2 +- internal/manifests/collector/rbac.go | 4 ++-- internal/manifests/collector/service.go | 4 ++-- internal/manifests/collector/service_test.go | 2 +- .../manifests/collector/serviceaccount.go | 2 +- .../manifests/collector/servicemonitor.go | 2 +- internal/manifests/collector/statefulset.go | 6 ++--- .../manifests/manifestutils/annotations.go | 15 +++++++----- .../manifestutils/annotations_test.go | 23 +++++++++++-------- internal/manifests/manifestutils/labels.go | 8 +++---- .../manifests/manifestutils/labels_test.go | 16 ++++++------- internal/manifests/opampbridge/annotations.go | 6 +++-- .../manifests/opampbridge/annotations_test.go | 2 +- internal/manifests/opampbridge/configmap.go | 2 +- internal/manifests/opampbridge/deployment.go | 4 ++-- internal/manifests/opampbridge/service.go | 2 +- .../manifests/opampbridge/serviceaccount.go | 2 +- .../manifests/targetallocator/annotations.go | 6 +++-- .../targetallocator/annotations_test.go | 6 ++--- .../manifests/targetallocator/configmap.go | 2 +- .../manifests/targetallocator/deployment.go | 4 ++-- .../targetallocator/poddisruptionbudget.go | 4 ++-- internal/manifests/targetallocator/service.go | 2 +- .../targetallocator/serviceaccount.go | 2 +- .../targetallocator/serviceaccount_test.go | 2 +- .../targetallocator/servicemonitor.go | 2 +- internal/status/collector/collector.go | 16 +++++++------ internal/status/collector/collector_test.go | 11 +++++---- internal/status/collector/handle.go | 2 +- 35 files changed, 99 insertions(+), 86 deletions(-) diff --git a/internal/manifests/collector/configmap.go b/internal/manifests/collector/configmap.go index 58ee4c1312..f9f380428e 100644 --- a/internal/manifests/collector/configmap.go +++ b/internal/manifests/collector/configmap.go @@ -25,7 +25,7 @@ import ( func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) { name := naming.ConfigMap(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) replacedConf, err := ReplaceConfig(params.OtelCol) if err != nil { diff --git a/internal/manifests/collector/daemonset.go b/internal/manifests/collector/daemonset.go index c006dbd10a..7524b20950 100644 --- a/internal/manifests/collector/daemonset.go +++ b/internal/manifests/collector/daemonset.go @@ -27,14 +27,14 @@ import ( // DaemonSet builds the deployment for the given instance. func DaemonSet(params manifests.Params) (*appsv1.DaemonSet, error) { name := naming.Collector(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) - annotations, err := manifestutils.Annotations(params.OtelCol, params.Config.AnnotationsFilter()) + annotations, err := manifestutils.Annotations(params.Log, params.OtelCol, params.Config.AnnotationsFilter()) if err != nil { return nil, err } - podAnnotations, err := manifestutils.PodAnnotations(params.OtelCol, params.Config.AnnotationsFilter()) + podAnnotations, err := manifestutils.PodAnnotations(params.Log, params.OtelCol, params.Config.AnnotationsFilter()) if err != nil { return nil, err } diff --git a/internal/manifests/collector/deployment.go b/internal/manifests/collector/deployment.go index 1cc105114b..c72c3344f9 100644 --- a/internal/manifests/collector/deployment.go +++ b/internal/manifests/collector/deployment.go @@ -27,13 +27,13 @@ import ( // Deployment builds the deployment for the given instance. func Deployment(params manifests.Params) (*appsv1.Deployment, error) { name := naming.Collector(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) - annotations, err := manifestutils.Annotations(params.OtelCol, params.Config.AnnotationsFilter()) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) + annotations, err := manifestutils.Annotations(params.Log, params.OtelCol, params.Config.AnnotationsFilter()) if err != nil { return nil, err } - podAnnotations, err := manifestutils.PodAnnotations(params.OtelCol, params.Config.AnnotationsFilter()) + podAnnotations, err := manifestutils.PodAnnotations(params.Log, params.OtelCol, params.Config.AnnotationsFilter()) if err != nil { return nil, err } diff --git a/internal/manifests/collector/horizontalpodautoscaler.go b/internal/manifests/collector/horizontalpodautoscaler.go index 1c6de43cae..e65cc34857 100644 --- a/internal/manifests/collector/horizontalpodautoscaler.go +++ b/internal/manifests/collector/horizontalpodautoscaler.go @@ -27,8 +27,8 @@ import ( func HorizontalPodAutoscaler(params manifests.Params) (*autoscalingv2.HorizontalPodAutoscaler, error) { name := naming.Collector(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) - annotations, err := manifestutils.Annotations(params.OtelCol, params.Config.AnnotationsFilter()) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) + annotations, err := manifestutils.Annotations(params.Log, params.OtelCol, params.Config.AnnotationsFilter()) if err != nil { return nil, err } diff --git a/internal/manifests/collector/ingress.go b/internal/manifests/collector/ingress.go index 18c6d0cb6c..8b2883a71f 100644 --- a/internal/manifests/collector/ingress.go +++ b/internal/manifests/collector/ingress.go @@ -31,7 +31,7 @@ import ( func Ingress(params manifests.Params) (*networkingv1.Ingress, error) { name := naming.Ingress(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) if params.OtelCol.Spec.Ingress.Type != v1beta1.IngressTypeNginx { return nil, nil } diff --git a/internal/manifests/collector/poddisruptionbudget.go b/internal/manifests/collector/poddisruptionbudget.go index 619fc7a110..dde91b26d7 100644 --- a/internal/manifests/collector/poddisruptionbudget.go +++ b/internal/manifests/collector/poddisruptionbudget.go @@ -31,8 +31,8 @@ func PodDisruptionBudget(params manifests.Params) (*policyV1.PodDisruptionBudget } name := naming.Collector(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) - annotations, err := manifestutils.Annotations(params.OtelCol, params.Config.AnnotationsFilter()) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) + annotations, err := manifestutils.Annotations(params.Log, params.OtelCol, params.Config.AnnotationsFilter()) if err != nil { return nil, err } diff --git a/internal/manifests/collector/podmonitor.go b/internal/manifests/collector/podmonitor.go index 86157c4138..c4dd0ad113 100644 --- a/internal/manifests/collector/podmonitor.go +++ b/internal/manifests/collector/podmonitor.go @@ -50,7 +50,7 @@ func PodMonitor(params manifests.Params) (*monitoringv1.PodMonitor, error) { return nil, nil } name := naming.PodMonitor(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, nil) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, nil) selectorLabels := manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector) pm = monitoringv1.PodMonitor{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/manifests/collector/rbac.go b/internal/manifests/collector/rbac.go index 4af9223996..26e0d7ad47 100644 --- a/internal/manifests/collector/rbac.go +++ b/internal/manifests/collector/rbac.go @@ -42,7 +42,7 @@ func ClusterRole(params manifests.Params) (*rbacv1.ClusterRole, error) { } name := naming.ClusterRole(params.OtelCol.Name, params.OtelCol.Namespace) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) return &rbacv1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{ @@ -71,7 +71,7 @@ func ClusterRoleBinding(params manifests.Params) (*rbacv1.ClusterRoleBinding, er } name := naming.ClusterRoleBinding(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) return &rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/manifests/collector/service.go b/internal/manifests/collector/service.go index 333b3e5254..b4f3699649 100644 --- a/internal/manifests/collector/service.go +++ b/internal/manifests/collector/service.go @@ -61,7 +61,7 @@ func HeadlessService(params manifests.Params) (*corev1.Service, error) { func MonitoringService(params manifests.Params) (*corev1.Service, error) { name := naming.MonitoringService(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) labels[monitoringLabel] = valueExists out, err := params.OtelCol.Spec.Config.Yaml() @@ -100,7 +100,7 @@ func MonitoringService(params manifests.Params) (*corev1.Service, error) { func Service(params manifests.Params) (*corev1.Service, error) { name := naming.Service(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) out, err := params.OtelCol.Spec.Config.Yaml() if err != nil { diff --git a/internal/manifests/collector/service_test.go b/internal/manifests/collector/service_test.go index 0498a071a8..eedb1acca6 100644 --- a/internal/manifests/collector/service_test.go +++ b/internal/manifests/collector/service_test.go @@ -283,7 +283,7 @@ func service(name string, ports []v1.ServicePort) v1.Service { func serviceWithInternalTrafficPolicy(name string, ports []v1.ServicePort, internalTrafficPolicy v1.ServiceInternalTrafficPolicyType) v1.Service { params := deploymentParams() - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) return v1.Service{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/manifests/collector/serviceaccount.go b/internal/manifests/collector/serviceaccount.go index 4ad29bf696..6cb7278de7 100644 --- a/internal/manifests/collector/serviceaccount.go +++ b/internal/manifests/collector/serviceaccount.go @@ -40,7 +40,7 @@ func ServiceAccount(params manifests.Params) (*corev1.ServiceAccount, error) { } name := naming.ServiceAccount(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) return &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/manifests/collector/servicemonitor.go b/internal/manifests/collector/servicemonitor.go index 1713ccfe50..468dd0d86e 100644 --- a/internal/manifests/collector/servicemonitor.go +++ b/internal/manifests/collector/servicemonitor.go @@ -50,7 +50,7 @@ func ServiceMonitor(params manifests.Params) (*monitoringv1.ServiceMonitor, erro return nil, nil } name := naming.ServiceMonitor(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, []string{}) selectorLabels := manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, ComponentOpenTelemetryCollector) selectorLabels[monitoringLabel] = valueExists diff --git a/internal/manifests/collector/statefulset.go b/internal/manifests/collector/statefulset.go index bfb3a70964..e89a41f54e 100644 --- a/internal/manifests/collector/statefulset.go +++ b/internal/manifests/collector/statefulset.go @@ -27,14 +27,14 @@ import ( // StatefulSet builds the statefulset for the given instance. func StatefulSet(params manifests.Params) (*appsv1.StatefulSet, error) { name := naming.Collector(params.OtelCol.Name) - labels := manifestutils.Labels(params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) + labels := manifestutils.Labels(params.Log, params.OtelCol.ObjectMeta, name, params.OtelCol.Spec.Image, ComponentOpenTelemetryCollector, params.Config.LabelsFilter()) - annotations, err := manifestutils.Annotations(params.OtelCol, params.Config.AnnotationsFilter()) + annotations, err := manifestutils.Annotations(params.Log, params.OtelCol, params.Config.AnnotationsFilter()) if err != nil { return nil, err } - podAnnotations, err := manifestutils.PodAnnotations(params.OtelCol, params.Config.AnnotationsFilter()) + podAnnotations, err := manifestutils.PodAnnotations(params.Log, params.OtelCol, params.Config.AnnotationsFilter()) if err != nil { return nil, err } diff --git a/internal/manifests/manifestutils/annotations.go b/internal/manifests/manifestutils/annotations.go index 06f1baed81..b49bb9e95f 100644 --- a/internal/manifests/manifestutils/annotations.go +++ b/internal/manifests/manifestutils/annotations.go @@ -15,15 +15,18 @@ package manifestutils import ( + "fmt" + "crypto/sha256" "encoding/json" - "fmt" + + "github.com/go-logr/logr" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" ) // Annotations return the annotations for OpenTelemetryCollector pod. -func Annotations(instance v1beta1.OpenTelemetryCollector, filterAnnotations []string) (map[string]string, error) { +func Annotations(logger logr.Logger, instance v1beta1.OpenTelemetryCollector, filterAnnotations []string) (map[string]string, error) { // new map every time, so that we don't touch the instance's annotations annotations := map[string]string{} @@ -38,7 +41,7 @@ func Annotations(instance v1beta1.OpenTelemetryCollector, filterAnnotations []st // allow override of prometheus annotations if nil != instance.ObjectMeta.Annotations { for k, v := range instance.ObjectMeta.Annotations { - if !IsFilteredSet(k, filterAnnotations) { + if !IsFilteredSet(logger, k, filterAnnotations) { annotations[k] = v } } @@ -56,18 +59,18 @@ func Annotations(instance v1beta1.OpenTelemetryCollector, filterAnnotations []st } // PodAnnotations return the spec annotations for OpenTelemetryCollector pod. -func PodAnnotations(instance v1beta1.OpenTelemetryCollector, filterAnnotations []string) (map[string]string, error) { +func PodAnnotations(logger logr.Logger, instance v1beta1.OpenTelemetryCollector, filterAnnotations []string) (map[string]string, error) { // new map every time, so that we don't touch the instance's annotations podAnnotations := map[string]string{} if nil != instance.Spec.PodAnnotations { for k, v := range instance.Spec.PodAnnotations { - if !IsFilteredSet(k, filterAnnotations) { + if !IsFilteredSet(logger, k, filterAnnotations) { podAnnotations[k] = v } } } - annotations, err := Annotations(instance, filterAnnotations) + annotations, err := Annotations(logger, instance, filterAnnotations) if err != nil { return nil, err } diff --git a/internal/manifests/manifestutils/annotations_test.go b/internal/manifests/manifestutils/annotations_test.go index 82d97eefff..48f25fcec2 100644 --- a/internal/manifests/manifestutils/annotations_test.go +++ b/internal/manifests/manifestutils/annotations_test.go @@ -19,12 +19,15 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + logf "sigs.k8s.io/controller-runtime/pkg/log" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" ) +var logger = logf.Log.WithName("unit-tests") + func TestDefaultAnnotations(t *testing.T) { // prepare otelcol := v1beta1.OpenTelemetryCollector{ @@ -45,9 +48,9 @@ func TestDefaultAnnotations(t *testing.T) { } // test - annotations, err := Annotations(otelcol, []string{}) + annotations, err := Annotations(logger, otelcol, []string{}) require.NoError(t, err) - podAnnotations, err := PodAnnotations(otelcol, []string{}) + podAnnotations, err := PodAnnotations(logger, otelcol, []string{}) require.NoError(t, err) //verify @@ -79,9 +82,9 @@ func TestNonDefaultPodAnnotation(t *testing.T) { } // test - annotations, err := Annotations(otelcol, []string{}) + annotations, err := Annotations(logger, otelcol, []string{}) require.NoError(t, err) - podAnnotations, err := PodAnnotations(otelcol, []string{}) + podAnnotations, err := PodAnnotations(logger, otelcol, []string{}) require.NoError(t, err) //verify @@ -121,9 +124,9 @@ func TestUserAnnotations(t *testing.T) { } // test - annotations, err := Annotations(otelcol, []string{}) + annotations, err := Annotations(logger, otelcol, []string{}) require.NoError(t, err) - podAnnotations, err := PodAnnotations(otelcol, []string{}) + podAnnotations, err := PodAnnotations(logger, otelcol, []string{}) require.NoError(t, err) //verify @@ -148,9 +151,9 @@ func TestAnnotationsPropagateDown(t *testing.T) { } // test - annotations, err := Annotations(otelcol, []string{}) + annotations, err := Annotations(logger, otelcol, []string{}) require.NoError(t, err) - podAnnotations, err := PodAnnotations(otelcol, []string{}) + podAnnotations, err := PodAnnotations(logger, otelcol, []string{}) require.NoError(t, err) // verify @@ -175,7 +178,7 @@ func TestAnnotationsSingleFilter(t *testing.T) { } // This requires the filter to be in regex match form and not the other simpler wildcard one. - annotations, err := Annotations(otelcol, []string{".*bar.io"}) + annotations, err := Annotations(logger, otelcol, []string{".*bar.io"}) // verify require.NoError(t, err) @@ -200,7 +203,7 @@ func TestAnnotationsFilter(t *testing.T) { } // This requires the filter to be in regex match form and not the other simpler wildcard one. - annotations, err := Annotations(otelcol, []string{".*bar.io", ".*otel/io", "config.*otel.test.*"}) + annotations, err := Annotations(logger, otelcol, []string{".*bar.io", ".*otel/io", "config.*otel.test.*"}) // verify require.NoError(t, err) diff --git a/internal/manifests/manifestutils/labels.go b/internal/manifests/manifestutils/labels.go index 6438191854..5ba09190fc 100644 --- a/internal/manifests/manifestutils/labels.go +++ b/internal/manifests/manifestutils/labels.go @@ -26,9 +26,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -var logger logr.Logger - -func IsFilteredSet(sourceSet string, filterSet []string) bool { +func IsFilteredSet(logger logr.Logger, sourceSet string, filterSet []string) bool { for _, basePattern := range filterSet { pattern, compileErr := regexp.Compile(basePattern) match := pattern.MatchString(sourceSet) @@ -43,13 +41,13 @@ func IsFilteredSet(sourceSet string, filterSet []string) bool { } // Labels return the common labels to all objects that are part of a managed CR. -func Labels(instance metav1.ObjectMeta, name string, image string, component string, filterLabels []string) map[string]string { +func Labels(logger logr.Logger, instance metav1.ObjectMeta, name string, image string, component string, filterLabels []string) map[string]string { var versionLabel string // new map every time, so that we don't touch the instance's label base := map[string]string{} if nil != instance.Labels { for k, v := range instance.Labels { - if !IsFilteredSet(k, filterLabels) { + if !IsFilteredSet(logger, k, filterLabels) { base[k] = v } } diff --git a/internal/manifests/manifestutils/labels_test.go b/internal/manifests/manifestutils/labels_test.go index 3333c541be..6ee2cae829 100644 --- a/internal/manifests/manifestutils/labels_test.go +++ b/internal/manifests/manifestutils/labels_test.go @@ -46,7 +46,7 @@ func TestLabelsCommonSet(t *testing.T) { } // test - labels := Labels(otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{}) + labels := Labels(logger, otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{}) assert.Equal(t, "opentelemetry-operator", labels["app.kubernetes.io/managed-by"]) assert.Equal(t, "my-ns.my-instance", labels["app.kubernetes.io/instance"]) assert.Equal(t, "0.47.0", labels["app.kubernetes.io/version"]) @@ -66,7 +66,7 @@ func TestLabelsSha256Set(t *testing.T) { } // test - labels := Labels(otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{}) + labels := Labels(logger, otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{}) assert.Equal(t, "opentelemetry-operator", labels["app.kubernetes.io/managed-by"]) assert.Equal(t, "my-ns.my-instance", labels["app.kubernetes.io/instance"]) assert.Equal(t, "c6671841470b83007e0553cdadbc9d05f6cfe17b3ebe9733728dc4a579a5b53", labels["app.kubernetes.io/version"]) @@ -85,7 +85,7 @@ func TestLabelsSha256Set(t *testing.T) { } // test - labelsTag := Labels(otelcolTag.ObjectMeta, collectorName, otelcolTag.Spec.Image, "opentelemetry-collector", []string{}) + labelsTag := Labels(logger, otelcolTag.ObjectMeta, collectorName, otelcolTag.Spec.Image, "opentelemetry-collector", []string{}) assert.Equal(t, "opentelemetry-operator", labelsTag["app.kubernetes.io/managed-by"]) assert.Equal(t, "my-ns.my-instance", labelsTag["app.kubernetes.io/instance"]) assert.Equal(t, "0.81.0", labelsTag["app.kubernetes.io/version"]) @@ -105,7 +105,7 @@ func TestLabelsTagUnset(t *testing.T) { } // test - labels := Labels(otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{}) + labels := Labels(logger, otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{}) assert.Equal(t, "opentelemetry-operator", labels["app.kubernetes.io/managed-by"]) assert.Equal(t, "my-ns.my-instance", labels["app.kubernetes.io/instance"]) assert.Equal(t, "latest", labels["app.kubernetes.io/version"]) @@ -128,7 +128,7 @@ func TestLabelsPropagateDown(t *testing.T) { } // test - labels := Labels(otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{}) + labels := Labels(logger, otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{}) // verify assert.Len(t, labels, 7) @@ -147,7 +147,7 @@ func TestLabelsFilter(t *testing.T) { } // This requires the filter to be in regex match form and not the other simpler wildcard one. - labels := Labels(otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{".*bar.io"}) + labels := Labels(logger, otelcol.ObjectMeta, collectorName, otelcol.Spec.Image, "opentelemetry-collector", []string{".*bar.io"}) // verify assert.Len(t, labels, 7) @@ -185,7 +185,7 @@ func TestLabelsTACommonSet(t *testing.T) { } // test - labels := Labels(tainstance.ObjectMeta, taname, tainstance.Spec.Image, "opentelemetry-targetallocator", nil) + labels := Labels(logger, tainstance.ObjectMeta, taname, tainstance.Spec.Image, "opentelemetry-targetallocator", nil) assert.Equal(t, "opentelemetry-operator", labels["app.kubernetes.io/managed-by"]) assert.Equal(t, "my-ns.my-instance", labels["app.kubernetes.io/instance"]) assert.Equal(t, "opentelemetry", labels["app.kubernetes.io/part-of"]) @@ -206,7 +206,7 @@ func TestLabelsTAPropagateDown(t *testing.T) { } // test - labels := Labels(tainstance.ObjectMeta, taname, tainstance.Spec.Image, "opentelemetry-targetallocator", nil) + labels := Labels(logger, tainstance.ObjectMeta, taname, tainstance.Spec.Image, "opentelemetry-targetallocator", nil) selectorLabels := TASelectorLabels(tainstance, "opentelemetry-targetallocator") diff --git a/internal/manifests/opampbridge/annotations.go b/internal/manifests/opampbridge/annotations.go index 2f29b69eb4..e321c19999 100644 --- a/internal/manifests/opampbridge/annotations.go +++ b/internal/manifests/opampbridge/annotations.go @@ -18,6 +18,8 @@ import ( "crypto/sha256" "fmt" + "github.com/go-logr/logr" + v1 "k8s.io/api/core/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -27,7 +29,7 @@ import ( const configMapHashAnnotationKey = "opentelemetry-opampbridge-config/hash" // Annotations returns the annotations for the OPAmpBridge Pod. -func Annotations(instance v1alpha1.OpAMPBridge, configMap *v1.ConfigMap, filterAnnotations []string) map[string]string { +func Annotations(logger logr.Logger, instance v1alpha1.OpAMPBridge, configMap *v1.ConfigMap, filterAnnotations []string) map[string]string { // Make a copy of PodAnnotations to be safe annotations := make(map[string]string, len(instance.Spec.PodAnnotations)) for key, value := range instance.Spec.PodAnnotations { @@ -35,7 +37,7 @@ func Annotations(instance v1alpha1.OpAMPBridge, configMap *v1.ConfigMap, filterA } if nil != instance.ObjectMeta.Annotations { for k, v := range instance.ObjectMeta.Annotations { - if !manifestutils.IsFilteredSet(k, filterAnnotations) { + if !manifestutils.IsFilteredSet(logger, k, filterAnnotations) { annotations[k] = v } } diff --git a/internal/manifests/opampbridge/annotations_test.go b/internal/manifests/opampbridge/annotations_test.go index cc4f030156..c2fe3c2be0 100644 --- a/internal/manifests/opampbridge/annotations_test.go +++ b/internal/manifests/opampbridge/annotations_test.go @@ -54,7 +54,7 @@ func TestConfigMapHash(t *testing.T) { expectedConfig := expectedConfigMap.Data[OpAMPBridgeFilename] require.NotEmpty(t, expectedConfig) expectedHash := sha256.Sum256([]byte(expectedConfig)) - annotations := Annotations(opampBridge, expectedConfigMap, []string{".*\\.bar\\.io"}) + annotations := Annotations(logger, opampBridge, expectedConfigMap, []string{".*\\.bar\\.io"}) require.Contains(t, annotations, configMapHashAnnotationKey) cmHash := annotations[configMapHashAnnotationKey] assert.Equal(t, fmt.Sprintf("%x", expectedHash), cmHash) diff --git a/internal/manifests/opampbridge/configmap.go b/internal/manifests/opampbridge/configmap.go index b5d0f4a6b9..5cc7145358 100644 --- a/internal/manifests/opampbridge/configmap.go +++ b/internal/manifests/opampbridge/configmap.go @@ -31,7 +31,7 @@ const ( func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) { name := naming.OpAMPBridgeConfigMap(params.OpAMPBridge.Name) - labels := manifestutils.Labels(params.OpAMPBridge.ObjectMeta, name, params.OpAMPBridge.Spec.Image, ComponentOpAMPBridge, []string{}) + labels := manifestutils.Labels(params.Log, params.OpAMPBridge.ObjectMeta, name, params.OpAMPBridge.Spec.Image, ComponentOpAMPBridge, []string{}) config := make(map[interface{}]interface{}) diff --git a/internal/manifests/opampbridge/deployment.go b/internal/manifests/opampbridge/deployment.go index 9b146c83eb..f8c4fb99d1 100644 --- a/internal/manifests/opampbridge/deployment.go +++ b/internal/manifests/opampbridge/deployment.go @@ -27,13 +27,13 @@ import ( // Deployment builds the deployment for the given instance. func Deployment(params manifests.Params) *appsv1.Deployment { name := naming.OpAMPBridge(params.OpAMPBridge.Name) - labels := manifestutils.Labels(params.OpAMPBridge.ObjectMeta, name, params.OpAMPBridge.Spec.Image, ComponentOpAMPBridge, params.Config.LabelsFilter()) + labels := manifestutils.Labels(params.Log, params.OpAMPBridge.ObjectMeta, name, params.OpAMPBridge.Spec.Image, ComponentOpAMPBridge, params.Config.LabelsFilter()) configMap, err := ConfigMap(params) if err != nil { params.Log.Info("failed to construct OpAMPBridge ConfigMap for annotations") configMap = nil } - annotations := Annotations(params.OpAMPBridge, configMap, params.Config.AnnotationsFilter()) + annotations := Annotations(params.Log, params.OpAMPBridge, configMap, params.Config.AnnotationsFilter()) return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: name, diff --git a/internal/manifests/opampbridge/service.go b/internal/manifests/opampbridge/service.go index 0c2a7dd9a0..4a2cf8345e 100644 --- a/internal/manifests/opampbridge/service.go +++ b/internal/manifests/opampbridge/service.go @@ -26,7 +26,7 @@ import ( func Service(params manifests.Params) *corev1.Service { name := naming.OpAMPBridgeService(params.OpAMPBridge.Name) - labels := manifestutils.Labels(params.OpAMPBridge.ObjectMeta, name, params.OpAMPBridge.Spec.Image, ComponentOpAMPBridge, []string{}) + labels := manifestutils.Labels(params.Log, params.OpAMPBridge.ObjectMeta, name, params.OpAMPBridge.Spec.Image, ComponentOpAMPBridge, []string{}) selector := manifestutils.SelectorLabels(params.OpAMPBridge.ObjectMeta, ComponentOpAMPBridge) ports := []corev1.ServicePort{{ diff --git a/internal/manifests/opampbridge/serviceaccount.go b/internal/manifests/opampbridge/serviceaccount.go index f5b0236923..c4f4f944fd 100644 --- a/internal/manifests/opampbridge/serviceaccount.go +++ b/internal/manifests/opampbridge/serviceaccount.go @@ -35,7 +35,7 @@ func ServiceAccountName(instance v1alpha1.OpAMPBridge) string { // ServiceAccount returns the service account for the given instance. func ServiceAccount(params manifests.Params) *corev1.ServiceAccount { name := naming.OpAMPBridgeServiceAccount(params.OpAMPBridge.Name) - labels := manifestutils.Labels(params.OpAMPBridge.ObjectMeta, name, params.OpAMPBridge.Spec.Image, ComponentOpAMPBridge, []string{}) + labels := manifestutils.Labels(params.Log, params.OpAMPBridge.ObjectMeta, name, params.OpAMPBridge.Spec.Image, ComponentOpAMPBridge, []string{}) return &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/manifests/targetallocator/annotations.go b/internal/manifests/targetallocator/annotations.go index 7666bd64ab..27834a1f7e 100644 --- a/internal/manifests/targetallocator/annotations.go +++ b/internal/manifests/targetallocator/annotations.go @@ -18,6 +18,8 @@ import ( "crypto/sha256" "fmt" + "github.com/go-logr/logr" + v1 "k8s.io/api/core/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" @@ -27,7 +29,7 @@ import ( const configMapHashAnnotationKey = "opentelemetry-targetallocator-config/hash" // Annotations returns the annotations for the TargetAllocator Pod. -func Annotations(instance v1beta1.TargetAllocator, configMap *v1.ConfigMap, filterAnnotations []string) map[string]string { +func Annotations(logger logr.Logger, instance v1beta1.TargetAllocator, configMap *v1.ConfigMap, filterAnnotations []string) map[string]string { // Make a copy of PodAnnotations to be safe annotations := make(map[string]string, len(instance.Spec.PodAnnotations)) for key, value := range instance.Spec.PodAnnotations { @@ -35,7 +37,7 @@ func Annotations(instance v1beta1.TargetAllocator, configMap *v1.ConfigMap, filt } if nil != instance.ObjectMeta.Annotations { for k, v := range instance.ObjectMeta.Annotations { - if !manifestutils.IsFilteredSet(k, filterAnnotations) { + if !manifestutils.IsFilteredSet(logger, k, filterAnnotations) { annotations[k] = v } } diff --git a/internal/manifests/targetallocator/annotations_test.go b/internal/manifests/targetallocator/annotations_test.go index 5f6c839a1d..722e945278 100644 --- a/internal/manifests/targetallocator/annotations_test.go +++ b/internal/manifests/targetallocator/annotations_test.go @@ -32,7 +32,7 @@ func TestPodAnnotations(t *testing.T) { instance.Spec.PodAnnotations = map[string]string{ "key": "value", } - annotations := Annotations(instance, nil, []string{".*\\.bar\\.io"}) + annotations := Annotations(logger, instance, nil, []string{".*\\.bar\\.io"}) assert.Subset(t, annotations, instance.Spec.PodAnnotations) } @@ -51,7 +51,7 @@ func TestConfigMapHash(t *testing.T) { expectedConfig := expectedConfigMap.Data[targetAllocatorFilename] require.NotEmpty(t, expectedConfig) expectedHash := sha256.Sum256([]byte(expectedConfig)) - annotations := Annotations(targetAllocator, expectedConfigMap, []string{".*\\.bar\\.io"}) + annotations := Annotations(logger, targetAllocator, expectedConfigMap, []string{".*\\.bar\\.io"}) require.Contains(t, annotations, configMapHashAnnotationKey) cmHash := annotations[configMapHashAnnotationKey] assert.Equal(t, fmt.Sprintf("%x", expectedHash), cmHash) @@ -59,6 +59,6 @@ func TestConfigMapHash(t *testing.T) { func TestInvalidConfigNoHash(t *testing.T) { instance := targetAllocatorInstance() - annotations := Annotations(instance, nil, []string{".*\\.bar\\.io"}) + annotations := Annotations(logger, instance, nil, []string{".*\\.bar\\.io"}) require.NotContains(t, annotations, configMapHashAnnotationKey) } diff --git a/internal/manifests/targetallocator/configmap.go b/internal/manifests/targetallocator/configmap.go index ebf99459d0..e748138b8a 100644 --- a/internal/manifests/targetallocator/configmap.go +++ b/internal/manifests/targetallocator/configmap.go @@ -32,7 +32,7 @@ const ( func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) { instance := params.TargetAllocator name := naming.TAConfigMap(instance.Name) - labels := manifestutils.Labels(instance.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) + labels := manifestutils.Labels(params.Log, instance.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) taSpec := instance.Spec taConfig := make(map[interface{}]interface{}) diff --git a/internal/manifests/targetallocator/deployment.go b/internal/manifests/targetallocator/deployment.go index db36d214fb..3c35941a38 100644 --- a/internal/manifests/targetallocator/deployment.go +++ b/internal/manifests/targetallocator/deployment.go @@ -27,14 +27,14 @@ import ( // Deployment builds the deployment for the given instance. func Deployment(params manifests.Params) (*appsv1.Deployment, error) { name := naming.TargetAllocator(params.TargetAllocator.Name) - labels := manifestutils.Labels(params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) + labels := manifestutils.Labels(params.Log, params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) configMap, err := ConfigMap(params) if err != nil { params.Log.Info("failed to construct target allocator config map for annotations") configMap = nil } - annotations := Annotations(params.TargetAllocator, configMap, params.Config.AnnotationsFilter()) + annotations := Annotations(params.Log, params.TargetAllocator, configMap, params.Config.AnnotationsFilter()) return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/manifests/targetallocator/poddisruptionbudget.go b/internal/manifests/targetallocator/poddisruptionbudget.go index 7746975f1b..e4858a8be8 100644 --- a/internal/manifests/targetallocator/poddisruptionbudget.go +++ b/internal/manifests/targetallocator/poddisruptionbudget.go @@ -42,13 +42,13 @@ func PodDisruptionBudget(params manifests.Params) (*policyV1.PodDisruptionBudget } name := naming.TAPodDisruptionBudget(params.TargetAllocator.Name) - labels := manifestutils.Labels(params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) + labels := manifestutils.Labels(params.Log, params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) configMap, err := ConfigMap(params) if err != nil { params.Log.Info("failed to construct target allocator config map for annotations") configMap = nil } - annotations := Annotations(params.TargetAllocator, configMap, params.Config.AnnotationsFilter()) + annotations := Annotations(params.Log, params.TargetAllocator, configMap, params.Config.AnnotationsFilter()) objectMeta := metav1.ObjectMeta{ Name: name, diff --git a/internal/manifests/targetallocator/service.go b/internal/manifests/targetallocator/service.go index 144213f56d..78a174856d 100644 --- a/internal/manifests/targetallocator/service.go +++ b/internal/manifests/targetallocator/service.go @@ -26,7 +26,7 @@ import ( func Service(params manifests.Params) *corev1.Service { name := naming.TAService(params.TargetAllocator.Name) - labels := manifestutils.Labels(params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) + labels := manifestutils.Labels(params.Log, params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) selector := manifestutils.TASelectorLabels(params.TargetAllocator, ComponentOpenTelemetryTargetAllocator) return &corev1.Service{ diff --git a/internal/manifests/targetallocator/serviceaccount.go b/internal/manifests/targetallocator/serviceaccount.go index e38e64b557..1d6a3d8367 100644 --- a/internal/manifests/targetallocator/serviceaccount.go +++ b/internal/manifests/targetallocator/serviceaccount.go @@ -39,7 +39,7 @@ func ServiceAccount(params manifests.Params) *corev1.ServiceAccount { return nil } name := naming.TargetAllocatorServiceAccount(params.TargetAllocator.Name) - labels := manifestutils.Labels(params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) + labels := manifestutils.Labels(params.Log, params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) return &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/manifests/targetallocator/serviceaccount_test.go b/internal/manifests/targetallocator/serviceaccount_test.go index f8bd532f9e..6017bb87bd 100644 --- a/internal/manifests/targetallocator/serviceaccount_test.go +++ b/internal/manifests/targetallocator/serviceaccount_test.go @@ -73,7 +73,7 @@ func TestServiceAccountDefault(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "my-instance-targetallocator", Namespace: params.OtelCol.Namespace, - Labels: manifestutils.Labels(params.TargetAllocator.ObjectMeta, "my-instance-targetallocator", params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil), + Labels: manifestutils.Labels(params.Log, params.TargetAllocator.ObjectMeta, "my-instance-targetallocator", params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil), Annotations: params.OtelCol.Annotations, }, } diff --git a/internal/manifests/targetallocator/servicemonitor.go b/internal/manifests/targetallocator/servicemonitor.go index a22e4b50f2..a556ca84f3 100644 --- a/internal/manifests/targetallocator/servicemonitor.go +++ b/internal/manifests/targetallocator/servicemonitor.go @@ -26,7 +26,7 @@ import ( // ServiceMonitor returns the service monitor for the given instance. func ServiceMonitor(params manifests.Params) *monitoringv1.ServiceMonitor { name := naming.TargetAllocator(params.TargetAllocator.Name) - labels := manifestutils.Labels(params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) + labels := manifestutils.Labels(params.Log, params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil) return &monitoringv1.ServiceMonitor{ ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/status/collector/collector.go b/internal/status/collector/collector.go index 091b36231c..5aa1edd52a 100644 --- a/internal/status/collector/collector.go +++ b/internal/status/collector/collector.go @@ -15,22 +15,24 @@ package collector import ( - "context" "fmt" - "strconv" - appsv1 "k8s.io/api/apps/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" + "context" + "strconv" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" "github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils" "github.com/open-telemetry/opentelemetry-operator/internal/naming" "github.com/open-telemetry/opentelemetry-operator/internal/version" + + "github.com/go-logr/logr" + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" ) -func UpdateCollectorStatus(ctx context.Context, cli client.Client, changed *v1alpha1.OpenTelemetryCollector) error { +func UpdateCollectorStatus(logger logr.Logger, ctx context.Context, cli client.Client, changed *v1alpha1.OpenTelemetryCollector) error { if changed.Status.Version == "" { // a version is not set, otherwise let the upgrade mechanism take care of it! changed.Status.Version = version.OpenTelemetryCollector() @@ -47,7 +49,7 @@ func UpdateCollectorStatus(ctx context.Context, cli client.Client, changed *v1al name := naming.Collector(changed.Name) // Set the scale selector - labels := manifestutils.Labels(changed.ObjectMeta, name, changed.Spec.Image, collector.ComponentOpenTelemetryCollector, []string{}) + labels := manifestutils.Labels(logger, changed.ObjectMeta, name, changed.Spec.Image, collector.ComponentOpenTelemetryCollector, []string{}) selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{MatchLabels: labels}) if err != nil { return fmt.Errorf("failed to get selector for labelSelector: %w", err) diff --git a/internal/status/collector/collector_test.go b/internal/status/collector/collector_test.go index 83312187dd..c1e0b136cb 100644 --- a/internal/status/collector/collector_test.go +++ b/internal/status/collector/collector_test.go @@ -24,10 +24,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + logf "sigs.k8s.io/controller-runtime/pkg/log" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" ) +var logger = logf.Log.WithName("unit-tests") + func TestUpdateCollectorStatusUnsupported(t *testing.T) { ctx := context.TODO() cli := client.Client(fake.NewFakeClient()) @@ -42,7 +45,7 @@ func TestUpdateCollectorStatusUnsupported(t *testing.T) { }, } - err := UpdateCollectorStatus(ctx, cli, changed) + err := UpdateCollectorStatus(logger, ctx, cli, changed) assert.NoError(t, err) assert.Equal(t, int32(0), changed.Status.Scale.Replicas, "expected replicas to be 0") @@ -89,7 +92,7 @@ func TestUpdateCollectorStatusDeploymentMode(t *testing.T) { }, } - err := UpdateCollectorStatus(ctx, cli, changed) + err := UpdateCollectorStatus(logger, ctx, cli, changed) assert.NoError(t, err) assert.Equal(t, int32(1), changed.Status.Scale.Replicas, "expected replicas to be 1") @@ -137,7 +140,7 @@ func TestUpdateCollectorStatusStatefulset(t *testing.T) { }, } - err := UpdateCollectorStatus(ctx, cli, changed) + err := UpdateCollectorStatus(logger, ctx, cli, changed) assert.NoError(t, err) assert.Equal(t, int32(1), changed.Status.Scale.Replicas, "expected replicas to be 1") @@ -184,7 +187,7 @@ func TestUpdateCollectorStatusDaemonsetMode(t *testing.T) { }, } - err := UpdateCollectorStatus(ctx, cli, changed) + err := UpdateCollectorStatus(logger, ctx, cli, changed) assert.NoError(t, err) assert.Contains(t, changed.Status.Scale.Selector, "customLabel=customValue", "expected selector to contain customlabel=customValue") diff --git a/internal/status/collector/handle.go b/internal/status/collector/handle.go index 9620d37975..8963fbe13d 100644 --- a/internal/status/collector/handle.go +++ b/internal/status/collector/handle.go @@ -60,7 +60,7 @@ func HandleReconcileStatus(ctx context.Context, log logr.Logger, params manifest log.V(2).Error(upgradeErr, "failed to upgrade the OpenTelemetry CR") } changed = &upgraded - statusErr := UpdateCollectorStatus(ctx, params.Client, changed) + statusErr := UpdateCollectorStatus(log, ctx, params.Client, changed) if statusErr != nil { params.Recorder.Event(changed, eventTypeWarning, reasonStatusFailure, statusErr.Error()) return ctrl.Result{}, statusErr