feat: support translate BackendTrafficPolicy CRD #92

AlinsRan · 2025-04-15T00:11:06Z

Type of change:

What this PR does / why we need it:

Only a portion of the conversion logic has been implemented.

TODO:

HTTPRoute support BackendTrafficPolicy
Ingress support BackendTrafficPolicy

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 · 2025-04-15T00:25:42Z

conformance test report

apiVersion: gateway.networking.k8s.io/v1
date: "2025-04-16T08:44:19Z"
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.

Copilot

Pull Request Overview

This PR adds support for translating the new BackendTrafficPolicy CRD within the ingress controller by extending the translation context, translator, and controller logic while also updating the CRD definitions and associated types.

Changes to the TranslateContext to encapsulate context and include BackendTrafficPolicies and StatusUpdaters.
Enhancements in translator and controller logic to attach BackendTrafficPolicy configurations and update indexers.
Updates to CRD YAML defaults and deepcopy functions to support the new CRD.

Reviewed Changes

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

Show a summary per file

File	Description
internal/provider/provider.go	Updated TranslateContext and constructor to accept a context and initialize maps.
internal/provider/adc/translator/policies.go	Added functions to attach BackendTrafficPolicy to upstreams.
internal/provider/adc/translator/ingress.go & httproute.go	Updated to invoke attachBackendTrafficPolicyToUpstream in translation routines.
internal/controller/*	Propagated context usage in status, policies, and ingress reconciler logic.
internal/controller/indexer/indexer.go	Added indexer support for BackendTrafficPolicy target references.
CRD YAML and api files	Updated CRD defaults, deep copy functions, and type definitions for Back......

Comments suppressed due to low confidence (1)

internal/provider/adc/translator/policies.go:31

Consider using a lower log level (e.g. Info or Debug) here since attaching a BackendTrafficPolicy appears to be a normal operation; using an error log might mislead monitoring systems.

log.Errorw("attach backend traffic policy to upstream", zap.String("policy", policy.Name))

Copilot

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

Comments suppressed due to low confidence (2)

internal/provider/adc/translator/ingress.go:112

Calling attachBackendTrafficPolicyToUpstream with a nil policy results in a no-op. Consider adding a conditional to only invoke this function when a valid policy exists, or document the rationale for this call.

t.attachBackendTrafficPolicyToUpstream(nil, upstream)

internal/provider/adc/translator/httproute.go:293

Using a nil policy in attachBackendTrafficPolicyToUpstream may be redundant. Review whether this call is necessary or if it should be conditioned on a non-nil policy.

t.attachBackendTrafficPolicyToUpstream(nil, upstream)

dspo

api7-ingress-controller/config/crd/bases/gateway.apisix.io_backendtrafficpolicies.yaml

Lines 161 to 165 in c9fa9f4

    
           x-kubernetes-list-map-keys: 
        
           - group 
        
           - kind 
        
           - name 
        
           x-kubernetes-list-type: map

这里会以这些字段为 key，也就不能出现同样的 group kind name 组合，符合期望吗？

internal/controller/policies.go

dspo · 2025-04-16T03:38:11Z

internal/controller/indexer/indexer.go

 	SecretIndexRef     = "secretRefs"
 	IngressClassRef    = "ingressClassRef"
 	ConsumerGatewayRef = "consumerGatewayRef"
+	PolicyTargetRefs   = "targetRefs"


Can it be renamed to BackendTrafficPolicyTargetRefs or BTPolicyTargetRefs to distinguish it from HTTPRoutePolicyTargetRefs ?

Do we need to distinguish? This index key is for resources with targetrefs.

dspo · 2025-04-16T03:39:32Z

internal/controller/policies.go

+		backendTrafficPolicyList := &v1alpha1.BackendTrafficPolicyList{}
+		if err := c.List(tctx, backendTrafficPolicyList,
+			client.MatchingFields{
+				indexer.PolicyTargetRefs: indexer.GenIndexKeyWithGK("", "Service", service.Namespace, service.Name),


Why is empty string for group ?

The group of the service is an empty string.

dspo · 2025-04-16T03:44:04Z

internal/controller/policies.go

+				condition := NewPolicyCondition(policy.Generation, true, "Policy has been accepted")
+				if sectionName != nil && !portNameExist[string(*sectionName)] {
+					condition = NewPolicyCondition(policy.Generation, false, fmt.Sprintf("SectionName %s not found in Service %s/%s", *sectionName, service.Namespace, service.Name))
+					goto record_status


Can the use of goto be avoided?

dspo · 2025-04-16T03:50:00Z

internal/provider/adc/translator/policies.go

+			Connect: policy.Spec.Timeout.Connect.Duration.Milliseconds(),
+			Read:    policy.Spec.Timeout.Read.Duration.Milliseconds(),
+			Send:    policy.Spec.Timeout.Send.Duration.Milliseconds(),


In dashboard API, the unit is second. It is millsecond in adc ？

feat: support translate backendtrafficpolicy CRD

7716bfc

AlinsRan marked this pull request as ready for review April 15, 2025 00:25

AlinsRan added 2 commits April 15, 2025 11:01

fix status

0a99a0b

fix

9223a02

AlinsRan requested review from Copilot and dspo April 15, 2025 09:18

Copilot AI reviewed Apr 15, 2025

View reviewed changes

remove debug log

a935f15

AlinsRan requested a review from Copilot April 15, 2025 09:26

Copilot AI reviewed Apr 15, 2025

View reviewed changes

AlinsRan added 3 commits April 15, 2025 10:21

add rbac

12ac6db

fix test

eff13da

fix lint

c9fa9f4

dspo reviewed Apr 16, 2025

View reviewed changes

ronething reviewed Apr 16, 2025

View reviewed changes

internal/controller/policies.go Show resolved Hide resolved

dspo approved these changes Apr 16, 2025

View reviewed changes

AlinsRan added 3 commits April 16, 2025 16:06

resolve coment

3152257

f2

065133b

f3

8663b82

ronething approved these changes Apr 16, 2025

View reviewed changes

AlinsRan requested a review from dspo April 16, 2025 09:02

AlinsRan merged commit 9e4e35b into release-v2-dev Apr 16, 2025
8 checks passed

AlinsRan deleted the feat/support-backendtrafficpolicy-translate branch April 16, 2025 12:52

	x-kubernetes-list-map-keys:
	- group
	- kind
	- name
	x-kubernetes-list-type: map

feat: support translate BackendTrafficPolicy CRD #92

feat: support translate BackendTrafficPolicy CRD #92

Uh oh!

Conversation

AlinsRan commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change:

What this PR does / why we need it:

Pre-submission checklist:

Uh oh!

github-actions bot commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

conformance test report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

dspo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dspo Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlinsRan Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

dspo Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlinsRan Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

dspo Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlinsRan Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

dspo Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlinsRan Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AlinsRan commented Apr 15, 2025 •

edited

Loading

github-actions bot commented Apr 15, 2025 •

edited

Loading

dspo left a comment •

edited

Loading

dspo Apr 16, 2025 •

edited

Loading

dspo Apr 16, 2025 •

edited

Loading

dspo Apr 16, 2025 •

edited

Loading

dspo Apr 16, 2025 •

edited

Loading