Remove duplicated Sidecar mode validations

.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.

.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 }}
+        path: ${{ github.event_path }}

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
@@ -419,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

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

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 {