Skip to content

Commit

Permalink
refactor error processing logic for EEP & CTP
Browse files Browse the repository at this point in the history
Signed-off-by: shawnh2 <shawnhxh@outlook.com>
  • Loading branch information
shawnh2 committed May 11, 2024
1 parent 17929c5 commit 2bdf980
Show file tree
Hide file tree
Showing 14 changed files with 82 additions and 63 deletions.
79 changes: 41 additions & 38 deletions internal/gatewayapi/backendtrafficpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,22 +287,23 @@ func resolveBTPolicyRouteTargetRef(policy *egv1a1.BackendTrafficPolicy, routes m

func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.BackendTrafficPolicy, route RouteContext, xdsIR XdsIRMap) error {
var (
rl *ir.RateLimit
lb *ir.LoadBalancer
pp *ir.ProxyProtocol
hc *ir.HealthCheck
cb *ir.CircuitBreaker
fi *ir.FaultInjection
to *ir.Timeout
ka *ir.TCPKeepalive
rt *ir.Retry
err error
rl *ir.RateLimit
lb *ir.LoadBalancer
pp *ir.ProxyProtocol
hc *ir.HealthCheck
cb *ir.CircuitBreaker
fi *ir.FaultInjection
to *ir.Timeout
ka *ir.TCPKeepalive
rt *ir.Retry
err, errs error
)

// Build IR
if policy.Spec.RateLimit != nil {
if rl, err = t.buildRateLimit(policy); err != nil {
return perr.WithMessage(err, "RateLimit")
err = perr.WithMessage(err, "RateLimit")
errs = errors.Join(errs, err)
}
}
if policy.Spec.LoadBalancer != nil {
Expand All @@ -316,7 +317,8 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
}
if policy.Spec.CircuitBreaker != nil {
if cb, err = t.buildCircuitBreaker(policy); err != nil {
return perr.WithMessage(err, "CircuitBreaker")
err = perr.WithMessage(err, "CircuitBreaker")
errs = errors.Join(errs, err)
}
}

Expand All @@ -325,7 +327,8 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
}
if policy.Spec.TCPKeepalive != nil {
if ka, err = t.buildTCPKeepAlive(policy); err != nil {
return perr.WithMessage(err, "TCPKeepalive")
err = perr.WithMessage(err, "TCPKeepalive")
errs = errors.Join(errs, err)
}
}
if policy.Spec.Retry != nil {
Expand All @@ -336,7 +339,8 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen

if policy.Spec.Timeout != nil {
if to, err = t.buildTimeout(policy, nil); err != nil {
return perr.WithMessage(err, "Timeout")
err = perr.WithMessage(err, "Timeout")
errs = errors.Join(errs, err)
}
}

Expand Down Expand Up @@ -378,10 +382,9 @@ 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 perr.WithMessage(err, "Timeout")
if to, err = t.buildTimeout(policy, r); err == nil {
r.Timeout = to
}
r.Timeout = to
}

if policy.Spec.UseClientProtocol != nil {
Expand All @@ -392,27 +395,28 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
}
}

return nil
return errs
}

func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.BackendTrafficPolicy, gateway *GatewayContext, xdsIR XdsIRMap) error {
var (
rl *ir.RateLimit
lb *ir.LoadBalancer
pp *ir.ProxyProtocol
hc *ir.HealthCheck
cb *ir.CircuitBreaker
fi *ir.FaultInjection
ct *ir.Timeout
ka *ir.TCPKeepalive
rt *ir.Retry
err error
rl *ir.RateLimit
lb *ir.LoadBalancer
pp *ir.ProxyProtocol
hc *ir.HealthCheck
cb *ir.CircuitBreaker
fi *ir.FaultInjection
ct *ir.Timeout
ka *ir.TCPKeepalive
rt *ir.Retry
err, errs error
)

// Build IR
if policy.Spec.RateLimit != nil {
if rl, err = t.buildRateLimit(policy); err != nil {
return perr.WithMessage(err, "RateLimit")
err = perr.WithMessage(err, "RateLimit")
errs = errors.Join(errs, err)
}
}
if policy.Spec.LoadBalancer != nil {
Expand All @@ -426,15 +430,17 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
}
if policy.Spec.CircuitBreaker != nil {
if cb, err = t.buildCircuitBreaker(policy); err != nil {
return perr.WithMessage(err, "CircuitBreaker")
err = perr.WithMessage(err, "CircuitBreaker")
errs = errors.Join(errs, err)
}
}
if policy.Spec.FaultInjection != nil {
fi = t.buildFaultInjection(policy)
}
if policy.Spec.TCPKeepalive != nil {
if ka, err = t.buildTCPKeepAlive(policy); err != nil {
return perr.WithMessage(err, "TCPKeepalive")
err = perr.WithMessage(err, "TCPKeepalive")
errs = errors.Join(errs, err)
}
}
if policy.Spec.Retry != nil {
Expand All @@ -452,7 +458,8 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back

if policy.Spec.Timeout != nil {
if ct, err = t.buildTimeout(policy, nil); err != nil {
return perr.WithMessage(err, "Timeout")
err = perr.WithMessage(err, "Timeout")
errs = errors.Join(errs, err)
}
}

Expand Down Expand Up @@ -549,11 +556,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
}

if policy.Spec.Timeout != nil {
if ct, err = t.buildTimeout(policy, r); err != nil {
return perr.WithMessage(err, "Timeout")
}

if r.Timeout == nil {
if ct, err = t.buildTimeout(policy, r); err == nil && r.Timeout == nil {
r.Timeout = ct
}
}
Expand All @@ -566,7 +569,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
}
}

return nil
return errs
}

func (t *Translator) buildRateLimit(policy *egv1a1.BackendTrafficPolicy) (*ir.RateLimit, error) {
Expand Down
33 changes: 21 additions & 12 deletions internal/gatewayapi/clienttrafficpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"
"time"

perr "github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -360,6 +361,8 @@ func validatePortOverlapForClientTrafficPolicy(l *ListenerContext, xds *ir.Xds,
func (t *Translator) translateClientTrafficPolicyForListener(policy *egv1a1.ClientTrafficPolicy, l *ListenerContext,
xdsIR XdsIRMap, infraIR InfraIRMap, resources *Resources,
) error {
var err, errs error

// Find IR
irKey := t.getIRKey(l.gateway)
// It must exist since we've already finished processing the gateways
Expand All @@ -384,8 +387,9 @@ func (t *Translator) translateClientTrafficPolicyForListener(policy *egv1a1.Clie
translateListenerTCPKeepalive(policy.Spec.TCPKeepalive, httpIR)

// Translate Connection
if err := translateListenerConnection(policy.Spec.Connection, httpIR); err != nil {
return err
if err = translateListenerConnection(policy.Spec.Connection, httpIR); err != nil {
err = perr.WithMessage(err, "Connection")
errs = errors.Join(errs, err)
}

// Translate Proxy Protocol
Expand All @@ -401,18 +405,21 @@ func (t *Translator) translateClientTrafficPolicyForListener(policy *egv1a1.Clie
translatePathSettings(policy.Spec.Path, httpIR)

// Translate Client Timeout Settings
if err := translateClientTimeout(policy.Spec.Timeout, httpIR); err != nil {
return err
if err = translateClientTimeout(policy.Spec.Timeout, httpIR); err != nil {
err = perr.WithMessage(err, "Timeout")
errs = errors.Join(errs, err)
}

// Translate HTTP1 Settings
if err := translateHTTP1Settings(policy.Spec.HTTP1, httpIR); err != nil {
return err
if err = translateHTTP1Settings(policy.Spec.HTTP1, httpIR); err != nil {
err = perr.WithMessage(err, "HTTP1")
errs = errors.Join(errs, err)
}

// Translate HTTP2 Settings
if err := translateHTTP2Settings(policy.Spec.HTTP2, httpIR); err != nil {
return err
if err = translateHTTP2Settings(policy.Spec.HTTP2, httpIR); err != nil {
err = perr.WithMessage(err, "HTTP2")
errs = errors.Join(errs, err)
}

// enable http3 if set and TLS is enabled
Expand All @@ -434,11 +441,13 @@ func (t *Translator) translateClientTrafficPolicyForListener(policy *egv1a1.Clie
}

// Translate TLS parameters
if err := t.translateListenerTLSParameters(policy, httpIR, resources); err != nil {
return err
if err = t.translateListenerTLSParameters(policy, httpIR, resources); err != nil {
err = perr.WithMessage(err, "TLS")
errs = errors.Join(errs, err)
}
}
return nil

return errs
}

func translateListenerTCPKeepalive(tcpKeepAlive *egv1a1.TCPKeepalive, httpIR *ir.HTTPListener) {
Expand Down Expand Up @@ -570,7 +579,7 @@ func translateHTTP1Settings(http1Settings *egv1a1.HTTP1Settings, httpIR *ir.HTTP
}
}
if defaultHost == nil {
return fmt.Errorf("can't set http10 default host on listener with only wildcard hostnames")
return fmt.Errorf("cannot set http10 default host on listener with only wildcard hostnames")
}
}
// If useDefaultHost was set, then defaultHost will have the hostname to use.
Expand Down
5 changes: 5 additions & 0 deletions internal/gatewayapi/envoyextensionpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strings"
"time"

perr "github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -293,9 +294,11 @@ func (t *Translator) translateEnvoyExtensionPolicyForRoute(policy *egv1a1.EnvoyE
)

if extProcs, err = t.buildExtProcs(policy, resources); err != nil {
err = perr.WithMessage(err, "ExtProcs")
errs = errors.Join(errs, err)
}
if wasms, err = t.buildWasms(policy); err != nil {
err = perr.WithMessage(err, "WASMs")
errs = errors.Join(errs, err)
}

Expand Down Expand Up @@ -350,9 +353,11 @@ func (t *Translator) translateEnvoyExtensionPolicyForGateway(policy *egv1a1.Envo
policyTarget := irStringKey(policy.Namespace, string(policy.Spec.TargetRef.Name))

if extProcs, err = t.buildExtProcs(policy, resources); err != nil {
err = perr.WithMessage(err, "ExtProcs")
errs = errors.Join(errs, err)
}
if wasms, err = t.buildWasms(policy); err != nil {
err = perr.WithMessage(err, "WASMs")
errs = errors.Join(errs, err)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ clientTrafficPolicies:
sectionName: http-1
conditions:
- lastTransitionTime: null
message: Invalid BufferLimit value 500m.
message: 'Connection: invalid BufferLimit value 500m.'
reason: Invalid
status: "False"
type: Accepted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ clientTrafficPolicies:
sectionName: http-1
conditions:
- lastTransitionTime: null
message: BufferLimit value 100G is out of range, must be between 0 and 4294967295.
message: 'Connection: BufferLimit value 100G is out of range, must be between
0 and 4294967295.'
reason: Invalid
status: "False"
type: Accepted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ clientTrafficPolicies:
sectionName: http-1
conditions:
- lastTransitionTime: null
message: Invalid CloseDelay value 10mib.
message: 'Connection: invalid CloseDelay value 10mib.'
reason: Invalid
status: "False"
type: Accepted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ clientTrafficPolicies:
sectionName: http-3
conditions:
- lastTransitionTime: null
message: Can't set http10 default host on listener with only wildcard hostnames.
message: 'HTTP1: cannot set http10 default host on listener with only wildcard
hostnames.'
reason: Invalid
status: "False"
type: Accepted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ clientTrafficPolicies:
conditions:
- lastTransitionTime: null
message: |-
InitialStreamWindowSize value 1Ki is out of range, must be between 65535 and 2147483647
HTTP2: InitialStreamWindowSize value 1Ki is out of range, must be between 65535 and 2147483647
InitialConnectionWindowSize value 1Ti is out of range, must be between 65535 and 2147483647.
reason: Invalid
status: "False"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ clientTrafficPolicies:
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: 'Time: unknown unit "sec" in duration "10sec".'
message: 'Timeout: time: unknown unit "sec" in duration "10sec".'
reason: Invalid
status: "False"
type: Accepted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ clientTrafficPolicies:
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: 'Time: unknown unit "sec" in duration "5sec".'
message: 'Timeout: time: unknown unit "sec" in duration "5sec".'
reason: Invalid
status: "False"
type: Accepted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ envoyExtensionPolicies:
namespace: default
conditions:
- lastTransitionTime: null
message: TCP Port 4000 not found on service default/grpc-backend.
message: 'ExtProcs: TCP Port 4000 not found on service default/grpc-backend.'
reason: Invalid
status: "False"
type: Accepted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ envoyExtensionPolicies:
namespace: default
conditions:
- lastTransitionTime: null
message: A valid port number corresponding to a port on the Service must be
specified.
message: 'ExtProcs: a valid port number corresponding to a port on the Service
must be specified.'
reason: Invalid
status: "False"
type: Accepted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ envoyExtensionPolicies:
namespace: default
conditions:
- lastTransitionTime: null
message: Backend ref to Service envoy-gateway/grpc-backend not permitted by
any ReferenceGrant.
message: 'ExtProcs: backend ref to Service envoy-gateway/grpc-backend not
permitted by any ReferenceGrant.'
reason: Invalid
status: "False"
type: Accepted
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ envoyExtensionPolicies:
namespace: default
conditions:
- lastTransitionTime: null
message: Service envoy-gateway/grpc-backend not found.
message: 'ExtProcs: service envoy-gateway/grpc-backend not found.'
reason: Invalid
status: "False"
type: Accepted
Expand Down

0 comments on commit 2bdf980

Please sign in to comment.