Skip to content

feat: add L4RoutePolicy CRD for attaching stream plugins to Gateway API L4 routes#416

Open
AlinsRan wants to merge 4 commits into
masterfrom
feat/l4routepolicy
Open

feat: add L4RoutePolicy CRD for attaching stream plugins to Gateway API L4 routes#416
AlinsRan wants to merge 4 commits into
masterfrom
feat/l4routepolicy

Conversation

@AlinsRan
Copy link
Copy Markdown
Contributor

@AlinsRan AlinsRan commented May 14, 2026

Summary

Closes #403

Add a new L4RoutePolicy custom resource that enables attaching APISIX stream plugins to Gateway API L4 routes (TCPRoute, UDPRoute, TLSRoute). This follows the Gateway API Policy Attachment pattern (GEP-713), consistent with the existing BackendTrafficPolicy and HTTPRoutePolicy CRDs.

Motivation

AIPSIX supports attaching stream plugins (limit-conn, ip-restriction, mqtt-proxy, syslog, traffic-split, etc.) to stream_routes. However, the controller had no mechanism to express this through Gateway API resources.

Changes

New CRD: L4RoutePolicy

apiVersion: apisix.apache.org/v1alpha1
kind: L4RoutePolicy
metadata:
  name: tcp-rate-limit
  namespace: default
spec:
  targetRefs:
    - group: gateway.networking.k8s.io
      kind: TCPRoute   # also UDPRoute / TLSRoute
      name: my-tcp-route
  plugins:
    - name: limit-conn
      config:
        conn: 100
        burst: 50
        default_conn_delay: 0.1
        key_type: "var"
        key: "remote_addr"
    - name: ip-restriction
      config:
        whitelist:
          - "192.168.1.0/24"

Implementation details

  • Plugin attachment level: plugins are attached to service.Plugins (one service per L4 rule), giving per-rule granularity and avoiding duplication in TLS multi-SNI scenarios
  • Conflict resolution: when multiple policies target the same route, the oldest (creationTimestamp + namespace/name tie-break) wins; losers receive Accepted=False, Reason=Conflicted
  • Lifecycle: L4 route controllers watch L4RoutePolicy changes and trigger reconciliation; ancestor status entries are cleaned up on route deletion
  • LocalPolicyTargetReferenceWithSectionName is used in the spec to preserve the option for future per-rule targeting via sectionName

Files changed

File Change
api/v1alpha1/l4routepolicy_types.go New CRD type
api/v1alpha1/zz_generated.deepcopy.go DeepCopy methods
internal/provider/provider.go Add L4RoutePolicies to TranslateContext
internal/controller/indexer/indexer.go Register L4RoutePolicy indexer
internal/controller/policies.go ProcessL4RoutePolicy, updateL4RoutePolicyStatusOnDeleting
internal/adc/translator/policies.go AttachL4RoutePolicyPlugins helper
internal/adc/translator/{tcp,udp,tls}route.go Call plugin attachment
internal/controller/{tcp,udp,tls}route_controller.go Watch + reconcile + deletion cleanup
internal/manager/controllers.go RBAC markers
internal/adc/translator/l4routepolicy_test.go Unit tests

Summary by CodeRabbit

  • New Features

    • Added L4RoutePolicy CRD to attach plugins to TCPRoute, UDPRoute, and TLSRoute; controller translation now applies matching policy plugins.
    • Deterministic conflict resolution with accepted vs. conflicted ancestor status and index-based policy-to-route mapping.
  • Chores

    • RBAC updated to allow managing L4RoutePolicy resources.
  • Tests

    • New unit and e2e tests covering policy matching, plugin attachment, status updates, and connectivity.
  • Documentation

    • API docs added for L4RoutePolicy and its spec.

Review Change Stack

…PI L4 routes

Add a new L4RoutePolicy custom resource that enables attaching APISIX stream
plugins to Gateway API L4 routes (TCPRoute, UDPRoute, TLSRoute). This follows
the Gateway API Policy Attachment pattern (GEP-713) consistent with the existing
BackendTrafficPolicy and HTTPRoutePolicy CRDs.

Changes:
- Add L4RoutePolicy CRD type (api/v1alpha1/l4routepolicy_types.go)
  - targetRefs support TCPRoute, UDPRoute, TLSRoute (validated via CEL rule)
  - plugins list reuses the existing Plugin type (name + config)
  - LocalPolicyTargetReferenceWithSectionName for future per-rule targeting
- Add DeepCopy methods in zz_generated.deepcopy.go
- Add L4RoutePolicies to TranslateContext in provider.go
- Register L4RoutePolicy indexer (by group+kind+namespace+name) in indexer.go
- Add ProcessL4RoutePolicy in policies.go with deterministic conflict resolution
  (oldest creationTimestamp wins; tie-break by namespace/name; losers get
  Accepted=False, Reason=Conflicted)
- Add updateL4RoutePolicyStatusOnDeleting for route deletion cleanup
- Add AttachL4RoutePolicyPlugins translator helper; plugins are attached at the
  service level (one service per L4 rule) to avoid duplication in TLS multi-SNI
- Wire up TCPRoute, UDPRoute, TLSRoute controllers:
  - Watch L4RoutePolicy and enqueue affected routes
  - Call ProcessL4RoutePolicy during reconcile
  - Clear policy ancestor status on route deletion
- Add RBAC markers for l4routepolicies resources
- Add unit tests for AttachL4RoutePolicyPlugins

Fixes: #403

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 14, 2026 04:48
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 48433a5e-5be4-47d2-825c-7e90202013e6

📥 Commits

Reviewing files that changed from the base of the PR and between cbea9a4 and 69bacc9.

📒 Files selected for processing (1)
  • docs/en/latest/reference/api-reference.md
✅ Files skipped from review due to trivial changes (1)
  • docs/en/latest/reference/api-reference.md

📝 Walkthrough

Walkthrough

Adds a new L4RoutePolicy API and full controller/translator/runtime support: CRD, deepcopy, RBAC, indexer, controller watches and processing, translator plugin attachment, provider/translate-context caching, unit and e2e tests, scaffold helpers, and documentation updates.

Changes

L4RoutePolicy feature (CRD, controller, translator, tests, docs)

Layer / File(s) Summary
API types & CRD
api/v1alpha1/l4routepolicy_types.go, config/crd/bases/apisix.apache.org_l4routepolicies.yaml
Adds L4RoutePolicy v1alpha1 API types (Spec, Status, List), kubebuilder markers/validation, and the CRD YAML describing spec.targetRefs, spec.plugins, and status.ancestors.
Deepcopy generation
api/v1alpha1/zz_generated.deepcopy.go, api/v2/zz_generated.deepcopy.go
Adds autogenerated deepcopy methods for new API types; updates license header in v2 deepcopy file.
RBAC
config/rbac/role.yaml, internal/manager/controllers.go
Grants controller get,list,watch for l4routepolicies and get,update for l4routepolicies/status via manifests and kubebuilder markers.
Indexer
internal/controller/indexer/indexer.go
Registers a field index for L4RoutePolicy on PolicyTargetRefs and exposes L4RoutePolicyIndexFunc to generate deduplicated index keys from Spec.TargetRefs.
Controller: enqueueing, processing, deletion handling
internal/controller/tcproute_controller.go, internal/controller/tlsroute_controller.go, internal/controller/udproute_controller.go, internal/controller/policies.go
Wires watches for L4RoutePolicy to enqueue referenced L4 routes, adds ProcessL4RoutePolicy to list and deterministically select the winning policy (set accepted/conflicted ancestor conditions), queue status updaters, and deletion-path helpers to clear ancestors.
Provider / translate context
internal/provider/provider.go
Adds TranslateContext.L4RoutePolicies map and initializes it in NewDefaultTranslateContext for translator access.
Translator: attach/merge L4 plugins
internal/adc/translator/policies.go, internal/adc/translator/tcproute.go, internal/adc/translator/udproute.go, internal/adc/translator/tlsroute.go
Adds AttachL4RoutePolicyPlugins and mergeL4PolicyPlugins to parse plugin JSON configs and merge them into adctypes.Plugins. Translator paths for TCP/UDP/TLS routes now initialize service.Plugins and call this attachment logic.
Unit tests (translator)
internal/adc/translator/l4routepolicy_test.go, internal/adc/translator/l4route_test.go, internal/adc/translator/apisixconsumer_test.go
Adds unit tests validating plugin attachment logic across matching/mismatched kinds/namespaces and route translations; minor test fix for ApisixConsumer auth parameter pointer.
E2E support & harness
test/e2e/framework/assertion.go, test/e2e/scaffold/k8s.go, test/e2e/gatewayapi/tcproute.go
Adds assertion helpers to poll L4RoutePolicy status ancestors, scaffold helper to apply policies and wait for acceptance, and a TCPRoute E2E that applies an ip-restriction L4RoutePolicy to block and then restore TCP connectivity.
Docs
docs/en/latest/reference/api-reference.md
Documents the new L4RoutePolicy and L4RoutePolicySpec in the v1alpha1 API reference.

Sequence Diagram

sequenceDiagram
    participant User as rgba(10,80,160,0.5)
    participant K8sAPI as rgba(0,120,60,0.5)
    participant Controller as rgba(160,80,10,0.5)
    participant Translator as rgba(120,10,140,0.5)
    participant APISIX as rgba(200,40,40,0.5)

    User->>K8sAPI: create L4RoutePolicy + TCPRoute/UDPRoute/TLSRoute
    K8sAPI->>Controller: inform via watch / indexer keys
    Controller->>Controller: ProcessL4RoutePolicy (select winner, set ancestor status)
    Controller->>Translator: include winning L4RoutePolicy in TranslateContext
    Translator->>Translator: AttachL4RoutePolicyPlugins(parse plugin config → map)
    Translator->>APISIX: render service/stream_route with plugins
    User->>K8sAPI: delete L4RoutePolicy
    K8sAPI->>Controller: deletion event
    Controller->>Controller: updateL4RoutePolicyStatusOnDeleting (clear ancestors) 
    Controller->>APISIX: retranslate/update APISIX to remove plugins
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • nic-6443
  • shreemaan-abhishek
🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review ⚠️ Warning E2E test coverage is incomplete. Only TCPRoute has L4RoutePolicy E2E tests; UDPRoute and TLSRoute lack E2E scenarios. ProcessL4RoutePolicy (conflict resolution logic) lacks unit tests. Add E2E tests for UDPRoute/TLSRoute with L4RoutePolicy. Add unit tests for ProcessL4RoutePolicy: multiple conflicting policies, tie-breaking, status conditions. Test all route kinds per PR scope.
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately and specifically describes the main change: adding an L4RoutePolicy CRD for attaching stream plugins to Gateway API L4 routes.
Linked Issues check ✅ Passed All coding requirements from issue #403 are met: L4RoutePolicy CRD added [#403], conflict resolution implemented [#403], plugin attachment to TCPRoute/UDPRoute/TLSRoute via translators [#403], controller integration with reconciliation and status handling [#403], and Gateway API Policy Attachment pattern alignment [#403].
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing L4RoutePolicy CRD and its integration. Minor file updates (license header in zz_generated.deepcopy.go, test data structure adjustment) are necessary supporting changes.
Security Check ✅ Passed No security vulnerabilities found. Sensitive configs not logged. Namespace validation prevents cross-namespace attacks. Proper RBAC. Deterministic conflict resolution with status tracking.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/l4routepolicy

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

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 a new L4RoutePolicy CRD and controller/translator plumbing to attach APISIX stream (L4) plugins to Gateway API L4 routes (TCPRoute, UDPRoute, TLSRoute) using the Policy Attachment pattern.

Changes:

  • Introduces api/v1alpha1.L4RoutePolicy type (plus deepcopy) and extends TranslateContext to carry selected policies.
  • Indexes L4RoutePolicy.spec.targetRefs, watches L4RoutePolicy from L4 route controllers, resolves conflicts deterministically, and updates policy status.
  • Attaches matched policy plugins onto translated L4 “service” objects; includes unit tests for the attachment helper.

Reviewed changes

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

Show a summary per file
File Description
api/v1alpha1/l4routepolicy_types.go New CRD Go types + kubebuilder validations/markers for L4RoutePolicy.
api/v1alpha1/zz_generated.deepcopy.go Generated DeepCopy methods for the new CRD types.
internal/provider/provider.go Adds L4RoutePolicies storage to translation context.
internal/manager/controllers.go Adds RBAC markers for l4routepolicies and /status.
internal/controller/indexer/indexer.go Registers field indexer for L4RoutePolicy.spec.targetRefs.
internal/controller/policies.go Implements ProcessL4RoutePolicy and deletion-time status cleanup.
internal/controller/tcproute_controller.go Watches L4RoutePolicy, triggers reconcile, and calls ProcessL4RoutePolicy.
internal/controller/udproute_controller.go Same as above for UDPRoute.
internal/controller/tlsroute_controller.go Same as above for TLSRoute.
internal/adc/translator/policies.go Adds AttachL4RoutePolicyPlugins helper and JSON config unmarshalling.
internal/adc/translator/tcproute.go Calls plugin attachment during TCPRoute translation.
internal/adc/translator/udproute.go Calls plugin attachment during UDPRoute translation.
internal/adc/translator/tlsroute.go Calls plugin attachment during TLSRoute translation.
internal/adc/translator/l4routepolicy_test.go Unit tests for L4RoutePolicy plugin attachment behavior.
Files not reviewed (1)
  • api/v1alpha1/zz_generated.deepcopy.go: Language not supported

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

Comment on lines +261 to +298
winner := list.Items[0].DeepCopy()
tctx.L4RoutePolicies[types.NamespacedName{Namespace: winner.Namespace, Name: winner.Name}] = winner

for i := range list.Items {
policy := list.Items[i]
var condition metav1.Condition
if i == 0 {
condition = metav1.Condition{
Type: string(gatewayv1alpha2.PolicyConditionAccepted),
Status: metav1.ConditionTrue,
ObservedGeneration: policy.GetGeneration(),
LastTransitionTime: metav1.Now(),
Reason: string(gatewayv1alpha2.PolicyReasonAccepted),
Message: "Policy has been accepted",
}
} else {
condition = metav1.Condition{
Type: string(gatewayv1alpha2.PolicyConditionAccepted),
Status: metav1.ConditionFalse,
ObservedGeneration: policy.GetGeneration(),
LastTransitionTime: metav1.Now(),
Reason: string(gatewayv1alpha2.PolicyReasonConflicted),
Message: fmt.Sprintf("Conflicts with L4RoutePolicy %s/%s which was created earlier", winner.Namespace, winner.Name),
}
}

if updated := SetAncestors(&policy.Status, tctx.RouteParentRefs, condition); updated {
policyCopy := policy.DeepCopy()
tctx.StatusUpdaters = append(tctx.StatusUpdaters, status.Update{
NamespacedName: utils.NamespacedName(policyCopy),
Resource: policyCopy,
Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
cp := obj.(*v1alpha1.L4RoutePolicy).DeepCopy()
cp.Status = policyCopy.Status
return cp
}),
})
}
Comment on lines +302 to +329
// updateL4RoutePolicyStatusOnDeleting clears ancestor status entries for L4RoutePolicy
// resources that target the deleted route.
func updateL4RoutePolicyStatusOnDeleting(ctx context.Context, c client.Client, updater status.Updater, log logr.Logger, nn types.NamespacedName, routeKind string) {
var list v1alpha1.L4RoutePolicyList
key := indexer.GenIndexKeyWithGK(gatewayv1alpha2.GroupName, routeKind, nn.Namespace, nn.Name)
if err := c.List(ctx, &list, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil {
log.Error(err, "failed to list L4RoutePolicy on route deletion", "namespace", nn.Namespace, "name", nn.Name)
return
}
for _, policy := range list.Items {
updateL4RoutePolicyDeleteAncestors(updater, policy)
}
}

func updateL4RoutePolicyDeleteAncestors(updater status.Updater, policy v1alpha1.L4RoutePolicy) {
if len(policy.Status.Ancestors) == 0 {
return
}
policy.Status.Ancestors = nil
updater.Update(status.Update{
NamespacedName: utils.NamespacedName(&policy),
Resource: policy.DeepCopy(),
Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
cp := obj.(*v1alpha1.L4RoutePolicy).DeepCopy()
cp.Status = policy.Status
return cp
}),
})
//
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:validation:XValidation:rule="self.all(r, r.kind == 'TCPRoute' || r.kind == 'UDPRoute' || r.kind == 'TLSRoute')",message="targetRefs kind must be TCPRoute, UDPRoute, or TLSRoute"
Comment on lines +520 to +527
for _, ref := range policy.Spec.TargetRefs {
if string(ref.Kind) != KindTCPRoute {
continue
}
nn := k8stypes.NamespacedName{
Namespace: policy.Namespace,
Name: string(ref.Name),
}
Comment on lines +520 to +527
for _, ref := range policy.Spec.TargetRefs {
if string(ref.Kind) != KindUDPRoute {
continue
}
nn := k8stypes.NamespacedName{
Namespace: policy.Namespace,
Name: string(ref.Name),
}
Comment on lines +520 to +527
for _, ref := range policy.Spec.TargetRefs {
if string(ref.Kind) != types.KindTLSRoute {
continue
}
nn := k8stypes.NamespacedName{
Namespace: policy.Namespace,
Name: string(ref.Name),
}
Comment on lines +42 to +68
// +kubebuilder:object:root=true
// +kubebuilder:subresource:status

// L4RoutePolicy defines plugin configuration for Gateway API L4 routes (TCPRoute, UDPRoute, TLSRoute).
// It follows the Gateway API Policy Attachment pattern and attaches APISIX stream plugins
// to the targeted L4 route resources.
type L4RoutePolicy struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

// Spec defines the desired state of L4RoutePolicy.
Spec L4RoutePolicySpec `json:"spec,omitempty"`
Status PolicyStatus `json:"status,omitempty"`
}

// +kubebuilder:object:root=true

// L4RoutePolicyList contains a list of L4RoutePolicy.
type L4RoutePolicyList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []L4RoutePolicy `json:"items"`
}

func init() {
SchemeBuilder.Register(&L4RoutePolicy{}, &L4RoutePolicyList{})
}
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
internal/controller/policies.go (1)

233-300: ⚡ Quick win

Use tctx instead of context.Background() for consistency.

Line 241 uses context.Background() while other similar code in this file (e.g., line 111) passes tctx directly as the context. Since TranslateContext embeds context.Context, use tctx for consistency with the rest of the codebase.

♻️ Proposed fix
-	if err := c.List(context.Background(), &list, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil {
+	if err := c.List(tctx, &list, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/controller/policies.go` around lines 233 - 300, In
ProcessL4RoutePolicy replace the call that uses context.Background() when
listing policies with the TranslateContext value (tctx) so the c.List call uses
tctx as the context; update the c.List invocation (the call that passes
client.MatchingFields{indexer.PolicyTargetRefs: key}) to pass tctx
(TranslateContext) instead of context.Background() to be consistent with other
handlers that use tctx as the request context.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/controller/indexer/indexer.go`:
- Around line 522-532: The file fails gofmt/formatting check; run gofmt (or
goimports if preferred) on the file and reformat the function
setupL4RoutePolicyIndexer and the other affected blocks (around the 900-914
region) so that spacing, imports, and indentation conform to gofmt rules; ensure
the signatures and calls for mgr.GetFieldIndexer().IndexField,
v1alpha1.L4RoutePolicy{}, PolicyTargetRefs, and L4RoutePolicyIndexFunc remain
unchanged except for whitespace/formatting, then stage and commit the formatted
file to unblock CI.

---

Nitpick comments:
In `@internal/controller/policies.go`:
- Around line 233-300: In ProcessL4RoutePolicy replace the call that uses
context.Background() when listing policies with the TranslateContext value
(tctx) so the c.List call uses tctx as the context; update the c.List invocation
(the call that passes client.MatchingFields{indexer.PolicyTargetRefs: key}) to
pass tctx (TranslateContext) instead of context.Background() to be consistent
with other handlers that use tctx as the request context.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 48e8aad9-1b5e-4ec8-9a48-c6e3e7b6c9d2

📥 Commits

Reviewing files that changed from the base of the PR and between 382cc0f and 242ce9c.

📒 Files selected for processing (14)
  • api/v1alpha1/l4routepolicy_types.go
  • api/v1alpha1/zz_generated.deepcopy.go
  • internal/adc/translator/l4routepolicy_test.go
  • internal/adc/translator/policies.go
  • internal/adc/translator/tcproute.go
  • internal/adc/translator/tlsroute.go
  • internal/adc/translator/udproute.go
  • internal/controller/indexer/indexer.go
  • internal/controller/policies.go
  • internal/controller/tcproute_controller.go
  • internal/controller/tlsroute_controller.go
  • internal/controller/udproute_controller.go
  • internal/manager/controllers.go
  • internal/provider/provider.go

Comment thread internal/controller/indexer/indexer.go
- Generate config/crd/bases/apisix.apache.org_l4routepolicies.yaml via make manifests
- Regenerate config/rbac/role.yaml with l4routepolicies RBAC entries
- Regenerate api/v2/zz_generated.deepcopy.go via make generate
- Fix pre-existing go vet error in apisixconsumer_test.go: AuthParameter is a pointer field
- Fix import ordering in indexer.go (goimports-reviser)
- Fix prealloc lint warnings in tcproute/udproute/tlsroute controllers
- Add translator-level tests: TestTranslateTCPRouteWithL4RoutePolicy,
  TestTranslateUDPRouteWithL4RoutePolicy, TestTranslateTLSRouteWithL4RoutePolicy

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
internal/adc/translator/l4route_test.go (1)

259-260: 💤 Low value

Assertion does not verify the stated intent.

The comment says "plugins should be on service level, not duplicated" but the assertion only checks that there's exactly one service. To verify plugins aren't duplicated across stream routes, consider asserting that individual stream routes don't have plugins attached (if that's how the structure works), or remove the misleading comment.

Suggested clarification
-				// Plugins are on the service, not duplicated per stream route
-				assert.Len(t, result.Services, 1, "plugins should be on service level, not duplicated")
+				// Verify single service with plugins (plugins attached at service level)
+				assert.Len(t, result.Services, 1, "expected exactly one service")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/adc/translator/l4route_test.go` around lines 259 - 260, The test
comment says "plugins should be on service level, not duplicated" but the
assertion assert.Len(t, result.Services, 1) only verifies the number of
services; update the test in l4route_test.go to actually assert plugin
placement: either remove or revise the misleading comment, and add checks such
as asserting that each stream route (e.g., elements in
result.Services[0].StreamRoutes or the route objects returned by the translator)
has no Plugins (assert.Empty(t, route.Plugins) for each route) or assert that
only result.Services[0].Plugins contains the plugins and that none of the
individual routes contain duplicates; keep the existing assert.Len if still
relevant but add the plugin-level assertions to verify the intended behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/adc/translator/l4route_test.go`:
- Around line 259-260: The test comment says "plugins should be on service
level, not duplicated" but the assertion assert.Len(t, result.Services, 1) only
verifies the number of services; update the test in l4route_test.go to actually
assert plugin placement: either remove or revise the misleading comment, and add
checks such as asserting that each stream route (e.g., elements in
result.Services[0].StreamRoutes or the route objects returned by the translator)
has no Plugins (assert.Empty(t, route.Plugins) for each route) or assert that
only result.Services[0].Plugins contains the plugins and that none of the
individual routes contain duplicates; keep the existing assert.Len if still
relevant but add the plugin-level assertions to verify the intended behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b1a4a092-ab61-46e7-b78a-ed7d9431361d

📥 Commits

Reviewing files that changed from the base of the PR and between 242ce9c and 261bfc9.

📒 Files selected for processing (9)
  • api/v2/zz_generated.deepcopy.go
  • config/crd/bases/apisix.apache.org_l4routepolicies.yaml
  • config/rbac/role.yaml
  • internal/adc/translator/apisixconsumer_test.go
  • internal/adc/translator/l4route_test.go
  • internal/controller/indexer/indexer.go
  • internal/controller/tcproute_controller.go
  • internal/controller/tlsroute_controller.go
  • internal/controller/udproute_controller.go
✅ Files skipped from review due to trivial changes (2)
  • api/v2/zz_generated.deepcopy.go
  • config/rbac/role.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
  • internal/controller/tlsroute_controller.go
  • internal/controller/indexer/indexer.go
  • internal/controller/tcproute_controller.go
  • internal/controller/udproute_controller.go

- Add L4RoutePolicyMustHaveCondition and PollUntilL4RoutePolicyHaveStatus
  framework helpers (mirroring HTTPRoutePolicy pattern)
- Add ApplyL4RoutePolicy scaffold helper
- Add e2e test 'L4RoutePolicy blocks traffic via ip-restriction plugin':
  1. Create TCPRoute, verify traffic works
  2. Apply L4RoutePolicy with ip-restriction blacklist 0.0.0.0/0
  3. Verify traffic is blocked
  4. Delete L4RoutePolicy, verify traffic recovers

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 15, 2026 00:48
Copy link
Copy Markdown
Contributor

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

Copilot reviewed 20 out of 22 changed files in this pull request and generated 3 comments.

Files not reviewed (2)
  • api/v1alpha1/zz_generated.deepcopy.go: Language not supported
  • api/v2/zz_generated.deepcopy.go: Language not supported

Comment on lines +316 to +330
func updateL4RoutePolicyDeleteAncestors(updater status.Updater, policy v1alpha1.L4RoutePolicy) {
if len(policy.Status.Ancestors) == 0 {
return
}
policy.Status.Ancestors = nil
updater.Update(status.Update{
NamespacedName: utils.NamespacedName(&policy),
Resource: policy.DeepCopy(),
Mutator: status.MutatorFunc(func(obj client.Object) client.Object {
cp := obj.(*v1alpha1.L4RoutePolicy).DeepCopy()
cp.Status = policy.Status
return cp
}),
})
}
) {
var list v1alpha1.L4RoutePolicyList
key := indexer.GenIndexKeyWithGK(gatewayv1alpha2.GroupName, routeKind, routeNamespace, routeName)
if err := c.List(context.Background(), &list, client.MatchingFields{indexer.PolicyTargetRefs: key}); err != nil {
//
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:validation:XValidation:rule="self.all(r, r.kind == 'TCPRoute' || r.kind == 'UDPRoute' || r.kind == 'TLSRoute')",message="targetRefs kind must be TCPRoute, UDPRoute, or TLSRoute"
Run 'make generate-crd-docs' to add L4RoutePolicy and L4RoutePolicySpec
entries to the auto-generated CRD reference documentation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

conformance test report - apisix mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-05-15T01:13:23Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
  contact: null
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    result: success
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests succeeded.
- core:
    failedTests:
    - HTTPRouteInvalidBackendRefUnknownKind
    result: failure
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 1
      Passed: 31
      Skipped: 1
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 11
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - GatewayHTTPListenerIsolation
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
  name: GATEWAY-HTTP
  summary: Core tests failed with 1 test failures. Extended tests partially succeeded
    with 1 test skips.
- core:
    failedTests:
    - TLSRouteInvalidReferenceGrant
    result: failure
    skippedTests:
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 1
      Passed: 9
      Skipped: 1
  name: GATEWAY-TLS
  summary: Core tests failed with 1 test failures.

@github-actions
Copy link
Copy Markdown
Contributor

conformance test report - apisix-standalone mode

apiVersion: gateway.networking.k8s.io/v1
date: "2026-05-15T01:14:00Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
  contact: null
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    result: success
    statistics:
      Failed: 0
      Passed: 12
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests succeeded.
- core:
    failedTests:
    - HTTPRouteInvalidBackendRefUnknownKind
    result: failure
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 1
      Passed: 31
      Skipped: 1
  extended:
    result: partial
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 0
      Passed: 11
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - GatewayHTTPListenerIsolation
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
  name: GATEWAY-HTTP
  summary: Core tests failed with 1 test failures. Extended tests partially succeeded
    with 1 test skips.
- core:
    failedTests:
    - TLSRouteInvalidReferenceGrant
    result: failure
    skippedTests:
    - TLSRouteSimpleSameNamespace
    statistics:
      Failed: 1
      Passed: 9
      Skipped: 1
  name: GATEWAY-TLS
  summary: Core tests failed with 1 test failures.

@github-actions
Copy link
Copy Markdown
Contributor

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2026-05-15T01:25:38Z"
gatewayAPIChannel: experimental
gatewayAPIVersion: v1.3.0
implementation:
  contact: null
  organization: APISIX
  project: apisix-ingress-controller
  url: https://github.com/apache/apisix-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    failedTests:
    - GRPCExactMethodMatching
    - GRPCRouteHeaderMatching
    - GRPCRouteListenerHostnameMatching
    result: failure
    statistics:
      Failed: 3
      Passed: 9
      Skipped: 0
  name: GATEWAY-GRPC
  summary: Core tests failed with 3 test failures.
- core:
    result: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 0
      Passed: 32
      Skipped: 1
  extended:
    failedTests:
    - HTTPRouteBackendProtocolWebSocket
    result: failure
    skippedTests:
    - HTTPRouteRedirectPortAndScheme
    statistics:
      Failed: 1
      Passed: 10
      Skipped: 1
    supportedFeatures:
    - GatewayAddressEmpty
    - GatewayPort8080
    - HTTPRouteBackendProtocolWebSocket
    - HTTPRouteDestinationPortMatching
    - HTTPRouteHostRewrite
    - HTTPRouteMethodMatching
    - HTTPRoutePathRewrite
    - HTTPRoutePortRedirect
    - HTTPRouteQueryParamMatching
    - HTTPRouteRequestMirror
    - HTTPRouteResponseHeaderModification
    - HTTPRouteSchemeRedirect
    unsupportedFeatures:
    - GatewayHTTPListenerIsolation
    - GatewayInfrastructurePropagation
    - GatewayStaticAddresses
    - HTTPRouteBackendProtocolH2C
    - HTTPRouteBackendRequestHeaderModification
    - HTTPRouteBackendTimeout
    - HTTPRouteParentRefPort
    - HTTPRoutePathRedirect
    - HTTPRouteRequestMultipleMirrors
    - HTTPRouteRequestPercentageMirror
    - HTTPRouteRequestTimeout
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 1 test skips. Extended tests failed
    with 1 test failures.
- core:
    failedTests:
    - TLSRouteInvalidReferenceGrant
    - TLSRouteSimpleSameNamespace
    result: failure
    statistics:
      Failed: 2
      Passed: 9
      Skipped: 0
  name: GATEWAY-TLS
  summary: Core tests failed with 2 test failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

enhancement: support attaching L4 plugins with Gateway API routes

2 participants