From a146e0f063dd964f09f0f8c8585a6673f17af0a1 Mon Sep 17 00:00:00 2001 From: Ravi Verma Date: Sat, 3 May 2025 13:22:42 +0530 Subject: [PATCH 1/4] Add custom extension server policies to PostTranslateContext and eventually pass to PostTranslateModify() method Signed-off-by: Ravi Verma --- internal/extension/registry/xds_hook.go | 16 +++- internal/extension/types/hooks.go | 2 +- internal/gatewayapi/extensionserverpolicy.go | 3 + .../extensionpolicy-tcp-listener.out.yaml | 53 ++++++++++++ .../extensionpolicy-udp-listener.out.yaml | 53 ++++++++++++ ...ionpolicy-with-valid-target-array.out.yaml | 82 +++++++++++++++++++ ...extensionpolicy-with-valid-target.out.yaml | 53 ++++++++++++ internal/gatewayapi/translator_test.go | 43 +++++----- internal/ir/xds.go | 2 + internal/ir/zz_generated.deepcopy.go | 8 ++ internal/xds/translator/extension.go | 8 +- internal/xds/translator/translator.go | 2 +- proto/extension/context.pb.go | 48 +++++++---- proto/extension/context.proto | 4 +- release-notes/current.yaml | 3 +- 15 files changed, 334 insertions(+), 46 deletions(-) diff --git a/internal/extension/registry/xds_hook.go b/internal/extension/registry/xds_hook.go index d21e29f448..51d75169df 100644 --- a/internal/extension/registry/xds_hook.go +++ b/internal/extension/registry/xds_hook.go @@ -105,14 +105,22 @@ func (h *XDSHook) PostHTTPListenerModifyHook(l *listener.Listener, extensionReso return resp.Listener, nil } -func (h *XDSHook) PostTranslateModifyHook(clusters []*cluster.Cluster, secrets []*tls.Secret) ([]*cluster.Cluster, []*tls.Secret, error) { +func (h *XDSHook) PostTranslateModifyHook(clusters []*cluster.Cluster, secrets []*tls.Secret, extensionPolicies []*unstructured.Unstructured) ([]*cluster.Cluster, []*tls.Secret, error) { // Make the request to the extension server + // Take all of the unstructured resources for the extension and package them into bytes + extensionPoliciesBytes, err := translateUnstructuredToUnstructuredBytes(extensionPolicies) + if err != nil { + return clusters, secrets, err + } + ctx := context.Background() resp, err := h.grpcClient.PostTranslateModify(ctx, &extension.PostTranslateModifyRequest{ - PostTranslateContext: &extension.PostTranslateExtensionContext{}, - Clusters: clusters, - Secrets: secrets, + PostTranslateContext: &extension.PostTranslateExtensionContext{ + ExtensionResources: extensionPoliciesBytes, + }, + Clusters: clusters, + Secrets: secrets, }) if err != nil { return nil, nil, err diff --git a/internal/extension/types/hooks.go b/internal/extension/types/hooks.go index 08d8c4ede2..d823a88cf7 100644 --- a/internal/extension/types/hooks.go +++ b/internal/extension/types/hooks.go @@ -41,5 +41,5 @@ type XDSHookClient interface { // An example of how this may be used is to inject a cluster that will be used by an ext_authz http filter created by the extension. // The list of clusters and secrets returned by the extension are used as the final list of all clusters and secrets // PostTranslateModifyHook is always executed when an extension is loaded - PostTranslateModifyHook([]*cluster.Cluster, []*tls.Secret) ([]*cluster.Cluster, []*tls.Secret, error) + PostTranslateModifyHook([]*cluster.Cluster, []*tls.Secret, []*unstructured.Unstructured) ([]*cluster.Cluster, []*tls.Secret, error) } diff --git a/internal/gatewayapi/extensionserverpolicy.go b/internal/gatewayapi/extensionserverpolicy.go index 395a30ef33..cd710f0048 100644 --- a/internal/gatewayapi/extensionserverpolicy.go +++ b/internal/gatewayapi/extensionserverpolicy.go @@ -67,6 +67,9 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst // unable to find a matching Gateway for policy continue } + // Appped policy extension server policy list for related gateway. + gatewayKey := t.getIRKey(gateway.Gateway) + xdsIR[gatewayKey].ExtensionServerPolicies = append(xdsIR[gatewayKey].ExtensionServerPolicies, *policy) // Set conditions for translation if it got any if t.translateExtServerPolicyForGateway(policy, gateway, currTarget, xdsIR) { diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml index d1d6fb4101..42f1dbcadd 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml @@ -146,6 +146,59 @@ xdsIR: accessLog: json: - path: /dev/stdout + extensionServerPolicies: + - apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test1 + namespace: envoy-gateway + spec: + data: attached to all listeners + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached only to listener on port 80 + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: tcp1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: tcp1 + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller readyListener: address: 0.0.0.0 ipFamily: IPv4 diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml index cea577c54a..84fccff522 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml @@ -146,6 +146,59 @@ xdsIR: accessLog: json: - path: /dev/stdout + extensionServerPolicies: + - apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test1 + namespace: envoy-gateway + spec: + data: attached to all listeners + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached only to listener on port 162 + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: udp1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: udp1 + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller readyListener: address: 0.0.0.0 ipFamily: IPv4 diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml index 2f9c597bb5..ce7bed03c1 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml @@ -158,6 +158,47 @@ xdsIR: accessLog: json: - path: /dev/stdout + extensionServerPolicies: + - apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached to multiple gateways + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller http: - address: 0.0.0.0 extensionRefs: @@ -224,6 +265,47 @@ xdsIR: accessLog: json: - path: /dev/stdout + extensionServerPolicies: + - apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached to multiple gateways + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + - group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller http: - address: 0.0.0.0 extensionRefs: diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml index 9dea789d18..30869890bb 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml @@ -150,6 +150,59 @@ xdsIR: accessLog: json: - path: /dev/stdout + extensionServerPolicies: + - apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test1 + namespace: envoy-gateway + spec: + data: attached to all listeners + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached only to listener on port 80 + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: http2 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http2 + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller http: - address: 0.0.0.0 extensionRefs: diff --git a/internal/gatewayapi/translator_test.go b/internal/gatewayapi/translator_test.go index e468245949..4d62026073 100644 --- a/internal/gatewayapi/translator_test.go +++ b/internal/gatewayapi/translator_test.go @@ -26,6 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -891,27 +892,29 @@ func (m *mockWasmCache) Cleanup() {} // This allows us to use cmp.Diff to compare the types with field-level cmpopts. func xdsWithoutEqual(a *ir.Xds) any { ret := struct { - ReadyListener *ir.ReadyListener - AccessLog *ir.AccessLog - Tracing *ir.Tracing - Metrics *ir.Metrics - HTTP []*ir.HTTPListener - TCP []*ir.TCPListener - UDP []*ir.UDPListener - EnvoyPatchPolicies []*ir.EnvoyPatchPolicy - FilterOrder []egv1a1.FilterPosition - GlobalResources *ir.GlobalResources + ReadyListener *ir.ReadyListener + AccessLog *ir.AccessLog + Tracing *ir.Tracing + Metrics *ir.Metrics + HTTP []*ir.HTTPListener + TCP []*ir.TCPListener + UDP []*ir.UDPListener + EnvoyPatchPolicies []*ir.EnvoyPatchPolicy + FilterOrder []egv1a1.FilterPosition + GlobalResources *ir.GlobalResources + ExtensionServerPolicies []unstructured.Unstructured }{ - ReadyListener: a.ReadyListener, - AccessLog: a.AccessLog, - Tracing: a.Tracing, - Metrics: a.Metrics, - HTTP: a.HTTP, - TCP: a.TCP, - UDP: a.UDP, - EnvoyPatchPolicies: a.EnvoyPatchPolicies, - FilterOrder: a.FilterOrder, - GlobalResources: a.GlobalResources, + ReadyListener: a.ReadyListener, + AccessLog: a.AccessLog, + Tracing: a.Tracing, + Metrics: a.Metrics, + HTTP: a.HTTP, + TCP: a.TCP, + UDP: a.UDP, + EnvoyPatchPolicies: a.EnvoyPatchPolicies, + FilterOrder: a.FilterOrder, + GlobalResources: a.GlobalResources, + ExtensionServerPolicies: a.ExtensionServerPolicies, } // Ensure we didn't drop an exported field. diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 08cc1cb07f..8f2c21f1b1 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -153,6 +153,8 @@ type Xds struct { FilterOrder []egv1a1.FilterPosition `json:"filterOrder,omitempty" yaml:"filterOrder,omitempty"` // GlobalResources holds the global resources used by the Envoy, for example, the envoy client certificate and the OIDC HMAC secret GlobalResources *GlobalResources `json:"globalResources,omitempty" yaml:"globalResources,omitempty"` + // extensionServerPolicies is the intermediate representation of the ExtensionServerPolicy resource + ExtensionServerPolicies []unstructured.Unstructured `json:"extensionServerPolicies,omitempty" yaml:"extensionServerPolicies,omitempty"` } // Equal implements the Comparable interface used by watchable.DeepEqual to skip unnecessary updates. diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 5dda160daa..26fd5fcf90 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -13,6 +13,7 @@ import ( "github.com/envoyproxy/gateway/api/v1alpha1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -4091,6 +4092,13 @@ func (in *Xds) DeepCopyInto(out *Xds) { *out = new(GlobalResources) (*in).DeepCopyInto(*out) } + if in.ExtensionServerPolicies != nil { + in, out := &in.ExtensionServerPolicies, &out.ExtensionServerPolicies + *out = make([]unstructured.Unstructured, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Xds. diff --git a/internal/xds/translator/extension.go b/internal/xds/translator/extension.go index 77162e14f7..53948ba0b1 100644 --- a/internal/xds/translator/extension.go +++ b/internal/xds/translator/extension.go @@ -139,7 +139,7 @@ func processExtensionPostListenerHook(tCtx *types.ResourceVersionTable, xdsListe return nil } -func processExtensionPostTranslationHook(tCtx *types.ResourceVersionTable, em *extensionTypes.Manager) error { +func processExtensionPostTranslationHook(tCtx *types.ResourceVersionTable, em *extensionTypes.Manager, policies []unstructured.Unstructured) error { // Do nothing unless there is an extension manager if em == nil { return nil @@ -169,7 +169,11 @@ func processExtensionPostTranslationHook(tCtx *types.ResourceVersionTable, em *e oldSecrets[idx] = secret.(*tlsv3.Secret) } - newClusters, newSecrets, err := extensionInsertHookClient.PostTranslateModifyHook(oldClusters, oldSecrets) + unstructuredExtensionPolicies := make([]*unstructured.Unstructured, len(policies)) + for id, policy := range policies { + unstructuredExtensionPolicies[id] = policy.DeepCopy() + } + newClusters, newSecrets, err := extensionInsertHookClient.PostTranslateModifyHook(oldClusters, oldSecrets, unstructuredExtensionPolicies) if err != nil { return err } diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go index 79e59b39ad..b89731fc13 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -142,7 +142,7 @@ func (t *Translator) Translate(xdsIR *ir.Xds) (*types.ResourceVersionTable, erro // Check if an extension want to inject any clusters/secrets // If no extension exists (or it doesn't subscribe to this hook) then this is a quick no-op - if err := processExtensionPostTranslationHook(tCtx, t.ExtensionManager); err != nil { + if err := processExtensionPostTranslationHook(tCtx, t.ExtensionManager, xdsIR.ExtensionServerPolicies); err != nil { // If the extension server returns an error, and the extension server is not configured to fail open, // then propagate the error if !(*t.ExtensionManager).FailOpen() { diff --git a/proto/extension/context.pb.go b/proto/extension/context.pb.go index f39a09711e..707c782ace 100644 --- a/proto/extension/context.pb.go +++ b/proto/extension/context.pb.go @@ -174,9 +174,12 @@ func (x *PostHTTPListenerExtensionContext) GetExtensionResources() []*ExtensionR // breaking any clients that use the API // additional context information can be added to this message as more use-cases are discovered type PostTranslateExtensionContext struct { - state protoimpl.MessageState `protogen:"open.v1"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + state protoimpl.MessageState `protogen:"open.v1"` + // Resources/Policies introduced by the extension that were used as extension server + // policies targeting the clusters + ExtensionResources []*ExtensionResource `protobuf:"bytes,1,rep,name=extension_resources,json=extensionResources,proto3" json:"extension_resources,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *PostTranslateExtensionContext) Reset() { @@ -209,6 +212,13 @@ func (*PostTranslateExtensionContext) Descriptor() ([]byte, []int) { return file_proto_extension_context_proto_rawDescGZIP(), []int{3} } +func (x *PostTranslateExtensionContext) GetExtensionResources() []*ExtensionResource { + if x != nil { + return x.ExtensionResources + } + return nil +} + // ExtensionResource stores the data for a K8s API object referenced in an HTTPRouteFilter // extensionRef. It is constructed from an unstructured.Unstructured marshalled to JSON. An extension // can marshal the bytes from this resource back into an unstructured.Unstructured and then @@ -283,15 +293,20 @@ var file_proto_extension_context_proto_rawDesc = string([]byte{ 0x61, 0x79, 0x2e, 0x65, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x52, 0x12, 0x65, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, - 0x65, 0x73, 0x22, 0x1f, 0x0a, 0x1d, 0x50, 0x6f, 0x73, 0x74, 0x54, 0x72, 0x61, 0x6e, 0x73, 0x6c, + 0x65, 0x73, 0x22, 0x7b, 0x0a, 0x1d, 0x50, 0x6f, 0x73, 0x74, 0x54, 0x72, 0x61, 0x6e, 0x73, 0x6c, 0x61, 0x74, 0x65, 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x43, 0x6f, 0x6e, 0x74, - 0x65, 0x78, 0x74, 0x22, 0x42, 0x0a, 0x11, 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, - 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x12, 0x2d, 0x0a, 0x12, 0x75, 0x6e, 0x73, 0x74, - 0x72, 0x75, 0x63, 0x74, 0x75, 0x72, 0x65, 0x64, 0x5f, 0x62, 0x79, 0x74, 0x65, 0x73, 0x18, 0x01, - 0x20, 0x01, 0x28, 0x0c, 0x52, 0x11, 0x75, 0x6e, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, 0x75, 0x72, - 0x65, 0x64, 0x42, 0x79, 0x74, 0x65, 0x73, 0x42, 0x11, 0x5a, 0x0f, 0x70, 0x72, 0x6f, 0x74, 0x6f, - 0x2f, 0x65, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, - 0x6f, 0x33, + 0x65, 0x78, 0x74, 0x12, 0x5a, 0x0a, 0x13, 0x65, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, + 0x5f, 0x72, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, + 0x32, 0x29, 0x2e, 0x65, 0x6e, 0x76, 0x6f, 0x79, 0x67, 0x61, 0x74, 0x65, 0x77, 0x61, 0x79, 0x2e, + 0x65, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x2e, 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, + 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x52, 0x12, 0x65, 0x78, 0x74, + 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x73, 0x22, + 0x42, 0x0a, 0x11, 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x73, 0x6f, + 0x75, 0x72, 0x63, 0x65, 0x12, 0x2d, 0x0a, 0x12, 0x75, 0x6e, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, + 0x75, 0x72, 0x65, 0x64, 0x5f, 0x62, 0x79, 0x74, 0x65, 0x73, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, + 0x52, 0x11, 0x75, 0x6e, 0x73, 0x74, 0x72, 0x75, 0x63, 0x74, 0x75, 0x72, 0x65, 0x64, 0x42, 0x79, + 0x74, 0x65, 0x73, 0x42, 0x11, 0x5a, 0x0f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x65, 0x78, 0x74, + 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, }) var ( @@ -317,11 +332,12 @@ var file_proto_extension_context_proto_goTypes = []any{ var file_proto_extension_context_proto_depIdxs = []int32{ 4, // 0: envoygateway.extension.PostRouteExtensionContext.extension_resources:type_name -> envoygateway.extension.ExtensionResource 4, // 1: envoygateway.extension.PostHTTPListenerExtensionContext.extension_resources:type_name -> envoygateway.extension.ExtensionResource - 2, // [2:2] is the sub-list for method output_type - 2, // [2:2] is the sub-list for method input_type - 2, // [2:2] is the sub-list for extension type_name - 2, // [2:2] is the sub-list for extension extendee - 0, // [0:2] is the sub-list for field type_name + 4, // 2: envoygateway.extension.PostTranslateExtensionContext.extension_resources:type_name -> envoygateway.extension.ExtensionResource + 3, // [3:3] is the sub-list for method output_type + 3, // [3:3] is the sub-list for method input_type + 3, // [3:3] is the sub-list for extension type_name + 3, // [3:3] is the sub-list for extension extendee + 0, // [0:3] is the sub-list for field type_name } func init() { file_proto_extension_context_proto_init() } diff --git a/proto/extension/context.proto b/proto/extension/context.proto index cff232343c..6981c5ebab 100644 --- a/proto/extension/context.proto +++ b/proto/extension/context.proto @@ -43,7 +43,9 @@ message PostHTTPListenerExtensionContext { // breaking any clients that use the API // additional context information can be added to this message as more use-cases are discovered message PostTranslateExtensionContext { - + // Resources/Policies introduced by the extension that were used as extension server + // policies targeting the clusters + repeated ExtensionResource extension_resources = 1; } diff --git a/release-notes/current.yaml b/release-notes/current.yaml index e58dfcc562..d85af17303 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -1,4 +1,4 @@ -date: Pending +date: May 4, 2025 # Changes that are expected to cause an incompatibility with previous versions, such as deletions or modifications to existing APIs. breaking changes: | @@ -17,6 +17,7 @@ new features: | Added support for setting ownerreference to infra resources when enable gateway namespace mode. Added support for configuring hostname in active HTTP healthchecks. Added support for GatewayInfrastructure in gateway namespace mode. + Added support for using externsion server policies to in PostTranslateModify hook. bug fixes: | Handle integer zone annotation values From 430b7f23a498d5efe1067e5c9ed3e7a2fba82faa Mon Sep 17 00:00:00 2001 From: Ravi Verma Date: Thu, 29 May 2025 14:50:19 +0530 Subject: [PATCH 2/4] review comment fix Signed-off-by: Ravi Verma --- .../registry/extension_manager_test.go | 180 ++++++++++++++++++ internal/extension/registry/xds_hook.go | 12 +- internal/extension/types/hooks.go | 4 +- internal/gatewayapi/extensionserverpolicy.go | 8 +- .../extensionpolicy-tcp-listener.out.yaml | 92 ++++----- .../extensionpolicy-udp-listener.out.yaml | 92 ++++----- ...ionpolicy-with-valid-target-array.out.yaml | 146 +++++++------- ...extensionpolicy-with-valid-target.out.yaml | 92 ++++----- internal/gatewayapi/translator_test.go | 3 +- internal/ir/xds.go | 4 +- internal/ir/zz_generated.deepcopy.go | 9 +- internal/xds/translator/extension.go | 8 +- internal/xds/translator/translator_test.go | 133 +++++++++++++ release-notes/current.yaml | 4 +- 14 files changed, 559 insertions(+), 228 deletions(-) diff --git a/internal/extension/registry/extension_manager_test.go b/internal/extension/registry/extension_manager_test.go index fc63af64af..c11bb39292 100644 --- a/internal/extension/registry/extension_manager_test.go +++ b/internal/extension/registry/extension_manager_test.go @@ -9,6 +9,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "errors" "fmt" "math" "net" @@ -17,6 +18,7 @@ import ( "sync" "testing" + clusterv3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" "github.com/stretchr/testify/require" "google.golang.org/grpc" @@ -26,12 +28,14 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/utils/ptr" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/envoyproxy/gateway/internal/envoygateway" + "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/proto/extension" ) @@ -546,3 +550,179 @@ func Test_Integration_RetryPolicy_MaxAttempts(t *testing.T) { }) } } + +type clusterUpdateTestServer struct { + extension.UnimplementedEnvoyGatewayExtensionServer +} + +func getTargetRefKind(obj *unstructured.Unstructured) (string, error) { + targetRef, found, err := unstructured.NestedMap(obj.Object, "spec", "targetRef") + if err != nil || !found { + return "", errors.New("targetRef not found or error") + } + + kind, ok := targetRef["kind"].(string) + if !ok { + return "", errors.New("kind is not a string or missing in targetRef") + } + + return kind, nil +} + +func (s *clusterUpdateTestServer) PostTranslateModify(ctx context.Context, req *extension.PostTranslateModifyRequest) (*extension.PostTranslateModifyResponse, error) { + clusters := req.GetClusters() + if clusters == nil { + return &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + }, errors.New("No clusters found") + } + + if len(req.PostTranslateContext.ExtensionResources) == 0 { + return &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + }, errors.New("No policy found") + } + + for _, extensionResourceBytes := range req.PostTranslateContext.ExtensionResources { + extensionResource := unstructured.Unstructured{} + if err := extensionResource.UnmarshalJSON(extensionResourceBytes.UnstructuredBytes); err != nil { + return &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + }, err + } + + targetKind, err := getTargetRefKind(&extensionResource) + if err != nil || extensionResource.GetObjectKind().GroupVersionKind().Kind != "ExampleExtPolicy" || targetKind != "Gateway" { + return &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + }, errors.New("No matching policy found") + } + } + + ret := &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + } + + return ret, nil +} + +func Test_Integration_ClusterUpdateExtensionServer(t *testing.T) { + testCases := []struct { + name string + extensionPolicies []*ir.UnstructuredRef + errorExpected bool + }{ + { + name: "valid extension policy with targetRef", + extensionPolicies: []*ir.UnstructuredRef{ + { + Object: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "gateway.example.io/v1", + "kind": "ExampleExtPolicy", + "metadata": map[string]any{ + "name": "test", + "namespace": "test", + }, + "spec": map[string]any{ + "targetRef": map[string]any{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "test", + }, + "data": "some data", + }, + }, + }, + }, + }, + errorExpected: false, + }, + + { + name: "invalid extension policy - no target", + extensionPolicies: []*ir.UnstructuredRef{ + { + Object: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "gateway.example.io/v1alpha1", + "kind": "ExampleExtPolicy", + "metadata": map[string]any{ + "name": "test", + "namespace": "test", + }, + "spec": map[string]any{ + "data": "some data", + }, + }, + }, + }, + }, + errorExpected: true, + }, + { + name: "invalid extension policy - no spec", + extensionPolicies: []*ir.UnstructuredRef{ + { + Object: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "gateway.example.io/v1alpha1", + "kind": "ExampleExtPolicy", + "metadata": map[string]any{ + "name": "test", + "namespace": "test", + }, + }, + }, + }, + }, + errorExpected: true, + }, + } + + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + extManager := egv1a1.ExtensionManager{ + Hooks: &egv1a1.ExtensionHooks{ + XDSTranslator: &egv1a1.XDSTranslatorHooks{ + Post: []egv1a1.XDSTranslatorHook{ + egv1a1.XDSTranslation, + }, + }, + }, + Service: &egv1a1.ExtensionService{ + BackendEndpoint: egv1a1.BackendEndpoint{ + FQDN: &egv1a1.FQDNEndpoint{ + Hostname: "example.foo", + Port: 44344, + }, + }, + }, + } + + mgr, _, err := NewInMemoryManager(extManager, &clusterUpdateTestServer{}) + require.NoError(t, err) + + hook, err := mgr.GetPostXDSHookClient(egv1a1.XDSTranslation) + require.NoError(t, err) + require.NotNil(t, hook) + + _, _, err = hook.PostTranslateModifyHook( + []*clusterv3.Cluster{ + { + Name: "test-cluster", + }, + }, nil, tt.extensionPolicies) + + if (err != nil) != tt.errorExpected { + t.Errorf("PostRouteModifyHook() error = %v, errorExpected %v", err, tt.errorExpected) + return + } + }) + } +} diff --git a/internal/extension/registry/xds_hook.go b/internal/extension/registry/xds_hook.go index 51d75169df..976dac6faa 100644 --- a/internal/extension/registry/xds_hook.go +++ b/internal/extension/registry/xds_hook.go @@ -15,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/envoyproxy/gateway/internal/extension/types" + "github.com/envoyproxy/gateway/internal/ir" "github.com/envoyproxy/gateway/proto/extension" ) @@ -105,12 +106,17 @@ func (h *XDSHook) PostHTTPListenerModifyHook(l *listener.Listener, extensionReso return resp.Listener, nil } -func (h *XDSHook) PostTranslateModifyHook(clusters []*cluster.Cluster, secrets []*tls.Secret, extensionPolicies []*unstructured.Unstructured) ([]*cluster.Cluster, []*tls.Secret, error) { +func (h *XDSHook) PostTranslateModifyHook(clusters []*cluster.Cluster, secrets []*tls.Secret, extensionPolicies []*ir.UnstructuredRef) ([]*cluster.Cluster, []*tls.Secret, error) { // Make the request to the extension server // Take all of the unstructured resources for the extension and package them into bytes - extensionPoliciesBytes, err := translateUnstructuredToUnstructuredBytes(extensionPolicies) + unstructuredPolicies := make([]*unstructured.Unstructured, len(extensionPolicies)) + for i, policy := range extensionPolicies { + unstructuredPolicies[i] = policy.Object + } + // Convert the unstructured policies to bytes + extensionPoliciesBytes, err := translateUnstructuredToUnstructuredBytes(unstructuredPolicies) if err != nil { - return clusters, secrets, err + return nil, nil, err } ctx := context.Background() diff --git a/internal/extension/types/hooks.go b/internal/extension/types/hooks.go index d823a88cf7..21350696bf 100644 --- a/internal/extension/types/hooks.go +++ b/internal/extension/types/hooks.go @@ -11,6 +11,8 @@ import ( route "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" tls "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/envoyproxy/gateway/internal/ir" ) type XDSHookClient interface { @@ -41,5 +43,5 @@ type XDSHookClient interface { // An example of how this may be used is to inject a cluster that will be used by an ext_authz http filter created by the extension. // The list of clusters and secrets returned by the extension are used as the final list of all clusters and secrets // PostTranslateModifyHook is always executed when an extension is loaded - PostTranslateModifyHook([]*cluster.Cluster, []*tls.Secret, []*unstructured.Unstructured) ([]*cluster.Cluster, []*tls.Secret, error) + PostTranslateModifyHook([]*cluster.Cluster, []*tls.Secret, []*ir.UnstructuredRef) ([]*cluster.Cluster, []*tls.Secret, error) } diff --git a/internal/gatewayapi/extensionserverpolicy.go b/internal/gatewayapi/extensionserverpolicy.go index cd710f0048..70fe2f7e1f 100644 --- a/internal/gatewayapi/extensionserverpolicy.go +++ b/internal/gatewayapi/extensionserverpolicy.go @@ -67,9 +67,13 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst // unable to find a matching Gateway for policy continue } - // Appped policy extension server policy list for related gateway. + + // Append policy extension server policy list for related gateway. gatewayKey := t.getIRKey(gateway.Gateway) - xdsIR[gatewayKey].ExtensionServerPolicies = append(xdsIR[gatewayKey].ExtensionServerPolicies, *policy) + unstructuredPolicy := &ir.UnstructuredRef{ + Object: policy, + } + xdsIR[gatewayKey].ExtensionServerPolicies = append(xdsIR[gatewayKey].ExtensionServerPolicies, unstructuredPolicy) // Set conditions for translation if it got any if t.translateExtServerPolicyForGateway(policy, gateway, currTarget, xdsIR) { diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml index 42f1dbcadd..19259715e4 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml @@ -147,58 +147,60 @@ xdsIR: json: - path: /dev/stdout extensionServerPolicies: - - apiVersion: foo.example.io/v1alpha1 - kind: Bar - metadata: - name: test1 - namespace: envoy-gateway - spec: - data: attached to all listeners - targetRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - status: - ancestors: - - ancestorRef: + - object: + apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test1 + namespace: envoy-gateway + spec: + data: attached to all listeners + targetRef: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - - apiVersion: foo.example.io/v1alpha1 - kind: Bar - metadata: - name: test2 - namespace: envoy-gateway - spec: - data: attached only to listener on port 80 - targetRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - sectionName: tcp1 - status: - ancestors: - - ancestorRef: + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - object: + apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached only to listener on port 80 + targetRef: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - namespace: envoy-gateway sectionName: tcp1 - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: tcp1 + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller readyListener: address: 0.0.0.0 ipFamily: IPv4 diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml index 84fccff522..3065b88810 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml @@ -147,58 +147,60 @@ xdsIR: json: - path: /dev/stdout extensionServerPolicies: - - apiVersion: foo.example.io/v1alpha1 - kind: Bar - metadata: - name: test1 - namespace: envoy-gateway - spec: - data: attached to all listeners - targetRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - status: - ancestors: - - ancestorRef: + - object: + apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test1 + namespace: envoy-gateway + spec: + data: attached to all listeners + targetRef: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - - apiVersion: foo.example.io/v1alpha1 - kind: Bar - metadata: - name: test2 - namespace: envoy-gateway - spec: - data: attached only to listener on port 162 - targetRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - sectionName: udp1 - status: - ancestors: - - ancestorRef: + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - object: + apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached only to listener on port 162 + targetRef: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - namespace: envoy-gateway sectionName: udp1 - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: udp1 + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller readyListener: address: 0.0.0.0 ipFamily: IPv4 diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml index ce7bed03c1..b1d2c6b9a1 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml @@ -159,46 +159,47 @@ xdsIR: json: - path: /dev/stdout extensionServerPolicies: - - apiVersion: foo.example.io/v1alpha1 - kind: Bar - metadata: - name: test2 - namespace: envoy-gateway - spec: - data: attached to multiple gateways - targetRefs: - - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-2 - - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io + - object: + apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached to multiple gateways + targetRefs: + - group: gateway.networking.k8s.io kind: Gateway name: gateway-2 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - - ancestorRef: - group: gateway.networking.k8s.io + - group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller http: - address: 0.0.0.0 extensionRefs: @@ -266,46 +267,47 @@ xdsIR: json: - path: /dev/stdout extensionServerPolicies: - - apiVersion: foo.example.io/v1alpha1 - kind: Bar - metadata: - name: test2 - namespace: envoy-gateway - spec: - data: attached to multiple gateways - targetRefs: - - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-2 - - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io + - object: + apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached to multiple gateways + targetRefs: + - group: gateway.networking.k8s.io kind: Gateway name: gateway-2 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - - ancestorRef: - group: gateway.networking.k8s.io + - group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller http: - address: 0.0.0.0 extensionRefs: diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml index 30869890bb..b9a61c6945 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml @@ -151,58 +151,60 @@ xdsIR: json: - path: /dev/stdout extensionServerPolicies: - - apiVersion: foo.example.io/v1alpha1 - kind: Bar - metadata: - name: test1 - namespace: envoy-gateway - spec: - data: attached to all listeners - targetRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - status: - ancestors: - - ancestorRef: + - object: + apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test1 + namespace: envoy-gateway + spec: + data: attached to all listeners + targetRef: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - - apiVersion: foo.example.io/v1alpha1 - kind: Bar - metadata: - name: test2 - namespace: envoy-gateway - spec: - data: attached only to listener on port 80 - targetRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - sectionName: http2 - status: - ancestors: - - ancestorRef: + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller + - object: + apiVersion: foo.example.io/v1alpha1 + kind: Bar + metadata: + name: test2 + namespace: envoy-gateway + spec: + data: attached only to listener on port 80 + targetRef: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - namespace: envoy-gateway sectionName: http2 - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http2 + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller http: - address: 0.0.0.0 extensionRefs: diff --git a/internal/gatewayapi/translator_test.go b/internal/gatewayapi/translator_test.go index 4d62026073..4ae505cc6c 100644 --- a/internal/gatewayapi/translator_test.go +++ b/internal/gatewayapi/translator_test.go @@ -26,7 +26,6 @@ import ( corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" @@ -902,7 +901,7 @@ func xdsWithoutEqual(a *ir.Xds) any { EnvoyPatchPolicies []*ir.EnvoyPatchPolicy FilterOrder []egv1a1.FilterPosition GlobalResources *ir.GlobalResources - ExtensionServerPolicies []unstructured.Unstructured + ExtensionServerPolicies []*ir.UnstructuredRef }{ ReadyListener: a.ReadyListener, AccessLog: a.AccessLog, diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 8f2c21f1b1..c2c3c39cc5 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -153,8 +153,8 @@ type Xds struct { FilterOrder []egv1a1.FilterPosition `json:"filterOrder,omitempty" yaml:"filterOrder,omitempty"` // GlobalResources holds the global resources used by the Envoy, for example, the envoy client certificate and the OIDC HMAC secret GlobalResources *GlobalResources `json:"globalResources,omitempty" yaml:"globalResources,omitempty"` - // extensionServerPolicies is the intermediate representation of the ExtensionServerPolicy resource - ExtensionServerPolicies []unstructured.Unstructured `json:"extensionServerPolicies,omitempty" yaml:"extensionServerPolicies,omitempty"` + // ExtensionServerPolicies is the intermediate representation of the ExtensionServerPolicy resource + ExtensionServerPolicies []*UnstructuredRef `json:"extensionServerPolicies,omitempty" yaml:"extensionServerPolicies,omitempty"` } // Equal implements the Comparable interface used by watchable.DeepEqual to skip unnecessary updates. diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 26fd5fcf90..de924f1d52 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -13,7 +13,6 @@ import ( "github.com/envoyproxy/gateway/api/v1alpha1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/gateway-api/apis/v1alpha2" ) @@ -4094,9 +4093,13 @@ func (in *Xds) DeepCopyInto(out *Xds) { } if in.ExtensionServerPolicies != nil { in, out := &in.ExtensionServerPolicies, &out.ExtensionServerPolicies - *out = make([]unstructured.Unstructured, len(*in)) + *out = make([]*UnstructuredRef, len(*in)) for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(UnstructuredRef) + (*in).DeepCopyInto(*out) + } } } } diff --git a/internal/xds/translator/extension.go b/internal/xds/translator/extension.go index 53948ba0b1..771a4d7beb 100644 --- a/internal/xds/translator/extension.go +++ b/internal/xds/translator/extension.go @@ -139,7 +139,7 @@ func processExtensionPostListenerHook(tCtx *types.ResourceVersionTable, xdsListe return nil } -func processExtensionPostTranslationHook(tCtx *types.ResourceVersionTable, em *extensionTypes.Manager, policies []unstructured.Unstructured) error { +func processExtensionPostTranslationHook(tCtx *types.ResourceVersionTable, em *extensionTypes.Manager, policies []*ir.UnstructuredRef) error { // Do nothing unless there is an extension manager if em == nil { return nil @@ -169,11 +169,7 @@ func processExtensionPostTranslationHook(tCtx *types.ResourceVersionTable, em *e oldSecrets[idx] = secret.(*tlsv3.Secret) } - unstructuredExtensionPolicies := make([]*unstructured.Unstructured, len(policies)) - for id, policy := range policies { - unstructuredExtensionPolicies[id] = policy.DeepCopy() - } - newClusters, newSecrets, err := extensionInsertHookClient.PostTranslateModifyHook(oldClusters, oldSecrets, unstructuredExtensionPolicies) + newClusters, newSecrets, err := extensionInsertHookClient.PostTranslateModifyHook(oldClusters, oldSecrets, policies) if err != nil { return err } diff --git a/internal/xds/translator/translator_test.go b/internal/xds/translator/translator_test.go index 1ddd3d5ad2..5485447b52 100644 --- a/internal/xds/translator/translator_test.go +++ b/internal/xds/translator/translator_test.go @@ -6,7 +6,9 @@ package translator import ( + "context" "embed" + "errors" "path/filepath" "runtime" "sort" @@ -21,6 +23,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/yaml" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -32,6 +35,7 @@ import ( "github.com/envoyproxy/gateway/internal/utils/test" xtypes "github.com/envoyproxy/gateway/internal/xds/types" "github.com/envoyproxy/gateway/internal/xds/utils" + "github.com/envoyproxy/gateway/proto/extension" ) var ( @@ -217,6 +221,135 @@ func TestTranslateXds(t *testing.T) { } } +type clusterUpdateTestServer struct { + extension.UnimplementedEnvoyGatewayExtensionServer +} + +func getTargetRefKind(obj *unstructured.Unstructured) (string, error) { + targetRef, found, err := unstructured.NestedMap(obj.Object, "spec", "targetRef") + if err != nil || !found { + return "", errors.New("targetRef not found or error") + } + + kind, ok := targetRef["kind"].(string) + if !ok { + return "", errors.New("kind is not a string or missing in targetRef") + } + + return kind, nil +} + +func (s *clusterUpdateTestServer) PostTranslateModify(ctx context.Context, req *extension.PostTranslateModifyRequest) (*extension.PostTranslateModifyResponse, error) { + clusters := req.GetClusters() + if clusters == nil { + return &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + }, errors.New("No clusters found") + } + + if len(req.PostTranslateContext.ExtensionResources) == 0 { + return &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + }, errors.New("No policy found") + } + + for _, extensionResourceBytes := range req.PostTranslateContext.ExtensionResources { + extensionResource := unstructured.Unstructured{} + if err := extensionResource.UnmarshalJSON(extensionResourceBytes.UnstructuredBytes); err != nil { + return &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + }, err + } + + targetKind, err := getTargetRefKind(&extensionResource) + if err != nil || extensionResource.GetObjectKind().GroupVersionKind().Kind != "ExampleExtPolicy" || targetKind != "Gateway" { + return &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + }, errors.New("No matching policy found") + } + } + + ret := &extension.PostTranslateModifyResponse{ + Clusters: clusters, + Secrets: req.GetSecrets(), + } + + return ret, nil +} + +func TestTranslateXdsTranslateModify(t *testing.T) { + testConfigs := map[string]testFileConfig{ + "extension-server-policy": { + errMsg: "No clusters found", + }, + } + + t.Run("extension-server-policy", func(t *testing.T) { + cfg := testConfigs["extension-server-policy"] + + extManager := egv1a1.ExtensionManager{ + Hooks: &egv1a1.ExtensionHooks{ + XDSTranslator: &egv1a1.XDSTranslatorHooks{ + Post: []egv1a1.XDSTranslatorHook{ + egv1a1.XDSTranslation, + }, + }, + }, + Service: &egv1a1.ExtensionService{ + BackendEndpoint: egv1a1.BackendEndpoint{ + FQDN: &egv1a1.FQDNEndpoint{ + Hostname: "example.foo", + Port: 44344, + }, + }, + }, + } + + mgr, _, err := registry.NewInMemoryManager(extManager, &clusterUpdateTestServer{}) + if err != nil { + t.Fatalf("failed to create extension manager: %v", err) + } + + x := &ir.Xds{} + x.ExtensionServerPolicies = []*ir.UnstructuredRef{ + { + Object: &unstructured.Unstructured{ + Object: map[string]any{ + "apiVersion": "gateway.networking.k8s.io/v1", + "kind": "ExampleExtPolicy", + "metadata": map[string]any{ + "name": "ext-server-policy-test", + "namespace": "test", + }, + "spec": map[string]any{ + "targetRef": map[string]any{ + "group": "gateway.networking.k8s.io", + "kind": "Gateway", + "name": "test-gtw", + }, + "data": "some data", + }, + }, + }, + }, + } + tr := &Translator{ + ControllerNamespace: "envoy-gateway-system", + ExtensionManager: &mgr, + } + _, err = tr.Translate(x) + if len(cfg.errMsg) > 0 { + require.Error(t, err) + require.Contains(t, err.Error(), cfg.errMsg) + return + } + }) +} + func TestTranslateRateLimitConfig(t *testing.T) { inputFiles, err := filepath.Glob(filepath.Join("testdata", "in", "ratelimit-config", "*.yaml")) require.NoError(t, err) diff --git a/release-notes/current.yaml b/release-notes/current.yaml index d85af17303..62338fa9ed 100644 --- a/release-notes/current.yaml +++ b/release-notes/current.yaml @@ -1,4 +1,4 @@ -date: May 4, 2025 +date: Pending # Changes that are expected to cause an incompatibility with previous versions, such as deletions or modifications to existing APIs. breaking changes: | @@ -17,7 +17,7 @@ new features: | Added support for setting ownerreference to infra resources when enable gateway namespace mode. Added support for configuring hostname in active HTTP healthchecks. Added support for GatewayInfrastructure in gateway namespace mode. - Added support for using externsion server policies to in PostTranslateModify hook. + Added support for using extension server policies to in PostTranslateModify hook. bug fixes: | Handle integer zone annotation values From d24a52df20ca2fe3f93bfe09a6ba4166791c4a63 Mon Sep 17 00:00:00 2001 From: Ravi Verma Date: Wed, 4 Jun 2025 01:34:18 +0530 Subject: [PATCH 3/4] updated logic to store pointer of original extension server policies instead of creating another copy Signed-off-by: Ravi Verma --- internal/gatewayapi/extensionserverpolicy.go | 4 +- .../extensionpolicy-tcp-listener.out.yaml | 29 ----------- .../extensionpolicy-udp-listener.out.yaml | 29 ----------- ...ionpolicy-with-valid-target-array.out.yaml | 52 ------------------- ...extensionpolicy-with-valid-target.out.yaml | 29 ----------- 5 files changed, 3 insertions(+), 140 deletions(-) diff --git a/internal/gatewayapi/extensionserverpolicy.go b/internal/gatewayapi/extensionserverpolicy.go index 70fe2f7e1f..a60e5d0ff8 100644 --- a/internal/gatewayapi/extensionserverpolicy.go +++ b/internal/gatewayapi/extensionserverpolicy.go @@ -44,9 +44,11 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst } var errs error + policyIndex := -1 // Process the policies targeting Gateways. Only update the policy status if it was accepted. // A policy is considered accepted if at least one targetRef contained inside matched a listener. for _, policy := range policies { + policyIndex++ policy := policy.DeepCopy() var policyStatus gwapiv1a2.PolicyStatus accepted := false @@ -71,7 +73,7 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst // Append policy extension server policy list for related gateway. gatewayKey := t.getIRKey(gateway.Gateway) unstructuredPolicy := &ir.UnstructuredRef{ - Object: policy, + Object: &policies[policyIndex], } xdsIR[gatewayKey].ExtensionServerPolicies = append(xdsIR[gatewayKey].ExtensionServerPolicies, unstructuredPolicy) diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml index 19259715e4..fe833ea2ba 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-tcp-listener.out.yaml @@ -159,20 +159,6 @@ xdsIR: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - object: apiVersion: foo.example.io/v1alpha1 kind: Bar @@ -186,21 +172,6 @@ xdsIR: kind: Gateway name: gateway-1 sectionName: tcp1 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - namespace: envoy-gateway - sectionName: tcp1 - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller readyListener: address: 0.0.0.0 ipFamily: IPv4 diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml index 3065b88810..56e227eaba 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-udp-listener.out.yaml @@ -159,20 +159,6 @@ xdsIR: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - object: apiVersion: foo.example.io/v1alpha1 kind: Bar @@ -186,21 +172,6 @@ xdsIR: kind: Gateway name: gateway-1 sectionName: udp1 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - namespace: envoy-gateway - sectionName: udp1 - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller readyListener: address: 0.0.0.0 ipFamily: IPv4 diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml index b1d2c6b9a1..203efa8067 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target-array.out.yaml @@ -174,32 +174,6 @@ xdsIR: - group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-2 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller http: - address: 0.0.0.0 extensionRefs: @@ -282,32 +256,6 @@ xdsIR: - group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-2 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller http: - address: 0.0.0.0 extensionRefs: diff --git a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml index b9a61c6945..9dcb168f84 100644 --- a/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml +++ b/internal/gatewayapi/testdata/extensions/extensionpolicy-with-valid-target.out.yaml @@ -163,20 +163,6 @@ xdsIR: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - namespace: envoy-gateway - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller - object: apiVersion: foo.example.io/v1alpha1 kind: Bar @@ -190,21 +176,6 @@ xdsIR: kind: Gateway name: gateway-1 sectionName: http2 - status: - ancestors: - - ancestorRef: - group: gateway.networking.k8s.io - kind: Gateway - name: gateway-1 - namespace: envoy-gateway - sectionName: http2 - conditions: - - lastTransitionTime: null - message: Policy has been accepted. - reason: Accepted - status: "True" - type: Accepted - controllerName: gateway.envoyproxy.io/gatewayclass-controller http: - address: 0.0.0.0 extensionRefs: From 48d054b2e77d4c8763c3cd7ccb193d4e349e220e Mon Sep 17 00:00:00 2001 From: Ravi Verma Date: Thu, 5 Jun 2025 20:14:14 +0530 Subject: [PATCH 4/4] added test case to handle success and failure case of extension server policy Signed-off-by: Ravi Verma --- internal/gatewayapi/extensionserverpolicy.go | 4 +- .../xds/translator/extensionserver_test.go | 74 +++++++++ ...xtensionpolicy-extension-server-error.yaml | 13 ++ .../extensionpolicy-extension-server.yaml | 13 ++ ...olicy-extension-server-error.clusters.yaml | 1 + ...licy-extension-server-error.endpoints.yaml | 1 + ...licy-extension-server-error.listeners.yaml | 1 + ...npolicy-extension-server-error.routes.yaml | 1 + ...policy-extension-server-error.secrets.yaml | 4 + ...nsionpolicy-extension-server.clusters.yaml | 23 +++ ...sionpolicy-extension-server.endpoints.yaml | 1 + ...sionpolicy-extension-server.listeners.yaml | 1 + ...tensionpolicy-extension-server.routes.yaml | 1 + ...ensionpolicy-extension-server.secrets.yaml | 4 + internal/xds/translator/translator_test.go | 146 ++---------------- 15 files changed, 152 insertions(+), 136 deletions(-) create mode 100644 internal/xds/translator/testdata/in/extension-xds-ir/extensionpolicy-extension-server-error.yaml create mode 100644 internal/xds/translator/testdata/in/extension-xds-ir/extensionpolicy-extension-server.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.clusters.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.endpoints.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.listeners.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.routes.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.secrets.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.clusters.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.endpoints.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.listeners.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.routes.yaml create mode 100644 internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.secrets.yaml diff --git a/internal/gatewayapi/extensionserverpolicy.go b/internal/gatewayapi/extensionserverpolicy.go index a60e5d0ff8..e7f65f57a4 100644 --- a/internal/gatewayapi/extensionserverpolicy.go +++ b/internal/gatewayapi/extensionserverpolicy.go @@ -44,11 +44,9 @@ func (t *Translator) ProcessExtensionServerPolicies(policies []unstructured.Unst } var errs error - policyIndex := -1 // Process the policies targeting Gateways. Only update the policy status if it was accepted. // A policy is considered accepted if at least one targetRef contained inside matched a listener. - for _, policy := range policies { - policyIndex++ + for policyIndex, policy := range policies { policy := policy.DeepCopy() var policyStatus gwapiv1a2.PolicyStatus accepted := false diff --git a/internal/xds/translator/extensionserver_test.go b/internal/xds/translator/extensionserver_test.go index 0579b0ca33..fc1449bf05 100644 --- a/internal/xds/translator/extensionserver_test.go +++ b/internal/xds/translator/extensionserver_test.go @@ -18,6 +18,7 @@ import ( routeV3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" tlsV3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/anypb" "google.golang.org/protobuf/types/known/durationpb" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -264,6 +265,65 @@ func (t *testingExtensionServer) PostTranslateModify(_ context.Context, req *pb. }, }) + for _, extensionResourceBytes := range req.PostTranslateContext.ExtensionResources { + extensionResource := unstructured.Unstructured{} + if err := extensionResource.UnmarshalJSON(extensionResourceBytes.UnstructuredBytes); err != nil { + return response, err + } + + targetKind, err := getTargetRefKind(&extensionResource) + if err != nil { + return response, fmt.Errorf("failed to get targetRef kind: %w", err) + } + + if extensionResource.GetObjectKind().GroupVersionKind().Kind == "ExampleExtPolicy" && targetKind == "Gateway" { + upstreamTLS := &tlsV3.UpstreamTlsContext{ + CommonTlsContext: &tlsV3.CommonTlsContext{ + TlsCertificateSdsSecretConfigs: []*tlsV3.SdsSecretConfig{ + { + Name: "default", + SdsConfig: &coreV3.ConfigSource{ + ConfigSourceSpecifier: &coreV3.ConfigSource_ApiConfigSource{ + ApiConfigSource: &coreV3.ApiConfigSource{ + ApiType: coreV3.ApiConfigSource_GRPC, + GrpcServices: []*coreV3.GrpcService{ + { + TargetSpecifier: &coreV3.GrpcService_EnvoyGrpc_{ + EnvoyGrpc: &coreV3.GrpcService_EnvoyGrpc{ + ClusterName: "sds-cluster", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + typedConfig, err := anypb.New(upstreamTLS) + if err != nil { + return nil, err + } + + for _, cluster := range response.Clusters { + if cluster.Name == "mock-extension-injected-cluster" { + cluster.TransportSocket = &coreV3.TransportSocket{ + Name: "envoy.transport_sockets.tls", + ConfigType: &coreV3.TransportSocket_TypedConfig{ + TypedConfig: typedConfig, + }, + } + } + } + } else if extensionResource.GetObjectKind().GroupVersionKind().Kind == "ExampleExtPolicy" && targetKind != "Gateway" { + // This simulates an extension server that returns an error. It allows verifying that fail-close is working. + return response, fmt.Errorf("invalid extension policy : %s", extensionResource.GetName()) + } + } + for idx, secret := range req.Secrets { response.Secrets[idx] = proto.Clone(secret).(*tlsV3.Secret) } @@ -282,3 +342,17 @@ func (t *testingExtensionServer) PostTranslateModify(_ context.Context, req *pb. return response, nil } + +func getTargetRefKind(obj *unstructured.Unstructured) (string, error) { + targetRef, found, err := unstructured.NestedMap(obj.Object, "spec", "targetRef") + if err != nil || !found { + return "", errors.New("targetRef not found or error") + } + + kind, ok := targetRef["kind"].(string) + if !ok { + return "", errors.New("kind is not a string or missing in targetRef") + } + + return kind, nil +} diff --git a/internal/xds/translator/testdata/in/extension-xds-ir/extensionpolicy-extension-server-error.yaml b/internal/xds/translator/testdata/in/extension-xds-ir/extensionpolicy-extension-server-error.yaml new file mode 100644 index 0000000000..33f5d37949 --- /dev/null +++ b/internal/xds/translator/testdata/in/extension-xds-ir/extensionpolicy-extension-server-error.yaml @@ -0,0 +1,13 @@ +extensionServerPolicies: +- Object: + apiVersion: security.example.io/v1alpha1 + kind: ExampleExtPolicy + metadata: + name: ext-server-policy-invalid-test + namespace: test + spec: + data: attached to all clusters + targetRef: + group: gateway.networking.k8s.io + kind: Service + name: Service-1 diff --git a/internal/xds/translator/testdata/in/extension-xds-ir/extensionpolicy-extension-server.yaml b/internal/xds/translator/testdata/in/extension-xds-ir/extensionpolicy-extension-server.yaml new file mode 100644 index 0000000000..2ad12b9f3d --- /dev/null +++ b/internal/xds/translator/testdata/in/extension-xds-ir/extensionpolicy-extension-server.yaml @@ -0,0 +1,13 @@ +extensionServerPolicies: +- Object: + apiVersion: security.example.io/v1alpha1 + kind: ExampleExtPolicy + metadata: + name: ext-server-policy-test + namespace: test + spec: + data: attached to all clusters + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.clusters.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.clusters.yaml new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.clusters.yaml @@ -0,0 +1 @@ +[] diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.endpoints.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.endpoints.yaml new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.endpoints.yaml @@ -0,0 +1 @@ +[] diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.listeners.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.listeners.yaml new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.listeners.yaml @@ -0,0 +1 @@ +[] diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.routes.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.routes.yaml new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.routes.yaml @@ -0,0 +1 @@ +[] diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.secrets.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.secrets.yaml new file mode 100644 index 0000000000..b04034756d --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server-error.secrets.yaml @@ -0,0 +1,4 @@ +- genericSecret: + secret: + inlineString: super-secret-extension-secret + name: mock-extension-injected-secret diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.clusters.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.clusters.yaml new file mode 100644 index 0000000000..875b5acbaf --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.clusters.yaml @@ -0,0 +1,23 @@ +- loadAssignment: + clusterName: mock-extension-injected-cluster + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: exampleservice.examplenamespace.svc.cluster.local + portValue: 5000 + name: mock-extension-injected-cluster + transportSocket: + name: envoy.transport_sockets.tls + typedConfig: + '@type': type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext + commonTlsContext: + tlsCertificateSdsSecretConfigs: + - name: default + sdsConfig: + apiConfigSource: + apiType: GRPC + grpcServices: + - envoyGrpc: + clusterName: sds-cluster diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.endpoints.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.endpoints.yaml new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.endpoints.yaml @@ -0,0 +1 @@ +[] diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.listeners.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.listeners.yaml new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.listeners.yaml @@ -0,0 +1 @@ +[] diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.routes.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.routes.yaml new file mode 100644 index 0000000000..fe51488c70 --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.routes.yaml @@ -0,0 +1 @@ +[] diff --git a/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.secrets.yaml b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.secrets.yaml new file mode 100644 index 0000000000..b04034756d --- /dev/null +++ b/internal/xds/translator/testdata/out/extension-xds-ir/extensionpolicy-extension-server.secrets.yaml @@ -0,0 +1,4 @@ +- genericSecret: + secret: + inlineString: super-secret-extension-secret + name: mock-extension-injected-secret diff --git a/internal/xds/translator/translator_test.go b/internal/xds/translator/translator_test.go index 5485447b52..1c8c14e6f7 100644 --- a/internal/xds/translator/translator_test.go +++ b/internal/xds/translator/translator_test.go @@ -6,9 +6,7 @@ package translator import ( - "context" "embed" - "errors" "path/filepath" "runtime" "sort" @@ -23,7 +21,6 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/yaml" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -35,7 +32,6 @@ import ( "github.com/envoyproxy/gateway/internal/utils/test" xtypes "github.com/envoyproxy/gateway/internal/xds/types" "github.com/envoyproxy/gateway/internal/xds/utils" - "github.com/envoyproxy/gateway/proto/extension" ) var ( @@ -221,135 +217,6 @@ func TestTranslateXds(t *testing.T) { } } -type clusterUpdateTestServer struct { - extension.UnimplementedEnvoyGatewayExtensionServer -} - -func getTargetRefKind(obj *unstructured.Unstructured) (string, error) { - targetRef, found, err := unstructured.NestedMap(obj.Object, "spec", "targetRef") - if err != nil || !found { - return "", errors.New("targetRef not found or error") - } - - kind, ok := targetRef["kind"].(string) - if !ok { - return "", errors.New("kind is not a string or missing in targetRef") - } - - return kind, nil -} - -func (s *clusterUpdateTestServer) PostTranslateModify(ctx context.Context, req *extension.PostTranslateModifyRequest) (*extension.PostTranslateModifyResponse, error) { - clusters := req.GetClusters() - if clusters == nil { - return &extension.PostTranslateModifyResponse{ - Clusters: clusters, - Secrets: req.GetSecrets(), - }, errors.New("No clusters found") - } - - if len(req.PostTranslateContext.ExtensionResources) == 0 { - return &extension.PostTranslateModifyResponse{ - Clusters: clusters, - Secrets: req.GetSecrets(), - }, errors.New("No policy found") - } - - for _, extensionResourceBytes := range req.PostTranslateContext.ExtensionResources { - extensionResource := unstructured.Unstructured{} - if err := extensionResource.UnmarshalJSON(extensionResourceBytes.UnstructuredBytes); err != nil { - return &extension.PostTranslateModifyResponse{ - Clusters: clusters, - Secrets: req.GetSecrets(), - }, err - } - - targetKind, err := getTargetRefKind(&extensionResource) - if err != nil || extensionResource.GetObjectKind().GroupVersionKind().Kind != "ExampleExtPolicy" || targetKind != "Gateway" { - return &extension.PostTranslateModifyResponse{ - Clusters: clusters, - Secrets: req.GetSecrets(), - }, errors.New("No matching policy found") - } - } - - ret := &extension.PostTranslateModifyResponse{ - Clusters: clusters, - Secrets: req.GetSecrets(), - } - - return ret, nil -} - -func TestTranslateXdsTranslateModify(t *testing.T) { - testConfigs := map[string]testFileConfig{ - "extension-server-policy": { - errMsg: "No clusters found", - }, - } - - t.Run("extension-server-policy", func(t *testing.T) { - cfg := testConfigs["extension-server-policy"] - - extManager := egv1a1.ExtensionManager{ - Hooks: &egv1a1.ExtensionHooks{ - XDSTranslator: &egv1a1.XDSTranslatorHooks{ - Post: []egv1a1.XDSTranslatorHook{ - egv1a1.XDSTranslation, - }, - }, - }, - Service: &egv1a1.ExtensionService{ - BackendEndpoint: egv1a1.BackendEndpoint{ - FQDN: &egv1a1.FQDNEndpoint{ - Hostname: "example.foo", - Port: 44344, - }, - }, - }, - } - - mgr, _, err := registry.NewInMemoryManager(extManager, &clusterUpdateTestServer{}) - if err != nil { - t.Fatalf("failed to create extension manager: %v", err) - } - - x := &ir.Xds{} - x.ExtensionServerPolicies = []*ir.UnstructuredRef{ - { - Object: &unstructured.Unstructured{ - Object: map[string]any{ - "apiVersion": "gateway.networking.k8s.io/v1", - "kind": "ExampleExtPolicy", - "metadata": map[string]any{ - "name": "ext-server-policy-test", - "namespace": "test", - }, - "spec": map[string]any{ - "targetRef": map[string]any{ - "group": "gateway.networking.k8s.io", - "kind": "Gateway", - "name": "test-gtw", - }, - "data": "some data", - }, - }, - }, - }, - } - tr := &Translator{ - ControllerNamespace: "envoy-gateway-system", - ExtensionManager: &mgr, - } - _, err = tr.Translate(x) - if len(cfg.errMsg) > 0 { - require.Error(t, err) - require.Contains(t, err.Error(), cfg.errMsg) - return - } - }) -} - func TestTranslateRateLimitConfig(t *testing.T) { inputFiles, err := filepath.Glob(filepath.Join("testdata", "in", "ratelimit-config", "*.yaml")) require.NoError(t, err) @@ -421,6 +288,11 @@ func TestTranslateXdsWithExtensionErrorsWhenFailOpen(t *testing.T) { Version: "v1alpha1", Kind: "Bar", }, + { + Group: "security.example.io", + Version: "v1alpha1", + Kind: "ExampleExtPolicy", + }, }, Hooks: &egv1a1.ExtensionHooks{ XDSTranslator: &egv1a1.XDSTranslatorHooks{ @@ -490,6 +362,9 @@ func TestTranslateXdsWithExtensionErrorsWhenFailClosed(t *testing.T) { "multiple-listeners-same-port-error": { errMsg: "rpc error: code = Unknown desc = simulate error when there is no default filter chain in the original resources", }, + "extensionpolicy-extension-server-error": { + errMsg: "rpc error: code = Unknown desc = invalid extension policy : ext-server-policy-invalid-test", + }, } inputFiles, err := filepath.Glob(filepath.Join("testdata", "in", "extension-xds-ir", "*-error.yaml")) @@ -531,6 +406,11 @@ func TestTranslateXdsWithExtensionErrorsWhenFailClosed(t *testing.T) { Version: "v1alpha1", Kind: "Bar", }, + { + Group: "security.example.io", + Version: "v1alpha1", + Kind: "ExampleExtPolicy", + }, }, Hooks: &egv1a1.ExtensionHooks{ XDSTranslator: &egv1a1.XDSTranslatorHooks{