From f8064661a6d62fa0a3983dbcc756dcb829aa36c9 Mon Sep 17 00:00:00 2001 From: Shuhei Kitagawa Date: Sun, 2 Nov 2025 16:04:58 +0100 Subject: [PATCH 1/3] Remove duplicated Sidecar mode validations --- ...remove-duplicated-sidecar-validations.yaml | 16 +++ apis/v1beta1/collector_webhook.go | 28 ---- apis/v1beta1/collector_webhook_test.go | 136 +++++++++--------- 3 files changed, 84 insertions(+), 96 deletions(-) create mode 100644 .chloggen/remove-duplicated-sidecar-validations.yaml diff --git a/.chloggen/remove-duplicated-sidecar-validations.yaml b/.chloggen/remove-duplicated-sidecar-validations.yaml new file mode 100644 index 0000000000..41fca601a2 --- /dev/null +++ b/.chloggen/remove-duplicated-sidecar-validations.yaml @@ -0,0 +1,16 @@ +# 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: collector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Removed duplicate Sidecar mode validations from the collector webhook in favor of the CRD CEL checks. + +# One or more tracking issues related to the change +issues: [3319] + +# (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: Kubernetes 1.23 and 1.24 support was dropped in v0.131.0, where custom resource validation expressions were disabled by default. diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go index 0ea26c59f8..cdf4a412b5 100644 --- a/apis/v1beta1/collector_webhook.go +++ b/apis/v1beta1/collector_webhook.go @@ -202,29 +202,6 @@ func (c CollectorWebhook) Validate(ctx context.Context, r *OpenTelemetryCollecto return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'persistentVolumeClaimRetentionPolicy'", r.Spec.Mode) } - // validate tolerations - // NOTE: this validation is also implemented in CRDs using CEL (Common Expression Language) - if r.Spec.Mode == ModeSidecar && len(r.Spec.Tolerations) > 0 { - return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'tolerations'", r.Spec.Mode) - } - - // validate priorityClassName - // NOTE: this validation is also implemented in CRDs using CEL (Common Expression Language) - if r.Spec.Mode == ModeSidecar && r.Spec.PriorityClassName != "" { - return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'priorityClassName'", r.Spec.Mode) - } - - // validate affinity - // NOTE: this validation is also implemented in CRDs using CEL (Common Expression Language) - if r.Spec.Mode == ModeSidecar && r.Spec.Affinity != nil { - return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'affinity'", r.Spec.Mode) - } - - // NOTE: this validation is also implemented in CRDs using CEL (Common Expression Language) - if r.Spec.Mode == ModeSidecar && len(r.Spec.AdditionalContainers) > 0 { - return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'AdditionalContainers'", r.Spec.Mode) - } - // validate target allocator configs if r.Spec.TargetAllocator.Enabled { taWarnings, err := c.validateTargetAllocatorConfig(ctx, r) @@ -295,11 +272,6 @@ func (c CollectorWebhook) Validate(ctx context.Context, r *OpenTelemetryCollecto ) } - if r.Spec.Ingress.Type == IngressTypeIngress && r.Spec.Mode == ModeSidecar { - return warnings, fmt.Errorf("the OpenTelemetry Spec Ingress configuiration is incorrect. Ingress can only be used in combination with the modes: %s, %s, %s", - ModeDeployment, ModeDaemonSet, ModeStatefulSet, - ) - } if r.Spec.Ingress.RuleType == IngressRuleTypeSubdomain && (r.Spec.Ingress.Hostname == "" || r.Spec.Ingress.Hostname == "*") { return warnings, fmt.Errorf("a valid Ingress hostname has to be defined for subdomain ruleType") } diff --git a/apis/v1beta1/collector_webhook_test.go b/apis/v1beta1/collector_webhook_test.go index 2a80859a43..dc74fc8cb7 100644 --- a/apis/v1beta1/collector_webhook_test.go +++ b/apis/v1beta1/collector_webhook_test.go @@ -806,18 +806,6 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "does not support the attribute 'persistentVolumeClaimRetentionPolicy'", }, - { - name: "invalid mode with tolerations", - otelcol: v1beta1.OpenTelemetryCollector{ - Spec: v1beta1.OpenTelemetryCollectorSpec{ - Mode: v1beta1.ModeSidecar, - OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ - Tolerations: []v1.Toleration{{}, {}}, - }, - }, - }, - expectedErr: "does not support the attribute 'tolerations'", - }, { name: "invalid mode with target allocator", otelcol: v1beta1.OpenTelemetryCollector{ @@ -1145,62 +1133,6 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: fmt.Sprintf("Ingress can only be used in combination with the modes: %s, %s, %s", v1beta1.ModeDeployment, v1beta1.ModeDaemonSet, v1beta1.ModeStatefulSet), }, - { - name: "invalid mode with priorityClassName", - otelcol: v1beta1.OpenTelemetryCollector{ - Spec: v1beta1.OpenTelemetryCollectorSpec{ - Mode: v1beta1.ModeSidecar, - OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ - PriorityClassName: "test-class", - }, - }, - }, - expectedErr: "does not support the attribute 'priorityClassName'", - }, - { - name: "invalid mode with affinity", - otelcol: v1beta1.OpenTelemetryCollector{ - Spec: v1beta1.OpenTelemetryCollectorSpec{ - Mode: v1beta1.ModeSidecar, - OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ - Affinity: &v1.Affinity{ - NodeAffinity: &v1.NodeAffinity{ - RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ - NodeSelectorTerms: []v1.NodeSelectorTerm{ - { - MatchExpressions: []v1.NodeSelectorRequirement{ - { - Key: "node", - Operator: v1.NodeSelectorOpIn, - Values: []string{"test-node"}, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - expectedErr: "does not support the attribute 'affinity'", - }, - { - name: "invalid AdditionalContainers", - otelcol: v1beta1.OpenTelemetryCollector{ - Spec: v1beta1.OpenTelemetryCollectorSpec{ - Mode: v1beta1.ModeSidecar, - OpenTelemetryCommonFields: v1beta1.OpenTelemetryCommonFields{ - AdditionalContainers: []v1.Container{ - { - Name: "test", - }, - }, - }, - }, - }, - expectedErr: "the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'AdditionalContainers'", - }, { name: "missing ingress hostname for subdomain ruleType", otelcol: v1beta1.OpenTelemetryCollector{ @@ -1753,6 +1685,74 @@ func TestValidationViaCRDAnnotations(t *testing.T) { }, expectedErr: "spec.ports[0].hostPort in body should be less than or equal to 65535", }, + { + name: "Sidecar mode with tolerations", + collector: func(namespace string) *v1beta1.OpenTelemetryCollector { + c := minimalCollector(namespace) + c.Spec.Mode = v1beta1.ModeSidecar + c.Spec.Tolerations = []v1.Toleration{ + { + Key: "key", + Operator: v1.TolerationOpEqual, + Value: "value", + Effect: v1.TaintEffectNoSchedule, + }, + } + return c + }, + expectedErr: "the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'tolerations'", + }, + { + name: "Sidecar mode with priorityClassName", + collector: func(namespace string) *v1beta1.OpenTelemetryCollector { + c := minimalCollector(namespace) + c.Spec.Mode = v1beta1.ModeSidecar + c.Spec.PriorityClassName = "test-class" + return c + }, + expectedErr: "the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'priorityClassName'", + }, + { + name: "Sidecar mode with affinity", + collector: func(namespace string) *v1beta1.OpenTelemetryCollector { + c := minimalCollector(namespace) + c.Spec.Mode = v1beta1.ModeSidecar + c.Spec.Affinity = &v1.Affinity{ + NodeAffinity: &v1.NodeAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{ + NodeSelectorTerms: []v1.NodeSelectorTerm{ + { + MatchExpressions: []v1.NodeSelectorRequirement{ + { + Key: "node", + Operator: v1.NodeSelectorOpIn, + Values: []string{"test-node"}, + }, + }, + }, + }, + }, + }, + } + return c + }, + expectedErr: "the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'affinity'", + }, + { + name: "Sidecar mode with additionalContainers", + collector: func(namespace string) *v1beta1.OpenTelemetryCollector { + c := minimalCollector(namespace) + c.Spec.Mode = v1beta1.ModeSidecar + c.Spec.AdditionalContainers = []v1.Container{ + { + Name: "test", + Image: "test-image", + }, + } + return c + }, + expectedErr: "the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'additionalContainers'", + }, } for _, tc := range cases { From 5a620f8618fc8e07b6af1024307427529529c469 Mon Sep 17 00:00:00 2001 From: Shuhei Kitagawa Date: Wed, 5 Nov 2025 07:01:38 +0100 Subject: [PATCH 2/3] Run tests on the minimum supported Kubernetes version in CI --- Makefile | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 8be5d02209..03568e5a3c 100644 --- a/Makefile +++ b/Makefile @@ -87,16 +87,17 @@ ENABLE_WEBHOOKS ?= false # Additional flags for go test command GOTEST_EXTRA_OPTS ?= -# If we are running in CI, run go test in verbose mode +# If we are running in CI, run go test in verbose mode without caching ifeq (,$(CI)) GOTEST_OPTS=-race $(if $(GOTEST_EXTRA_OPTS),$(GOTEST_EXTRA_OPTS)) else -GOTEST_OPTS=-race -v $(if $(GOTEST_EXTRA_OPTS),$(GOTEST_EXTRA_OPTS)) +GOTEST_OPTS=-race -v -count=1 $(if $(GOTEST_EXTRA_OPTS),$(GOTEST_EXTRA_OPTS)) endif START_KIND_CLUSTER ?= true KUBE_VERSION ?= 1.33 +MIN_KUBE_VERSION = 1.25 KIND_CONFIG ?= kind-$(KUBE_VERSION).yaml KIND_CLUSTER_NAME ?= "otel-operator" CHAINSAW_SELECTOR := $(shell [ "$(shell printf '%s\n' "$(KUBE_VERSION)" "1.29" | sort -V | head -n1)" = "1.29" ] && echo "--selector sidecar=native" || echo "--selector sidecar=legacy") @@ -174,7 +175,7 @@ all: manager targetallocator operator-opamp-bridge # No lint here, as CI runs it separately .PHONY: ci -ci: generate fmt vet test ensure-update-is-noop +ci: generate fmt vet test test-min-k8s ensure-update-is-noop .PHONY: update update: generate manifests bundle api-docs reset @@ -296,11 +297,21 @@ release-artifacts: set-image-controller manifests: controller-gen $(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=${MANIFEST_DIR} +# Internal target for running tests with a specific Kubernetes version +.PHONY: run-envtest +run-envtest: envtest gotestsum + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(TEST_K8S_VERSION) -p path)" $(GOTESTSUM) -- ${GOTEST_OPTS} ./... + # Run tests # setup-envtest uses KUBEBUILDER_ASSETS which points to a directory with binaries (api-server, etcd and kubectl) .PHONY: test -test: envtest gotestsum - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(KUBE_VERSION) -p path)" $(GOTESTSUM) -- ${GOTEST_OPTS} ./... +test: + @$(MAKE) run-envtest TEST_K8S_VERSION=$(KUBE_VERSION) + +# Run tests with the minimum supported Kubernetes version +.PHONY: test-min-k8s +test-min-k8s: + @$(MAKE) run-envtest TEST_K8S_VERSION=$(MIN_KUBE_VERSION) .PHONY: precommit precommit: fmt vet lint test ensure-update-is-noop From 0dfa7fe1222a8deb4636c960a4a98243752be662 Mon Sep 17 00:00:00 2001 From: Shuhei Kitagawa Date: Wed, 5 Nov 2025 09:51:35 +0100 Subject: [PATCH 3/3] Remove e2e-crd-validations --- .github/workflows/e2e-reusable.yaml | 3 +- Makefile | 5 -- .../sidecar/00-install-priority-class.yaml | 22 ------ .../sidecar/01-install-tolerations.yaml | 26 ------- .../sidecar/02-install-affinity.yaml | 31 -------- .../03-install-additional-containers.yaml | 23 ------ .../sidecar/chainsaw-test.yaml | 72 ------------------- 7 files changed, 1 insertion(+), 181 deletions(-) delete mode 100644 tests/e2e-crd-validations/sidecar/00-install-priority-class.yaml delete mode 100644 tests/e2e-crd-validations/sidecar/01-install-tolerations.yaml delete mode 100644 tests/e2e-crd-validations/sidecar/02-install-affinity.yaml delete mode 100644 tests/e2e-crd-validations/sidecar/03-install-additional-containers.yaml delete mode 100755 tests/e2e-crd-validations/sidecar/chainsaw-test.yaml diff --git a/.github/workflows/e2e-reusable.yaml b/.github/workflows/e2e-reusable.yaml index 71e1c61908..df3c0bb2e6 100644 --- a/.github/workflows/e2e-reusable.yaml +++ b/.github/workflows/e2e-reusable.yaml @@ -81,7 +81,6 @@ jobs: - e2e-multi-instrumentation - e2e-metadata-filters - e2e-ta-collector-mtls - - e2e-crd-validations include: - group: e2e-instrumentation-default setup: "add-instrumentation-params prepare-e2e" @@ -181,4 +180,4 @@ jobs: uses: actions/upload-artifact@v5 with: name: Event File - path: ${{ github.event_path }} \ No newline at end of file + path: ${{ github.event_path }} diff --git a/Makefile b/Makefile index 03568e5a3c..9834692b21 100644 --- a/Makefile +++ b/Makefile @@ -430,11 +430,6 @@ e2e-upgrade: undeploy chainsaw go run hack/check-operator-ready.go $(CHAINSAW) test --test-dir ./tests/e2e-upgrade --report-name e2e-upgrade -# end-to-end tests to test crd validations -.PHONY: e2e-crd-validations -e2e-crd-validations: chainsaw - $(CHAINSAW) test --test-dir ./tests/e2e-crd-validations - .PHONY: prepare-e2e prepare-e2e: chainsaw set-image-controller add-image-targetallocator add-image-opampbridge start-kind cert-manager install-metrics-server install-targetallocator-prometheus-crds load-image-all deploy @mkdir -p ./.testresults/e2e diff --git a/tests/e2e-crd-validations/sidecar/00-install-priority-class.yaml b/tests/e2e-crd-validations/sidecar/00-install-priority-class.yaml deleted file mode 100644 index 5698059c60..0000000000 --- a/tests/e2e-crd-validations/sidecar/00-install-priority-class.yaml +++ /dev/null @@ -1,22 +0,0 @@ -apiVersion: opentelemetry.io/v1beta1 -kind: OpenTelemetryCollector -metadata: - name: sidecar-priorityclass -spec: - mode: sidecar - priorityClassName: "priority" - config: - receivers: - otlp: - protocols: - grpc: {} - http: {} - - exporters: - debug: {} - - service: - pipelines: - traces: - receivers: [otlp] - exporters: [debug] diff --git a/tests/e2e-crd-validations/sidecar/01-install-tolerations.yaml b/tests/e2e-crd-validations/sidecar/01-install-tolerations.yaml deleted file mode 100644 index 5cf502dde0..0000000000 --- a/tests/e2e-crd-validations/sidecar/01-install-tolerations.yaml +++ /dev/null @@ -1,26 +0,0 @@ -apiVersion: opentelemetry.io/v1beta1 -kind: OpenTelemetryCollector -metadata: - name: sidecar-tolerations -spec: - mode: sidecar - tolerations: - - key: "key1" - operator: "Equal" - value: "value1" - effect: "NoSchedule" - config: - receivers: - otlp: - protocols: - grpc: {} - http: {} - - exporters: - debug: {} - - service: - pipelines: - traces: - receivers: [otlp] - exporters: [debug] diff --git a/tests/e2e-crd-validations/sidecar/02-install-affinity.yaml b/tests/e2e-crd-validations/sidecar/02-install-affinity.yaml deleted file mode 100644 index d903014109..0000000000 --- a/tests/e2e-crd-validations/sidecar/02-install-affinity.yaml +++ /dev/null @@ -1,31 +0,0 @@ -apiVersion: opentelemetry.io/v1beta1 -kind: OpenTelemetryCollector -metadata: - name: sidecar-affinity -spec: - mode: sidecar - affinity: - nodeAffinity: - preferredDuringSchedulingIgnoredDuringExecution: - - weight: 1 - preference: - matchExpressions: - - key: kubernetes.io/os - operator: In - values: - - linux - config: - receivers: - otlp: - protocols: - grpc: {} - http: {} - - exporters: - debug: {} - - service: - pipelines: - traces: - receivers: [otlp] - exporters: [debug] diff --git a/tests/e2e-crd-validations/sidecar/03-install-additional-containers.yaml b/tests/e2e-crd-validations/sidecar/03-install-additional-containers.yaml deleted file mode 100644 index 6c93d6ce29..0000000000 --- a/tests/e2e-crd-validations/sidecar/03-install-additional-containers.yaml +++ /dev/null @@ -1,23 +0,0 @@ -apiVersion: opentelemetry.io/v1beta1 -kind: OpenTelemetryCollector -metadata: - name: sidecar-additional-containers -spec: - mode: sidecar - additionalContainers: - - name: some - config: - receivers: - otlp: - protocols: - grpc: {} - http: {} - - exporters: - debug: {} - - service: - pipelines: - traces: - receivers: [otlp] - exporters: [debug] diff --git a/tests/e2e-crd-validations/sidecar/chainsaw-test.yaml b/tests/e2e-crd-validations/sidecar/chainsaw-test.yaml deleted file mode 100755 index 94e26500a3..0000000000 --- a/tests/e2e-crd-validations/sidecar/chainsaw-test.yaml +++ /dev/null @@ -1,72 +0,0 @@ -# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json -apiVersion: chainsaw.kyverno.io/v1alpha1 -kind: Test -metadata: - name: cel-validations-test -spec: - bindings: - - name: version - value: (x_k8s_server_version($config)) - - name: minorVersion - value: (to_number($version.minor)) - steps: - - name: step-00 - bindings: - - name: priorityClassErrorMessageCEL - value: |- - OpenTelemetryCollector.opentelemetry.io "sidecar-priorityclass" is invalid: spec: Invalid value: "object": the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'priorityClassName' - - name: priorityClassErrorMessageWebhook - value: |- - admission webhook "vopentelemetrycollectorcreateupdatebeta.kb.io" denied the request: the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'priorityClassName' - try: - - apply: - file: 00-install-priority-class.yaml - expect: - - check: - ($error != null): true - ($error): (($minorVersion > `24` && $priorityClassErrorMessageCEL) || $priorityClassErrorMessageWebhook) - - name: step-01 - bindings: - - name: tolerationErrorMessageCEL - value: |- - OpenTelemetryCollector.opentelemetry.io "sidecar-tolerations" is invalid: spec: Invalid value: "object": the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'tolerations' - - name: tolerationErrorMessageWebhook - value: |- - admission webhook "vopentelemetrycollectorcreateupdatebeta.kb.io" denied the request: the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'tolerations' - try: - - apply: - file: 01-install-tolerations.yaml - expect: - - check: - ($error != null): true - ($error): (($minorVersion > `24` && $tolerationErrorMessageCEL) || $tolerationErrorMessageWebhook) - - name: step-02 - bindings: - - name: affinityErrorMessageCEL - value: |- - OpenTelemetryCollector.opentelemetry.io "sidecar-affinity" is invalid: spec: Invalid value: "object": the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'affinity' - - name: affinityErrorMessageWebhook - value: |- - admission webhook "vopentelemetrycollectorcreateupdatebeta.kb.io" denied the request: the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'affinity' - try: - - apply: - file: 02-install-affinity.yaml - expect: - - check: - ($error != null): true - ($error): (($minorVersion > `24` && $affinityErrorMessageCEL) || $affinityErrorMessageWebhook) - - name: step-03 - bindings: - - name: affinityErrorMessageCEL - value: |- - OpenTelemetryCollector.opentelemetry.io "sidecar-additional-containers" is invalid: spec: Invalid value: "object": the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'additionalContainers' - - name: affinityErrorMessageWebhook - value: |- - admission webhook "vopentelemetrycollectorcreateupdatebeta.kb.io" denied the request: the OpenTelemetry Collector mode is set to sidecar, which does not support the attribute 'AdditionalContainers' - try: - - apply: - file: 03-install-additional-containers.yaml - expect: - - check: - ($error != null): true - ($error): (($minorVersion > `24` && $affinityErrorMessageCEL) || $affinityErrorMessageWebhook)