Skip to content

Commit

Permalink
gateway-api: RequestRedirect picks wrong port with multiple listeners
Browse files Browse the repository at this point in the history 
[ upstream commit 74119be ]

Fixes: 29099

If RequestRedirect does not specify a port and schem is empty. The port of the listener is used by default.

Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
  • Loading branch information
@chaunceyjiang @sayboras
chaunceyjiang authored and sayboras committed Apr 4, 2024
1 parent dcfe7bd commit e546df1
Show file tree
Hide file tree
Showing 4 changed files with 262 additions and 3 deletions.
17 changes: 14 additions & 3 deletions operator/pkg/model/ingestion/gateway.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func toHTTPRoutes(listener gatewayv1.Listener, input []gatewayv1.HTTPRoute, serv
HeadersToRemove: f.ResponseHeaderModifier.Remove,
}
case gatewayv1.HTTPRouteFilterRequestRedirect:
requestRedirectFilter = toHTTPRequestRedirectFilter(f.RequestRedirect)
requestRedirectFilter = toHTTPRequestRedirectFilter(listener, f.RequestRedirect)
case gatewayv1.HTTPRouteFilterURLRewrite:
rewriteFilter = toHTTPRewriteFilter(f.URLRewrite)
case gatewayv1.HTTPRouteFilterRequestMirror:
Expand Down Expand Up @@ -408,7 +408,7 @@ func toTLSRoutes(listener gatewayv1beta1.Listener, input []gatewayv1alpha2.TLSRo
return tlsRoutes
}

func toHTTPRequestRedirectFilter(redirect *gatewayv1.HTTPRequestRedirectFilter) *model.HTTPRequestRedirectFilter {
func toHTTPRequestRedirectFilter(listener gatewayv1beta1.Listener, redirect *gatewayv1.HTTPRequestRedirectFilter) *model.HTTPRequestRedirectFilter {
if redirect == nil {
return nil
}
Expand All @@ -423,11 +423,22 @@ func toHTTPRequestRedirectFilter(redirect *gatewayv1.HTTPRequestRedirectFilter)
pathModifier.Prefix = *redirect.Path.ReplacePrefixMatch
}
}
var redirectPort *int32
if redirect.Port == nil {
if redirect.Scheme == nil {
// If redirect scheme is empty, the redirect port MUST be the Gateway
// Listener port.
// Refer to: https://github.com/kubernetes-sigs/gateway-api/blob/35fe25d1384a41c9b89dd5af7ae3214c431f008c/apis/v1/httproute_types.go#L1040-L1041
redirectPort = model.AddressOf(int32(listener.Port))
}
} else {
redirectPort = (*int32)(redirect.Port)
}
return &model.HTTPRequestRedirectFilter{
Scheme: redirect.Scheme,
Hostname: (*string)(redirect.Hostname),
Path: pathModifier,
Port: (*int32)(redirect.Port),
Port: redirectPort,
StatusCode: redirect.StatusCode,
}
}
Expand Down
3 changes: 3 additions & 0 deletions operator/pkg/model/ingestion/gateway_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1349,6 +1349,7 @@ var requestRedirectHTTPListeners = []model.HTTPListener{
},
RequestRedirect: &model.HTTPRequestRedirectFilter{
Hostname: model.AddressOf("example.com"),
Port: model.AddressOf(int32(80)),
},
},
{
Expand All @@ -1359,6 +1360,7 @@ var requestRedirectHTTPListeners = []model.HTTPListener{
},
RequestRedirect: &model.HTTPRequestRedirectFilter{
StatusCode: model.AddressOf(301),
Port: model.AddressOf(int32(80)),
},
},
{
Expand All @@ -1375,6 +1377,7 @@ var requestRedirectHTTPListeners = []model.HTTPListener{
RequestRedirect: &model.HTTPRequestRedirectFilter{
Hostname: model.AddressOf("example.com"),
StatusCode: model.AddressOf(301),
Port: model.AddressOf(int32(80)),
},
},
},
Expand Down
236 changes: 236 additions & 0 deletions operator/pkg/model/translation/gateway-api/translator_fixture_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
envoy_extensions_listener_tls_inspector_v3 "github.com/cilium/proxy/go/envoy/extensions/filters/listener/tls_inspector/v3"
http_connection_manager_v3 "github.com/cilium/proxy/go/envoy/extensions/filters/network/http_connection_manager/v3"
envoy_extensions_filters_network_tcp_v3 "github.com/cilium/proxy/go/envoy/extensions/filters/network/tcp_proxy/v3"
envoy_extensions_transport_sockets_tls_v3 "github.com/cilium/proxy/go/envoy/extensions/transport_sockets/tls/v3"
envoy_upstreams_http_v3 "github.com/cilium/proxy/go/envoy/extensions/upstreams/http/v3"
envoy_type_matcher_v3 "github.com/cilium/proxy/go/envoy/type/matcher/v3"
envoy_type_v3 "github.com/cilium/proxy/go/envoy/type/v3"
Expand Down Expand Up @@ -67,6 +68,47 @@ var httpInsecureListenerXDSResource = toAny(&envoy_config_listener.Listener{
SocketOptions: toSocketOptions(),
})

var httpSecureListenerXDSResource = toAny(&envoy_config_listener.Listener{
Name: "listener",
FilterChains: []*envoy_config_listener.FilterChain{
{
FilterChainMatch: &envoy_config_listener.FilterChainMatch{TransportProtocol: "raw_buffer"},
Filters: []*envoy_config_listener.Filter{
toListenerFilter("listener-insecure"),
},
},
{
FilterChainMatch: &envoy_config_listener.FilterChainMatch{TransportProtocol: "tls", ServerNames: []string{"example.com"}},
Filters: []*envoy_config_listener.Filter{
toListenerFilter("listener-secure"),
},
TransportSocket: &envoy_config_core_v3.TransportSocket{
Name: "envoy.transport_sockets.tls",
ConfigType: &envoy_config_core_v3.TransportSocket_TypedConfig{
TypedConfig: toAny(&envoy_extensions_transport_sockets_tls_v3.DownstreamTlsContext{
CommonTlsContext: &envoy_extensions_transport_sockets_tls_v3.CommonTlsContext{
TlsCertificateSdsSecretConfigs: []*envoy_extensions_transport_sockets_tls_v3.SdsSecretConfig{
{
Name: "/gateway-conformance-infra-tls-secure",
},
},
},
}),
},
},
},
},
ListenerFilters: []*envoy_config_listener.ListenerFilter{
{
Name: "envoy.filters.listener.tls_inspector",
ConfigType: &envoy_config_listener.ListenerFilter_TypedConfig{
TypedConfig: toAny(&envoy_extensions_listener_tls_inspector_v3.TlsInspector{}),
},
},
},
SocketOptions: toSocketOptions(),
})

// basicHTTPListeners is the internal model representation of the simple HTTP listeners
var basicHTTPListeners = []model.HTTPListener{
{
Expand Down Expand Up @@ -3810,6 +3852,200 @@ var requestRedirectHTTPListenersCiliumEnvoyConfig = &ciliumv2.CiliumEnvoyConfig{
},
}

// requestRedirectWithMultiHTTPListeners is the internal representation of the Conformance/HTTPRouteRequestRedirect
var requestRedirectWithMultiHTTPListeners = []model.HTTPListener{
{
Name: "http",
Sources: []model.FullyQualifiedResource{
{
Kind: "Gateway",
Name: "same-namespace",
Namespace: "gateway-conformance-infra",
},
},
Port: 80,
Hostname: "example.com",
Routes: []model.HTTPRoute{
{
PathMatch: model.StringMatch{Prefix: "/request-redirect"},
Backends: []model.Backend{
{
Name: "infra-backend-v1",
Namespace: "gateway-conformance-infra",
Port: &model.BackendPort{
Port: 8080,
},
},
},
},
{
PathMatch: model.StringMatch{Prefix: "/"},
DirectResponse: &model.DirectResponse{
StatusCode: 500,
},
RequestRedirect: &model.HTTPRequestRedirectFilter{
Hostname: model.AddressOf("example.com"),
Path: &model.StringMatch{Prefix: "/request-redirect"},
StatusCode: model.AddressOf(302),
Port: model.AddressOf(int32(80)),
},
},
},
},
{
Name: "https",
Sources: []model.FullyQualifiedResource{
{
Kind: "Gateway",
Name: "same-namespace",
Namespace: "gateway-conformance-infra",
},
},
Port: 443,
Hostname: "example.com",
Routes: []model.HTTPRoute{
{
PathMatch: model.StringMatch{Prefix: "/request-redirect"},
Backends: []model.Backend{
{
Name: "infra-backend-v1",
Namespace: "gateway-conformance-infra",
Port: &model.BackendPort{
Port: 8080,
},
},
},
},
{
PathMatch: model.StringMatch{Prefix: "/"},
DirectResponse: &model.DirectResponse{
StatusCode: 500,
},
RequestRedirect: &model.HTTPRequestRedirectFilter{
Hostname: model.AddressOf("example.com"),
Path: &model.StringMatch{Prefix: "/request-redirect"},
StatusCode: model.AddressOf(302),
Port: model.AddressOf(int32(443)),
},
},
},
TLS: []model.TLSSecret{
{
Name: "tls-secure",
Namespace: "gateway-conformance-infra",
},
},
},
}
var requestRedirectWithMultiHTTPListenersCiliumEnvoyConfig = &ciliumv2.CiliumEnvoyConfig{
ObjectMeta: metav1.ObjectMeta{
Name: "cilium-gateway-same-namespace",
Namespace: "gateway-conformance-infra",
Labels: map[string]string{
"cilium.io/use-original-source-address": "false",
},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "gateway.networking.k8s.io/v1",
Kind: "Gateway",
Name: "same-namespace",
Controller: model.AddressOf(true),
},
},
},
Spec: ciliumv2.CiliumEnvoyConfigSpec{
Services: []*ciliumv2.ServiceListener{
{
Name: "cilium-gateway-same-namespace",
Namespace: "gateway-conformance-infra",
},
},
BackendServices: []*ciliumv2.Service{
{
Name: "infra-backend-v1",
Namespace: "gateway-conformance-infra",
Ports: []string{"8080"},
},
},
Resources: []ciliumv2.XDSResource{
{Any: httpSecureListenerXDSResource},
{
Any: toAny(&envoy_config_route_v3.RouteConfiguration{
Name: "listener-insecure",
VirtualHosts: []*envoy_config_route_v3.VirtualHost{
{
Name: "example.com",
Domains: []string{"example.com", "example.com:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_PathSeparatedPrefix{
PathSeparatedPrefix: "/request-redirect",
},
},
Action: routeActionBackendV1,
},
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{
Prefix: "/",
},
},
Action: &envoy_config_route_v3.Route_Redirect{
Redirect: &envoy_config_route_v3.RedirectAction{
PortRedirect: 80,
HostRedirect: "example.com",
ResponseCode: envoy_config_route_v3.RedirectAction_FOUND,
PathRewriteSpecifier: &envoy_config_route_v3.RedirectAction_PrefixRewrite{PrefixRewrite: "/request-redirect"},
},
},
},
},
},
},
}),
},
{
Any: toAny(&envoy_config_route_v3.RouteConfiguration{
Name: "listener-secure",
VirtualHosts: []*envoy_config_route_v3.VirtualHost{
{
Name: "example.com",
Domains: []string{"example.com", "example.com:*"},
Routes: []*envoy_config_route_v3.Route{
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_PathSeparatedPrefix{
PathSeparatedPrefix: "/request-redirect",
},
},
Action: routeActionBackendV1,
},
{
Match: &envoy_config_route_v3.RouteMatch{
PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{
Prefix: "/",
},
},
Action: &envoy_config_route_v3.Route_Redirect{
Redirect: &envoy_config_route_v3.RedirectAction{
PortRedirect: 443,
HostRedirect: "example.com",
ResponseCode: envoy_config_route_v3.RedirectAction_FOUND,
PathRewriteSpecifier: &envoy_config_route_v3.RedirectAction_PrefixRewrite{PrefixRewrite: "/request-redirect"},
},
},
},
},
},
},
}),
},
{Any: backendV1XDSResource},
},
},
}

// responseHeaderModifierHTTPListeners is the internal representation of the Conformance/HTTPRouteResponseHeaderModifier
var responseHeaderModifierHTTPListeners = []model.HTTPListener{
{
Expand Down
9 changes: 9 additions & 0 deletions operator/pkg/model/translation/gateway-api/translator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,15 @@ func Test_translator_Translate(t *testing.T) {
},
want: mirrorHTTPListenersCiliumEnvoyConfig,
},
{
name: "Conformance/HTTPRouteRequestRedirectWithMultiHTTPListeners",
args: args{
m: &model.Model{
HTTP: requestRedirectWithMultiHTTPListeners,
},
},
want: requestRedirectWithMultiHTTPListenersCiliumEnvoyConfig,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit e546df1

Please sign in to comment.