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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@
`sticky_sessions_cookie` is set when `algorithm` is `sticky-sessions`.
[#3555](https://github.com/Kong/kong-operator/pull/3555)

### Changed

- HybridGateway: generate one KongRoute per HTTPRouteMatch to honor Gateway API OR semantics across matches within a rule.
This enables the `HTTPRouteMatching` conformance test for Hybrid mode.
Note: routes count per rule may increase.
[#3577](https://github.com/Kong/kong-operator/pull/3577)

### Fixes

- Admission webhook now validates HTTPRoute regex patterns before sending
Expand Down
69 changes: 38 additions & 31 deletions controller/hybridgateway/converter/http_route.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,23 +425,27 @@ func (c *httpRouteConverter) translate(ctx context.Context, logger logr.Logger)
log.Debug(logger, "Successfully translated KongService resource",
"service", serviceName)

// Build the KongRoute resource.
routePtr, err := kongroute.RouteForRule(ctx, logger, c.Client, c.route, rule, &pRef, cp, serviceName, hostnames)
// Build one KongRoute per match in the rule.
// Gateway API semantics require OR across matches within a rule
// and AND within a single match. Generating a route per match
// preserves the OR semantics for Hybrid Gateway.
routes, err := kongroute.RoutesForRule(ctx, logger, c.Client, c.route, rule, &pRef, cp, serviceName, hostnames)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please explain why one route is required for each rule?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Added explanations in the RoutesForRule godoc and at the call site

https://github.com/kubernetes-sigs/gateway-api/blob/913c416869573abb70cee0636c6c5395a412687a/apis/v1/httproute_types.go#L149-L210

The key reason: Gateway API specifies matches within a rule are ORed, but Kong combines all matchers on a single route with AND. Splitting to one route per match preserves the OR semantics, which is required by conformance tests like HTTPRouteMatching.

if err != nil {
log.Error(logger, err, "Failed to translate KongRoute resource, skipping rule",
log.Error(logger, err, "Failed to translate KongRoute resources for rule, skipping rule",
"service", serviceName,
"hostnames", hostnames)
translationErrors = append(translationErrors, fmt.Errorf("failed to translate KongRoute for rule: %w", err))
translationErrors = append(translationErrors, fmt.Errorf("failed to translate KongRoutes for rule: %w", err))
continue
}
routeName := routePtr.Name
c.outputStore = append(c.outputStore, routePtr)
log.Debug(logger, "Successfully translated KongRoute resource",
"route", routeName)
for _, r := range routes {
routeName := r.Name
c.outputStore = append(c.outputStore, r)
log.Debug(logger, "Successfully translated KongRoute resource",
"route", routeName)
}

// Build the KongPlugin and KongPluginBinding resources.
log.Debug(logger, "Processing filters for rule",
"kongRoute", routeName,
"filterCount", len(rule.Filters))

for _, filter := range rule.Filters {
Expand All @@ -456,30 +460,33 @@ func (c *httpRouteConverter) translate(ctx context.Context, logger logr.Logger)
for _, plugin := range plugins {
pluginName := plugin.Name
c.outputStore = append(c.outputStore, &plugin)
// Create a KongPluginBinding to bind the KongPlugin to each KongRoute.
bindingPtr, err := pluginbinding.BindingForPluginAndRoute(
ctx,
logger,
c.Client,
c.route,
&pRef,
cp,
pluginName,
routeName,
)
if err != nil {
log.Error(logger, err, "Failed to build KongPluginBinding resource, skipping binding",
// Create a KongPluginBinding to bind the KongPlugin to each KongRoute generated for the rule.
for _, r := range routes {
bindingPtr, err := pluginbinding.BindingForPluginAndRoute(
ctx,
logger,
c.Client,
c.route,
&pRef,
cp,
pluginName,
r.Name,
)
if err != nil {
log.Error(logger, err, "Failed to build KongPluginBinding resource, skipping binding",
"plugin", pluginName,
"kongRoute", r.Name)
translationErrors = append(translationErrors, fmt.Errorf("failed to build KongPluginBinding for plugin %s: %w", pluginName, err))
continue
}
bindingName := bindingPtr.Name
c.outputStore = append(c.outputStore, bindingPtr)

log.Debug(logger, "Successfully translated KongPlugin and KongPluginBinding resources",
"plugin", pluginName,
"kongRoute", routeName)
translationErrors = append(translationErrors, fmt.Errorf("failed to build KongPluginBinding for plugin %s: %w", pluginName, err))
continue
"binding", bindingName,
"route", r.Name)
}
bindingName := bindingPtr.Name
c.outputStore = append(c.outputStore, bindingPtr)

log.Debug(logger, "Successfully translated KongPlugin and KongPluginBinding resources",
"plugin", pluginName,
"binding", bindingName)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ func TestHTTPRouteConverter_Translate(t *testing.T) {
return newHTTPRouteConverter(route, fakeClient, false, "").(*httpRouteConverter)
},
wantErr: true,
wantErrSub: "failed to translate KongRoute for rule",
wantErrSub: "failed to translate KongRoutes for rule",
wantOutputs: outputCount{
upstreams: 1,
services: 1,
Expand Down
101 changes: 58 additions & 43 deletions controller/hybridgateway/kongroute/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"

"github.com/go-logr/logr"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"

Expand All @@ -18,30 +19,32 @@ import (
gwtypes "github.com/kong/kong-operator/v2/internal/types"
)

// RouteForRule creates or updates a KongRoute for the given HTTPRoute rule.
// RoutesForRule creates or updates one KongRoute per HTTPRouteMatch in the given rule.
//
// The function performs the following operations:
// 1. Generates the KongRoute name using the namegen package
// 2. Checks if a KongRoute with that name already exists in the cluster
// 3. If it exists, updates the KongRoute
// 4. If it doesn't exist, creates a new KongRoute
// 5. Returns the KongRoute resource for use by the caller
// Gateway API semantics are:
// - Within a single HTTPRouteRule, entries in .Matches are ORed
// - Within a single HTTPRouteMatch, individual criteria (path/method/headers) are ANDed
//
// To faithfully represent this in Kong, we generate one KongRoute for each HTTPRouteMatch
// and attach only that match's criteria to the route. All routes point to the same KongService.
// This fixes Hybrid Gateway conformance failures such as HTTPRouteMatching, which includes
// rules that combine independent path-only and header-only matches.
//
// Parameters:
// - ctx: The context for API calls and cancellation
// - logger: Logger for structured logging
// - cl: Kubernetes client for API operations
// - httpRoute: The HTTPRoute resource from which the KongRoute is derived
// - rule: The specific rule within the HTTPRoute from which the KongRoute is derived
// - httpRoute: The HTTPRoute resource from which the KongRoutes are derived
// - rule: The specific rule within the HTTPRoute
// - pRef: The parent reference (Gateway) for the HTTPRoute
// - cp: The control plane reference for the KongRoute
// - serviceName: The name of the KongService this KongRoute should point to
// - hostnames: The hostnames for the KongRoute
// - cp: The control plane reference for the KongRoutes
// - serviceName: The name of the KongService these KongRoutes should point to
// - hostnames: The hostnames for the KongRoutes
//
// Returns:
// - kongRoute: The created or updated KongRoute resource
// - kongRoutes: The created or updated KongRoute resources (one per match)
// - err: Any error that occurred during the process
func RouteForRule(
func RoutesForRule(
ctx context.Context,
logger logr.Logger,
cl client.Client,
Expand All @@ -51,41 +54,53 @@ func RouteForRule(
cp *commonv1alpha1.ControlPlaneRef,
serviceName string,
hostnames []string,
) (kongRoute *configurationv1alpha1.KongRoute, err error) {
routeName := namegen.NewKongRouteName(httpRoute, cp, rule)
logger = logger.WithValues("kongroute", routeName)
log.Debug(logger, "Creating KongRoute for HTTPRoute rule")

routeBuilder := builder.NewKongRoute().
WithName(routeName).
WithNamespace(metadata.NamespaceFromParentRef(httpRoute, pRef)).
WithLabels(httpRoute, pRef).
WithAnnotations(httpRoute, pRef).
WithSpecName(routeName).
WithHosts(hostnames).
WithStripPath(metadata.ExtractStripPath(httpRoute.Annotations)).
WithPreserveHost(metadata.ExtractPreserveHost(httpRoute.Annotations)).
WithKongService(serviceName)
) (kongRoutes []*configurationv1alpha1.KongRoute, err error) {
// If the rule has no matches, create a single catch-all route.
// Kong requires at least one matcher; use "/" path to represent catch-all.
if len(rule.Matches) == 0 {
match := gatewayv1.HTTPRouteMatch{
Path: &gatewayv1.HTTPPathMatch{Type: ptr.To(gatewayv1.PathMatchPathPrefix), Value: new("/")},
}
rule.Matches = append(rule.Matches, match)
}

// Check if the rule contains a URLRewrite or RequestRedirect filter with ReplacePrefixMatch:
// if so, we need to set a capture group on the KongRoute paths.
// Check filters to determine if we need capture groups in paths.
setCaptureGroup := needsCaptureGroup(rule)

// Add HTTPRoute matches
for _, match := range rule.Matches {
routeBuilder = routeBuilder.WithHTTPRouteMatch(match, setCaptureGroup)
}
newRoute, err := routeBuilder.Build()
if err != nil {
log.Error(logger, err, "Failed to build KongRoute resource")
return nil, fmt.Errorf("failed to build KongRoute %s: %w", routeName, err)
}
for i, match := range rule.Matches {
routeName := namegen.NewKongRouteNameForMatch(httpRoute, cp, match, i)
mLog := logger.WithValues("kongroute", routeName, "matchIndex", i)
log.Debug(mLog, "Creating KongRoute for HTTPRoute match")

routeBuilder := builder.NewKongRoute().
WithName(routeName).
WithNamespace(metadata.NamespaceFromParentRef(httpRoute, pRef)).
WithLabels(httpRoute, pRef).
WithAnnotations(httpRoute, pRef).
WithSpecName(routeName).
WithHosts(hostnames).
WithStripPath(metadata.ExtractStripPath(httpRoute.Annotations)).
WithPreserveHost(metadata.ExtractPreserveHost(httpRoute.Annotations)).
WithKongService(serviceName).
WithHTTPRouteMatch(match, setCaptureGroup)

newRoute, buildErr := routeBuilder.Build()
if buildErr != nil {
log.Error(mLog, buildErr, "Failed to build KongRoute resource")
return nil, fmt.Errorf("failed to build KongRoute %s: %w", routeName, buildErr)
}

if _, updErr := translator.VerifyAndUpdate(ctx, mLog, cl, &newRoute, httpRoute, true); updErr != nil {
return nil, updErr
}

if _, err = translator.VerifyAndUpdate(ctx, logger, cl, &newRoute, httpRoute, true); err != nil {
return nil, err
// Add to result slice as an explicit copy for clarity.
// Using DeepCopy expresses the intent that each match yields an
// independent KongRoute object.
kongRoutes = append(kongRoutes, newRoute.DeepCopy())
}

return &newRoute, nil
return kongRoutes, nil
}

// needsCaptureGroup checks if the given HTTPRoute rule requires a capture group
Expand Down
80 changes: 46 additions & 34 deletions controller/hybridgateway/kongroute/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,11 @@ import (

commonv1alpha1 "github.com/kong/kong-operator/v2/api/common/v1alpha1"
configurationv1alpha1 "github.com/kong/kong-operator/v2/api/configuration/v1alpha1"
"github.com/kong/kong-operator/v2/controller/hybridgateway/builder"
gwtypes "github.com/kong/kong-operator/v2/internal/types"
"github.com/kong/kong-operator/v2/pkg/consts"
)

func TestRouteForRule(t *testing.T) {
func TestRoutesForRule(t *testing.T) {
ctx := context.Background()
logger := logr.Discard()

Expand All @@ -43,15 +42,22 @@ func TestRouteForRule(t *testing.T) {
},
}

// Create test rule
// Create test rule with two matches so that two KongRoutes are generated
prefix := gatewayv1.PathMatchPathPrefix
rule := gwtypes.HTTPRouteRule{
Matches: []gatewayv1.HTTPRouteMatch{
{
Path: &gatewayv1.HTTPPathMatch{
Type: (*gatewayv1.PathMatchType)(new("PathPrefix")),
Type: &prefix,
Value: new("/test"),
},
},
{
Headers: []gatewayv1.HTTPHeaderMatch{{
Name: "X-Foo",
Value: "bar",
}},
},
},
}

Expand All @@ -75,13 +81,14 @@ func TestRouteForRule(t *testing.T) {
serviceName string
hostnames []string
expectError bool
expectUpdate bool
expectRoutes int
}{
{
name: "create new route",
serviceName: "test-service",
hostnames: []string{"example.com"},
expectError: false,
name: "create new route",
serviceName: "test-service",
hostnames: []string{"example.com"},
expectError: false,
expectRoutes: 2,
},
}

Expand All @@ -93,38 +100,43 @@ func TestRouteForRule(t *testing.T) {
}
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build()

result, err := RouteForRule(ctx, logger, fakeClient, httpRoute, rule, pRef, cpRef, tt.serviceName, tt.hostnames)
results, err := RoutesForRule(ctx, logger, fakeClient, httpRoute, rule, pRef, cpRef, tt.serviceName, tt.hostnames)

if tt.expectError {
assert.Error(t, err)
assert.Nil(t, result)
assert.Nil(t, results)
return
}

require.NoError(t, err)
require.NotNil(t, result)

// Verify the route has correct properties
assert.Equal(t, "test-namespace", result.Namespace)
assert.NotEmpty(t, result.Name)
assert.Equal(t, tt.hostnames, result.Spec.Hosts)

// Verify service reference
if tt.serviceName != "" {
require.NotNil(t, result.Spec.ServiceRef)
assert.Equal(t, configurationv1alpha1.ServiceRefNamespacedRef, result.Spec.ServiceRef.Type)
require.NotNil(t, result.Spec.ServiceRef.NamespacedRef)
assert.Equal(t, tt.serviceName, result.Spec.ServiceRef.NamespacedRef.Name)
}

// Verify HTTPRoute annotation
expectedAnnotation := httpRoute.Namespace + "/" + httpRoute.Name
assert.Contains(t, result.Annotations[consts.GatewayOperatorHybridRoutesAnnotation], expectedAnnotation)

// Verify paths from rule matches
if len(rule.Matches) > 0 && rule.Matches[0].Path != nil {
paths, _ := builder.GenerateKongRoutePathFromHTTPRouteMatch(rule.Matches[0].Path, false)
assert.Subset(t, result.Spec.Paths, paths)
require.NotNil(t, results)
assert.Len(t, results, tt.expectRoutes)

for _, result := range results {
// Verify the route has correct properties
assert.Equal(t, "test-namespace", result.Namespace)
assert.NotEmpty(t, result.Name)
assert.Equal(t, tt.hostnames, result.Spec.Hosts)

// Verify service reference
if tt.serviceName != "" {
require.NotNil(t, result.Spec.ServiceRef)
assert.Equal(t, configurationv1alpha1.ServiceRefNamespacedRef, result.Spec.ServiceRef.Type)
require.NotNil(t, result.Spec.ServiceRef.NamespacedRef)
assert.Equal(t, tt.serviceName, result.Spec.ServiceRef.NamespacedRef.Name)
}

// Verify HTTPRoute annotation
expectedAnnotation := httpRoute.Namespace + "/" + httpRoute.Name
assert.Contains(t, result.Annotations[consts.GatewayOperatorHybridRoutesAnnotation], expectedAnnotation)

// Verify at least one path/header/method is set based on the match.
// For the first match with path, we expect Paths to be non-empty.
// For the second header-only match, Paths may be empty; Headers must be set.
if len(result.Spec.Paths) == 0 {
// header-only route
assert.Contains(t, result.Spec.Headers, "X-Foo")
}
}
})
}
Expand Down
Loading
Loading