Error aggregation for Validation Webhooks

api/v1alpha1/fenceagentsremediation_webhook.go

@@ -19,9 +19,8 @@ package v1alpha1
 import (
 	"fmt"
 
-	"github.com/pkg/errors"
-
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -71,30 +70,35 @@ func (far *FenceAgentsRemediation) ValidateDelete() (admission.Warnings, error)
 }
 
 func validateFAR(farSpec *FenceAgentsRemediationSpec) (admission.Warnings, error) {
-	if _, err := validateAgentName(farSpec.Agent); err != nil {
-		return nil, err
-	}
-	return validateStrategy(farSpec.RemediationStrategy)
+	aggregated := errors.NewAggregate([]error{
+		validateAgentName(farSpec.Agent),
+		validateStrategy(farSpec.RemediationStrategy),
+	})
+
+	return admission.Warnings{}, aggregated
 }
 
 func InitOutOfServiceTaintSupportedFlag(outOfServiceTaintSupported bool) {
 	isOutOfServiceTaintSupported = outOfServiceTaintSupported
 }
 
-func validateAgentName(agent string) (admission.Warnings, error) {
+func validateAgentName(agent string) error {
 	exists, err := agentValidator.ValidateAgentName(agent)
 	if err != nil {
-		return nil, errors.WithMessagef(err, "Failed to validate fence agent: %s. You might want to try again.", agent)
+		return errors.NewAggregate([]error{
+			fmt.Errorf("Failed to validate fence agent: %s. You might want to try again.", agent),
+			err,
+		})
 	}
 	if !exists {
-		return nil, fmt.Errorf("unsupported fence agent: %s", agent)
+		return fmt.Errorf("unsupported fence agent: %s", agent)
 	}
-	return nil, nil
+	return nil
 }
 
-func validateStrategy(farRemStrategy RemediationStrategyType) (admission.Warnings, error) {
+func validateStrategy(farRemStrategy RemediationStrategyType) error {
 	if farRemStrategy == OutOfServiceTaintRemediationStrategy && !isOutOfServiceTaintSupported {
-		return nil, fmt.Errorf("%s remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy", OutOfServiceTaintRemediationStrategy)
+		return fmt.Errorf("%s remediation strategy is not supported at kubernetes version lower than 1.26, please use a different remediation strategy", OutOfServiceTaintRemediationStrategy)
 	}
-	return nil, nil
+	return nil
 }

api/v1alpha1/fenceagentsremediation_webhook_test.go

@@ -21,7 +21,9 @@ var _ = Describe("FenceAgentsRemediation Validation", func() {
 		When("agent name was not found ", func() {
 			It("should be rejected", func() {
 				far := getTestFAR(invalidAgentName)
-				Expect(far.ValidateCreate()).Error().To(MatchError(ContainSubstring("unsupported fence agent: %s", invalidAgentName)))
+				warnings, err := far.ValidateCreate()
+				ExpectWithOffset(1, warnings).To(BeEmpty())
+				Expect(err).To(MatchError(ContainSubstring("unsupported fence agent: %s", invalidAgentName)))
 			})
 		})
 
@@ -47,7 +49,9 @@ var _ = Describe("FenceAgentsRemediation Validation", func() {
 					isOutOfServiceTaintSupported = false
 				})
 				It("should be denied", func() {
-					Expect(outOfServiceStrategy.ValidateCreate()).Error().To(MatchError(ContainSubstring(outOfServiceTaintUnsupportedMsg)))
+					warnings, err := outOfServiceStrategy.ValidateCreate()
+					ExpectWithOffset(1, warnings).To(BeEmpty())
+					Expect(err).To(MatchError(ContainSubstring(outOfServiceTaintUnsupportedMsg)))
 				})
 			})
 		})
@@ -71,7 +75,9 @@ var _ = Describe("FenceAgentsRemediation Validation", func() {
 			})
 			It("should be rejected", func() {
 				far := getTestFAR(invalidAgentName)
-				Expect(far.ValidateUpdate(oldFAR)).Error().To(MatchError(ContainSubstring("unsupported fence agent: %s", invalidAgentName)))
+				warnings, err := far.ValidateUpdate(oldFAR)
+				ExpectWithOffset(1, warnings).To(BeEmpty())
+				Expect(err).To(MatchError(ContainSubstring("unsupported fence agent: %s", invalidAgentName)))
 			})
 		})
 
@@ -99,7 +105,9 @@ var _ = Describe("FenceAgentsRemediation Validation", func() {
 					isOutOfServiceTaintSupported = false
 				})
 				It("should be denied", func() {
-					Expect(outOfServiceStrategy.ValidateUpdate(resourceDeletionStrategy)).Error().To(MatchError(ContainSubstring(outOfServiceTaintUnsupportedMsg)))
+					warnings, err := outOfServiceStrategy.ValidateUpdate(resourceDeletionStrategy)
+					ExpectWithOffset(1, warnings).To(BeEmpty())
+					Expect(err).To(MatchError(ContainSubstring(outOfServiceTaintUnsupportedMsg)))
 				})
 			})
 		})

api/v1alpha1/fenceagentsremediationtemplate_webhook_test.go

@@ -21,7 +21,9 @@ var _ = Describe("FenceAgentsRemediationTemplate Validation", func() {
 		When("agent name was not found ", func() {
 			It("should be rejected", func() {
 				farTemplate := getTestFARTemplate(invalidAgentName)
-				Expect(farTemplate.ValidateCreate()).Error().To(MatchError(ContainSubstring("unsupported fence agent: %s", invalidAgentName)))
+				warnings, err := farTemplate.ValidateCreate()
+				ExpectWithOffset(1, warnings).To(BeEmpty())
+				Expect(err).To(MatchError(ContainSubstring("unsupported fence agent: %s", invalidAgentName)))
 			})
 		})
 
@@ -49,7 +51,9 @@ var _ = Describe("FenceAgentsRemediationTemplate Validation", func() {
 					isOutOfServiceTaintSupported = false
 				})
 				It("should be denied", func() {
-					Expect(outOfServiceStrategy.ValidateCreate()).Error().To(MatchError(ContainSubstring(outOfServiceTaintUnsupportedMsg)))
+					warnings, err := outOfServiceStrategy.ValidateCreate()
+					ExpectWithOffset(1, warnings).To(BeEmpty())
+					Expect(err).To(MatchError(ContainSubstring(outOfServiceTaintUnsupportedMsg)))
 				})
 			})
 		})
@@ -73,7 +77,9 @@ var _ = Describe("FenceAgentsRemediationTemplate Validation", func() {
 			})
 			It("should be rejected", func() {
 				farTemplate := getTestFARTemplate(invalidAgentName)
-				Expect(farTemplate.ValidateUpdate(oldFARTemplate)).Error().To(MatchError(ContainSubstring("unsupported fence agent: %s", invalidAgentName)))
+				warnings, err := farTemplate.ValidateUpdate(oldFARTemplate)
+				ExpectWithOffset(1, warnings).To(BeEmpty())
+				Expect(err).To(MatchError(ContainSubstring("unsupported fence agent: %s", invalidAgentName)))
 			})
 		})
 
@@ -103,7 +109,9 @@ var _ = Describe("FenceAgentsRemediationTemplate Validation", func() {
 					isOutOfServiceTaintSupported = false
 				})
 				It("should be denied", func() {
-					Expect(outOfServiceStrategy.ValidateUpdate(resourceDeletionStrategy)).Error().To(MatchError(ContainSubstring(outOfServiceTaintUnsupportedMsg)))
+					warnings, err := outOfServiceStrategy.ValidateUpdate(resourceDeletionStrategy)
+					ExpectWithOffset(1, warnings).To(BeEmpty())
+					Expect(err).To(MatchError(ContainSubstring(outOfServiceTaintUnsupportedMsg)))
 				})
 			})
 		})

go.mod

@@ -9,7 +9,6 @@ require (
 	github.com/onsi/gomega v1.30.0
 	github.com/openshift/api v0.0.0-20230621174358-ea40115b9fa6
 	github.com/openshift/client-go v0.0.0-20230626133714-296133fbf75e
-	github.com/pkg/errors v0.9.1
 	go.uber.org/zap v1.26.0
 	k8s.io/api v0.29.0
 	k8s.io/apimachinery v0.29.0
@@ -49,6 +48,7 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
 	github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f // indirect
+	github.com/pkg/errors v0.9.1 // indirect
 	github.com/prometheus/client_golang v1.17.0 // indirect
 	github.com/prometheus/client_model v0.5.0 // indirect
 	github.com/prometheus/common v0.45.0 // indirect