Skip to content

refactor: return 500 when BackendTLSPolicy translation fails #4363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arkodg merged 3 commits into from
Dec 8, 2024
Merged

refactor: return 500 when BackendTLSPolicy translation fails #4363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f71e441
updating
alexwo Dec 7, 2024
1263d48
updating
alexwo Dec 7, 2024
c2c8b4b
updating
alexwo Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 30 additions & 44 deletions internal/gatewayapi/backendtlspolicy.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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