Skip to content

fix: honor BackendTrafficPolicy targetRefs.sectionName for Service ports #428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AlinsRan wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: honor BackendTrafficPolicy targetRefs.sectionName for Service ports #428

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/adc/translator/grpcroute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ func (t *Translator) TranslateGRPCRoute(tctx *provider.TranslateContext, grpcRou
continue
}

t.AttachBackendTrafficPolicyToUpstream(backend.BackendRef, tctx.BackendTrafficPolicies, upstream)
t.AttachBackendTrafficPolicyToUpstream(backend.BackendRef, tctx.BackendTrafficPolicies, upstream, tctx.Services)
upstream.Nodes = upNodes

var (
Expand Down
2 changes: 1 addition & 1 deletion internal/adc/translator/httproute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ func (t *Translator) TranslateHTTPRoute(tctx *provider.TranslateContext, httpRou
enableWebsocket = ptr.To(true)
}

t.AttachBackendTrafficPolicyToUpstream(backend.BackendRef, tctx.BackendTrafficPolicies, upstream)
t.AttachBackendTrafficPolicyToUpstream(backend.BackendRef, tctx.BackendTrafficPolicies, upstream, tctx.Services)
upstream.Nodes = upNodes
if upstream.Scheme == "" {
upstream.Scheme = appProtocolToUpstreamScheme(protocol)
Expand Down
103 changes: 103 additions & 0 deletions internal/adc/translator/httproute_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,3 +309,106 @@ func TestAttachBackendTrafficPolicyHealthCheck(t *testing.T) {
})
}
}

func TestAttachBackendTrafficPolicyToUpstreamSectionName(t *testing.T) {
const (
namespace = "default"
serviceName = "backend"
webPort = int32(80)
webName = "web"
adminPort = int32(9000)
adminName = "admin"
)

serviceKey := types.NamespacedName{Namespace: namespace, Name: serviceName}
services := map[types.NamespacedName]*corev1.Service{
serviceKey: {
ObjectMeta: metav1.ObjectMeta{Name: serviceName, Namespace: namespace},
Spec: corev1.ServiceSpec{
Ports: []corev1.ServicePort{
{Name: webName, Port: webPort},
{Name: adminName, Port: adminPort},
},
},
},
}

newRef := func(port int32) gatewayv1.BackendRef {
return gatewayv1.BackendRef{
BackendObjectReference: gatewayv1.BackendObjectReference{
Name: gatewayv1.ObjectName(serviceName),
Namespace: ptr.To(gatewayv1.Namespace(namespace)),
Port: ptr.To(gatewayv1.PortNumber(port)),
},
}
}

newPolicy := func(name, sectionName, scheme string) *v1alpha1.BackendTrafficPolicy {
targetRef := v1alpha1.BackendPolicyTargetReferenceWithSectionName{
LocalPolicyTargetReference: gatewayv1alpha2.LocalPolicyTargetReference{
Name: gatewayv1alpha2.ObjectName(serviceName),
Kind: gatewayv1alpha2.Kind(internaltypes.KindService),
},
}
if sectionName != "" {
targetRef.SectionName = ptr.To(gatewayv1alpha2.SectionName(sectionName))
}
return &v1alpha1.BackendTrafficPolicy{
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace},
Spec: v1alpha1.BackendTrafficPolicySpec{
TargetRefs: []v1alpha1.BackendPolicyTargetReferenceWithSectionName{targetRef},
Scheme: scheme,
},
}
}

tests := []struct {
name string
ref gatewayv1.BackendRef
policies map[types.NamespacedName]*v1alpha1.BackendTrafficPolicy
wantScheme string
}{
{
name: "sectionName matches the backend port name",
ref: newRef(webPort),
policies: map[types.NamespacedName]*v1alpha1.BackendTrafficPolicy{
{Namespace: namespace, Name: "p"}: newPolicy("p", webName, apiv2.SchemeHTTPS),
},
wantScheme: apiv2.SchemeHTTPS,
},
{
name: "sectionName does not match the backend port name",
ref: newRef(adminPort),
policies: map[types.NamespacedName]*v1alpha1.BackendTrafficPolicy{
{Namespace: namespace, Name: "p"}: newPolicy("p", webName, apiv2.SchemeHTTPS),
},
wantScheme: "",
},
{
name: "no sectionName applies to the whole service",
ref: newRef(adminPort),
policies: map[types.NamespacedName]*v1alpha1.BackendTrafficPolicy{
{Namespace: namespace, Name: "p"}: newPolicy("p", "", apiv2.SchemeHTTPS),
},
wantScheme: apiv2.SchemeHTTPS,
},
{
name: "port-specific policy takes precedence over whole-service policy",
ref: newRef(adminPort),
policies: map[types.NamespacedName]*v1alpha1.BackendTrafficPolicy{
{Namespace: namespace, Name: "generic"}: newPolicy("generic", "", apiv2.SchemeHTTP),
{Namespace: namespace, Name: "specific"}: newPolicy("specific", adminName, apiv2.SchemeHTTPS),
},
wantScheme: apiv2.SchemeHTTPS,
},
}

translator := NewTranslator(logr.Discard())
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
upstream := adctypes.NewDefaultUpstream()
translator.AttachBackendTrafficPolicyToUpstream(tt.ref, tt.policies, upstream, services)
assert.Equal(t, tt.wantScheme, upstream.Scheme)
})
}
}
2 changes: 1 addition & 1 deletion internal/adc/translator/ingress.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func (t *Translator) resolveIngressUpstream(
ns = config.ServiceNamespace
}
backendRef := convertBackendRef(ns, backendService.Name, internaltypes.KindService)
t.AttachBackendTrafficPolicyToUpstream(backendRef, tctx.BackendTrafficPolicies, upstream)
t.AttachBackendTrafficPolicyToUpstream(backendRef, tctx.BackendTrafficPolicies, upstream, tctx.Services)
if config != nil {
upConfig := config.Upstream
if upConfig.Scheme != "" {
Expand Down
43 changes: 38 additions & 5 deletions internal/adc/translator/policies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package translator
import (
"encoding/json"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
Expand All @@ -38,28 +39,60 @@ func convertBackendRef(namespace, name, kind string) gatewayv1.BackendRef {
return backendRef
}

func (t *Translator) AttachBackendTrafficPolicyToUpstream(ref gatewayv1.BackendRef, policies map[types.NamespacedName]*v1alpha1.BackendTrafficPolicy, upstream *adctypes.Upstream) {
func (t *Translator) AttachBackendTrafficPolicyToUpstream(ref gatewayv1.BackendRef, policies map[types.NamespacedName]*v1alpha1.BackendTrafficPolicy, upstream *adctypes.Upstream, services map[types.NamespacedName]*corev1.Service) {
if len(policies) == 0 {
return
}
var policy *v1alpha1.BackendTrafficPolicy
// A targetRef with sectionName scopes the policy to a specific Service port
// (matched by port name). It takes precedence over a whole-Service targetRef
// (no sectionName) that matches the same backend.
var genericPolicy, specificPolicy *v1alpha1.BackendTrafficPolicy
for _, po := range policies {
if ref.Namespace != nil && string(*ref.Namespace) != po.Namespace {
continue
}
for _, targetRef := range po.Spec.TargetRefs {
if ref.Name == targetRef.Name {
policy = po
break
if ref.Name != targetRef.Name {
continue
}
if targetRef.SectionName != nil && *targetRef.SectionName != "" {
if backendRefMatchesSectionName(ref, po.Namespace, string(*targetRef.SectionName), services) {
specificPolicy = po
Comment on lines 54 to +60

@coderabbitai coderabbitai Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate targetRef group/kind before attaching by name.

At Line 55, matching only by Name allows cross-kind attachment when backend names collide (e.g., a ServiceImport policy attaching to a Service backend). This breaks attachment scoping and can apply the wrong policy.

💡 Suggested fix 
 import (
 	"encoding/json"

 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

 	adctypes "github.com/apache/apisix-ingress-controller/api/adc"
 	"github.com/apache/apisix-ingress-controller/api/v1alpha1"
 	apiv2 "github.com/apache/apisix-ingress-controller/api/v2"
+	internaltypes "github.com/apache/apisix-ingress-controller/internal/types"
 )
@@
 func (t *Translator) AttachBackendTrafficPolicyToUpstream(ref gatewayv1.BackendRef, policies map[types.NamespacedName]*v1alpha1.BackendTrafficPolicy, upstream *adctypes.Upstream, services map[types.NamespacedName]*corev1.Service) {
@@
+	refKind := internaltypes.KindService
+	if ref.Kind != nil {
+		refKind = string(*ref.Kind)
+	}
+	refGroup := ""
+	if ref.Group != nil {
+		refGroup = string(*ref.Group)
+	}
 	for _, po := range policies {
@@
 		for _, targetRef := range po.Spec.TargetRefs {
-			if ref.Name != targetRef.Name {
+			if string(ref.Name) != string(targetRef.Name) {
+				continue
+			}
+			if targetRef.Kind != "" && string(targetRef.Kind) != refKind {
+				continue
+			}
+			if targetRef.Group != "" && string(targetRef.Group) != refGroup {
 				continue
 			}

As per coding guidelines, "Resource access must be scoped to the correct parent entity; validate that child resources (subscription, upstream, route, credential) belong to the requested parent before access or modification."

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/adc/translator/policies.go` around lines 54 - 60, The policy
matching logic in the loop iterating through po.Spec.TargetRefs only validates
the Name field of the targetRef against ref.Name, which allows policies for
different resource kinds (e.g., ServiceImport) to incorrectly attach to
resources with the same name but different kinds (e.g., Service). Add validation
to also check that the targetRef's group and kind match the corresponding fields
of ref before proceeding with the name comparison and the subsequent
backendRefMatchesSectionName check. This ensures policies are scoped to the
correct resource type and prevents cross-kind attachment when names collide.

Source: Coding guidelines

}
continue
}
genericPolicy = po
}
}
policy := specificPolicy
if policy == nil {
policy = genericPolicy
}
if policy == nil {
return
}
t.attachBackendTrafficPolicyToUpstream(policy, upstream)
}

// backendRefMatchesSectionName reports whether the backend ref resolves to the
// Service port named sectionName. Per the Gateway API policy semantics, when a
// sectionName is specified but cannot be resolved, the policy must not attach.
func backendRefMatchesSectionName(ref gatewayv1.BackendRef, namespace, sectionName string, services map[types.NamespacedName]*corev1.Service) bool {
if ref.Port == nil {
return false
}
svc, ok := services[types.NamespacedName{Namespace: namespace, Name: string(ref.Name)}]
if !ok || svc == nil {
return false
}
for _, port := range svc.Spec.Ports {
if port.Port == int32(*ref.Port) {
return port.Name == sectionName
}
}
return false
}

func (t *Translator) attachBackendTrafficPolicyToUpstream(policy *v1alpha1.BackendTrafficPolicy, upstream *adctypes.Upstream) {
if policy == nil {
return
Expand Down
2 changes: 1 addition & 1 deletion internal/adc/translator/tcproute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (t *Translator) TranslateTCPRoute(tctx *provider.TranslateContext, tcpRoute
continue
}
// TODO: Confirm BackendTrafficPolicy attachment with e2e test case.
t.AttachBackendTrafficPolicyToUpstream(backend, tctx.BackendTrafficPolicies, upstream)
t.AttachBackendTrafficPolicyToUpstream(backend, tctx.BackendTrafficPolicies, upstream, tctx.Services)
upstream.Nodes = upNodes
var (
kind string
Expand Down
2 changes: 1 addition & 1 deletion internal/adc/translator/tlsroute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (t *Translator) TranslateTLSRoute(tctx *provider.TranslateContext, tlsRoute
continue
}
// TODO: Confirm BackendTrafficPolicy attachment with e2e test case.
t.AttachBackendTrafficPolicyToUpstream(backend, tctx.BackendTrafficPolicies, upstream)
t.AttachBackendTrafficPolicyToUpstream(backend, tctx.BackendTrafficPolicies, upstream, tctx.Services)
upstream.Nodes = upNodes
var (
kind string
Expand Down
2 changes: 1 addition & 1 deletion internal/adc/translator/udproute.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (t *Translator) TranslateUDPRoute(tctx *provider.TranslateContext, udpRoute
continue
}
// TODO: Confirm BackendTrafficPolicy attachment with e2e test case.
t.AttachBackendTrafficPolicyToUpstream(backend, tctx.BackendTrafficPolicies, upstream)
t.AttachBackendTrafficPolicyToUpstream(backend, tctx.BackendTrafficPolicies, upstream, tctx.Services)
upstream.Nodes = upNodes
var (
kind string
Expand Down
Loading