refactor xpolicy err message

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

internal/gatewayapi/backendtrafficpolicy.go

@@ -6,14 +6,15 @@
 package gatewayapi
 
 import (
+	"errors"
 	"fmt"
 	"math"
 	"net"
 	"sort"
 	"strings"
 	"time"
 
-	"github.com/pkg/errors"
+	perr "github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -310,7 +311,7 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
 	// Build IR
 	if policy.Spec.RateLimit != nil {
 		if rl, err = t.buildRateLimit(policy); err != nil {
-			return errors.Wrap(err, "RateLimit")
+			return perr.WithMessage(err, "RateLimit")
 		}
 	}
 	if policy.Spec.LoadBalancer != nil {
@@ -324,7 +325,7 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
 	}
 	if policy.Spec.CircuitBreaker != nil {
 		if cb, err = t.buildCircuitBreaker(policy); err != nil {
-			return errors.Wrap(err, "CircuitBreaker")
+			return perr.WithMessage(err, "CircuitBreaker")
 		}
 	}
 
@@ -333,7 +334,7 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
 	}
 	if policy.Spec.TCPKeepalive != nil {
 		if ka, err = t.buildTCPKeepAlive(policy); err != nil {
-			return errors.Wrap(err, "TCPKeepalive")
+			return perr.WithMessage(err, "TCPKeepalive")
 		}
 	}
 	if policy.Spec.Retry != nil {
@@ -344,7 +345,7 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
 
 	if policy.Spec.Timeout != nil {
 		if to, err = t.buildTimeout(policy, nil); err != nil {
-			return errors.Wrap(err, "Timeout")
+			return perr.WithMessage(err, "Timeout")
 		}
 	}
 
@@ -385,7 +386,7 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
 					// some timeout setting originate from the route
 					if policy.Spec.Timeout != nil {
 						if to, err = t.buildTimeout(policy, r); err != nil {
-							return errors.Wrap(err, "Timeout")
+							return perr.WithMessage(err, "Timeout")
 						}
 						r.Timeout = to
 					}
@@ -418,7 +419,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
 	// Build IR
 	if policy.Spec.RateLimit != nil {
 		if rl, err = t.buildRateLimit(policy); err != nil {
-			return errors.Wrap(err, "RateLimit")
+			return perr.WithMessage(err, "RateLimit")
 		}
 	}
 	if policy.Spec.LoadBalancer != nil {
@@ -432,15 +433,15 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
 	}
 	if policy.Spec.CircuitBreaker != nil {
 		if cb, err = t.buildCircuitBreaker(policy); err != nil {
-			return errors.Wrap(err, "CircuitBreaker")
+			return perr.WithMessage(err, "CircuitBreaker")
 		}
 	}
 	if policy.Spec.FaultInjection != nil {
 		fi = t.buildFaultInjection(policy)
 	}
 	if policy.Spec.TCPKeepalive != nil {
 		if ka, err = t.buildTCPKeepAlive(policy); err != nil {
-			return errors.Wrap(err, "TCPKeepalive")
+			return perr.WithMessage(err, "TCPKeepalive")
 		}
 	}
 	if policy.Spec.Retry != nil {
@@ -461,7 +462,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
 
 	if policy.Spec.Timeout != nil {
 		if ct, err = t.buildTimeout(policy, nil); err != nil {
-			return errors.Wrap(err, "Timeout")
+			return perr.WithMessage(err, "Timeout")
 		}
 	}
 
@@ -557,7 +558,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
 
 			if policy.Spec.Timeout != nil {
 				if ct, err = t.buildTimeout(policy, r); err != nil {
-					return errors.Wrap(err, "Timeout")
+					return perr.WithMessage(err, "Timeout")
 				}
 
 				if r.Timeout == nil {
@@ -1014,20 +1015,23 @@ func (t *Translator) buildCircuitBreaker(policy *egv1a1.BackendTrafficPolicy) (*
 
 func (t *Translator) buildTimeout(policy *egv1a1.BackendTrafficPolicy, r *ir.HTTPRoute) (*ir.Timeout, error) {
 	var (
-		tto *ir.TCPTimeout
-		hto *ir.HTTPTimeout
+		tto  *ir.TCPTimeout
+		hto  *ir.HTTPTimeout
+		terr bool
+		errs error
 	)
 
 	pto := policy.Spec.Timeout
 
 	if pto.TCP != nil && pto.TCP.ConnectTimeout != nil {
 		d, err := time.ParseDuration(string(*pto.TCP.ConnectTimeout))
 		if err != nil {
-			return nil, fmt.Errorf("invalid ConnectTimeout value %s", *pto.TCP.ConnectTimeout)
-		}
-
-		tto = &ir.TCPTimeout{
-			ConnectTimeout: ptr.To(metav1.Duration{Duration: d}),
+			terr = true
+			errs = errors.Join(errs, fmt.Errorf("invalid ConnectTimeout value %s", *pto.TCP.ConnectTimeout))
+		} else {
+			tto = &ir.TCPTimeout{
+				ConnectTimeout: ptr.To(metav1.Duration{Duration: d}),
+			}
 		}
 	}
 
@@ -1038,19 +1042,21 @@ func (t *Translator) buildTimeout(policy *egv1a1.BackendTrafficPolicy, r *ir.HTT
 		if pto.HTTP.ConnectionIdleTimeout != nil {
 			d, err := time.ParseDuration(string(*pto.HTTP.ConnectionIdleTimeout))
 			if err != nil {
-				return nil, fmt.Errorf("invalid ConnectionIdleTimeout value %s", *pto.HTTP.ConnectionIdleTimeout)
+				terr = true
+				errs = errors.Join(errs, fmt.Errorf("invalid ConnectionIdleTimeout value %s", *pto.HTTP.ConnectionIdleTimeout))
+			} else {
+				cit = ptr.To(metav1.Duration{Duration: d})
 			}
-
-			cit = ptr.To(metav1.Duration{Duration: d})
 		}
 
 		if pto.HTTP.MaxConnectionDuration != nil {
 			d, err := time.ParseDuration(string(*pto.HTTP.MaxConnectionDuration))
 			if err != nil {
-				return nil, fmt.Errorf("invalid MaxConnectionDuration value %s", *pto.HTTP.MaxConnectionDuration)
+				terr = true
+				errs = errors.Join(errs, fmt.Errorf("invalid MaxConnectionDuration value %s", *pto.HTTP.MaxConnectionDuration))
+			} else {
+				mcd = ptr.To(metav1.Duration{Duration: d})
 			}
-
-			mcd = ptr.To(metav1.Duration{Duration: d})
 		}
 
 		hto = &ir.HTTPTimeout{
@@ -1061,24 +1067,32 @@ func (t *Translator) buildTimeout(policy *egv1a1.BackendTrafficPolicy, r *ir.HTT
 
 	// http request timeout is translated during the gateway-api route resource translation
 	// merge route timeout setting with backendtrafficpolicy timeout settings
-	if r != nil && r.Timeout != nil && r.Timeout.HTTP != nil && r.Timeout.HTTP.RequestTimeout != nil {
-		if hto == nil {
-			hto = &ir.HTTPTimeout{
-				RequestTimeout: r.Timeout.HTTP.RequestTimeout,
+	if terr {
+		if r != nil && r.Timeout != nil {
+			return r.Timeout.DeepCopy(), errs
+		}
+	} else {
+		// http request timeout is translated during the gateway-api route resource translation
+		// merge route timeout setting with backendtrafficpolicy timeout settings
+		if r != nil && r.Timeout != nil && r.Timeout.HTTP != nil && r.Timeout.HTTP.RequestTimeout != nil {
+			if hto == nil {
+				hto = &ir.HTTPTimeout{
+					RequestTimeout: r.Timeout.HTTP.RequestTimeout,
+				}
+			} else {
+				hto.RequestTimeout = r.Timeout.HTTP.RequestTimeout
 			}
-		} else {
-			hto.RequestTimeout = r.Timeout.HTTP.RequestTimeout
 		}
-	}
 
-	if hto != nil || tto != nil {
-		return &ir.Timeout{
-			TCP:  tto,
-			HTTP: hto,
-		}, nil
+		if hto != nil || tto != nil {
+			return &ir.Timeout{
+				TCP:  tto,
+				HTTP: hto,
+			}, nil
+		}
 	}
 
-	return nil, nil
+	return nil, errs
 }
 
 func int64ToUint32(in int64) (uint32, bool) {

internal/gatewayapi/securitypolicy.go

@@ -16,6 +16,7 @@ import (
 	"strconv"
 	"strings"
 
+	perr "github.com/pkg/errors"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -356,6 +357,7 @@ func (t *Translator) translateSecurityPolicyForRoute(
 		if oidc, err = t.buildOIDC(
 			policy,
 			resources); err != nil {
+			err = perr.WithMessage(err, "OIDC")
 			errs = errors.Join(errs, err)
 		}
 	}
@@ -364,6 +366,7 @@ func (t *Translator) translateSecurityPolicyForRoute(
 		if basicAuth, err = t.buildBasicAuth(
 			policy,
 			resources); err != nil {
+			err = perr.WithMessage(err, "BasicAuth")
 			errs = errors.Join(errs, err)
 		}
 	}
@@ -372,6 +375,7 @@ func (t *Translator) translateSecurityPolicyForRoute(
 		if extAuth, err = t.buildExtAuth(
 			policy,
 			resources); err != nil {
+			err = perr.WithMessage(err, "ExtAuth")
 			errs = errors.Join(errs, err)
 		}
 	}
@@ -427,6 +431,7 @@ func (t *Translator) translateSecurityPolicyForGateway(
 		if oidc, err = t.buildOIDC(
 			policy,
 			resources); err != nil {
+			err = perr.WithMessage(err, "OIDC")
 			errs = errors.Join(errs, err)
 		}
 	}
@@ -435,6 +440,7 @@ func (t *Translator) translateSecurityPolicyForGateway(
 		if basicAuth, err = t.buildBasicAuth(
 			policy,
 			resources); err != nil {
+			err = perr.WithMessage(err, "BasicAuth")
 			errs = errors.Join(errs, err)
 		}
 	}
@@ -443,6 +449,7 @@ func (t *Translator) translateSecurityPolicyForGateway(
 		if extAuth, err = t.buildExtAuth(
 			policy,
 			resources); err != nil {
+			err = perr.WithMessage(err, "ExtAuth")
 			errs = errors.Join(errs, err)
 		}
 	}
@@ -591,7 +598,7 @@ func (t *Translator) buildOIDC(
 	// Generate a unique cookie suffix for oauth filters
 	suffix := utils.Digest32(string(policy.UID))
 
-	// Get the HMAC secret
+	// Get the HMAC secret.
 	// HMAC secret is generated by the CertGen job and stored in a secret
 	// We need to rotate the HMAC secret in the future, probably the same
 	// way we rotate the certs generated by the CertGen job.

internal/gatewayapi/testdata/securitypolicy-with-extauth-invalid-no-matching-port.out.yaml

@@ -126,7 +126,7 @@ securityPolicies:
         namespace: default
       conditions:
       - lastTransitionTime: null
-        message: TCP Port 80 not found on service default/http-backend.
+        message: 'ExtAuth: TCP Port 80 not found on service default/http-backend.'
         reason: Invalid
         status: "False"
         type: Accepted

internal/gatewayapi/testdata/securitypolicy-with-extauth-invalid-no-port.out.yaml

@@ -125,8 +125,8 @@ securityPolicies:
         namespace: default
       conditions:
       - lastTransitionTime: null
-        message: A valid port number corresponding to a port on the Service must be
-          specified.
+        message: 'ExtAuth: a valid port number corresponding to a port on the Service
+          must be specified.'
         reason: Invalid
         status: "False"
         type: Accepted

internal/gatewayapi/testdata/securitypolicy-with-extauth-invalid-no-reference-grant.out.yaml

@@ -126,8 +126,8 @@ securityPolicies:
         namespace: default
       conditions:
       - lastTransitionTime: null
-        message: Backend ref to Service envoy-gateway/http-backend not permitted by
-          any ReferenceGrant.
+        message: 'ExtAuth: backend ref to Service envoy-gateway/http-backend not permitted
+          by any ReferenceGrant.'
         reason: Invalid
         status: "False"
         type: Accepted

internal/gatewayapi/testdata/securitypolicy-with-extauth-invalid-no-service.out.yaml

@@ -126,7 +126,7 @@ securityPolicies:
         namespace: default
       conditions:
       - lastTransitionTime: null
-        message: Service default/http-backend not found.
+        message: 'ExtAuth: service default/http-backend not found.'
         reason: Invalid
         status: "False"
         type: Accepted

internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml

@@ -174,7 +174,7 @@ securityPolicies:
         sectionName: http
       conditions:
       - lastTransitionTime: null
-        message: Secret default/client2-secret does not exist.
+        message: 'OIDC: secret default/client2-secret does not exist.'
         reason: Invalid
         status: "False"
         type: Accepted
@@ -219,7 +219,7 @@ securityPolicies:
         namespace: envoy-gateway
       conditions:
       - lastTransitionTime: null
-        message: Secret envoy-gateway/client1-secret does not exist.
+        message: 'OIDC: secret envoy-gateway/client1-secret does not exist.'
         reason: Invalid
         status: "False"
         type: Accepted

internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-issuer.out.yaml

@@ -85,8 +85,8 @@ securityPolicies:
         namespace: default
       conditions:
       - lastTransitionTime: null
-        message: 'Error fetching endpoints from issuer: invalid character ''<'' looking
-          for beginning of value.'
+        message: 'OIDC: error fetching endpoints from issuer: invalid character ''<''
+          looking for beginning of value.'
         reason: Invalid
         status: "False"
         type: Accepted

internal/gatewayapi/testdata/securitypolicy-with-oidc-invalid-secretref.out.yaml

@@ -197,7 +197,7 @@ securityPolicies:
         namespace: default
       conditions:
       - lastTransitionTime: null
-        message: Secret default/client1-secret does not exist.
+        message: 'OIDC: secret default/client1-secret does not exist.'
         reason: Invalid
         status: "False"
         type: Accepted
@@ -234,7 +234,7 @@ securityPolicies:
         namespace: default
       conditions:
       - lastTransitionTime: null
-        message: Secret ref namespace must be unspecified/empty or default.
+        message: 'OIDC: secret ref namespace must be unspecified/empty or default.'
         reason: Invalid
         status: "False"
         type: Accepted
@@ -270,7 +270,7 @@ securityPolicies:
         namespace: default
       conditions:
       - lastTransitionTime: null
-        message: Client secret not found in secret default/client3-secret.
+        message: 'OIDC: client secret not found in secret default/client3-secret.'
         reason: Invalid
         status: "False"
         type: Accepted