From 07b6494968215825c72a2c5c601f5c7c07cf9e0b Mon Sep 17 00:00:00 2001 From: bitliu Date: Fri, 4 Jul 2025 20:45:59 +0800 Subject: [PATCH] fix: custom backend weight not set properly Signed-off-by: bitliu --- internal/gatewayapi/route.go | 6 +++- ...custom-backend-invalid-apiversion.out.yaml | 24 ++++++++++++--- ...ith-custom-backend-mixed-multiple.out.yaml | 12 ++++++++ ...tproute-with-custom-backend-mixed.out.yaml | 6 ++++ ...oute-with-custom-backend-multiple.out.yaml | 30 ++++++++++++++++--- .../httproute-with-custom-backend.out.yaml | 24 ++++++++++++--- internal/ir/xds.go | 5 ++++ internal/provider/kubernetes/controller.go | 2 +- .../provider/kubernetes/controller_test.go | 14 ++------- 9 files changed, 97 insertions(+), 26 deletions(-) diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 69ea451eb02..974f2cebc40 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -1427,7 +1427,11 @@ func (t *Translator) processDestination(name string, backendRefContext BackendRe if t.isCustomBackendResource(backendRef.Group, KindDerefOr(backendRef.Kind, resource.KindService)) { // Add the custom backend resource to ExtensionRefFilters so it can be processed by the extension system unstructuredRef = t.processBackendExtensions(backendRef.BackendObjectReference, backendNamespace, resources) - return nil, unstructuredRef, nil + return &ir.DestinationSetting{ + Name: name, + Weight: &weight, + IsCustomBackend: true, + }, unstructuredRef, nil } } diff --git a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-invalid-apiversion.out.yaml b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-invalid-apiversion.out.yaml index eb6e27c75d1..c4a91292b66 100644 --- a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-invalid-apiversion.out.yaml +++ b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-invalid-apiversion.out.yaml @@ -130,8 +130,16 @@ xdsIR: mergeSlashes: true port: 10080 routes: - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/1 + settings: + - isCustomBackend: true + name: httproute/default/httproute-1/rule/1/backend/0 + weight: 1 hostname: gateway.envoyproxy.io isHTTP2: false metadata: @@ -143,8 +151,16 @@ xdsIR: distinct: false name: "" prefix: /lambda - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - isCustomBackend: true + name: httproute/default/httproute-1/rule/0/backend/0 + weight: 1 hostname: gateway.envoyproxy.io isHTTP2: false metadata: diff --git a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-mixed-multiple.out.yaml b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-mixed-multiple.out.yaml index ca4c13b7631..605ff6b2eff 100644 --- a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-mixed-multiple.out.yaml +++ b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-mixed-multiple.out.yaml @@ -160,6 +160,12 @@ xdsIR: name: httproute/default/httproute-1/rule/0/backend/0 protocol: HTTP weight: 1 + - isCustomBackend: true + name: httproute/default/httproute-1/rule/0/backend/1 + weight: 1 + - isCustomBackend: true + name: httproute/default/httproute-1/rule/0/backend/2 + weight: 1 extensionRefs: - object: apiVersion: storage.example.io/v1alpha1 @@ -199,6 +205,12 @@ xdsIR: namespace: default name: httproute/default/httproute-1/rule/1 settings: + - isCustomBackend: true + name: httproute/default/httproute-1/rule/1/backend/0 + weight: 1 + - isCustomBackend: true + name: httproute/default/httproute-1/rule/1/backend/1 + weight: 1 - addressType: IP endpoints: - host: 7.7.7.7 diff --git a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-mixed.out.yaml b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-mixed.out.yaml index 0aac167ee66..eb0053db425 100644 --- a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-mixed.out.yaml +++ b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-mixed.out.yaml @@ -152,6 +152,9 @@ xdsIR: name: httproute/default/httproute-1/rule/0/backend/0 protocol: HTTP weight: 1 + - isCustomBackend: true + name: httproute/default/httproute-1/rule/0/backend/1 + weight: 1 extensionRefs: - object: apiVersion: storage.example.io/v1alpha1 @@ -181,6 +184,9 @@ xdsIR: namespace: default name: httproute/default/httproute-1/rule/1 settings: + - isCustomBackend: true + name: httproute/default/httproute-1/rule/1/backend/0 + weight: 1 - addressType: IP endpoints: - host: 7.7.7.7 diff --git a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-multiple.out.yaml b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-multiple.out.yaml index ba10662f95c..b782b65aa99 100644 --- a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-multiple.out.yaml +++ b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend-multiple.out.yaml @@ -138,8 +138,19 @@ xdsIR: mergeSlashes: true port: 10080 routes: - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - isCustomBackend: true + name: httproute/default/httproute-1/rule/0/backend/0 + weight: 1 + - isCustomBackend: true + name: httproute/default/httproute-1/rule/0/backend/1 + weight: 1 extensionRefs: - object: apiVersion: storage.example.io/v1alpha1 @@ -172,8 +183,19 @@ xdsIR: distinct: false name: "" prefix: /service - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/1 + settings: + - isCustomBackend: true + name: httproute/default/httproute-1/rule/1/backend/0 + weight: 1 + - isCustomBackend: true + name: httproute/default/httproute-1/rule/1/backend/1 + weight: 1 extensionRefs: - object: apiVersion: storage.example.io/v1alpha1 diff --git a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend.out.yaml b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend.out.yaml index f2d88f6f551..1484045d8ec 100644 --- a/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend.out.yaml +++ b/internal/gatewayapi/testdata/extensions/httproute-with-custom-backend.out.yaml @@ -130,8 +130,16 @@ xdsIR: mergeSlashes: true port: 10080 routes: - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/1 + settings: + - isCustomBackend: true + name: httproute/default/httproute-1/rule/1/backend/0 + weight: 1 extensionRefs: - object: apiVersion: compute.example.io/v1alpha1 @@ -154,8 +162,16 @@ xdsIR: distinct: false name: "" prefix: /lambda - - directResponse: - statusCode: 500 + - destination: + metadata: + kind: HTTPRoute + name: httproute-1 + namespace: default + name: httproute/default/httproute-1/rule/0 + settings: + - isCustomBackend: true + name: httproute/default/httproute-1/rule/0/backend/0 + weight: 1 extensionRefs: - object: apiVersion: storage.example.io/v1alpha1 diff --git a/internal/ir/xds.go b/internal/ir/xds.go index f9bac24c8bf..c1e60990970 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -1600,6 +1600,8 @@ func (r *RouteDestination) ToBackendWeights() *BackendWeights { switch { case s.IsDynamicResolver: // Dynamic resolver has no endpoints w.Valid += *s.Weight + case s.IsCustomBackend: // Custom backends has no endpoints + w.Valid += *s.Weight case len(s.Endpoints) > 0: w.Valid += *s.Weight default: @@ -1620,6 +1622,9 @@ type DestinationSetting struct { // A dynamic resolver is a destination that is resolved dynamically using the request's host header. IsDynamicResolver bool `json:"isDynamicResolver,omitempty" yaml:"isDynamicResolver,omitempty"` + // IsCustomBackend specifies whether the destination is a custom backend. + IsCustomBackend bool `json:"isCustomBackend,omitempty" yaml:"isCustomBackend,omitempty"` + // Weight associated with this destination, // invalid endpoints are represents with a // non-zero weight with an empty endpoints list diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 2ea4900aa85..a4b809f6827 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -657,7 +657,7 @@ func (r *gatewayAPIReconciler) processBackendRefs(ctx context.Context, gwcResour } if !resourceMappings.allAssociatedBackendRefExtensionFilters.Has(key) { resourceMappings.allAssociatedBackendRefExtensionFilters.Insert(key) - if extRefFilter, exists := resourceMappings.extensionRefFilters[key]; !exists { + if extRefFilter, exists := resourceMappings.extensionRefFilters[key]; exists { resourceMappings.extensionRefFilters[key] = extRefFilter gwcResource.ExtensionRefFilters = append(gwcResource.ExtensionRefFilters, extRefFilter) r.log.Info("added custom backend resource to resource tree", diff --git a/internal/provider/kubernetes/controller_test.go b/internal/provider/kubernetes/controller_test.go index da8d36cf95b..68888116c56 100644 --- a/internal/provider/kubernetes/controller_test.go +++ b/internal/provider/kubernetes/controller_test.go @@ -318,18 +318,8 @@ func TestProcessBackendRefsWithCustomBackends(t *testing.T) { // Call the function under test require.NoError(t, r.processBackendRefs(ctx, gwcResource, resourceMappings)) - - // Verify results - // Note: Due to a bug in the current implementation (line 542 in controller.go), - // custom backends are not properly added to ExtensionRefFilters when they exist - // in the extensionRefFilters map. The logic should be `exists` instead of `!exists`. - // For now, we test the current (buggy) behavior. - if tc.name == "skip non-custom backends" { - require.Len(t, gwcResource.ExtensionRefFilters, tc.expectedExtFiltersCount) - } else { - // Current buggy behavior: custom backends are not added to ExtensionRefFilters - require.Empty(t, gwcResource.ExtensionRefFilters) - } + // Compare the results + require.Len(t, gwcResource.ExtensionRefFilters, tc.expectedExtFiltersCount) for _, expectedNS := range tc.expectedNamespaces { require.True(t, resourceMappings.allAssociatedNamespaces.Has(expectedNS))