Run job from validating webhook

apis/v1alpha1/opentelemetrycollector_webhook.go

@@ -18,11 +18,8 @@ import (
 	"fmt"
 
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation"
-	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters"
@@ -32,15 +29,9 @@ import (
 // log is for logging in this package.
 var opentelemetrycollectorlog = logf.Log.WithName("opentelemetrycollector-resource")
 
-func (r *OpenTelemetryCollector) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).
-		For(r).
-		Complete()
-}
-
 // +kubebuilder:webhook:path=/mutate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=true,failurePolicy=fail,groups=opentelemetry.io,resources=opentelemetrycollectors,verbs=create;update,versions=v1alpha1,name=mopentelemetrycollector.kb.io,sideEffects=none,admissionReviewVersions=v1
-
-var _ webhook.Defaulter = &OpenTelemetryCollector{}
+// +kubebuilder:webhook:verbs=create;update,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=fail,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectorcreateupdate.kb.io,sideEffects=none,admissionReviewVersions=v1
+// +kubebuilder:webhook:verbs=delete,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=ignore,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectordelete.kb.io,sideEffects=none,admissionReviewVersions=v1
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type.
 func (r *OpenTelemetryCollector) Default() {
@@ -99,72 +90,51 @@ func (r *OpenTelemetryCollector) Default() {
 	}
 }
 
-// +kubebuilder:webhook:verbs=create;update,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=fail,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectorcreateupdate.kb.io,sideEffects=none,admissionReviewVersions=v1
-// +kubebuilder:webhook:verbs=delete,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=ignore,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectordelete.kb.io,sideEffects=none,admissionReviewVersions=v1
-
-var _ webhook.Validator = &OpenTelemetryCollector{}
-
-// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (r *OpenTelemetryCollector) ValidateCreate() (admission.Warnings, error) {
-	opentelemetrycollectorlog.Info("validate create", "name", r.Name)
-	return nil, r.validateCRDSpec()
-}
-
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (r *OpenTelemetryCollector) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	opentelemetrycollectorlog.Info("validate update", "name", r.Name)
-	return nil, r.validateCRDSpec()
-}
-
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (r *OpenTelemetryCollector) ValidateDelete() (admission.Warnings, error) {
-	opentelemetrycollectorlog.Info("validate delete", "name", r.Name)
-	return nil, nil
-}
-
-func (r *OpenTelemetryCollector) validateCRDSpec() error {
+// ValidateCRDSpec adheres closely to the admission.Validate spec to allow the collector to validate its CRD spec.
+func (r *OpenTelemetryCollector) ValidateCRDSpec() (admission.Warnings, error) {
+	warnings := admission.Warnings{}
 	// validate volumeClaimTemplates
 	if r.Spec.Mode != ModeStatefulSet && len(r.Spec.VolumeClaimTemplates) > 0 {
-		return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'volumeClaimTemplates'", r.Spec.Mode)
+		return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'volumeClaimTemplates'", r.Spec.Mode)
 	}
 
 	// validate tolerations
 	if r.Spec.Mode == ModeSidecar && len(r.Spec.Tolerations) > 0 {
-		return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'tolerations'", r.Spec.Mode)
+		return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'tolerations'", r.Spec.Mode)
 	}
 
 	// validate priorityClassName
 	if r.Spec.Mode == ModeSidecar && r.Spec.PriorityClassName != "" {
-		return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'priorityClassName'", r.Spec.Mode)
+		return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'priorityClassName'", r.Spec.Mode)
 	}
 
 	// validate affinity
 	if r.Spec.Mode == ModeSidecar && r.Spec.Affinity != nil {
-		return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'affinity'", r.Spec.Mode)
+		return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'affinity'", r.Spec.Mode)
 	}
 
 	if r.Spec.Mode == ModeSidecar && len(r.Spec.AdditionalContainers) > 0 {
-		return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'AdditionalContainers'", r.Spec.Mode)
+		return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'AdditionalContainers'", r.Spec.Mode)
 	}
 
 	// validate target allocation
 	if r.Spec.TargetAllocator.Enabled && r.Spec.Mode != ModeStatefulSet {
-		return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the target allocation deployment", r.Spec.Mode)
+		return warnings, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the target allocation deployment", r.Spec.Mode)
 	}
 
 	// validate Prometheus config for target allocation
 	if r.Spec.TargetAllocator.Enabled {
 		promCfg, err := ta.ConfigToPromConfig(r.Spec.Config)
 		if err != nil {
-			return fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
+			return warnings, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
 		}
 		err = ta.ValidatePromConfig(promCfg, r.Spec.TargetAllocator.Enabled, featuregate.EnableTargetAllocatorRewrite.IsEnabled())
 		if err != nil {
-			return fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
+			return warnings, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
 		}
 		err = ta.ValidateTargetAllocatorConfig(r.Spec.TargetAllocator.PrometheusCR.Enabled, promCfg)
 		if err != nil {
-			return fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
+			return warnings, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
 		}
 	}
 
@@ -173,95 +143,97 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error {
 		nameErrs := validation.IsValidPortName(p.Name)
 		numErrs := validation.IsValidPortNum(int(p.Port))
 		if len(nameErrs) > 0 || len(numErrs) > 0 {
-			return fmt.Errorf("the OpenTelemetry Spec Ports configuration is incorrect, port name '%s' errors: %s, num '%d' errors: %s",
+			return warnings, fmt.Errorf("the OpenTelemetry Spec Ports configuration is incorrect, port name '%s' errors: %s, num '%d' errors: %s",
 				p.Name, nameErrs, p.Port, numErrs)
 		}
 	}
 
-	maxReplicas := new(int32)
+	var maxReplicas *int32
 	if r.Spec.Autoscaler != nil && r.Spec.Autoscaler.MaxReplicas != nil {
 		maxReplicas = r.Spec.Autoscaler.MaxReplicas
 	}
 
 	// check deprecated .Spec.MaxReplicas if maxReplicas is not set
-	if *maxReplicas == 0 {
+	if maxReplicas == nil && r.Spec.MaxReplicas != nil {
+		warnings = append(warnings, "MaxReplicas is deprecated")
 		maxReplicas = r.Spec.MaxReplicas
 	}
 
-	minReplicas := new(int32)
+	var minReplicas *int32
 	if r.Spec.Autoscaler != nil && r.Spec.Autoscaler.MinReplicas != nil {
 		minReplicas = r.Spec.Autoscaler.MinReplicas
 	}
 
 	// check deprecated .Spec.MinReplicas if minReplicas is not set
-	if *minReplicas == 0 {
+	if minReplicas == nil {
 		if r.Spec.MinReplicas != nil {
+			warnings = append(warnings, "MinReplicas is deprecated")
 			minReplicas = r.Spec.MinReplicas
 		} else {
 			minReplicas = r.Spec.Replicas
 		}
 	}
 
 	if r.Spec.Ingress.Type == IngressTypeNginx && r.Spec.Mode == ModeSidecar {
-		return fmt.Errorf("the OpenTelemetry Spec Ingress configuration is incorrect. Ingress can only be used in combination with the modes: %s, %s, %s",
+		return warnings, fmt.Errorf("the OpenTelemetry Spec Ingress configuration is incorrect. Ingress can only be used in combination with the modes: %s, %s, %s",
 			ModeDeployment, ModeDaemonSet, ModeStatefulSet,
 		)
 	}
 
 	// validate autoscale with horizontal pod autoscaler
 	if maxReplicas != nil {
 		if *maxReplicas < int32(1) {
-			return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maxReplicas should be defined and one or more")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maxReplicas should be defined and one or more")
 		}
 
 		if r.Spec.Replicas != nil && *r.Spec.Replicas > *maxReplicas {
-			return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, replicas must not be greater than maxReplicas")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, replicas must not be greater than maxReplicas")
 		}
 
 		if minReplicas != nil && *minReplicas > *maxReplicas {
-			return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas must not be greater than maxReplicas")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas must not be greater than maxReplicas")
 		}
 
 		if minReplicas != nil && *minReplicas < int32(1) {
-			return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas should be one or more")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas should be one or more")
 		}
 
 		if r.Spec.Autoscaler != nil {
-			return checkAutoscalerSpec(r.Spec.Autoscaler)
+			return warnings, checkAutoscalerSpec(r.Spec.Autoscaler)
 		}
 	}
 
 	if r.Spec.Ingress.Type == IngressTypeNginx && r.Spec.Mode == ModeSidecar {
-		return fmt.Errorf("the OpenTelemetry Spec Ingress configuiration is incorrect. Ingress can only be used in combination with the modes: %s, %s, %s",
+		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 fmt.Errorf("a valid Ingress hostname has to be defined for subdomain ruleType")
+		return warnings, fmt.Errorf("a valid Ingress hostname has to be defined for subdomain ruleType")
 	}
 
 	if r.Spec.LivenessProbe != nil {
 		if r.Spec.LivenessProbe.InitialDelaySeconds != nil && *r.Spec.LivenessProbe.InitialDelaySeconds < 0 {
-			return fmt.Errorf("the OpenTelemetry Spec LivenessProbe InitialDelaySeconds configuration is incorrect. InitialDelaySeconds should be greater than or equal to 0")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec LivenessProbe InitialDelaySeconds configuration is incorrect. InitialDelaySeconds should be greater than or equal to 0")
 		}
 		if r.Spec.LivenessProbe.PeriodSeconds != nil && *r.Spec.LivenessProbe.PeriodSeconds < 1 {
-			return fmt.Errorf("the OpenTelemetry Spec LivenessProbe PeriodSeconds configuration is incorrect. PeriodSeconds should be greater than or equal to 1")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec LivenessProbe PeriodSeconds configuration is incorrect. PeriodSeconds should be greater than or equal to 1")
 		}
 		if r.Spec.LivenessProbe.TimeoutSeconds != nil && *r.Spec.LivenessProbe.TimeoutSeconds < 1 {
-			return fmt.Errorf("the OpenTelemetry Spec LivenessProbe TimeoutSeconds configuration is incorrect. TimeoutSeconds should be greater than or equal to 1")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec LivenessProbe TimeoutSeconds configuration is incorrect. TimeoutSeconds should be greater than or equal to 1")
 		}
 		if r.Spec.LivenessProbe.SuccessThreshold != nil && *r.Spec.LivenessProbe.SuccessThreshold < 1 {
-			return fmt.Errorf("the OpenTelemetry Spec LivenessProbe SuccessThreshold configuration is incorrect. SuccessThreshold should be greater than or equal to 1")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec LivenessProbe SuccessThreshold configuration is incorrect. SuccessThreshold should be greater than or equal to 1")
 		}
 		if r.Spec.LivenessProbe.FailureThreshold != nil && *r.Spec.LivenessProbe.FailureThreshold < 1 {
-			return fmt.Errorf("the OpenTelemetry Spec LivenessProbe FailureThreshold configuration is incorrect. FailureThreshold should be greater than or equal to 1")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec LivenessProbe FailureThreshold configuration is incorrect. FailureThreshold should be greater than or equal to 1")
 		}
 		if r.Spec.LivenessProbe.TerminationGracePeriodSeconds != nil && *r.Spec.LivenessProbe.TerminationGracePeriodSeconds < 1 {
-			return fmt.Errorf("the OpenTelemetry Spec LivenessProbe TerminationGracePeriodSeconds configuration is incorrect. TerminationGracePeriodSeconds should be greater than or equal to 1")
+			return warnings, fmt.Errorf("the OpenTelemetry Spec LivenessProbe TerminationGracePeriodSeconds configuration is incorrect. TerminationGracePeriodSeconds should be greater than or equal to 1")
 		}
 	}
 
-	return nil
+	return warnings, nil
 }
 
 func checkAutoscalerSpec(autoscaler *AutoscalerSpec) error {

apis/v1alpha1/opentelemetrycollector_webhook_test.go

@@ -180,9 +180,10 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 	five := int32(5)
 
 	tests := []struct { //nolint:govet
-		name        string
-		otelcol     OpenTelemetryCollector
-		expectedErr string
+		name             string
+		otelcol          OpenTelemetryCollector
+		expectedErr      string
+		expectedWarnings []string
 	}{
 		{
 			name:    "valid empty spec",
@@ -335,7 +336,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					MaxReplicas: &zero,
 				},
 			},
-			expectedErr: "maxReplicas should be defined and one or more",
+			expectedErr:      "maxReplicas should be defined and one or more",
+			expectedWarnings: []string{"MaxReplicas is deprecated"},
 		},
 		{
 			name: "invalid replicas, greater than max",
@@ -345,7 +347,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					Replicas:    &five,
 				},
 			},
-			expectedErr: "replicas must not be greater than maxReplicas",
+			expectedErr:      "replicas must not be greater than maxReplicas",
+			expectedWarnings: []string{"MaxReplicas is deprecated"},
 		},
 		{
 			name: "invalid min replicas, greater than max",
@@ -355,7 +358,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					MinReplicas: &five,
 				},
 			},
-			expectedErr: "minReplicas must not be greater than maxReplicas",
+			expectedErr:      "minReplicas must not be greater than maxReplicas",
+			expectedWarnings: []string{"MaxReplicas is deprecated", "MinReplicas is deprecated"},
 		},
 		{
 			name: "invalid min replicas, lesser than 1",
@@ -365,7 +369,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					MinReplicas: &zero,
 				},
 			},
-			expectedErr: "minReplicas should be one or more",
+			expectedErr:      "minReplicas should be one or more",
+			expectedWarnings: []string{"MaxReplicas is deprecated", "MinReplicas is deprecated"},
 		},
 		{
 			name: "invalid autoscaler scale down",
@@ -381,7 +386,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "scaleDown should be one or more",
+			expectedErr:      "scaleDown should be one or more",
+			expectedWarnings: []string{"MaxReplicas is deprecated"},
 		},
 		{
 			name: "invalid autoscaler scale up",
@@ -397,7 +403,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "scaleUp should be one or more",
+			expectedErr:      "scaleUp should be one or more",
+			expectedWarnings: []string{"MaxReplicas is deprecated"},
 		},
 		{
 			name: "invalid autoscaler target cpu utilization",
@@ -409,7 +416,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "targetCPUUtilization should be greater than 0 and less than 100",
+			expectedErr:      "targetCPUUtilization should be greater than 0 and less than 100",
+			expectedWarnings: []string{"MaxReplicas is deprecated"},
 		},
 		{
 			name: "autoscaler minReplicas is less than maxReplicas",
@@ -437,7 +445,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "the OpenTelemetry Spec autoscale configuration is incorrect, metric type unsupported. Expected metric of source type Pod",
+			expectedErr:      "the OpenTelemetry Spec autoscale configuration is incorrect, metric type unsupported. Expected metric of source type Pod",
+			expectedWarnings: []string{"MaxReplicas is deprecated"},
 		},
 		{
 			name: "invalid pod metric average value",
@@ -462,7 +471,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "the OpenTelemetry Spec autoscale configuration is incorrect, average value should be greater than 0",
+			expectedErr:      "the OpenTelemetry Spec autoscale configuration is incorrect, average value should be greater than 0",
+			expectedWarnings: []string{"MaxReplicas is deprecated"},
 		},
 		{
 			name: "utilization target is not valid with pod metrics",
@@ -487,7 +497,8 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 					},
 				},
 			},
-			expectedErr: "the OpenTelemetry Spec autoscale configuration is incorrect, invalid pods target type",
+			expectedErr:      "the OpenTelemetry Spec autoscale configuration is incorrect, invalid pods target type",
+			expectedWarnings: []string{"MaxReplicas is deprecated"},
 		},
 		{
 			name: "invalid deployment mode incompabible with ingress settings",
@@ -634,11 +645,16 @@ func TestOTELColValidatingWebhook(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err := test.otelcol.validateCRDSpec()
+			warnings, err := test.otelcol.ValidateCRDSpec()
 			if test.expectedErr == "" {
 				assert.NoError(t, err)
 				return
 			}
+			if len(test.expectedWarnings) == 0 {
+				assert.Empty(t, warnings, test.expectedWarnings)
+			} else {
+				assert.ElementsMatch(t, warnings, test.expectedWarnings)
+			}
 			assert.ErrorContains(t, err, test.expectedErr)
 		})
 	}

bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml

@@ -31,7 +31,7 @@ metadata:
     categories: Logging & Tracing,Monitoring
     certified: "false"
     containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator
-    createdAt: "2023-09-20T15:05:15Z"
+    createdAt: "2023-10-03T15:28:54Z"
     description: Provides the OpenTelemetry components, including the Collector
     operators.operatorframework.io/builder: operator-sdk-v1.29.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
@@ -168,6 +168,18 @@ spec:
           - patch
           - update
           - watch
+        - apiGroups:
+          - batch
+          resources:
+          - jobs
+          verbs:
+          - create
+          - delete
+          - get
+          - list
+          - patch
+          - update
+          - watch
         - apiGroups:
           - coordination.k8s.io
           resources: