From 94aa125b716917e0e9d45561965d1a4dcb278bdb Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 26 Nov 2024 07:36:39 +0000 Subject: [PATCH 1/8] fix btlsp section name Signed-off-by: Huabing Zhao --- internal/gatewayapi/backendtlspolicy.go | 44 ++++++++++++++++++- .../backendtlspolicy-across-ns.in.yaml | 2 +- .../backendtlspolicy-across-ns.out.yaml | 2 +- .../backendtlspolicy-ca-only-secret.in.yaml | 2 +- .../backendtlspolicy-ca-only-secret.out.yaml | 2 +- .../testdata/backendtlspolicy-ca-only.in.yaml | 2 +- .../backendtlspolicy-ca-only.out.yaml | 2 +- ...endtlspolicy-default-ns-targetrefs.in.yaml | 2 +- ...ndtlspolicy-default-ns-targetrefs.out.yaml | 2 +- .../backendtlspolicy-default-ns.in.yaml | 2 +- .../backendtlspolicy-default-ns.out.yaml | 2 +- .../backendtlspolicy-invalid-ca.in.yaml | 2 +- .../backendtlspolicy-invalid-ca.out.yaml | 2 +- .../backendtlspolicy-multiple-targets.in.yaml | 6 +-- ...backendtlspolicy-multiple-targets.out.yaml | 2 +- ...backendtlspolicy-system-truststore.in.yaml | 2 +- ...ackendtlspolicy-system-truststore.out.yaml | 2 +- test/e2e/testdata/backend-tls-settings.yaml | 5 ++- test/e2e/testdata/backend-tls.yaml | 5 ++- 19 files changed, 66 insertions(+), 24 deletions(-) diff --git a/internal/gatewayapi/backendtlspolicy.go b/internal/gatewayapi/backendtlspolicy.go index b76e215f99a..48b53d065ce 100644 --- 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,13 +157,53 @@ func backendTLSTargetMatched(policy gwapiv1a3.BackendTLSPolicy, target gwapiv1a2 return false } -func getBackendTLSPolicy(policies []*gwapiv1a3.BackendTLSPolicy, backendRef gwapiv1a2.BackendObjectReference, backendNamespace string) *gwapiv1a3.BackendTLSPolicy { +// 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 ref.SectionName == 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) for _, policy := range policies { if backendTLSTargetMatched(*policy, target, backendNamespace) { return policy } } + + // SectionName can be port name for Kubernetes Service + target = getTargetBackendReferenceWithPortName(backendRef, backendNamespace, resources) + for _, policy := range policies { + if backendTLSTargetMatched(*policy, target, backendNamespace) { + return policy + } + } return nil } diff --git a/internal/gatewayapi/testdata/backendtlspolicy-across-ns.in.yaml b/internal/gatewayapi/testdata/backendtlspolicy-across-ns.in.yaml index e87b3ad1cb9..efd69116641 100644 --- 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 index 7d776a1784f..2e6c7d9dd97 100644 --- 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 index b701ad9800f..fd4caad15e4 100644 --- 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 index a65ea66d0ab..7d6a8237be1 100644 --- 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 index cc6c0f17c8f..2b6701762f7 100644 --- 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 index f85b9c73c3f..b17f6528118 100644 --- 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 index a86b1a25930..2fd3adc48e7 100644 --- 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 index 3467422f204..1056bc483ae 100644 --- 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 index 5a13fba2fc2..10ac7095127 100644 --- 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 index c8898169624..bfe19432905 100644 --- 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 index 7abc20d19c1..a5484a20358 100644 --- 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 index cb968f9a6a0..2a7b7ab89d4 100644 --- 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 index d3458d06da8..73ed3aa49bc 100644 --- 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 @@ -114,7 +114,7 @@ backendTLSPolicies: - 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 index 207713455e8..50d243d3e66 100644 --- a/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.out.yaml +++ b/internal/gatewayapi/testdata/backendtlspolicy-multiple-targets.out.yaml @@ -14,7 +14,7 @@ backendTLSPolicies: - 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 index 3b20aa31ee5..520065b82a4 100644 --- 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 index 8438c8551ce..659d092df7f 100644 --- 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/test/e2e/testdata/backend-tls-settings.yaml b/test/e2e/testdata/backend-tls-settings.yaml index 749255f82e5..b78ace739fe 100644 --- a/test/e2e/testdata/backend-tls-settings.yaml +++ b/test/e2e/testdata/backend-tls-settings.yaml @@ -62,7 +62,8 @@ spec: selector: app: tls-backend ports: - - protocol: TCP + - name: https + protocol: TCP port: 443 targetPort: 8443 --- @@ -137,7 +138,7 @@ spec: - group: "" kind: Service name: tls-backend - sectionName: "443" + sectionName: https validation: caCertificateRefs: - name: backend-tls-certificate diff --git a/test/e2e/testdata/backend-tls.yaml b/test/e2e/testdata/backend-tls.yaml index f00218ab99c..5616e144ede 100644 --- a/test/e2e/testdata/backend-tls.yaml +++ b/test/e2e/testdata/backend-tls.yaml @@ -8,7 +8,7 @@ spec: - group: "" kind: Service name: tls-backend-2 - sectionName: "443" + sectionName: https validation: caCertificateRefs: - name: backend-tls-checks-certificate @@ -42,7 +42,8 @@ spec: selector: app: tls-backend-2 ports: - - protocol: TCP + - name: https + protocol: TCP port: 443 targetPort: 8443 --- From 41e3ba95a8467d8acbc37cb938be36c506a7cfe1 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 26 Nov 2024 08:21:30 +0000 Subject: [PATCH 2/8] add release note Signed-off-by: Huabing Zhao --- release-notes/current.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/release-notes/current.yaml b/release-notes/current.yaml index c0f8dfc0e5b..cbfc5177c1d 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -20,6 +20,7 @@ bug fixes: | Fixed failed to update SecurityPolicy resources with the `backendRef` field specified Fixed Envoy rejecting TCP Listeners that have no attached TCPRoutes Fixed xDS translation failed when oidc tokenEndpoint and jwt remoteJWKS are specified in the same SecurityPolicy and using the same hostname + Fixed BackendTLSPolicy didn't support using port name as the sectionName in the targetRefs # Enhancements that improve performance. performance improvements: | From 1536c2a3538da9f6322351f5276c0453a7a34e24 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 26 Nov 2024 11:31:23 +0000 Subject: [PATCH 3/8] optmize service search Signed-off-by: Huabing Zhao --- internal/cmd/egctl/translate_test.go | 9 +++++++-- internal/gatewayapi/backendtlspolicy.go | 3 ++- internal/gatewayapi/resource/resource.go | 19 ++++++++++++++----- .../resource/zz_generated.deepcopy.go | 17 +++++++++++++++++ internal/gatewayapi/translator_test.go | 9 ++++++--- 5 files changed, 46 insertions(+), 11 deletions(-) diff --git a/internal/cmd/egctl/translate_test.go b/internal/cmd/egctl/translate_test.go index 20cf76d0162..e87167ce305 100644 --- 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 index 48b53d065ce..98a195c5d01 100644 --- a/internal/gatewayapi/backendtlspolicy.go +++ b/internal/gatewayapi/backendtlspolicy.go @@ -162,7 +162,8 @@ func backendTLSTargetMatched(policy gwapiv1a3.BackendTLSPolicy, target gwapiv1a2 func getTargetBackendReferenceWithPortName( backendRef gwapiv1a2.BackendObjectReference, backendNamespace string, - resources *resource.Resources) gwapiv1a2.LocalPolicyTargetReferenceWithSectionName { + resources *resource.Resources, +) gwapiv1a2.LocalPolicyTargetReferenceWithSectionName { ref := getTargetBackendReference(backendRef) if ref.SectionName == nil { return ref diff --git a/internal/gatewayapi/resource/resource.go b/internal/gatewayapi/resource/resource.go index 97468511fa8..749e2efeef6 100644 --- 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 index 06925b1467d..3caecc292c8 100644 --- a/internal/gatewayapi/resource/zz_generated.deepcopy.go +++ b/internal/gatewayapi/resource/zz_generated.deepcopy.go @@ -14,6 +14,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" "sigs.k8s.io/gateway-api/apis/v1" "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/apis/v1alpha3" @@ -290,6 +291,22 @@ func (in *Resources) DeepCopyInto(out *Resources) { } } } + if in.serviceMap != nil { + in, out := &in.serviceMap, &out.serviceMap + *out = make(map[types.NamespacedName]*corev1.Service, len(*in)) + for key, val := range *in { + var outVal *corev1.Service + if val == nil { + (*out)[key] = nil + } else { + inVal := (*in)[key] + in, out := &inVal, &outVal + *out = new(corev1.Service) + (*in).DeepCopyInto(*out) + } + (*out)[key] = outVal + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Resources. diff --git a/internal/gatewayapi/translator_test.go b/internal/gatewayapi/translator_test.go index 61e0025fbdd..96a88bfdec9 100644 --- a/internal/gatewayapi/translator_test.go +++ b/internal/gatewayapi/translator_test.go @@ -320,11 +320,11 @@ func TestTranslate(t *testing.T) { opts := []cmp.Option{ cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"), + cmpopts.IgnoreFields(resource.Resources{}, "serviceMap"), cmp.Transformer("ClearXdsEqual", xdsWithoutEqual), cmpopts.IgnoreTypes(ir.PrivateBytes{}), cmpopts.EquateEmpty(), } - require.Empty(t, cmp.Diff(want, got, opts...)) }) } @@ -519,8 +519,11 @@ func TestTranslateWithExtensionKinds(t *testing.T) { want := &TranslateResult{} mustUnmarshal(t, output, want) - 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...)) }) } } From dfd2daca1ac85749ce5f77625540c8cd8c8d2f5c Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Wed, 27 Nov 2024 02:06:06 +0000 Subject: [PATCH 4/8] address comment Signed-off-by: Huabing Zhao --- internal/gatewayapi/backendtlspolicy.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/gatewayapi/backendtlspolicy.go b/internal/gatewayapi/backendtlspolicy.go index 98a195c5d01..98706e74bea 100644 --- a/internal/gatewayapi/backendtlspolicy.go +++ b/internal/gatewayapi/backendtlspolicy.go @@ -165,9 +165,10 @@ func getTargetBackendReferenceWithPortName( resources *resource.Resources, ) gwapiv1a2.LocalPolicyTargetReferenceWithSectionName { ref := getTargetBackendReference(backendRef) - if ref.SectionName == nil { + if backendRef.Port == nil { return ref } + if backendRef.Kind != nil && *backendRef.Kind != resource.KindService { return ref } @@ -199,10 +200,13 @@ func getBackendTLSPolicy( } // SectionName can be port name for Kubernetes Service - target = getTargetBackendReferenceWithPortName(backendRef, backendNamespace, resources) - for _, policy := range policies { - if backendTLSTargetMatched(*policy, target, backendNamespace) { - return policy + 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 From 271d5d4ff6d0c6185ffa28cc030a965525817624 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Wed, 27 Nov 2024 11:32:23 +0000 Subject: [PATCH 5/8] fix test Signed-off-by: Huabing Zhao --- internal/gatewayapi/backendtlspolicy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/gatewayapi/backendtlspolicy.go b/internal/gatewayapi/backendtlspolicy.go index 98706e74bea..e2f07f8da86 100644 --- a/internal/gatewayapi/backendtlspolicy.go +++ b/internal/gatewayapi/backendtlspolicy.go @@ -201,7 +201,7 @@ func getBackendTLSPolicy( // SectionName can be port name for Kubernetes Service if backendRef.Port != nil && - (backendRef.Kind != nil && *backendRef.Kind == resource.KindService) { + (backendRef.Kind == nil || *backendRef.Kind == resource.KindService) { target = getTargetBackendReferenceWithPortName(backendRef, backendNamespace, resources) for _, policy := range policies { if backendTLSTargetMatched(*policy, target, backendNamespace) { From 28b13f2f9788e14afb1ddffe0a9f31539848a336 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Wed, 27 Nov 2024 14:58:09 +0000 Subject: [PATCH 6/8] fix test Signed-off-by: Huabing Zhao --- test/e2e/base/manifests.yaml | 3 ++- test/e2e/testdata/backend-tls.yaml | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/base/manifests.yaml b/test/e2e/base/manifests.yaml index 34ccc08390a..ef3e9841c3b 100644 --- a/test/e2e/base/manifests.yaml +++ b/test/e2e/base/manifests.yaml @@ -480,7 +480,8 @@ spec: selector: app: tls-backend-2 ports: - - protocol: TCP + - name: https + protocol: TCP port: 443 targetPort: 8443 --- diff --git a/test/e2e/testdata/backend-tls.yaml b/test/e2e/testdata/backend-tls.yaml index 5616e144ede..ad77871ea74 100644 --- a/test/e2e/testdata/backend-tls.yaml +++ b/test/e2e/testdata/backend-tls.yaml @@ -42,8 +42,7 @@ spec: selector: app: tls-backend-2 ports: - - name: https - protocol: TCP + - protocol: TCP port: 443 targetPort: 8443 --- From bd9c3e8d8a089997be87aef21a50614683a4e14c Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Mon, 2 Dec 2024 13:17:08 +0000 Subject: [PATCH 7/8] 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 From 6b762eb77b26fd24b87a9fced0928dde4745a775 Mon Sep 17 00:00:00 2001 From: Huabing Zhao Date: Tue, 3 Dec 2024 01:14:40 +0000 Subject: [PATCH 8/8] fix e2e Signed-off-by: Huabing Zhao --- test/e2e/testdata/ext-auth-grpc-securitypolicy.yaml | 2 +- test/e2e/testdata/ext-proc-service.yaml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/e2e/testdata/ext-auth-grpc-securitypolicy.yaml b/test/e2e/testdata/ext-auth-grpc-securitypolicy.yaml index c75ee250f09..2d49f69c50a 100644 --- a/test/e2e/testdata/ext-auth-grpc-securitypolicy.yaml +++ b/test/e2e/testdata/ext-auth-grpc-securitypolicy.yaml @@ -62,7 +62,7 @@ spec: - group: '' kind: Service name: grpc-ext-auth - sectionName: "9002" + sectionName: grpc validation: caCertificateRefs: - name: grpc-ext-auth-ca diff --git a/test/e2e/testdata/ext-proc-service.yaml b/test/e2e/testdata/ext-proc-service.yaml index 3dc4796e123..57581a80c38 100644 --- a/test/e2e/testdata/ext-proc-service.yaml +++ b/test/e2e/testdata/ext-proc-service.yaml @@ -95,3 +95,4 @@ spec: - protocol: TCP port: 9002 targetPort: 9002 + name: grpc