fix: btlsp section name doesn't support port name (envoyproxy#4784)

* fix btlsp section name Signed-off-by: Huabing Zhao <[email protected]> * add release note Signed-off-by: Huabing Zhao <[email protected]> * optmize service search Signed-off-by: Huabing Zhao <[email protected]> * address comment Signed-off-by: Huabing Zhao <[email protected]> * fix test Signed-off-by: Huabing Zhao <[email protected]> * fix test Signed-off-by: Huabing Zhao <[email protected]> * address comment Signed-off-by: Huabing Zhao <[email protected]> * fix e2e Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> Co-authored-by: zirain <[email protected]>
zhaohuabing · Dec 6, 2024 · b9f9a9f · b9f9a9f
1 parent 14fb56e
commit b9f9a9f
Show file tree

Hide file tree

Showing 41 changed files with 122 additions and 61 deletions.
diff --git a/internal/cmd/egctl/translate_test.go b/internal/cmd/egctl/translate_test.go
@@ -22,6 +22,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/yaml"
 
+	"github.com/envoyproxy/gateway/internal/gatewayapi/resource"
 	"github.com/envoyproxy/gateway/internal/utils/field"
 	"github.com/envoyproxy/gateway/internal/utils/file"
 )
@@ -368,8 +369,12 @@ func TestTranslate(t *testing.T) {
 			//	want.GatewayClass.Status.SupportedFeatures = status.GatewaySupportedFeatures
 			// }
 
-			opts := cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime")
-			require.Empty(t, cmp.Diff(want, got, opts))
+			opts := []cmp.Option{
+				cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"),
+				cmpopts.IgnoreFields(resource.Resources{}, "serviceMap"),
+			}
+
+			require.Empty(t, cmp.Diff(want, got, opts...))
 		})
 	}
 }

diff --git a/internal/gatewayapi/backendtlspolicy.go b/internal/gatewayapi/backendtlspolicy.go
@@ -32,7 +32,7 @@ func (t *Translator) processBackendTLSPolicy(
 	resources *resource.Resources,
 	envoyProxy *egv1a1.EnvoyProxy,
 ) (*ir.TLSUpstreamConfig, *gwapiv1a3.BackendTLSPolicy) {
-	policy := getBackendTLSPolicy(resources.BackendTLSPolicies, backendRef, backendNamespace)
+	policy := getBackendTLSPolicy(resources.BackendTLSPolicies, backendRef, backendNamespace, resources)
 	if policy == nil {
 		return nil, nil
 	}
@@ -157,8 +157,14 @@ func backendTLSTargetMatched(policy gwapiv1a3.BackendTLSPolicy, target gwapiv1a2
 	return false
 }
 
-func getBackendTLSPolicy(policies []*gwapiv1a3.BackendTLSPolicy, backendRef gwapiv1a2.BackendObjectReference, backendNamespace string) *gwapiv1a3.BackendTLSPolicy {
-	target := getTargetBackendReference(backendRef)
+func getBackendTLSPolicy(
+	policies []*gwapiv1a3.BackendTLSPolicy,
+	backendRef gwapiv1a2.BackendObjectReference,
+	backendNamespace string,
+	resources *resource.Resources,
+) *gwapiv1a3.BackendTLSPolicy {
+	// SectionName is port number for EG Backend object
+	target := getTargetBackendReference(backendRef, backendNamespace, resources)
 	for _, policy := range policies {
 		if backendTLSTargetMatched(*policy, target, backendNamespace) {
 			return policy

diff --git a/internal/gatewayapi/resource/resource.go b/internal/gatewayapi/resource/resource.go
@@ -13,6 +13,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	discoveryv1 "k8s.io/api/discovery/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/types"
 	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
 	gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 	gwapiv1a3 "sigs.k8s.io/gateway-api/apis/v1alpha3"
@@ -64,6 +65,8 @@ type Resources struct {
 	ExtensionServerPolicies []unstructured.Unstructured    `json:"extensionServerPolicies,omitempty" yaml:"extensionServerPolicies,omitempty"`
 	Backends                []*egv1a1.Backend              `json:"backends,omitempty" yaml:"backends,omitempty"`
 	HTTPRouteFilters        []*egv1a1.HTTPRouteFilter      `json:"httpFilters,omitempty" yaml:"httpFilters,omitempty"`
+
+	serviceMap map[types.NamespacedName]*corev1.Service
 }
 
 func NewResources() *Resources {
@@ -111,14 +114,20 @@ func (r *Resources) GetEnvoyProxy(namespace, name string) *egv1a1.EnvoyProxy {
 	return nil
 }
 
+// GetService returns the Service with the given namespace and name.
+// This function creates a HashMap of Services for faster lookup when it's called for the first time.
+// Subsequent calls will use the HashMap for lookup.
+// Note:
+// - This function is not thread-safe.
+// - This function should be called after all the Services are added to the Resources.
 func (r *Resources) GetService(namespace, name string) *corev1.Service {
-	for _, svc := range r.Services {
-		if svc.Namespace == namespace && svc.Name == name {
-			return svc
+	if r.serviceMap == nil {
+		r.serviceMap = make(map[types.NamespacedName]*corev1.Service)
+		for _, svc := range r.Services {
+			r.serviceMap[types.NamespacedName{Namespace: svc.Namespace, Name: svc.Name}] = svc
 		}
 	}
-
-	return nil
+	return r.serviceMap[types.NamespacedName{Namespace: namespace, Name: name}]
 }
 
 func (r *Resources) GetServiceImport(namespace, name string) *mcsapiv1a1.ServiceImport {

diff --git a/internal/gatewayapi/resource/zz_generated.deepcopy.go b/internal/gatewayapi/resource/zz_generated.deepcopy.go
diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
@@ -1598,30 +1598,45 @@ func getIREndpointsFromEndpointSlice(endpointSlice *discoveryv1.EndpointSlice, p
 	return endpoints
 }
 
-func getTargetBackendReference(backendRef gwapiv1a2.BackendObjectReference) gwapiv1a2.LocalPolicyTargetReferenceWithSectionName {
+func getTargetBackendReference(backendRef gwapiv1a2.BackendObjectReference, backendNamespace string, resources *resource.Resources) gwapiv1a2.LocalPolicyTargetReferenceWithSectionName {
 	ref := gwapiv1a2.LocalPolicyTargetReferenceWithSectionName{
 		LocalPolicyTargetReference: gwapiv1a2.LocalPolicyTargetReference{
 			Group: func() gwapiv1a2.Group {
-				if backendRef.Group == nil {
+				if backendRef.Group == nil || *backendRef.Group == "" {
 					return ""
 				}
 				return *backendRef.Group
 			}(),
 			Kind: func() gwapiv1.Kind {
-				if backendRef.Kind == nil {
+				if backendRef.Kind == nil || *backendRef.Kind == resource.KindService {
 					return "Service"
 				}
 				return *backendRef.Kind
 			}(),
 			Name: backendRef.Name,
 		},
-		SectionName: func() *gwapiv1.SectionName {
-			if backendRef.Port != nil {
-				return SectionNamePtr(strconv.Itoa(int(*backendRef.Port)))
+	}
+	if backendRef.Port == nil {
+		return ref
+	}
+
+	// Set the section name to the port name if the backend is a Kubernetes Service
+	if backendRef.Kind == nil || *backendRef.Kind == resource.KindService {
+		if service := resources.GetService(backendNamespace, string(backendRef.Name)); service != nil {
+			for _, port := range service.Spec.Ports {
+				if port.Port == int32(*backendRef.Port) {
+					if port.Name != "" {
+						ref.SectionName = SectionNamePtr(port.Name)
+						break
+					}
+				}
 			}
-			return nil
-		}(),
+		}
+	} else {
+		// Set the section name to the port number if the backend is a EG Backend
+		ref.SectionName = SectionNamePtr(strconv.Itoa(int(*backendRef.Port)))
 	}
+
 	return ref
 }
 

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-across-ns.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-across-ns.in.yaml
@@ -123,7 +123,7 @@ backendTLSPolicies:
         - group: ""
           kind: Service
           name: http-backend
-          sectionName: "8080"
+          sectionName: http
       validation:
         caCertificateRefs:
           - name: ca-cmap

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-across-ns.out.yaml b/internal/gatewayapi/testdata/backendtlspolicy-across-ns.out.yaml
@@ -10,7 +10,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: http-backend
-      sectionName: "8080"
+      sectionName: http
     validation:
       caCertificateRefs:
       - group: ""

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-ca-only-secret.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-ca-only-secret.in.yaml
@@ -108,7 +108,7 @@ backendTLSPolicies:
         - group: ""
           kind: Service
           name: http-backend
-          sectionName: "8080"
+          sectionName: http
       validation:
         caCertificateRefs:
           - name: ca-secret

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-ca-only-secret.out.yaml b/internal/gatewayapi/testdata/backendtlspolicy-ca-only-secret.out.yaml
@@ -10,7 +10,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: http-backend
-      sectionName: "8080"
+      sectionName: http
     validation:
       caCertificateRefs:
       - group: ""

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-ca-only.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-ca-only.in.yaml
@@ -123,7 +123,7 @@ backendTLSPolicies:
         - group: ""
           kind: Service
           name: http-backend
-          sectionName: "8080"
+          sectionName: http
       validation:
         caCertificateRefs:
           - name: ca-cmap

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-ca-only.out.yaml b/internal/gatewayapi/testdata/backendtlspolicy-ca-only.out.yaml
@@ -10,7 +10,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: http-backend
-      sectionName: "8080"
+      sectionName: http
     validation:
       caCertificateRefs:
       - group: ""

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-default-ns-targetrefs.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-default-ns-targetrefs.in.yaml
@@ -167,7 +167,7 @@ backendTLSPolicies:
         - group: ""
           kind: Service
           name: http-backend
-          sectionName: "8080"
+          sectionName: http
         - group: gateway.envoyproxy.io
           kind: Backend
           name: backend-ip-tls

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-default-ns-targetrefs.out.yaml b/internal/gatewayapi/testdata/backendtlspolicy-default-ns-targetrefs.out.yaml
@@ -10,7 +10,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: http-backend
-      sectionName: "8080"
+      sectionName: http
     - group: gateway.envoyproxy.io
       kind: Backend
       name: backend-ip-tls

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-default-ns.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-default-ns.in.yaml
@@ -134,7 +134,7 @@ backendTLSPolicies:
         - group: ""
           kind: Service
           name: http-backend
-          sectionName: "8080"
+          sectionName: http
       validation:
         caCertificateRefs:
           - name: ca-cmap

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-default-ns.out.yaml b/internal/gatewayapi/testdata/backendtlspolicy-default-ns.out.yaml
@@ -10,7 +10,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: http-backend
-      sectionName: "8080"
+      sectionName: http
     validation:
       caCertificateRefs:
       - group: ""

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-invalid-ca.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-invalid-ca.in.yaml
@@ -105,7 +105,7 @@ backendTLSPolicies:
         - group: ""
           kind: Service
           name: http-backend
-          sectionName: "8080"
+          sectionName: http
       validation:
         caCertificateRefs:
           - name: no-ca-cmap

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-invalid-ca.out.yaml b/internal/gatewayapi/testdata/backendtlspolicy-invalid-ca.out.yaml
@@ -10,7 +10,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: http-backend
-      sectionName: "8080"
+      sectionName: http
     validation:
       caCertificateRefs:
       - group: ""

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.in.yaml
@@ -64,11 +64,11 @@ services:
       clusterIP: 10.11.12.13
       ports:
         - port: 8080
-          name: http
+          name: http1
           protocol: TCP
           targetPort: 8080
         - port: 8081
-          name: http
+          name: http2
           protocol: TCP
           targetPort: 8081
 
@@ -110,11 +110,11 @@ backendTLSPolicies:
         - group: ""
           kind: Service
           name: http-backend
-          sectionName: "8080"
+          sectionName: http1
         - group: ""
           kind: Service
           name: http-backend
-          sectionName: "8081"
+          sectionName: http2
       validation:
         caCertificateRefs:
           - name: ca-cmap

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.out.yaml b/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.out.yaml
@@ -10,11 +10,11 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: http-backend
-      sectionName: "8080"
+      sectionName: http1
     - group: ""
       kind: Service
       name: http-backend
-      sectionName: "8081"
+      sectionName: http2
     validation:
       caCertificateRefs:
       - group: ""

diff --git a/internal/gatewayapi/testdata/backendtlspolicy-system-truststore.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-system-truststore.in.yaml
@@ -98,7 +98,7 @@ backendTLSPolicies:
         - group: ""
           kind: Service
           name: http-backend
-          sectionName: "8080"
+          sectionName: http
       validation:
         wellKnownCACertificates: System
         hostname: example.com
diff --git a/internal/gatewayapi/testdata/backendtlspolicy-system-truststore.out.yaml b/internal/gatewayapi/testdata/backendtlspolicy-system-truststore.out.yaml
@@ -10,7 +10,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: http-backend
-      sectionName: "8080"
+      sectionName: http
     validation:
       hostname: example.com
       wellKnownCACertificates: System

diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-backendtlspolicy.in.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-backendtlspolicy.in.yaml
@@ -160,7 +160,7 @@ backendTLSPolicies:
     - group: ''
       kind: Service
       name: grpc-backend
-      sectionName: "8000"
+      sectionName: grpc
     validation:
       caCertificateRefs:
       - name: ca-cmap
@@ -177,7 +177,7 @@ backendTLSPolicies:
     - group: ''
       kind: Service
       name: grpc-backend-2
-      sectionName: "9000"
+      sectionName: grpc
     validation:
       caCertificateRefs:
       - name: ca-cmap

diff --git a/...rnal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-backendtlspolicy.out.yaml b/...rnal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-backendtlspolicy.out.yaml
@@ -10,7 +10,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: grpc-backend
-      sectionName: "8000"
+      sectionName: grpc
     validation:
       caCertificateRefs:
       - group: ""
@@ -42,7 +42,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: grpc-backend-2
-      sectionName: "9000"
+      sectionName: grpc
     validation:
       caCertificateRefs:
       - group: ""

diff --git a/...l/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.in.yaml b/...l/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.in.yaml
@@ -162,7 +162,7 @@ backendTLSPolicies:
     - group: ''
       kind: Service
       name: grpc-backend
-      sectionName: "8000"
+      sectionName: grpc
     validation:
       caCertificateRefs:
       - name: ca-cmap

diff --git a/.../gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.out.yaml b/.../gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.out.yaml
@@ -10,7 +10,7 @@ backendTLSPolicies:
     - group: ""
       kind: Service
       name: grpc-backend
-      sectionName: "8000"
+      sectionName: grpc
     validation:
       caCertificateRefs:
       - group: ""