From bd9c3e8d8a089997be87aef21a50614683a4e14c Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Mon, 2 Dec 2024 13:17:08 +0000 Subject: [PATCH] address comment Signed-off-by: Huabing Zhao --- internal/gatewayapi/backendtlspolicy.go | 43 +------------------ internal/gatewayapi/route.go | 31 +++++++++---- .../backendtlspolicy-multiple-targets.in.yaml | 2 +- ...backendtlspolicy-multiple-targets.out.yaml | 2 +- ...with-extproc-with-backendtlspolicy.in.yaml | 4 +- ...ith-extproc-with-backendtlspolicy.out.yaml | 4 +- ...-extproc-with-multiple-backendrefs.in.yaml | 2 +- ...extproc-with-multiple-backendrefs.out.yaml | 2 +- ...with-extproc-with-traffic-features.in.yaml | 2 +- ...ith-extproc-with-traffic-features.out.yaml | 2 +- .../envoyproxy-priority-backend.in.yaml | 2 +- .../envoyproxy-priority-backend.out.yaml | 2 +- ...with-extauth-with-backendtlspolicy.in.yaml | 4 +- ...ith-extauth-with-backendtlspolicy.out.yaml | 4 +- test/e2e/testdata/ext-auth-grpc-service.yaml | 1 + test/e2e/testdata/ext-auth-http-service.yaml | 1 + .../ext-proc-envoyextensionpolicy.yaml | 2 +- 17 files changed, 44 insertions(+), 66 deletions(-) diff --git a/internal/gatewayapi/backendtlspolicy.go b/internal/gatewayapi/backendtlspolicy.go index e2f07f8da86..fbc9cafbf1a 100644 --- a/internal/gatewayapi/backendtlspolicy.go +++ b/internal/gatewayapi/backendtlspolicy.go @@ -157,58 +157,19 @@ func backendTLSTargetMatched(policy gwapiv1a3.BackendTLSPolicy, target gwapiv1a2 return false } -// getTargetBackendReferenceWithPortName returns the LocalPolicyTargetReference for the given BackendObjectReference, -// and sets the sectionName to the port name if the BackendObjectReference is a Kubernetes Service. -func getTargetBackendReferenceWithPortName( - backendRef gwapiv1a2.BackendObjectReference, - backendNamespace string, - resources *resource.Resources, -) gwapiv1a2.LocalPolicyTargetReferenceWithSectionName { - ref := getTargetBackendReference(backendRef) - if backendRef.Port == nil { - return ref - } - - if backendRef.Kind != nil && *backendRef.Kind != resource.KindService { - return ref - } - - 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) - } - } - } - } - - return ref -} - func getBackendTLSPolicy( policies []*gwapiv1a3.BackendTLSPolicy, backendRef gwapiv1a2.BackendObjectReference, backendNamespace string, resources *resource.Resources, ) *gwapiv1a3.BackendTLSPolicy { - target := getTargetBackendReference(backendRef) + // SectionName is port number for EG Backend object + target := getTargetBackendReference(backendRef, backendNamespace, resources) for _, policy := range policies { if backendTLSTargetMatched(*policy, target, backendNamespace) { return policy } } - - // SectionName can be port name for Kubernetes Service - if backendRef.Port != nil && - (backendRef.Kind == nil || *backendRef.Kind == resource.KindService) { - target = getTargetBackendReferenceWithPortName(backendRef, backendNamespace, resources) - for _, policy := range policies { - if backendTLSTargetMatched(*policy, target, backendNamespace) { - return policy - } - } - } return nil } diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 26627a07285..a93ba497611 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -1596,30 +1596,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-multiple-targets.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.in.yaml index 73ed3aa49bc..96a97fcb0ca 100644 --- a/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.in.yaml +++ b/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.in.yaml @@ -110,7 +110,7 @@ backendTLSPolicies: - group: "" kind: Service name: http-backend - sectionName: "8080" + sectionName: http1 - group: "" kind: Service name: http-backend diff --git a/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.out.yaml b/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.out.yaml index 50d243d3e66..50d9d9c9372 100644 --- a/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.out.yaml +++ b/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.out.yaml @@ -10,7 +10,7 @@ backendTLSPolicies: - group: "" kind: Service name: http-backend - sectionName: "8080" + sectionName: http1 - group: "" kind: Service name: http-backend diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-backendtlspolicy.in.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-backendtlspolicy.in.yaml index ca3297a5fae..bc367228c73 100644 --- 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/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-backendtlspolicy.out.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-backendtlspolicy.out.yaml index 81863d1acdf..68559dffb9d 100644 --- a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-backendtlspolicy.out.yaml +++ b/internal/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/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.in.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.in.yaml index dad20362396..89be7cac752 100644 --- a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.in.yaml +++ b/internal/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/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.out.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.out.yaml index 4789f8555e3..348977b316b 100644 --- a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-multiple-backendrefs.out.yaml +++ b/internal/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: "" diff --git a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-traffic-features.in.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-traffic-features.in.yaml index 1f25d8f7e0b..30af5a4dbd9 100644 --- a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-traffic-features.in.yaml +++ b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-traffic-features.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/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-traffic-features.out.yaml b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-traffic-features.out.yaml index 93c24363c31..16139f9c9dc 100644 --- a/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-traffic-features.out.yaml +++ b/internal/gatewayapi/testdata/envoyextensionpolicy-with-extproc-with-traffic-features.out.yaml @@ -10,7 +10,7 @@ backendTLSPolicies: - group: "" kind: Service name: grpc-backend - sectionName: "8000" + sectionName: grpc validation: caCertificateRefs: - group: "" diff --git a/internal/gatewayapi/testdata/envoyproxy-priority-backend.in.yaml b/internal/gatewayapi/testdata/envoyproxy-priority-backend.in.yaml index 64b0b7a3ae9..42e46b8990e 100644 --- a/internal/gatewayapi/testdata/envoyproxy-priority-backend.in.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-priority-backend.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/internal/gatewayapi/testdata/envoyproxy-priority-backend.out.yaml b/internal/gatewayapi/testdata/envoyproxy-priority-backend.out.yaml index f5c685bab24..1905a96a631 100644 --- a/internal/gatewayapi/testdata/envoyproxy-priority-backend.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-priority-backend.out.yaml @@ -10,7 +10,7 @@ backendTLSPolicies: - group: "" kind: Service name: grpc-backend - sectionName: "8000" + sectionName: grpc validation: caCertificateRefs: - group: "" diff --git a/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.in.yaml b/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.in.yaml index d2aee51b27e..abd7ed641b9 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.in.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.in.yaml @@ -160,7 +160,7 @@ backendTLSPolicies: - group: "" kind: Service name: http-backend - sectionName: "80" + sectionName: http validation: caCertificateRefs: - name: ca-cmap @@ -177,7 +177,7 @@ backendTLSPolicies: - group: "" kind: Service name: grpc-backend - sectionName: "9000" + sectionName: grpc validation: caCertificateRefs: - name: ca-cmap diff --git a/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.out.yaml index 4eca64d1a07..472ed5b7c3c 100644 --- a/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.out.yaml +++ b/internal/gatewayapi/testdata/securitypolicy-with-extauth-with-backendtlspolicy.out.yaml @@ -10,7 +10,7 @@ backendTLSPolicies: - group: "" kind: Service name: http-backend - sectionName: "80" + sectionName: http validation: caCertificateRefs: - group: "" @@ -42,7 +42,7 @@ backendTLSPolicies: - group: "" kind: Service name: grpc-backend - sectionName: "9000" + sectionName: grpc validation: caCertificateRefs: - group: "" diff --git a/test/e2e/testdata/ext-auth-grpc-service.yaml b/test/e2e/testdata/ext-auth-grpc-service.yaml index 587dad8a860..da74439592f 100644 --- a/test/e2e/testdata/ext-auth-grpc-service.yaml +++ b/test/e2e/testdata/ext-auth-grpc-service.yaml @@ -103,3 +103,4 @@ spec: - protocol: TCP port: 9002 targetPort: 9002 + name: grpc diff --git a/test/e2e/testdata/ext-auth-http-service.yaml b/test/e2e/testdata/ext-auth-http-service.yaml index a4e96928292..cada07e4712 100644 --- a/test/e2e/testdata/ext-auth-http-service.yaml +++ b/test/e2e/testdata/ext-auth-http-service.yaml @@ -39,3 +39,4 @@ spec: - protocol: TCP port: 9002 targetPort: 9002 + name: http diff --git a/test/e2e/testdata/ext-proc-envoyextensionpolicy.yaml b/test/e2e/testdata/ext-proc-envoyextensionpolicy.yaml index 3663e19b610..a3945a65f34 100644 --- a/test/e2e/testdata/ext-proc-envoyextensionpolicy.yaml +++ b/test/e2e/testdata/ext-proc-envoyextensionpolicy.yaml @@ -80,7 +80,7 @@ spec: - group: '' kind: Service name: grpc-ext-proc - sectionName: "9002" + sectionName: grpc validation: caCertificateRefs: - name: grpc-ext-proc-ca