Skip to content

Conversation

@dspo
Copy link
Contributor

@dspo dspo commented Apr 15, 2025

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 4 commits April 15, 2025 15:13
This commit updates the `HTTPRoutePolicy` CRD with more detailed validation for Kubernetes object references, adds status and ancestor information to the policy, and updates the corresponding controller and test files. Additionally, it refactors the `StringOrSlice` type and adjusts the indexing and translation logic.
This commit refactors the `common` package by moving the `Vars` and `StringOrSlice` types to the `adc` package. It also updates the `TargetReference` type to use `LocalPolicyTargetReferenceWithSectionName` from `gateway-api` and simplifies the CRD documentation. Additionally, it adjusts the import paths and type references in the `httproute.go` file to reflect these changes.
@github-actions
Copy link
Contributor

github-actions bot commented Apr 16, 2025

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2025-04-17T10:02:00Z"
gatewayAPIChannel: standard
gatewayAPIVersion: v1.2.0
implementation:
  contact: null
  organization: API7
  project: api7-ingress-controller
  url: https://github.com/api7/api7-ingress-controller.git
  version: v2.0.0
kind: ConformanceReport
mode: default
profiles:
- core:
    failedTests:
    - HTTPRoutePathMatchOrder
    - HTTPRouteSimpleSameNamespace
    result: failure
    skippedTests:
    - GatewayInvalidTLSConfiguration
    - GatewaySecretInvalidReferenceGrant
    - GatewaySecretMissingReferenceGrant
    - GatewaySecretReferenceGrantAllInNamespace
    - GatewaySecretReferenceGrantSpecific
    - HTTPRouteExactPathMatching
    - HTTPRouteHTTPSListener
    - HTTPRouteHeaderMatching
    - HTTPRouteHostnameIntersection
    - HTTPRouteInvalidBackendRefUnknownKind
    - HTTPRouteInvalidCrossNamespaceBackendRef
    - HTTPRouteInvalidCrossNamespaceParentRef
    - HTTPRouteInvalidNonExistentBackendRef
    - HTTPRouteInvalidParentRefNotMatchingSectionName
    - HTTPRouteInvalidReferenceGrant
    - HTTPRouteListenerHostnameMatching
    - HTTPRouteMatching
    - HTTPRouteMatchingAcrossRoutes
    - HTTPRoutePartiallyInvalidViaInvalidReferenceGrant
    - HTTPRouteReferenceGrant
    - HTTPRouteRequestHeaderModifier
    - HTTPRouteWeight
    statistics:
      Failed: 2
      Passed: 9
      Skipped: 22
  name: GATEWAY-HTTP
  summary: Core tests failed with 2 test failures.

dspo added 4 commits April 17, 2025 11:05
The `updateHTTPRoutePolicyStatus` function is replaced with `modifyHTTPRoutePolicyStatus` to streamline the process, and a new function `clearHTTPRoutePolicyRedundantAncestor` is introduced to remove redundant ancestors. Additionally, the group in the test's targetRefs is updated to `gateway.networking.k8s.io`.
Remove unnecessary `kind` fields and list map keys in the HTTPRoutePolicy CRD. Add a new generic event channel and update predicates for more efficient event handling in the HTTPRouteReconciler.
This commit refactors the handling of HTTPRoutePolicy, including moving the `httpRoutePolicyPredicateOnUpdate` function and updating the type definitions. It also updates the e2e test for HTTPRoute to correct a typo in the test description.
@dspo dspo changed the title (WIP)feat: Add support for HTTPRoutePolicy in HTTPRoute reconciliation feat: Add support for HTTPRoutePolicy in HTTPRoute reconciliation Apr 17, 2025
@dspo dspo requested review from AlinsRan and Copilot April 17, 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.

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Loop:
for _, items := range c.policies {
for _, item := range items {
if item.Spec.Priority != policy.Spec.Priority || *item.Spec.Priority != *policy.Spec.Priority {
Copy link

Copilot AI Apr 17, 2025

Choose a reason for hiding this comment

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

The condition combining a direct pointer comparison and a dereferenced value comparison is redundant and could potentially lead to a nil pointer dereference if either Spec.Priority is nil. Consider adding explicit nil checks and simplifying the comparison logic.

Suggested change
if item.Spec.Priority != policy.Spec.Priority || *item.Spec.Priority != *policy.Spec.Priority {
if item.Spec.Priority == nil || policy.Spec.Priority == nil {
if item.Spec.Priority != policy.Spec.Priority { // One is nil, the other is not
c.conflict = true
break Loop
}
} else if *item.Spec.Priority != *policy.Spec.Priority {

Copilot uses AI. Check for mistakes.
The import statements in `httproute_controller.go`, `httproutepolicy.go`, and `httproute.go` have been reordered to improve readability and maintain a consistent style across the files.
@dspo dspo requested a review from AlinsRan April 18, 2025 06:55
@dspo dspo merged commit ba5c304 into release-v2-dev Apr 20, 2025
7 of 8 checks passed
@AlinsRan AlinsRan deleted the feat/httproutepolicy branch May 8, 2025 07:24
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