Skip to content

Conversation

@dspo
Copy link
Contributor

@dspo dspo commented May 20, 2025

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.

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

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.
@github-actions
Copy link
Contributor

github-actions bot commented May 20, 2025

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2025-05-20T19:18:59Z"
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
    - HTTPRouteInvalidCrossNamespaceBackendRef
    - HTTPRouteInvalidReferenceGrant
    - HTTPRoutePartiallyInvalidViaInvalidReferenceGrant
    - HTTPRouteReferenceGrant
    statistics:
      Failed: 0
      Passed: 28
      Skipped: 5
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 5 test skips.

@dspo dspo requested a review from ronething May 21, 2025 01:01
@ronething ronething requested a review from Copilot May 21, 2025 02:42
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 Gateway API ReferenceGrant support to the controller, ensuring RBAC, controller logic, and tests reflect cross-namespace reference permissions.

  • Update RBAC manifests to allow referencegrants and referencegrants/status
  • Reinstate conformance tests that were skipped for ReferenceGrant
  • Enhance controller startup, watcher, status calculation, and permission checks for ReferenceGrant

Reviewed Changes

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

Show a summary per file
File Description
test/e2e/framework/manifests/ingress.yaml Add referencegrants and referencegrants/status verbs
test/conformance/conformance_test.go Remove skips for ReferenceGrant tests
internal/manager/run.go Install v1beta1 scheme and verify ReferenceGrant CRD
internal/controller/utils.go Extend getListenerStatus and add checkReferenceGrantBetweenGatewayAndSecret
internal/controller/gateway_controller.go Watch ReferenceGrant events and include grants in reconcile
charts/templates/cluster_role.yaml Add referencegrants and referencegrants/status verbs
Comments suppressed due to low confidence (5)

internal/controller/utils.go:1098

  • The new helper checkReferenceGrantBetweenGatewayAndSecret lacks unit tests; add tests to validate both permitted and denied cross-namespace scenarios.
func checkReferenceGrantBetweenGatewayAndSecret(gwNamespace string, certRef gatewayv1.SecretObjectReference, grants []v1beta1.ReferenceGrant) bool {

internal/controller/gateway_controller.go:380

  • The code calls errors.New but the standard "errors" package is not imported; add import "errors" to avoid a compile error.
errors.New("unexpected object type")

internal/controller/gateway_controller.go:36

  • [nitpick] The import of github.com/api7/gopkg/pkg/log appears unused in this file; consider removing it to keep imports tidy.
"github.com/api7/gopkg/pkg/log"

test/e2e/framework/manifests/ingress.yaml:232

  • The new RBAC rules appear mis-indented under rules:; verify that - apiGroups: lines align correctly to ensure valid YAML.
- apiGroups:

charts/templates/cluster_role.yaml:173

  • Ensure the inserted - apiGroups: for referencegrants is indented to match existing rule entries, otherwise the rendered ClusterRole may be invalid.
- apiGroups:

Copy link
Contributor

@ronething ronething left a comment

Choose a reason for hiding this comment

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

lgtm

@dspo dspo merged commit ecebace into release-v2-dev May 21, 2025
9 checks passed
@dspo dspo deleted the feat/reference-grant branch June 23, 2025 07:28
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.

3 participants