Skip to content

Conversation

@dspo
Copy link
Contributor

@dspo dspo commented May 21, 2025

This commit introduces support for Gateway API ReferenceGrant CRD, enabling cross-namespace references for HTTPRoutes. It refactors backend reference handling to validate Service references and check ReferenceGrants. Also includes minor code cleanups, added cluster role permissions for ReferenceGrants, and adjustments to e2e manifests.

Type of change:

  • Bugfix
  • New feature provided
  • Improve performance
  • Backport patches
  • Documentation
  • Refactor
  • Chore
  • CI/CD or Tests

What this PR does / why we need it:

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

dspo added 3 commits May 20, 2025 22:43
This commit introduces functionality to handle the ReferenceGrant resource in the Gateway API. It updates the Gateway controller logic, adds necessary permissions in RBAC manifests, and integrates condition handling for cross-namespace references. Additionally, skipped conformance tests related to ReferenceGrants are reinstated.
This commit introduces support for Gateway API ReferenceGrant CRD, enabling cross-namespace references for HTTPRoutes. It refactors backend reference handling to validate Service references and check ReferenceGrants. Also includes minor code cleanups, added cluster role permissions for ReferenceGrants, and adjustments to e2e manifests.
@github-actions
Copy link
Contributor

github-actions bot commented May 21, 2025

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2025-05-21T09:44:51Z"
gatewayAPIChannel: standard
gatewayAPIVersion: v1.2.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: partial
    skippedTests:
    - HTTPRouteHTTPSListener
    statistics:
      Failed: 0
      Passed: 32
      Skipped: 1
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 1 test skips.

@dspo dspo changed the title feat: Add ReferenceGrant support and refactor backend reference handling feat: Add ReferenceGrant support for HTTPRoute May 21, 2025
dspo added 7 commits May 21, 2025 10:06
…reference-grant-for-route

# Conflicts:
#	test/conformance/conformance_test.go
Reorganized and simplified the predicate logic for ReferenceGrant handling across gateway and HTTPRoute controllers. Consolidated duplicate code into reusable functions, reducing redundancy and improving maintainability. This centralization ensures consistent behavior and clearer code structure.
…nce-grant-for-route

# Conflicts:
#	internal/controller/gateway_controller.go
#	internal/controller/utils.go
#	test/conformance/conformance_test.go
@dspo dspo requested review from AlinsRan and ronething and removed request for ronething May 21, 2025 07:00
@dspo dspo requested a review from Copilot May 21, 2025 09:19
Copy link
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 support for Gateway API ReferenceGrant CRDs in HTTPRoute handling to enable cross-namespace backend references.

  • Refactors resolvedRefs condition logic and introduces ReasonError and shared ReferenceGrant utilities.
  • Enhances HTTPRoute controller to watch ReferenceGrant changes and validate cross-namespace Service refs.
  • Simplifies Gateway controller predicates for ReferenceGrant and cleans up event handling.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

File Description
test/conformance/conformance_test.go Removed skips for HTTPRoute cross-namespace tests, leaving a TODO.
internal/controller/utils.go Introduced ReasonError, predicate builders, and consolidated helpers.
internal/controller/httproute_controller.go Updated backend ref logic, added ReferenceGrant watching and checking.
internal/controller/gateway_controller.go Swapped custom predicates for generic referenceGrantPredicates.
Comments suppressed due to low confidence (5)

internal/controller/httproute_controller.go:641

  • [nitpick] The method name lisHTTPRoutesForReferenceGrant seems to have a typo; change it to listHTTPRoutesForReferenceGrant for consistency.
func (r *HTTPRouteReconciler) lisHTTPRoutesForReferenceGrant(ctx context.Context, obj client.Object) (requests []reconcile.Request) {

internal/controller/utils.go:956

  • The fmt.Sprintf call in newInvalidKindError requires importing the fmt package; please add "fmt" to the imports.
Message: fmt.Sprintf("Invalid kind %s, only Service is supported", kind),

internal/controller/utils.go:950

  • The slices.Contains call requires importing the slices package (Go 1.21+); please add "slices" to the imports or adjust accordingly.
return slices.Contains(reasons, Reason(re.Reason))

internal/controller/gateway_controller.go:179

  • The logged NamespacedName omits the Name field, so the gateway name will be empty; include Name: gateway.GetName() for complete context.
r.Log.Error(err, "failed to get listener status", "gateway", types.NamespacedName{Namespace: gateway.GetNamespace()})

internal/controller/httproute_controller.go:482

  • Double-check that v1beta1.RouteReasonRefNotPermitted is the correct constant for HTTPRoute resolvedRefs; if this constant belongs to the v1 package, switch to gatewayv1.RouteReasonRefNotPermitted.
Reason:  string(v1beta1.RouteReasonRefNotPermitted),

@dspo dspo closed this May 21, 2025
@dspo dspo reopened this May 21, 2025
@dspo
Copy link
Contributor Author

dspo commented May 21, 2025

This is closed, please view #149

@dspo dspo closed this May 21, 2025
@ronething ronething deleted the feat/reference-grant-for-route branch May 21, 2025 10:19
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.

2 participants