Skip to content

Commit

Permalink
refactor: return 500 when BackendTLSPolicy translation fails (#4363)
Browse files Browse the repository at this point in the history 
Signed-off-by: Alexander Volchok <alex.volchok@sap.com>
  • Loading branch information
@alexwo
alexwo authored Dec 8, 2024
1 parent b9f9a9f commit 05ee5f4
Show file tree
Hide file tree
Showing 43 changed files with 530 additions and 623 deletions.
74 changes: 30 additions & 44 deletions internal/gatewayapi/backendtlspolicy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"reflect"

"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
Expand All @@ -20,8 +21,11 @@ import (
"github.com/envoyproxy/gateway/internal/ir"
)

func (t *Translator) applyBackendTLSSetting(backendRef gwapiv1.BackendObjectReference, backendNamespace string, parent gwapiv1a2.ParentReference, resources *resource.Resources, envoyProxy *egv1a1.EnvoyProxy) *ir.TLSUpstreamConfig {
upstreamConfig, policy := t.processBackendTLSPolicy(backendRef, backendNamespace, parent, resources, envoyProxy)
func (t *Translator) applyBackendTLSSetting(backendRef gwapiv1.BackendObjectReference, backendNamespace string, parent gwapiv1a2.ParentReference, resources *resource.Resources, envoyProxy *egv1a1.EnvoyProxy) (*ir.TLSUpstreamConfig, error) {
upstreamConfig, policy, err := t.processBackendTLSPolicy(backendRef, backendNamespace, parent, resources)
if err != nil {
return nil, err
}
return t.applyEnvoyProxyBackendTLSSetting(policy, upstreamConfig, resources, parent, envoyProxy)
}

Expand All @@ -30,18 +34,13 @@ func (t *Translator) processBackendTLSPolicy(
backendNamespace string,
parent gwapiv1a2.ParentReference,
resources *resource.Resources,
envoyProxy *egv1a1.EnvoyProxy,
) (*ir.TLSUpstreamConfig, *gwapiv1a3.BackendTLSPolicy) {
) (*ir.TLSUpstreamConfig, *gwapiv1a3.BackendTLSPolicy, error) {
policy := getBackendTLSPolicy(resources.BackendTLSPolicies, backendRef, backendNamespace, resources)
if policy == nil {
return nil, nil
return nil, nil, nil
}

tlsBundle, err := getBackendTLSBundle(policy, resources)
if err == nil && tlsBundle == nil {
return nil, nil
}

ancestorRefs := getAncestorRefs(policy)
ancestorRefs = append(ancestorRefs, parent)

Expand All @@ -52,42 +51,16 @@ func (t *Translator) processBackendTLSPolicy(
policy.Generation,
status.Error2ConditionMsg(err),
)
return nil, nil
return nil, nil, err
}

status.SetAcceptedForPolicyAncestors(&policy.Status, ancestorRefs, t.GatewayControllerName)
// apply defaults as per envoyproxy
if envoyProxy != nil {
if envoyProxy.Spec.BackendTLS != nil {
if len(envoyProxy.Spec.BackendTLS.Ciphers) > 0 {
tlsBundle.Ciphers = envoyProxy.Spec.BackendTLS.Ciphers
}
if len(envoyProxy.Spec.BackendTLS.ECDHCurves) > 0 {
tlsBundle.ECDHCurves = envoyProxy.Spec.BackendTLS.ECDHCurves
}
if len(envoyProxy.Spec.BackendTLS.SignatureAlgorithms) > 0 {
tlsBundle.SignatureAlgorithms = envoyProxy.Spec.BackendTLS.SignatureAlgorithms
}
if envoyProxy.Spec.BackendTLS.MinVersion != nil {
tlsBundle.MinVersion = ptr.To(ir.TLSVersion(*envoyProxy.Spec.BackendTLS.MinVersion))
}
if envoyProxy.Spec.BackendTLS.MaxVersion != nil {
tlsBundle.MaxVersion = ptr.To(ir.TLSVersion(*envoyProxy.Spec.BackendTLS.MaxVersion))
}
if len(envoyProxy.Spec.BackendTLS.ALPNProtocols) > 0 {
tlsBundle.ALPNProtocols = make([]string, len(envoyProxy.Spec.BackendTLS.ALPNProtocols))
for i := range envoyProxy.Spec.BackendTLS.ALPNProtocols {
tlsBundle.ALPNProtocols[i] = string(envoyProxy.Spec.BackendTLS.ALPNProtocols[i])
}
}
}
}
return tlsBundle, policy
return tlsBundle, policy, nil
}

func (t *Translator) applyEnvoyProxyBackendTLSSetting(policy *gwapiv1a3.BackendTLSPolicy, tlsConfig *ir.TLSUpstreamConfig, resources *resource.Resources, parent gwapiv1a2.ParentReference, ep *egv1a1.EnvoyProxy) *ir.TLSUpstreamConfig {
func (t *Translator) applyEnvoyProxyBackendTLSSetting(policy *gwapiv1a3.BackendTLSPolicy, tlsConfig *ir.TLSUpstreamConfig, resources *resource.Resources, parent gwapiv1a2.ParentReference, ep *egv1a1.EnvoyProxy) (*ir.TLSUpstreamConfig, error) {
if ep == nil || ep.Spec.BackendTLS == nil || tlsConfig == nil {
return tlsConfig
return tlsConfig, nil
}

if len(ep.Spec.BackendTLS.Ciphers) > 0 {
Expand Down Expand Up @@ -116,28 +89,41 @@ func (t *Translator) applyEnvoyProxyBackendTLSSetting(policy *gwapiv1a3.BackendT
ancestorRefs := []gwapiv1a2.ParentReference{
parent,
}
var err error
if ns != ep.Namespace {
err = fmt.Errorf("ClientCertificateRef Secret is not located in the same namespace as Envoyproxy. Secret namespace: %s does not match Envoyproxy namespace: %s", ns, ep.Namespace)
status.SetTranslationErrorForPolicyAncestors(&policy.Status,
ancestorRefs,
t.GatewayControllerName,
policy.Generation,
status.Error2ConditionMsg(fmt.Errorf("client authentication TLS secret is not located in the same namespace as Envoyproxy. Secret namespace: %s does not match Envoyproxy namespace: %s", ns, ep.Namespace)))
return tlsConfig
status.Error2ConditionMsg(err))
return tlsConfig, err
}
secret := resources.GetSecret(ns, string(ep.Spec.BackendTLS.ClientCertificateRef.Name))
if secret == nil {
err = fmt.Errorf(
"failed to locate TLS secret for client auth: %s specified in EnvoyProxy %s",
types.NamespacedName{
Namespace: ep.Namespace,
Name: string(ep.Spec.BackendTLS.ClientCertificateRef.Name),
}.String(),
types.NamespacedName{
Namespace: ep.Namespace,
Name: ep.Name,
}.String(),
)
status.SetTranslationErrorForPolicyAncestors(&policy.Status,
ancestorRefs,
t.GatewayControllerName,
policy.Generation,
status.Error2ConditionMsg(fmt.Errorf("failed to locate TLS secret for client auth: %s in namespace: %s", ep.Spec.BackendTLS.ClientCertificateRef.Name, ns)),
status.Error2ConditionMsg(err),
)
return tlsConfig
return tlsConfig, err
}
tlsConf := irTLSConfigs(secret)
tlsConfig.ClientCertificates = tlsConf.Certificates
}
return tlsConfig
return tlsConfig, nil
}

func backendTLSTargetMatched(policy gwapiv1a3.BackendTLSPolicy, target gwapiv1a2.LocalPolicyTargetReferenceWithSectionName, backendNamespace string) bool {
Expand Down
6 changes: 5 additions & 1 deletion internal/gatewayapi/ext_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ func (t *Translator) processExtServiceDestination(
"mixed endpointslice address type for the same backendRef is not supported")
}

backendTLS = t.applyBackendTLSSetting(
var err error
backendTLS, err = t.applyBackendTLSSetting(
backendRef.BackendObjectReference,
backendNamespace,
// Gateway is not the appropriate parent reference here because the owner
Expand All @@ -126,6 +127,9 @@ func (t *Translator) processExtServiceDestination(
resources,
envoyProxy,
)
if err != nil {
return nil, err
}

ds.TLS = backendTLS

Expand Down
37 changes: 23 additions & 14 deletions internal/gatewayapi/filters.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ type HTTPFiltersTranslator interface {
processRedirectFilter(redirect *gwapiv1.HTTPRequestRedirectFilter, filterContext *HTTPFiltersContext)
processRequestHeaderModifierFilter(headerModifier *gwapiv1.HTTPHeaderFilter, filterContext *HTTPFiltersContext)
processResponseHeaderModifierFilter(headerModifier *gwapiv1.HTTPHeaderFilter, filterContext *HTTPFiltersContext)
processRequestMirrorFilter(filterIdx int, mirror *gwapiv1.HTTPRequestMirrorFilter, filterContext *HTTPFiltersContext, resources *resource.Resources)
processExtensionRefHTTPFilter(extRef *gwapiv1.LocalObjectReference, filterContext *HTTPFiltersContext, resources *resource.Resources)
processRequestMirrorFilter(filterIdx int, mirror *gwapiv1.HTTPRequestMirrorFilter, filterContext *HTTPFiltersContext, resources *resource.Resources) error
processUnsupportedHTTPFilter(filterType string, filterContext *HTTPFiltersContext)
}

Expand Down Expand Up @@ -69,13 +68,14 @@ func (t *Translator) ProcessHTTPFilters(parentRef *RouteParentContext,
filters []gwapiv1.HTTPRouteFilter,
ruleIdx int,
resources *resource.Resources,
) *HTTPFiltersContext {
) (*HTTPFiltersContext, error) {
httpFiltersContext := &HTTPFiltersContext{
ParentRef: parentRef,
Route: route,
RuleIdx: ruleIdx,
HTTPFilterIR: &HTTPFilterIR{},
}
var err error
for i := range filters {
filter := filters[i]
// If an invalid filter type has been configured then skip processing any more filters
Expand All @@ -97,29 +97,30 @@ func (t *Translator) ProcessHTTPFilters(parentRef *RouteParentContext,
case gwapiv1.HTTPRouteFilterResponseHeaderModifier:
t.processResponseHeaderModifierFilter(filter.ResponseHeaderModifier, httpFiltersContext)
case gwapiv1.HTTPRouteFilterRequestMirror:
t.processRequestMirrorFilter(i, filter.RequestMirror, httpFiltersContext, resources)
err = t.processRequestMirrorFilter(i, filter.RequestMirror, httpFiltersContext, resources)
case gwapiv1.HTTPRouteFilterExtensionRef:
t.processExtensionRefHTTPFilter(filter.ExtensionRef, httpFiltersContext, resources)
default:
t.processUnsupportedHTTPFilter(string(filter.Type), httpFiltersContext)
}
}

return httpFiltersContext
return httpFiltersContext, err
}

// ProcessGRPCFilters translates gateway api grpc filters to IRs.
func (t *Translator) ProcessGRPCFilters(parentRef *RouteParentContext,
route RouteContext,
filters []gwapiv1.GRPCRouteFilter,
resources *resource.Resources,
) *HTTPFiltersContext {
) (*HTTPFiltersContext, error) {
httpFiltersContext := &HTTPFiltersContext{
ParentRef: parentRef,
Route: route,

HTTPFilterIR: &HTTPFilterIR{},
}

for i := range filters {
filter := filters[i]
// If an invalid filter type has been configured then skip processing any more filters
Expand All @@ -137,15 +138,18 @@ func (t *Translator) ProcessGRPCFilters(parentRef *RouteParentContext,
case gwapiv1.GRPCRouteFilterResponseHeaderModifier:
t.processResponseHeaderModifierFilter(filter.ResponseHeaderModifier, httpFiltersContext)
case gwapiv1.GRPCRouteFilterRequestMirror:
t.processRequestMirrorFilter(i, filter.RequestMirror, httpFiltersContext, resources)
err := t.processRequestMirrorFilter(i, filter.RequestMirror, httpFiltersContext, resources)
if err != nil {
return nil, err
}
case gwapiv1.GRPCRouteFilterExtensionRef:
t.processExtensionRefHTTPFilter(filter.ExtensionRef, httpFiltersContext, resources)
default:
t.processUnsupportedHTTPFilter(string(filter.Type), httpFiltersContext)
}
}

return httpFiltersContext
return httpFiltersContext, nil
}

// Checks if the context and the rewrite both contain a core gw-api HTTP URL rewrite
Expand Down Expand Up @@ -968,10 +972,10 @@ func (t *Translator) processRequestMirrorFilter(
mirrorFilter *gwapiv1.HTTPRequestMirrorFilter,
filterContext *HTTPFiltersContext,
resources *resource.Resources,
) {
) error {
// Make sure the config actually exists
if mirrorFilter == nil {
return
return nil
}

mirrorBackend := mirrorFilter.BackendRef
Expand All @@ -988,18 +992,23 @@ func (t *Translator) processRequestMirrorFilter(
// This sets the status on the HTTPRoute, should the usage be changed so that the status message reflects that the backendRef is from the filter?
filterNs := filterContext.Route.GetNamespace()
serviceNamespace := NamespaceDerefOr(mirrorBackend.Namespace, filterNs)
if !t.validateBackendRef(mirrorBackendRef, filterContext.ParentRef, filterContext.Route,
resources, serviceNamespace, resource.KindHTTPRoute) {
return
err := t.validateBackendRef(mirrorBackendRef, filterContext.ParentRef, filterContext.Route,
resources, serviceNamespace, resource.KindHTTPRoute)
if err != nil {
return err
}

ds := t.processDestination(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources)
ds, err := t.processDestination(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources)
if err != nil {
return err
}

newMirror := &ir.RouteDestination{
Name: fmt.Sprintf("%s-mirror-%d", irRouteDestinationName(filterContext.Route, filterContext.RuleIdx), filterIdx),
Settings: []*ir.DestinationSetting{ds},
}
filterContext.Mirrors = append(filterContext.Mirrors, newMirror)
return nil
}

func (t *Translator) processUnresolvedHTTPFilter(errMsg string, filterContext *HTTPFiltersContext) {
Expand Down
Loading

0 comments on commit 05ee5f4

Please sign in to comment.