Enhanced Webhook Reconciliation Errors (#3180)

* initial commit

* added enhanced reconciliation errors

* tests

* update

* tests

* add test

* changelog

* initial fixes

* linting

* changing to warnings

* shadow declarations

* fixing errors

* kubebuilder test

* kubebuilder testing

* fixing file

* unit test

* initial changes

* clean up

* merge conflicts

* Update apis/v1beta1/collector_webhook_test.go

Co-authored-by: Israel Blancas <[email protected]>

* fixed test

* fixed test

* unneeded code

* linting

* linting and unit tests

* reuse code

* Update apis/v1beta1/collector_webhook.go

Co-authored-by: Jacob Aronoff <[email protected]>

* revert to not use getParams

* use config

* linting

* more linting

* validate as anon func

* linting

* linting

* testing without test

* added back test

* fix

* make generate

---------

Co-authored-by: Israel Blancas <[email protected]>
Co-authored-by: Jacob Aronoff <[email protected]>
Co-authored-by: Jacob Aronoff <[email protected]>

Author: 3 people

.chloggen/enhanced-webhook.yaml

@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
+component: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Added reconciliation errors for webhook events. The webhooks run the manifest generators to check for any errors.
+
+# One or more tracking issues related to the change
+issues: [2399]
+
+# (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:

apis/v1beta1/collector_webhook.go

@@ -47,6 +47,7 @@ type CollectorWebhook struct {
 	scheme   *runtime.Scheme
 	reviewer *rbac.Reviewer
 	metrics  *Metrics
+	bv       BuildValidator
 }
 
 func (c CollectorWebhook) Default(_ context.Context, obj runtime.Object) error {
@@ -108,14 +109,17 @@ func (c CollectorWebhook) ValidateCreate(ctx context.Context, obj runtime.Object
 		return nil, fmt.Errorf("expected an OpenTelemetryCollector, received %T", obj)
 	}
 
-	warnings, err := c.validate(ctx, otelcol)
+	warnings, err := c.Validate(ctx, otelcol)
 	if err != nil {
 		return warnings, err
 	}
 	if c.metrics != nil {
 		c.metrics.create(ctx, otelcol)
 	}
-
+	if c.bv != nil {
+		newWarnings := c.bv(*otelcol)
+		warnings = append(warnings, newWarnings...)
+	}
 	return warnings, nil
 }
 
@@ -133,7 +137,7 @@ func (c CollectorWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj run
 	if otelcolOld.Spec.Mode != otelcol.Spec.Mode {
 		return admission.Warnings{}, fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support modification", otelcolOld.Spec.Mode)
 	}
-	warnings, err := c.validate(ctx, otelcol)
+	warnings, err := c.Validate(ctx, otelcol)
 	if err != nil {
 		return warnings, err
 	}
@@ -142,6 +146,10 @@ func (c CollectorWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj run
 		c.metrics.update(ctx, otelcolOld, otelcol)
 	}
 
+	if c.bv != nil {
+		newWarnings := c.bv(*otelcol)
+		warnings = append(warnings, newWarnings...)
+	}
 	return warnings, nil
 }
 
@@ -151,7 +159,7 @@ func (c CollectorWebhook) ValidateDelete(ctx context.Context, obj runtime.Object
 		return nil, fmt.Errorf("expected an OpenTelemetryCollector, received %T", obj)
 	}
 
-	warnings, err := c.validate(ctx, otelcol)
+	warnings, err := c.Validate(ctx, otelcol)
 	if err != nil {
 		return warnings, err
 	}
@@ -163,7 +171,7 @@ func (c CollectorWebhook) ValidateDelete(ctx context.Context, obj runtime.Object
 	return warnings, nil
 }
 
-func (c CollectorWebhook) validate(ctx context.Context, r *OpenTelemetryCollector) (admission.Warnings, error) {
+func (c CollectorWebhook) Validate(ctx context.Context, r *OpenTelemetryCollector) (admission.Warnings, error) {
 	warnings := admission.Warnings{}
 
 	nullObjects := r.Spec.Config.nullObjects()
@@ -404,14 +412,30 @@ func checkAutoscalerSpec(autoscaler *AutoscalerSpec) error {
 	return nil
 }
 
-func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *Metrics) error {
-	cvw := &CollectorWebhook{
-		reviewer: reviewer,
-		logger:   mgr.GetLogger().WithValues("handler", "CollectorWebhook", "version", "v1beta1"),
-		scheme:   mgr.GetScheme(),
+// BuildValidator enables running the manifest generators for the collector reconciler
+// +kubebuilder:object:generate=false
+type BuildValidator func(c OpenTelemetryCollector) admission.Warnings
+
+func NewCollectorWebhook(
+	logger logr.Logger,
+	scheme *runtime.Scheme,
+	cfg config.Config,
+	reviewer *rbac.Reviewer,
+	metrics *Metrics,
+	bv BuildValidator,
+) *CollectorWebhook {
+	return &CollectorWebhook{
+		logger:   logger,
+		scheme:   scheme,
 		cfg:      cfg,
+		reviewer: reviewer,
 		metrics:  metrics,
+		bv:       bv,
 	}
+}
+
+func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer, metrics *Metrics, bv BuildValidator) error {
+	cvw := NewCollectorWebhook(mgr.GetLogger().WithValues("handler", "CollectorWebhook", "version", "v1beta1"), mgr.GetScheme(), cfg, reviewer, metrics, bv)
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&OpenTelemetryCollector{}).
 		WithValidator(cvw).