Skip to content

Conversation

@dspo
Copy link
Contributor

@dspo dspo commented Apr 25, 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:

Add logic to update HTTPRoutePolicy status when associated Ingress or HTTPRoute resources are deleted. Introduce new methods to handle status updates and remove ancestor references, along with corresponding e2e tests for ingress and gateway API scenarios.

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

@github-actions
Copy link
Contributor

github-actions bot commented Apr 25, 2025

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2025-04-27T04:55:12Z"
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:
    result: partial
    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: 0
      Passed: 11
      Skipped: 22
  name: GATEWAY-HTTP
  summary: Core tests partially succeeded with 22 test skips.

Add logic to update HTTPRoutePolicy status when associated Ingress or HTTPRoute resources are deleted. Introduce new methods to handle status updates and remove ancestor references, along with corresponding e2e tests for ingress and gateway API scenarios.
@dspo dspo changed the title (WIP)chore: httproutepolicy status on targetref deleting chore: httproutepolicy status on targetref deleting Apr 27, 2025
@dspo dspo requested a review from Copilot April 27, 2025 02:17
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

This PR is a chore that improves the handling and status updates of HTTPRoutePolicy resources when their associated Ingress or HTTPRoute is deleted. The key changes include:

  • Adding new e2e tests to verify that HTTPRoutePolicy status updates correctly on target resource deletion.
  • Refactoring the internal data structures and functions to use slices instead of maps for HTTPRoutePolicies.
  • Updating index functions and controller logic to accommodate these changes.

Reviewed Changes

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

Show a summary per file
File Description
test/e2e/ingress/ingress.go New tests for verifying HTTPRoutePolicy status on Ingress deletion
test/e2e/gatewayapi/httproute.go Updated tests for HTTPRoute deletion handling
internal/provider/provider.go Changed HTTPRoutePolicies from a map to a slice to support new processing logic
internal/provider/adc/translator/httproute.go Adjusted logic for filtering HTTPRoutePolicies for HTTPRoute and Ingress
internal/controller/* Updated indexing, reconciliation, and status update functions for HTTPRoutePolicy
Comments suppressed due to low confidence (3)

internal/provider/provider.go:41

  • Switching HTTPRoutePolicies from a map to a slice changes how policies are accessed; please verify that all methods relying on keyed lookups are updated correctly to iterate through the slice.
HTTPRoutePolicies:      []v1alpha1.HTTPRoutePolicy

internal/controller/httproute_controller.go:519

  • [nitpick] Ensure that using slices.DeleteFunc to filter discardsRefs preserves the intended order of targetRefs if the order is significant for subsequent processing.
discardsRefs := slices.DeleteFunc(oldPolicy.Spec.TargetRefs, func(oldRef v1alpha2.LocalPolicyTargetReferenceWithSectionName) bool {

internal/controller/httproutepolicy.go:183

  • [nitpick] Using metav1.Now() is consistent with Kubernetes API conventions, but please double-check that this timestamp approach aligns with other parts of the codebase that might use time.Now() for condition timestamps.
LastTransitionTime: metav1.Now(),

Rename `isPoliciesConflict` to `checkPoliciesConflict` for clarity and improve related functions for better readability and accuracy. Update method names and logic for deleting ancestors and finding matching policies, while also optimizing the status update process for deleted HTTPRoute resources.
@dspo dspo merged commit 90aff0b into release-v2-dev Apr 27, 2025
8 checks passed
@AlinsRan AlinsRan deleted the feat/httproutepolicy-status-on-targetref-deleting branch May 8, 2025 07:22
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