Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 12 additions & 3 deletions pkg/gateway/model/model_build_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (l listenerBuilderImpl) buildL4ListenerSpec(ctx context.Context, stack core

func (l listenerBuilderImpl) buildListenerRules(ctx context.Context, stack core.Stack, ls *elbv2model.Listener, ipAddressType elbv2model.IPAddressType, gw *gwv1.Gateway, port int32, lbCfg elbv2gw.LoadBalancerConfiguration, routes map[int32][]routeutils.RouteDescriptor) ([]types.NamespacedName, error) {
// sort all rules based on precedence
rulesWithPrecedenceOrder := routeutils.SortAllRulesByPrecedence(routes[port])
rulesWithPrecedenceOrder := routeutils.SortAllRulesByPrecedence(routes[port], port)
secrets := make([]types.NamespacedName, 0)
var albRules []elbv2model.Rule
for _, ruleWithPrecedence := range rulesWithPrecedenceOrder {
Expand Down Expand Up @@ -509,8 +509,17 @@ func mapGatewayListenerConfigsByPort(gw *gwv1.Gateway, routes map[int32][]routeu

if listenerRoutes != nil {
for _, route := range listenerRoutes {
for _, routeHostname := range route.GetHostnames() {
gwListenerConfigs[port].hostnames.Insert(string(routeHostname))
// Use compatible hostnames (intersection) instead of raw route hostnames
compatibleHostnamesByPort := route.GetCompatibleHostnamesByPort()[port]
if len(compatibleHostnamesByPort) > 0 {
for _, hostname := range compatibleHostnamesByPort {
gwListenerConfigs[port].hostnames.Insert(string(hostname))
}
} else {
// Fallback to route hostnames if no compatible hostnames
for _, routeHostname := range route.GetHostnames() {
gwListenerConfigs[port].hostnames.Insert(string(routeHostname))
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/gateway/model/model_build_listener_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ func Test_buildListenerTags(t *testing.T) {
},
externalManagedTags: []string{"ExternalTag", "ManagedByTeam"},
expectedTags: nil,
expectedErr: errors.New("external managed tag key ExternalTag cannot be specified"),
expectedErr: errors.New("external managed tag key"),
},
}

Expand All @@ -448,6 +448,7 @@ func Test_buildListenerTags(t *testing.T) {
if tt.expectedErr != nil {
assert.Error(t, err)
assert.Contains(t, err.Error(), tt.expectedErr.Error())
assert.True(t, strings.Contains(err.Error(), "ExternalTag") || strings.Contains(err.Error(), "ManagedByTeam"))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test has been flaky

--- FAIL: Test_buildListenerTags (0.00s)
--- FAIL: Test_buildListenerTags/external_managed_tags_specified_by_user_should_cause_error (0.00s)
model_build_listener_test.go:450:
Error Trace: /Users/shuqz/go/src/github.com/sigs.k8s.io/shuqz-aws-load-balancer-controller/pkg/gateway/model/model_build_listener_test.go:450
Error: "external managed tag key ManagedByTeam cannot be specified" does not contain "external managed tag key ExternalTag cannot be specified"
Test: Test_buildListenerTags/external_managed_tags_specified_by_user_should_cause_error
FAIL

assert.Nil(t, got)
} else {
assert.NoError(t, err)
Expand Down
12 changes: 11 additions & 1 deletion pkg/gateway/routeutils/descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package routeutils

import (
"context"
"time"

"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
"time"
)

// routeMetadataDescriptor a common set of functions that will describe a route.
Expand All @@ -21,6 +22,15 @@ type routeMetadataDescriptor interface {
GetRouteListenerRuleConfigRefs() []gwv1.LocalObjectReference
GetRouteGeneration() int64
GetRouteCreateTimestamp() time.Time
// GetCompatibleHostnamesByPort returns the compatible hostnames for each listener port.
// Compatible hostnames are computed during route attachment by intersecting listener hostnames
// with route hostnames (considering wildcards). The map key is the listener port number.
// When a route attaches to multiple listeners on the same port, hostnames are aggregated.
// When a route attaches to listeners on different ports, each port has its own hostname list.
GetCompatibleHostnamesByPort() map[int32][]gwv1.Hostname
// setCompatibleHostnamesByPort is a package-private method to set compatible hostnames.
// This is called by the loader after route attachment validation.
setCompatibleHostnamesByPort(map[int32][]gwv1.Hostname)
}

type routeLoadError struct {
Expand Down
15 changes: 12 additions & 3 deletions pkg/gateway/routeutils/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ func (t *convertedGRPCRouteRule) GetListenerRuleConfig() *elbv2gw.ListenerRuleCo
/* Route Description */

type grpcRouteDescription struct {
route *gwv1.GRPCRoute
rules []RouteRule
ruleAccumulator attachedRuleAccumulator[gwv1.GRPCRouteRule]
route *gwv1.GRPCRoute
rules []RouteRule
ruleAccumulator attachedRuleAccumulator[gwv1.GRPCRouteRule]
compatibleHostnamesByPort map[int32][]gwv1.Hostname
}

func (grpcRoute *grpcRouteDescription) loadAttachedRules(ctx context.Context, k8sClient client.Client) (RouteDescriptor, []routeLoadError) {
Expand Down Expand Up @@ -143,6 +144,14 @@ func (grpcRoute *grpcRouteDescription) GetRouteCreateTimestamp() time.Time {
return grpcRoute.route.CreationTimestamp.Time
}

func (grpcRoute *grpcRouteDescription) GetCompatibleHostnamesByPort() map[int32][]gwv1.Hostname {
return grpcRoute.compatibleHostnamesByPort
}

func (grpcRoute *grpcRouteDescription) setCompatibleHostnamesByPort(hostnamesByPort map[int32][]gwv1.Hostname) {
grpcRoute.compatibleHostnamesByPort = hostnamesByPort
}

var _ RouteDescriptor = &grpcRouteDescription{}

// Can we use an indexer here to query more efficiently?
Expand Down
15 changes: 12 additions & 3 deletions pkg/gateway/routeutils/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ func (t *convertedHTTPRouteRule) GetListenerRuleConfig() *elbv2gw.ListenerRuleCo
/* Route Description */

type httpRouteDescription struct {
route *gwv1.HTTPRoute
rules []RouteRule
ruleAccumulator attachedRuleAccumulator[gwv1.HTTPRouteRule]
route *gwv1.HTTPRoute
rules []RouteRule
ruleAccumulator attachedRuleAccumulator[gwv1.HTTPRouteRule]
compatibleHostnamesByPort map[int32][]gwv1.Hostname
}

func (httpRoute *httpRouteDescription) GetAttachedRules() []RouteRule {
Expand Down Expand Up @@ -136,6 +137,14 @@ func (httpRoute *httpRouteDescription) GetRouteCreateTimestamp() time.Time {
return httpRoute.route.CreationTimestamp.Time
}

func (httpRoute *httpRouteDescription) GetCompatibleHostnamesByPort() map[int32][]gwv1.Hostname {
return httpRoute.compatibleHostnamesByPort
}

func (httpRoute *httpRouteDescription) setCompatibleHostnamesByPort(hostnamesByPort map[int32][]gwv1.Hostname) {
httpRoute.compatibleHostnamesByPort = hostnamesByPort
}

func convertHTTPRoute(r gwv1.HTTPRoute) *httpRouteDescription {
return &httpRouteDescription{route: &r, ruleAccumulator: defaultHTTPRuleAccumulator}
}
Expand Down
62 changes: 40 additions & 22 deletions pkg/gateway/routeutils/listener_attachment_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
// listenerAttachmentHelper is an internal utility interface that can be used to determine if a listener will allow
// a route to attach to it.
type listenerAttachmentHelper interface {
listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, *RouteData, error)
listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error)
}

var _ listenerAttachmentHelper = &listenerAttachmentHelperImpl{}
Expand All @@ -35,33 +35,36 @@ func newListenerAttachmentHelper(k8sClient client.Client, logger logr.Logger) li

// listenerAllowsAttachment utility method to determine if a listener will allow a route to connect using
// Gateway API rules to determine compatibility between lister and route.
func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, *RouteData, error) {
// Returns: (compatibleHostnames, allowed, failedRouteData, error)
func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(ctx context.Context, gw gwv1.Gateway, listener gwv1.Listener, route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) ([]gwv1.Hostname, bool, *RouteData, error) {
// check namespace
namespaceOK, err := attachmentHelper.namespaceCheck(ctx, gw, listener, route)
if err != nil {
return false, nil, err
return nil, false, nil, err
}
if !namespaceOK {
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageNamespaceNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
return false, &rd, nil
return nil, false, &rd, nil
}

// check kind
kindOK := attachmentHelper.kindCheck(listener, route)
if !kindOK {
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), RouteStatusInfoRejectedMessageKindNotMatch, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
return false, &rd, nil
return nil, false, &rd, nil
}

// check hostname
if (route.GetRouteKind() == HTTPRouteKind || route.GetRouteKind() == GRPCRouteKind || route.GetRouteKind() == TLSRouteKind) && route.GetHostnames() != nil {
hostnameOK, err := attachmentHelper.hostnameCheck(listener, route)
// check hostname and get compatible hostnames
var compatibleHostnames []gwv1.Hostname
if route.GetRouteKind() == HTTPRouteKind || route.GetRouteKind() == GRPCRouteKind || route.GetRouteKind() == TLSRouteKind {
var hostnameOK bool
compatibleHostnames, hostnameOK, err = attachmentHelper.hostnameCheck(listener, route)
if err != nil {
return false, nil, err
return nil, false, nil, err
}
if !hostnameOK {
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNoMatchingListenerHostname), RouteStatusInfoRejectedMessageNoMatchingHostname, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
return false, &rd, nil
return nil, false, &rd, nil
}
}

Expand All @@ -71,11 +74,11 @@ func (attachmentHelper *listenerAttachmentHelperImpl) listenerAllowsAttachment(c
if !hostnameUniquenessOK {
message := fmt.Sprintf("HTTPRoute and GRPCRoute have overlap hostname, attachment is rejected. Conflict route: %s", conflictRoute)
rd := GenerateRouteData(false, true, string(gwv1.RouteReasonNotAllowedByListeners), message, route.GetRouteNamespacedName(), route.GetRouteKind(), route.GetRouteGeneration(), gw)
return false, &rd, nil
return nil, false, &rd, nil
}
}

return true, nil, nil
return compatibleHostnames, true, nil, nil
}

// namespaceCheck namespace check implements the Gateway API spec for namespace matching between listener
Expand Down Expand Up @@ -170,24 +173,33 @@ func (attachmentHelper *listenerAttachmentHelperImpl) kindCheck(listener gwv1.Li
return true
}

func (attachmentHelper *listenerAttachmentHelperImpl) hostnameCheck(listener gwv1.Listener, route preLoadRouteDescriptor) (bool, error) {
// A route can attach to listener if it does not have hostname or listener does not have hostname
if listener.Hostname == nil || len(route.GetHostnames()) == 0 {
return true, nil
func (attachmentHelper *listenerAttachmentHelperImpl) hostnameCheck(listener gwv1.Listener, route preLoadRouteDescriptor) ([]gwv1.Hostname, bool, error) {
// If route has no hostnames but listener does, use listener hostname
if route.GetHostnames() == nil || len(route.GetHostnames()) == 0 {
if listener.Hostname != nil {
return []gwv1.Hostname{*listener.Hostname}, true, nil
}
return nil, true, nil
}

// If listener has no hostname, route can attach
if listener.Hostname == nil {
return nil, true, nil
}

// validate listener hostname, return if listener hostname is not valid
isListenerHostnameValid, err := IsHostNameInValidFormat(string(*listener.Hostname))
if err != nil {
attachmentHelper.logger.Error(err, "listener hostname is not valid", "listener", listener.Name, "hostname", *listener.Hostname)
initialErrorMessage := fmt.Sprintf("listener hostname %s is not valid (listener name %s)", listener.Name, *listener.Hostname)
return false, wrapError(errors.Errorf("%s", initialErrorMessage), gwv1.GatewayReasonListenersNotValid, gwv1.RouteReasonUnsupportedValue, nil, nil)
return nil, false, wrapError(errors.Errorf("%s", initialErrorMessage), gwv1.GatewayReasonListenersNotValid, gwv1.RouteReasonUnsupportedValue, nil, nil)
}

if !isListenerHostnameValid {
return false, nil
return nil, false, nil
}

compatibleHostnames := []gwv1.Hostname{}
for _, hostname := range route.GetHostnames() {
// validate route hostname, skip invalid hostname
isHostnameValid, err := IsHostNameInValidFormat(string(hostname))
Expand All @@ -196,12 +208,18 @@ func (attachmentHelper *listenerAttachmentHelperImpl) hostnameCheck(listener gwv
continue
}

// check if two hostnames have overlap (compatible)
if isHostnameCompatible(string(hostname), string(*listener.Hostname)) {
return true, nil
// check if two hostnames have overlap (compatible) and get the more specific one
if compatible, ok := getCompatibleHostname(string(hostname), string(*listener.Hostname)); ok {
compatibleHostnames = append(compatibleHostnames, gwv1.Hostname(compatible))
}
}
return false, nil

if len(compatibleHostnames) == 0 {
return nil, false, nil
}

// Return computed compatible hostnames without storing in route
return compatibleHostnames, true, nil
}

func (attachmentHelper *listenerAttachmentHelperImpl) crossServingHostnameUniquenessCheck(route preLoadRouteDescriptor, hostnamesFromHttpRoutes map[types.NamespacedName][]gwv1.Hostname, hostnamesFromGrpcRoutes map[types.NamespacedName][]gwv1.Hostname) (bool, string) {
Expand Down
Loading
Loading