Skip to content

Feat(hybrid gateway): Add TLSRoute to hybrid gateway reconciler and implement translation#3708

Draft
randmonkey wants to merge 7 commits intomainfrom
feat/translate_tlsroute_for_hybrid_gateway
Draft

Feat(hybrid gateway): Add TLSRoute to hybrid gateway reconciler and implement translation#3708
randmonkey wants to merge 7 commits intomainfrom
feat/translate_tlsroute_for_hybrid_gateway

Conversation

@randmonkey
Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Which issue this PR fixes

Fixes #

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect significant changes

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds Gateway API TLSRoute support to the hybrid gateway reconciler by wiring it into controller setup, cache indexes, watches/ownership, translation (KongUpstream/KongService/KongRoute/KongTarget generation), and status/RBAC plumbing.

Changes:

  • Register TLSRoute in manager/controller setup, cache indexes, watches/owns/filtering, and RBAC/chart manifests.
  • Generalize existing HTTPRoute-only helpers (indexing, refs, status, translation building blocks) to work with both HTTPRoute and TLSRoute via generics.
  • Introduce a TLSRoute converter and TLSRoute-specific route translation behavior (e.g., SNI/protocol selection, passthrough detection).

Reviewed changes

Copilot reviewed 60 out of 60 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
modules/manager/controller_setup_test.go Updates expected controller count after adding TLSRoute controller.
modules/manager/controller_setup.go Registers TLSRoute cache indexes and hybrid controller.
internal/utils/index/tlsroute.go Adds TLSRoute field index options (services + gateways).
internal/utils/index/httproute_test.go Reworks HTTPRoute index tests (currently left as a stub).
internal/utils/index/httproute.go Switches gateway indexing to shared GatewaysOnRoute and refactors backend service extraction.
internal/utils/index/gateway_route_test.go Adds tests for shared gateway extraction from ParentRefs.
internal/utils/index/gateway_route.go Introduces shared GatewaysOnRoute and backendRefToServiceKey helpers.
internal/types/interfaces.go Adds generic type constraints for supported routes/rules/backendrefs.
internal/types/gatewaytypes.go Adds TLSRoute type aliases and GetSpecParentRefs helper.
controller/hybridgateway/watch/watch.go Adds TLSRoute watch wiring and generalizes HTTPRoute watchers to shared mappers.
controller/hybridgateway/watch/own_test.go Extends ownership tests to TLSRoute.
controller/hybridgateway/watch/own.go Adds owned resource set for TLSRoute.
controller/hybridgateway/watch/mapfuncs_tlsroute.go Adds TLSRoute list helpers for gateway/service mapping.
controller/hybridgateway/watch/mapfuncs_httproute.go Switches plugin mapfunc to shared route mapper.
controller/hybridgateway/watch/mapfuncs_gateway_routes.go Adds shared generic mapfuncs for gateway/gatewayclass/service/endpointslice/Kong resources.
controller/hybridgateway/watch/filter_test.go Updates tests to use generic route filter.
controller/hybridgateway/watch/filter.go Generalizes HTTPRoute filter predicate to filterByRoute and adds TLSRoute support.
controller/hybridgateway/upstream/upstream.go Generalizes upstream translation to accept supported routes/rules.
controller/hybridgateway/target/target_test.go Updates tests for generic backend ref handling and TypeMeta use.
controller/hybridgateway/target/target.go Generalizes target translation to accept supported routes/backendrefs.
controller/hybridgateway/service/service.go Generalizes service translation to accept supported routes/rules.
controller/hybridgateway/route/status_test.go Updates status tests for new reference grant checks and TypeMeta usage.
controller/hybridgateway/route/status.go Generalizes route status condition helpers for HTTPRoute + TLSRoute.
controller/hybridgateway/refs/get.go Generalizes gateway lookup by ParentRefs to supported routes.
controller/hybridgateway/refs/by.go Extends refs resolution to TLSRoute and generalizes route helpers.
controller/hybridgateway/reconciler_utils_test.go Updates enforceState test usage after API change.
controller/hybridgateway/reconciler_utils.go Extends “references supported gateway” logic to TLSRoute.
controller/hybridgateway/namegen/name.go Generalizes name generation for TLSRoute and refactors backend ref display names.
controller/hybridgateway/metadata/annotation.go Adds TLSRoute support to hybrid route annotations.
controller/hybridgateway/kongroute/route.go Adds TLSRoute KongRoute translation (protocols/SNIs) and refactors HTTPRoute route building.
controller/hybridgateway/converter/tls_route.go Introduces TLSRoute converter (translation + status updates + orphan handling).
controller/hybridgateway/converter/http_route_converter_test.go Ensures HTTPRoute test objects include TypeMeta.
controller/hybridgateway/converter/http_route.go Switches to shared parent resolution helper.
controller/hybridgateway/converter/converter.go Adds TLSRoute as a supported root object and registers TLSRoute converter.
controller/hybridgateway/converter/common.go Adds shared parent/hostname resolution used by route converters.
controller/hybridgateway/controller.go Adds kubebuilder RBAC marker for tlsroutes.
controller/hybridgateway/const/finalizers/finalizers.go Adds TLSRoute finalizer selection.
controller/hybridgateway/builder/kongupstream.go Generalizes builder label/annotation/owner inputs to client.Object.
controller/hybridgateway/builder/kongtarget.go Generalizes builder label/annotation/owner inputs to client.Object.
controller/hybridgateway/builder/kongservice.go Generalizes builder label/annotation inputs to client.Object.
controller/hybridgateway/builder/kongroute.go Adds builder cloning + SNI/protocol support and generalizes labels/annotations/owner.
controller/gateway/controller_watch.go Adds TLSRoute watch predicate to enqueue affected Gateways.
controller/gateway/controller.go Adds controller watch on TLSRoute resources.
config/rbac/role/role.yaml Adds explicit tlsroutes permissions (separate rule) and updates role manifests.
charts/kong-operator/templates/cluster-role.yaml Mirrors RBAC updates in Helm chart ClusterRole.
charts/kong-operator/ci/snapshots/webhooks-validating-and-conversion-disabled-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/webhook-conversion-enabled-cert-manager.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/webhook-conversion-disabled-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/validating-policies-dataplane-ports-disabled.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/tolerations-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/probes-and-args-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/pod-annotations-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/nightly-can-be-used-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/image-pull-secrets-and-image-digest-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/extra-labels-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/env-and-customenv-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/env-and-args-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/disable-gateway-controller-values.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/controlplane-config-dump.snap Updates rendered chart snapshot for RBAC change.
charts/kong-operator/ci/snapshots/affinity-values.snap Updates rendered chart snapshot for RBAC change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +453 to +456
for _, parentRef := range gwtypes.GetSpecParentRefs(tlsRoute) {
if parentRef.Group != nil && string(*parentRef.Group) == gatewayv1.GroupName &&
parentRef.Kind != nil && string(*parentRef.Kind) == "Gateway" &&
string(parentRef.Name) == gateway.Name {
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

This watch mapping only matches ParentRefs when group and kind are explicitly set and ignores the ParentRef namespace (including the defaulting behavior when it’s nil/empty). In Gateway API, group/kind commonly omit defaults and namespace defaulting matters; as written, many valid TLSRoute->Gateway attachments won’t enqueue updates, and same-named Gateways in other namespaces could be enqueued incorrectly. Handle nil/empty group+kind as defaults and compare the effective namespace (ParentRef.Namespace if set, else TLSRoute namespace).

Copilot uses AI. Check for mistakes.
HybridHTTPRouteFinalizer = "gateway-operator.konghq.com/hybrid-httproute-cleanup"

// HybridTLSRouteFinalizer is the finalizer added to TLSRoute objects to manage cleanup of generated resources.
HybridTLSRouteFinalizer = "gateway.operator.konghq.com/hybrid-tlsroute-cleanup"
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

HybridTLSRouteFinalizer uses a different domain format (gateway.operator.konghq.com) than the other hybrid finalizers (gateway-operator.konghq.com). This inconsistency will prevent the controller from recognizing/removing the finalizer correctly and can leave TLSRoute cleanups stuck. Align the TLSRoute finalizer string with the established gateway-operator.konghq.com/... pattern.

Suggested change
HybridTLSRouteFinalizer = "gateway.operator.konghq.com/hybrid-tlsroute-cleanup"
HybridTLSRouteFinalizer = "gateway-operator.konghq.com/hybrid-tlsroute-cleanup"

Copilot uses AI. Check for mistakes.
Comment on lines +481 to +485
// BackendRef group kind.
bRefGK := string(*backendRef.Kind)
if gr := string(*backendRef.Group); gr != "" {
bRefGK = fmt.Sprintf("%s/%s", gr, bRefGK)
}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

backendRefResolvedCondition dereferences backendRef.Kind and backendRef.Group without nil checks (*backendRef.Kind, *backendRef.Group). Both fields are optional in Gateway API and can be nil, which will panic during status calculation. Use defaulting (core/Service) and only dereference when non-nil.

Copilot uses AI. Check for mistakes.
Comment on lines +361 to +376

// Check if any translation errors occurred
if len(translationErrors) > 0 {
log.Error(logger, nil, "TLSRoute translation completed with errors",
"totalResourcesCreated", len(c.outputStore),
"errorCount", len(translationErrors))

// Join all errors using errors.Join for better error handling
return fmt.Errorf("translation failed with %d errors: %w", len(translationErrors), errors.Join(translationErrors...))
}

log.Debug(logger, "Successfully completed TLSRoute translation",
"totalResourcesCreated", len(c.outputStore))

}

Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

The aggregated translationErrors check/return is inside the loop over supportedParentRefs. This makes translation fail-fast after the first parent ref with any error and prevents processing remaining parents, which differs from the HTTPRoute converter’s stated strategy of collecting all errors. Move the aggregation/return after the outer loop (or reset translationErrors per parent if that’s the intent).

Suggested change
// Check if any translation errors occurred
if len(translationErrors) > 0 {
log.Error(logger, nil, "TLSRoute translation completed with errors",
"totalResourcesCreated", len(c.outputStore),
"errorCount", len(translationErrors))
// Join all errors using errors.Join for better error handling
return fmt.Errorf("translation failed with %d errors: %w", len(translationErrors), errors.Join(translationErrors...))
}
log.Debug(logger, "Successfully completed TLSRoute translation",
"totalResourcesCreated", len(c.outputStore))
}
}
// Check if any translation errors occurred after processing all parent references
if len(translationErrors) > 0 {
log.Error(logger, nil, "TLSRoute translation completed with errors",
"totalResourcesCreated", len(c.outputStore),
"errorCount", len(translationErrors))
// Join all errors using errors.Join for better error handling
return fmt.Errorf("translation failed with %d errors: %w", len(translationErrors), errors.Join(translationErrors...))
}
log.Debug(logger, "Successfully completed TLSRoute translation",
"totalResourcesCreated", len(c.outputStore))

Copilot uses AI. Check for mistakes.
Comment on lines +73 to +107
case *gwtypes.TLSRoute:
return []Watcher{
{
MapRouteForGateway(cl, &gwtypes.TLSRoute{}),
&gwtypes.Gateway{},
},
{
MapRouteForGatewayClass(cl, &gwtypes.TLSRoute{}),
&gwtypes.GatewayClass{},
},
{
MapRouteForService(cl, &gwtypes.TLSRoute{}),
&corev1.Service{},
},
{
MapRouteForEndpointSlice(cl, &gwtypes.TLSRoute{}),
&discoveryv1.EndpointSlice{},
},
{
MapRouteForKongResource[*configurationv1alpha1.KongUpstream](cl),
&configurationv1alpha1.KongUpstream{},
},
{
MapRouteForKongResource[*configurationv1alpha1.KongTarget](cl),
&configurationv1alpha1.KongTarget{},
},
{
MapRouteForKongResource[*configurationv1alpha1.KongService](cl),
&configurationv1alpha1.KongService{},
},
{
MapRouteForKongResource[*configurationv1alpha1.KongRoute](cl),
&configurationv1alpha1.KongRoute{},
},
}
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

TLSRoute reconciliation depends on ReferenceGrants for cross-namespace BackendRefs (status and target translation both call CheckReferenceGrant), but TLSRoute watches currently don’t include ReferenceGrant changes. Without a ReferenceGrant watch, TLSRoutes won’t be requeued when grants are added/removed/updated, leading to stale status/translation. Add a ReferenceGrant watcher/mapping for TLSRoute similar to HTTPRoute.

Copilot uses AI. Check for mistakes.
Comment on lines +2497 to 2500
route := tt.route
route.TypeMeta = httpRouteTypeMeta
cond, err := BuildResolvedRefsCondition(ctx, logger, cl, tt.route)
require.NoError(t, err)
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

route.TypeMeta is set on the local route variable, but BuildResolvedRefsCondition is called with tt.route, so the TypeMeta update is ignored. This makes the test inconsistent and can hide regressions around Kind/GVK handling; pass route to BuildResolvedRefsCondition here.

Copilot uses AI. Check for mistakes.
Comment on lines +2531 to +2532
fmt.Println("===", route.GetObjectKind().GroupVersionKind().Group)
cond, err := BuildResolvedRefsCondition(ctx, logger, cl, route)
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

Leftover debug output in tests (fmt.Println) will spam test logs and can break log-sensitive tooling. Please remove this print and rely on assertions for diagnostics.

Copilot uses AI. Check for mistakes.
Comment on lines +90 to +101
for i, match := range httpRule.Matches {
matchRouteName := namegen.NewKongRouteNameForMatch(r, cp, match, i)
mLog := logger.WithValues("kongroute", matchRouteName, "matchIndex", i)
log.Debug(mLog, "Creating KongRoute for HTTPRoute match")

matchRouteBuilder := routeBuilder.Clone().
WithName(routeName).
WithHosts(hostnames).
WithStripPath(metadata.ExtractStripPath(r.Annotations)).
WithPreserveHost(metadata.ExtractPreserveHost(r.Annotations)).
WithHTTPRouteMatch(match, setCaptureGroup)

Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

In the HTTPRoute branch, matchRouteName is computed per match but the builder uses WithName(routeName) (and retains WithSpecName(routeName) from the base builder). This will generate multiple KongRoute objects with the same name/spec.name for different matches, causing collisions/overwrites and breaking per-match semantics. Use the per-match name for both metadata.name and spec.name when building each match route.

Copilot uses AI. Check for mistakes.
Comment on lines 16 to 21
"github.com/kong/kong-operator/v2/controller/hybridgateway/metadata"
"github.com/kong/kong-operator/v2/controller/hybridgateway/namegen"
"github.com/kong/kong-operator/v2/controller/hybridgateway/translator"
"github.com/kong/kong-operator/v2/controller/pkg/log"
"github.com/kong/kong-operator/v2/ingress-controller/test/gatewayapi"
gwtypes "github.com/kong/kong-operator/v2/internal/types"
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

This production code imports a package under ingress-controller/test/... just to access TLSModePassthrough. Test helper packages shouldn’t be dependencies of controller code. Prefer using the Gateway API type/constant directly (e.g., gatewayv1.TLSModePassthrough) or define a local constant.

Copilot uses AI. Check for mistakes.

// Translate implements APIConverter.
//
// Performs the complete translation of an HTTPRoute resource into Kong-specific resources.
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

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

The doc comment for Translate still refers to translating an HTTPRoute, but this converter is for TLSRoute. This is misleading for future maintainers and should be corrected.

Suggested change
// Performs the complete translation of an HTTPRoute resource into Kong-specific resources.
// Performs the complete translation of a TLSRoute resource into Kong-specific resources.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants