From b9baf83f06a7e12a914e800e8960aec5e8747b80 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 29 Jun 2023 10:16:20 -0700 Subject: [PATCH 01/10] Implement JSON Patch in Xds Translator Relates to https://github.com/envoyproxy/gateway/issues/24 Signed-off-by: Arko Dasgupta --- internal/ir/xds.go | 28 ++++ internal/ir/zz_generated.deepcopy.go | 42 +++++ internal/xds/translator/jsonpatch.go | 146 ++++++++++++++++++ .../testdata/in/xds-ir/jsonpatch.yaml | 33 ++++ .../out/xds-ir/jsonpatch.clusters.yaml | 13 ++ .../out/xds-ir/jsonpatch.endpoints.yaml | 10 ++ .../out/xds-ir/jsonpatch.listeners.yaml | 33 ++++ .../testdata/out/xds-ir/jsonpatch.routes.yaml | 23 +++ internal/xds/translator/translator.go | 77 ++++++--- internal/xds/translator/translator_test.go | 3 + 10 files changed, 388 insertions(+), 20 deletions(-) create mode 100644 internal/xds/translator/jsonpatch.go create mode 100644 internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/jsonpatch.clusters.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/jsonpatch.listeners.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 33381172f9..2b967e49e3 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -54,6 +54,9 @@ type Xds struct { TCP []*TCPListener // UDP Listeners exposed by the gateway. UDP []*UDPListener + // JSONPatches are the JSON Patches that + // are to be applied to generaed Xds linked to the gateway. + JSONPatches []*JSONPatchConfig } // Validate the fields within the Xds structure. @@ -792,3 +795,28 @@ type OpenTelemetryAccessLog struct { Port uint32 `json:"port"` Resources map[string]string `json:"resources"` } + +// JSONPatchConfig defines the configuration for patching a Envoy xDS Resource +// using JSONPatch semantics +// +k8s:deepcopy-gen=true +type JSONPatchConfig struct { + // Type is the typed URL of the Envoy xDS Resource + Type string `json:"type"` + // Name is the name of the resource + Name string `json:"name"` + // Patch defines the JSON Patch Operation + Operation JSONPatchOperation `json:"operation"` +} + +// JSONPatchOperation defines the JSON Patch Operation as defined in +// https://datatracker.ietf.org/doc/html/rfc6902 +// +k8s:deepcopy-gen=true +type JSONPatchOperation struct { + // Op is the type of operation to perform + Op string `json:"op"` + // Path is the location of the target document/field where the operation will be performed + // Refer to https://datatracker.ietf.org/doc/html/rfc6901 for more details. + Path string `json:"path"` + // Value is the new value of the path location. + Value string `json:"value"` +} diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index d67ca2655d..90697be6b2 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -376,6 +376,37 @@ func (in *JSONAccessLog) DeepCopy() *JSONAccessLog { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *JSONPatchConfig) DeepCopyInto(out *JSONPatchConfig) { + *out = *in + out.Operation = in.Operation +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new JSONPatchConfig. +func (in *JSONPatchConfig) DeepCopy() *JSONPatchConfig { + if in == nil { + return nil + } + out := new(JSONPatchConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *JSONPatchOperation) DeepCopyInto(out *JSONPatchOperation) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new JSONPatchOperation. +func (in *JSONPatchOperation) DeepCopy() *JSONPatchOperation { + if in == nil { + return nil + } + out := new(JSONPatchOperation) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JwtRequestAuthentication) DeepCopyInto(out *JwtRequestAuthentication) { *out = *in @@ -928,6 +959,17 @@ func (in *Xds) DeepCopyInto(out *Xds) { } } } + if in.JSONPatches != nil { + in, out := &in.JSONPatches, &out.JSONPatches + *out = make([]*JSONPatchConfig, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(JSONPatchConfig) + **out = **in + } + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Xds. diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go new file mode 100644 index 0000000000..8d654333e0 --- /dev/null +++ b/internal/xds/translator/jsonpatch.go @@ -0,0 +1,146 @@ +package translator + +import ( + "fmt" + clusterv3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" + endpointv3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3" + listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" + routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" + resourcev3 "github.com/envoyproxy/go-control-plane/pkg/resource/v3" + jsonpatchv5 "github.com/evanphx/json-patch/v5" + "github.com/tetratelabs/multierror" + "google.golang.org/protobuf/encoding/protojson" + "sigs.k8s.io/yaml" + + "github.com/envoyproxy/gateway/internal/ir" + _ "github.com/envoyproxy/gateway/internal/xds/extensions" // register the generated types to support protojson unmarshalling + "github.com/envoyproxy/gateway/internal/xds/types" +) + +// processJSONPatches applies each JSONPatch to the Xds Resources for a specific type. +func processJSONPatches(tCtx *types.ResourceVersionTable, JSONPatches []*ir.JSONPatchConfig) error { + var errs error + for _, p := range JSONPatches { + var ( + listener *listenerv3.Listener + routeConfig *routev3.RouteConfiguration + cluster *clusterv3.Cluster + endpoint *endpointv3.ClusterLoadAssignment + resourceJSON []byte + err error + ) + m := protojson.MarshalOptions{ + UseProtoNames: true, + } + // Find the resource to patch and convert it to JSON + switch p.Type { + case string(resourcev3.ListenerType): + if listener = findXdsListener(tCtx, p.Name); listener == nil { + err = fmt.Errorf("unable to find xds resource %s: %s", p.Type, p.Name) + errs = multierror.Append(errs, err) + continue + } + + if resourceJSON, err = m.Marshal(listener); err != nil { + err = fmt.Errorf("unable to marshal xds resource %s: %s, err:%v", p.Type, p.Name, err) + errs = multierror.Append(errs, err) + continue + } + + case string(resourcev3.RouteType): + if routeConfig = findXdsRouteConfig(tCtx, p.Name); routeConfig == nil { + err := fmt.Errorf("unable to find xds resource %s: %s", p.Type, p.Name) + errs = multierror.Append(errs, err) + continue + } + + if resourceJSON, err = m.Marshal(routeConfig); err != nil { + err = fmt.Errorf("unable to marshal xds resource %s: %s, err:%v", p.Type, p.Name, err) + errs = multierror.Append(errs, err) + continue + } + + case string(resourcev3.ClusterType): + if cluster := findXdsCluster(tCtx, p.Name); cluster == nil { + err := fmt.Errorf("unable to find xds resource %s: %s", p.Type, p.Name) + errs = multierror.Append(errs, err) + continue + } + + if resourceJSON, err = m.Marshal(cluster); err != nil { + err = fmt.Errorf("unable to marshal xds resource %s: %s, err:%v", p.Type, p.Name, err) + errs = multierror.Append(errs, err) + continue + } + case string(resourcev3.EndpointType): + endpoint = findXdsEndpoint(tCtx, p.Name) + if endpoint == nil { + err = fmt.Errorf("unable to marshal xds resource %s: %s, err:%v", p.Type, p.Name, err) + errs = multierror.Append(errs, err) + continue + } + if resourceJSON, err = m.Marshal(endpoint); err != nil { + err = fmt.Errorf("unable to marshal xds resource %s: %s, err:%v", p.Type, p.Name, err) + errs = multierror.Append(errs, err) + continue + } + } + + // Convert patch to JSON + // The patch library expects an array so convert it into one + y, err := yaml.Marshal([]ir.JSONPatchOperation{p.Operation}) + if err != nil { + err := fmt.Errorf("unable to marshal patch %+v, err: %v", p.Operation, err) + errs = multierror.Append(errs, err) + continue + } + jsonBytes, err := yaml.YAMLToJSON(y) + + patchObj, err := jsonpatchv5.DecodePatch(jsonBytes) + if err != nil { + err := fmt.Errorf("unable to decode patch %s, err: %v", string(jsonBytes), err) + errs = multierror.Append(errs, err) + continue + } + + // Apply patch + opts := jsonpatchv5.NewApplyOptions() + opts.EnsurePathExistsOnAdd = true + modifiedJSON, err := patchObj.ApplyWithOptions(resourceJSON, opts) + if err != nil { + err := fmt.Errorf("unable to apply patch:\n%s on resource:\n%s, err: %v", string(jsonBytes), string(resourceJSON), err) + errs = multierror.Append(errs, err) + continue + } + my, _ := yaml.JSONToYAML(modifiedJSON) + fmt.Println(string(my)) + // Unmarshal back to typed resource + switch p.Type { + case string(resourcev3.ListenerType): + if err = protojson.Unmarshal(modifiedJSON, listener); err != nil { + err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } + case string(resourcev3.RouteType): + if err = protojson.Unmarshal(modifiedJSON, routeConfig); err != nil { + err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } + case string(resourcev3.ClusterType): + if err = protojson.Unmarshal(modifiedJSON, cluster); err != nil { + err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } + case string(resourcev3.EndpointType): + if err = protojson.Unmarshal(modifiedJSON, endpoint); err != nil { + err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } + } + } + return errs +} diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml new file mode 100644 index 0000000000..4edfde6109 --- /dev/null +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml @@ -0,0 +1,33 @@ +jsonPatches: +- type: "type.googleapis.com/envoy.config.listener.v3.Listener" + name: "first-listener" + operation: + op: "add" + path: "/default_filter_chain/filters/0/typed_config/http_filters/0" + value: | + name: "envoy.filters.http.ratelimit" + typed_config: + "@type": "type.googleapis.com/envoy.extensions.filters.http.ratelimit.v3.RateLimit" + domain: "eg-ratelimit" + failure_mode_deny: true + timeout: 1s + rate_limit_service: + grpc_service: + envoy_grpc: + cluster_name: rate-limit-cluster + transport_api_version: V3' +http: +- name: "first-listener" + address: "0.0.0.0" + port: 10080 + hostnames: + - "*" + routes: + - name: "first-route" + headerMatches: + - name: user + stringMatch: + exact: "jason" + destinations: + - host: "1.2.3.4" + port: 50000 diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.clusters.yaml new file mode 100644 index 0000000000..2c908f7ef6 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.clusters.yaml @@ -0,0 +1,13 @@ +- commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: first-route + name: first-route + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml new file mode 100644 index 0000000000..f5f137f937 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml @@ -0,0 +1,10 @@ +- clusterName: first-route + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + locality: {} diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.listeners.yaml new file mode 100644 index 0000000000..73ee1b42ef --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.listeners.yaml @@ -0,0 +1,33 @@ +- address: + socketAddress: + address: 0.0.0.0 + portValue: 10080 + defaultFilterChain: + filters: + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + commonHttpProtocolOptions: + headersWithUnderscoresAction: REJECT_REQUEST + http2ProtocolOptions: + initialConnectionWindowSize: 1048576 + initialStreamWindowSize: 65536 + maxConcurrentStreams: 100 + httpFilters: + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + mergeSlashes: true + normalizePath: true + pathWithEscapedSlashesAction: UNESCAPE_AND_REDIRECT + rds: + configSource: + ads: {} + resourceApiVersion: V3 + routeConfigName: first-listener + statPrefix: http + upgradeConfigs: + - upgradeType: websocket + useRemoteAddress: true + name: first-listener + perConnectionBufferLimitBytes: 32768 diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml new file mode 100644 index 0000000000..757bdd9e3e --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml @@ -0,0 +1,23 @@ +- ignorePortInHostMatching: true + name: first-listener + virtualHosts: + - domains: + - '*' + name: first-listener + routes: + - match: + headers: + - name: user + stringMatch: + exact: jason + - name: test + stringMatch: + suffix: end + prefix: / + queryParameters: + - name: debug + stringMatch: + exact: "yes" + name: first-route + route: + cluster: first-route diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go index 9b364dc774..185e2aa9e2 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -11,6 +11,7 @@ import ( clusterv3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" + endpointv3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3" listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" resourcev3 "github.com/envoyproxy/go-control-plane/pkg/resource/v3" @@ -58,6 +59,10 @@ func (t *Translator) Translate(ir *ir.Xds) (*types.ResourceVersionTable, error) return nil, err } + if err := processJSONPatches(tCtx, ir.JSONPatches); err != nil { + return nil, err + } + processClusterForAccessLog(tCtx, ir.AccessLog) // Check if an extension want to inject any clusters/secrets @@ -75,7 +80,7 @@ func (t *Translator) processHTTPListenerXdsTranslation(tCtx *types.ResourceVersi var xdsRouteCfg *routev3.RouteConfiguration // Search for an existing listener, if it does not exist, create one. - xdsListener := findXdsListener(tCtx, httpListener.Address, httpListener.Port, corev3.SocketAddress_TCP) + xdsListener := findXdsListenerByAddress(tCtx, httpListener.Address, httpListener.Port, corev3.SocketAddress_TCP) if xdsListener == nil { xdsListener = buildXdsTCPListener(httpListener.Name, httpListener.Address, httpListener.Port, accesslog) tCtx.AddXdsResource(resourcev3.ListenerType, xdsListener) @@ -217,7 +222,7 @@ func processTCPListenerXdsTranslation(tCtx *types.ResourceVersionTable, tcpListe } } // Search for an existing listener, if it does not exist, create one. - xdsListener := findXdsListener(tCtx, tcpListener.Address, tcpListener.Port, corev3.SocketAddress_TCP) + xdsListener := findXdsListenerByAddress(tCtx, tcpListener.Address, tcpListener.Port, corev3.SocketAddress_TCP) if xdsListener == nil { xdsListener = buildXdsTCPListener(tcpListener.Name, tcpListener.Address, tcpListener.Port, accesslog) tCtx.AddXdsResource(resourcev3.ListenerType, xdsListener) @@ -253,8 +258,8 @@ func processUDPListenerXdsTranslation(tCtx *types.ResourceVersionTable, udpListe } -// findXdsListener finds a xds listener with the same address, port and protocol, and returns nil if there is no match. -func findXdsListener(tCtx *types.ResourceVersionTable, address string, port uint32, +// findXdsListenerByAddress finds a xds listener with the same address, port and protocol, and returns nil if there is no match. +func findXdsListenerByAddress(tCtx *types.ResourceVersionTable, address string, port uint32, protocol corev3.SocketAddress_Protocol) *listenerv3.Listener { if tCtx == nil || tCtx.XdsResources == nil || tCtx.XdsResources[resourcev3.ListenerType] == nil { return nil @@ -272,6 +277,38 @@ func findXdsListener(tCtx *types.ResourceVersionTable, address string, port uint return nil } +// findXdsListener finds a xds listener with the same name and returns nil if there is no match. +func findXdsListener(tCtx *types.ResourceVersionTable, name string) *listenerv3.Listener { + if tCtx == nil || tCtx.XdsResources == nil || tCtx.XdsResources[resourcev3.ListenerType] == nil { + return nil + } + + for _, r := range tCtx.XdsResources[resourcev3.ListenerType] { + listener := r.(*listenerv3.Listener) + if listener.Name == name { + return listener + } + } + + return nil +} + +// findXdsRouteConfig finds an xds route with the name and returns nil if there is no match. +func findXdsRouteConfig(tCtx *types.ResourceVersionTable, name string) *routev3.RouteConfiguration { + if tCtx == nil || tCtx.XdsResources == nil || tCtx.XdsResources[resourcev3.RouteType] == nil { + return nil + } + + for _, r := range tCtx.XdsResources[resourcev3.RouteType] { + route := r.(*routev3.RouteConfiguration) + if route.Name == name { + return route + } + } + + return nil +} + // findXdsCluster finds a xds cluster with the same name, and returns nil if there is no match. func findXdsCluster(tCtx *types.ResourceVersionTable, name string) *clusterv3.Cluster { if tCtx == nil || tCtx.XdsResources == nil || tCtx.XdsResources[resourcev3.ClusterType] == nil { @@ -288,6 +325,22 @@ func findXdsCluster(tCtx *types.ResourceVersionTable, name string) *clusterv3.Cl return nil } +// findXdsEndpoint finds a xds endpoint with the same cluster name, and returns nil if there is no match. +func findXdsEndpoint(tCtx *types.ResourceVersionTable, name string) *endpointv3.ClusterLoadAssignment { + if tCtx == nil || tCtx.XdsResources == nil || tCtx.XdsResources[resourcev3.EndpointType] == nil { + return nil + } + + for _, r := range tCtx.XdsResources[resourcev3.EndpointType] { + endpoint := r.(*endpointv3.ClusterLoadAssignment) + if endpoint.ClusterName == name { + return endpoint + } + } + + return nil +} + func addXdsCluster(tCtx *types.ResourceVersionTable, args addXdsClusterArgs) { xdsCluster := buildXdsCluster(args.name, args.tSocket, args.protocol, args.endpoint) xdsEndpoints := buildXdsClusterLoadAssignment(args.name, args.destinations) @@ -300,22 +353,6 @@ func addXdsCluster(tCtx *types.ResourceVersionTable, args addXdsClusterArgs) { tCtx.AddXdsResource(resourcev3.ClusterType, xdsCluster) } -// findXdsRouteConfig finds an xds route with the name and returns nil if there is no match. -func findXdsRouteConfig(tCtx *types.ResourceVersionTable, name string) *routev3.RouteConfiguration { - if tCtx == nil || tCtx.XdsResources == nil || tCtx.XdsResources[resourcev3.RouteType] == nil { - return nil - } - - for _, r := range tCtx.XdsResources[resourcev3.RouteType] { - route := r.(*routev3.RouteConfiguration) - if route.Name == name { - return route - } - } - - return nil -} - type addXdsClusterArgs struct { name string destinations []*ir.RouteDestination diff --git a/internal/xds/translator/translator_test.go b/internal/xds/translator/translator_test.go index 2050201a1f..cff7c49bd5 100644 --- a/internal/xds/translator/translator_test.go +++ b/internal/xds/translator/translator_test.go @@ -148,6 +148,9 @@ func TestTranslateXds(t *testing.T) { { name: "accesslog", }, + { + name: "jsonpatch", + }, } for _, tc := range testCases { From 019bf45ca2e29587d8493d068a3129bbb3ccb3ab Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 29 Jun 2023 16:44:48 -0700 Subject: [PATCH 02/10] use temp variable to unmarshal into Signed-off-by: Arko Dasgupta --- go.mod | 2 +- internal/xds/translator/jsonpatch.go | 41 +++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 555058ca9f..6ac26b12e4 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/davecgh/go-spew v1.1.1 github.com/envoyproxy/go-control-plane v0.11.1 github.com/envoyproxy/ratelimit v1.4.1-0.20230427142404-e2a87f41d3a7 + github.com/evanphx/json-patch/v5 v5.6.0 github.com/go-logr/logr v1.2.4 github.com/go-logr/zapr v1.2.4 github.com/golang/protobuf v1.5.3 @@ -45,7 +46,6 @@ require ( github.com/emicklei/go-restful/v3 v3.9.0 // indirect github.com/envoyproxy/protoc-gen-validate v1.0.1 // indirect github.com/evanphx/json-patch v4.12.0+incompatible // indirect - github.com/evanphx/json-patch/v5 v5.6.0 // indirect github.com/exponent-io/jsonpath v0.0.0-20151013193312-d6023ce2651d // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-asn1-ber/asn1-ber v1.5.4 // indirect diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go index 8d654333e0..ee4797ac46 100644 --- a/internal/xds/translator/jsonpatch.go +++ b/internal/xds/translator/jsonpatch.go @@ -112,34 +112,67 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, JSONPatches []*ir.JSON errs = multierror.Append(errs, err) continue } + my, _ := yaml.JSONToYAML(modifiedJSON) fmt.Println(string(my)) + // Unmarshal back to typed resource + // Use a temp staging variable that can be marshalled + // into and validated before saving it into the xds output resource switch p.Type { case string(resourcev3.ListenerType): - if err = protojson.Unmarshal(modifiedJSON, listener); err != nil { + temp := listenerv3.Listener{} + if err = protojson.Unmarshal(modifiedJSON, &temp); err != nil { err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) errs = multierror.Append(errs, err) continue } + if err = temp.Validate(); err != nil { + err := fmt.Errorf("validation failed for xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } + *listener = temp case string(resourcev3.RouteType): - if err = protojson.Unmarshal(modifiedJSON, routeConfig); err != nil { + temp := routev3.RouteConfiguration{} + if err = protojson.Unmarshal(modifiedJSON, &temp); err != nil { err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) errs = multierror.Append(errs, err) continue } + if err = temp.Validate(); err != nil { + err := fmt.Errorf("validation failed for xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } + + *routeConfig = temp case string(resourcev3.ClusterType): - if err = protojson.Unmarshal(modifiedJSON, cluster); err != nil { + temp := clusterv3.Cluster{} + if err = protojson.Unmarshal(modifiedJSON, &temp); err != nil { err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) errs = multierror.Append(errs, err) continue } + if err = temp.Validate(); err != nil { + err := fmt.Errorf("validation failed for xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } + *cluster = temp case string(resourcev3.EndpointType): - if err = protojson.Unmarshal(modifiedJSON, endpoint); err != nil { + temp := endpointv3.ClusterLoadAssignment{} + if err = protojson.Unmarshal(modifiedJSON, &temp); err != nil { err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) errs = multierror.Append(errs, err) continue } + if err = temp.Validate(); err != nil { + err := fmt.Errorf("validation failed for xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } + *endpoint = temp } } return errs From 74ea031a977a5949dbe774cc3a262cc99feee4e9 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 29 Jun 2023 17:19:52 -0700 Subject: [PATCH 03/10] lint Signed-off-by: Arko Dasgupta --- internal/xds/translator/jsonpatch.go | 56 ++++++++++++++----- .../testdata/in/xds-ir/jsonpatch.yaml | 4 +- 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go index ee4797ac46..60abf1fdb7 100644 --- a/internal/xds/translator/jsonpatch.go +++ b/internal/xds/translator/jsonpatch.go @@ -1,7 +1,13 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + package translator import ( "fmt" + clusterv3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" endpointv3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3" listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" @@ -18,9 +24,9 @@ import ( ) // processJSONPatches applies each JSONPatch to the Xds Resources for a specific type. -func processJSONPatches(tCtx *types.ResourceVersionTable, JSONPatches []*ir.JSONPatchConfig) error { +func processJSONPatches(tCtx *types.ResourceVersionTable, jsonPatches []*ir.JSONPatchConfig) error { var errs error - for _, p := range JSONPatches { + for _, p := range jsonPatches { var ( listener *listenerv3.Listener routeConfig *routev3.RouteConfiguration @@ -95,6 +101,11 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, JSONPatches []*ir.JSON continue } jsonBytes, err := yaml.YAMLToJSON(y) + if err != nil { + err := fmt.Errorf("unable to convert patch to json %s, err: %v", string(y), err) + errs = multierror.Append(errs, err) + continue + } patchObj, err := jsonpatchv5.DecodePatch(jsonBytes) if err != nil { @@ -121,8 +132,8 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, JSONPatches []*ir.JSON // into and validated before saving it into the xds output resource switch p.Type { case string(resourcev3.ListenerType): - temp := listenerv3.Listener{} - if err = protojson.Unmarshal(modifiedJSON, &temp); err != nil { + temp := &listenerv3.Listener{} + if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) errs = multierror.Append(errs, err) continue @@ -132,10 +143,14 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, JSONPatches []*ir.JSON errs = multierror.Append(errs, err) continue } - *listener = temp + if err = deepCopyPtr(temp, listener); err != nil { + err := fmt.Errorf("unable to copy xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } case string(resourcev3.RouteType): - temp := routev3.RouteConfiguration{} - if err = protojson.Unmarshal(modifiedJSON, &temp); err != nil { + temp := &routev3.RouteConfiguration{} + if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) errs = multierror.Append(errs, err) continue @@ -145,11 +160,14 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, JSONPatches []*ir.JSON errs = multierror.Append(errs, err) continue } - - *routeConfig = temp + if err = deepCopyPtr(temp, routeConfig); err != nil { + err := fmt.Errorf("unable to copy xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } case string(resourcev3.ClusterType): - temp := clusterv3.Cluster{} - if err = protojson.Unmarshal(modifiedJSON, &temp); err != nil { + temp := &clusterv3.Cluster{} + if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) errs = multierror.Append(errs, err) continue @@ -159,10 +177,14 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, JSONPatches []*ir.JSON errs = multierror.Append(errs, err) continue } - *cluster = temp + if err = deepCopyPtr(temp, cluster); err != nil { + err := fmt.Errorf("unable to copy xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } case string(resourcev3.EndpointType): - temp := endpointv3.ClusterLoadAssignment{} - if err = protojson.Unmarshal(modifiedJSON, &temp); err != nil { + temp := &endpointv3.ClusterLoadAssignment{} + if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { err := fmt.Errorf("unable to unmarshal xds resource %s, err:%v", string(modifiedJSON), err) errs = multierror.Append(errs, err) continue @@ -172,7 +194,11 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, JSONPatches []*ir.JSON errs = multierror.Append(errs, err) continue } - *endpoint = temp + if err = deepCopyPtr(temp, endpoint); err != nil { + err := fmt.Errorf("unable to copy xds resource %s, err:%v", string(modifiedJSON), err) + errs = multierror.Append(errs, err) + continue + } } } return errs diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml index 4edfde6109..709afa52ab 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml @@ -3,8 +3,8 @@ jsonPatches: name: "first-listener" operation: op: "add" - path: "/default_filter_chain/filters/0/typed_config/http_filters/0" - value: | + path: "/default_filter_chain/filters/0/typed_config/http_filters/0" + value: | name: "envoy.filters.http.ratelimit" typed_config: "@type": "type.googleapis.com/envoy.extensions.filters.http.ratelimit.v3.RateLimit" From 631dace881925f52a7461bd1ab4b18085bec28cb Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 29 Jun 2023 20:25:14 -0700 Subject: [PATCH 04/10] fix test Signed-off-by: Arko Dasgupta --- internal/ir/xds.go | 2 +- internal/ir/zz_generated.deepcopy.go | 4 ++-- internal/xds/translator/jsonpatch.go | 4 ++-- .../xds/translator/testdata/in/xds-ir/jsonpatch.yaml | 4 ++-- .../testdata/out/xds-ir/jsonpatch.listeners.yaml | 11 +++++++++++ .../testdata/out/xds-ir/jsonpatch.routes.yaml | 7 ------- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 2b967e49e3..5f472d830f 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -818,5 +818,5 @@ type JSONPatchOperation struct { // Refer to https://datatracker.ietf.org/doc/html/rfc6901 for more details. Path string `json:"path"` // Value is the new value of the path location. - Value string `json:"value"` + Value interface{} `json:"value"` } diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 90697be6b2..acd52f9968 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -379,7 +379,7 @@ func (in *JSONAccessLog) DeepCopy() *JSONAccessLog { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JSONPatchConfig) DeepCopyInto(out *JSONPatchConfig) { *out = *in - out.Operation = in.Operation + in.Operation.DeepCopyInto(&out.Operation) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new JSONPatchConfig. @@ -966,7 +966,7 @@ func (in *Xds) DeepCopyInto(out *Xds) { if (*in)[i] != nil { in, out := &(*in)[i], &(*out)[i] *out = new(JSONPatchConfig) - **out = **in + (*in).DeepCopyInto(*out) } } } diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go index 60abf1fdb7..791232217d 100644 --- a/internal/xds/translator/jsonpatch.go +++ b/internal/xds/translator/jsonpatch.go @@ -124,8 +124,8 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, jsonPatches []*ir.JSON continue } - my, _ := yaml.JSONToYAML(modifiedJSON) - fmt.Println(string(my)) + //my, _ := yaml.JSONToYAML(modifiedJSON) + fmt.Println(string(modifiedJSON)) // Unmarshal back to typed resource // Use a temp staging variable that can be marshalled diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml index 709afa52ab..24ae9a5645 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml @@ -4,7 +4,7 @@ jsonPatches: operation: op: "add" path: "/default_filter_chain/filters/0/typed_config/http_filters/0" - value: | + value: name: "envoy.filters.http.ratelimit" typed_config: "@type": "type.googleapis.com/envoy.extensions.filters.http.ratelimit.v3.RateLimit" @@ -15,7 +15,7 @@ jsonPatches: grpc_service: envoy_grpc: cluster_name: rate-limit-cluster - transport_api_version: V3' + transport_api_version: V3 http: - name: "first-listener" address: "0.0.0.0" diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.listeners.yaml index 73ee1b42ef..f99ed72707 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.listeners.yaml @@ -14,6 +14,17 @@ initialStreamWindowSize: 65536 maxConcurrentStreams: 100 httpFilters: + - name: envoy.filters.http.ratelimit + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.ratelimit.v3.RateLimit + domain: eg-ratelimit + failureModeDeny: true + rateLimitService: + grpcService: + envoyGrpc: + clusterName: rate-limit-cluster + transportApiVersion: V3 + timeout: 1s - name: envoy.filters.http.router typedConfig: '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml index 757bdd9e3e..60009c41c8 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml @@ -10,14 +10,7 @@ - name: user stringMatch: exact: jason - - name: test - stringMatch: - suffix: end prefix: / - queryParameters: - - name: debug - stringMatch: - exact: "yes" name: first-route route: cluster: first-route From 5fca2d9de7d4a38fdd26b8c8dc901023f38b9a17 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 29 Jun 2023 20:44:52 -0700 Subject: [PATCH 05/10] use apiextensionsv1.JSON Signed-off-by: Arko Dasgupta --- go.mod | 2 +- internal/ir/xds.go | 3 ++- internal/ir/zz_generated.deepcopy.go | 1 + internal/xds/translator/jsonpatch.go | 3 --- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 6ac26b12e4..6b6c4014b8 100644 --- a/go.mod +++ b/go.mod @@ -26,6 +26,7 @@ require ( google.golang.org/protobuf v1.30.0 gopkg.in/yaml.v3 v3.0.1 k8s.io/api v0.27.3 + k8s.io/apiextensions-apiserver v0.27.2 k8s.io/apimachinery v0.27.3 k8s.io/cli-runtime v0.27.3 k8s.io/client-go v0.27.3 @@ -104,7 +105,6 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20230530153820-e85fd2cbaebc // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - k8s.io/apiextensions-apiserver v0.27.2 // indirect k8s.io/component-base v0.27.3 // indirect k8s.io/klog/v2 v2.100.1 // indirect k8s.io/kube-openapi v0.0.0-20230501164219-8b0f38b5fd1f // indirect diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 5f472d830f..ddc91492f9 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -11,6 +11,7 @@ import ( "github.com/tetratelabs/multierror" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -818,5 +819,5 @@ type JSONPatchOperation struct { // Refer to https://datatracker.ietf.org/doc/html/rfc6901 for more details. Path string `json:"path"` // Value is the new value of the path location. - Value interface{} `json:"value"` + Value apiextensionsv1.JSON `json:"value"` } diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index acd52f9968..8bc0dd89ae 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -395,6 +395,7 @@ func (in *JSONPatchConfig) DeepCopy() *JSONPatchConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JSONPatchOperation) DeepCopyInto(out *JSONPatchOperation) { *out = *in + in.Value.DeepCopyInto(&out.Value) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new JSONPatchOperation. diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go index 791232217d..50a41ddf53 100644 --- a/internal/xds/translator/jsonpatch.go +++ b/internal/xds/translator/jsonpatch.go @@ -124,9 +124,6 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, jsonPatches []*ir.JSON continue } - //my, _ := yaml.JSONToYAML(modifiedJSON) - fmt.Println(string(modifiedJSON)) - // Unmarshal back to typed resource // Use a temp staging variable that can be marshalled // into and validated before saving it into the xds output resource From b323f060cc06512c9ef930cb1aaa375def48d6b7 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Fri, 30 Jun 2023 00:26:27 -0700 Subject: [PATCH 06/10] routeConfig test Signed-off-by: Arko Dasgupta --- internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml | 8 ++++++++ .../translator/testdata/out/xds-ir/jsonpatch.routes.yaml | 3 +++ 2 files changed, 11 insertions(+) diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml index 24ae9a5645..77a9be23f1 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml @@ -16,6 +16,14 @@ jsonPatches: envoy_grpc: cluster_name: rate-limit-cluster transport_api_version: V3 +- type: "type.googleapis.com/envoy.config.route.v3.RouteConfiguration" + name: "first-listener" + operation: + op: add + path: "/virtual_hosts/0/rate_limits" + value: + - actions: + - remote_address: {} http: - name: "first-listener" address: "0.0.0.0" diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml index 60009c41c8..0482acadb0 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.routes.yaml @@ -4,6 +4,9 @@ - domains: - '*' name: first-listener + rateLimits: + - actions: + - remoteAddress: {} routes: - match: headers: From 5b91f56f2f7b66a5bfbdbd3805df3ea026d466d2 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Fri, 30 Jun 2023 12:05:49 -0700 Subject: [PATCH 07/10] add entire resource and more test cases Signed-off-by: Arko Dasgupta --- internal/xds/translator/jsonpatch.go | 77 +++++++++++++++++++ .../testdata/in/xds-ir/jsonpatch.yaml | 28 ++++++- .../out/xds-ir/jsonpatch.clusters.yaml | 13 ++++ .../out/xds-ir/jsonpatch.endpoints.yaml | 2 +- 4 files changed, 118 insertions(+), 2 deletions(-) diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go index 50a41ddf53..5a98fd0db4 100644 --- a/internal/xds/translator/jsonpatch.go +++ b/internal/xds/translator/jsonpatch.go @@ -38,6 +38,83 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, jsonPatches []*ir.JSON m := protojson.MarshalOptions{ UseProtoNames: true, } + + // If Path is "" and op is "add", unmarshal and add the patch as a complete + // resource + if p.Operation.Op == "add" && p.Operation.Path == "" { + // Convert patch to JSON + // The patch library expects an array so convert it into one + y, err := yaml.Marshal(p.Operation.Value) + if err != nil { + err := fmt.Errorf("unable to marshal patch %+v, err: %v", p.Operation.Value, err) + errs = multierror.Append(errs, err) + continue + } + jsonBytes, err := yaml.YAMLToJSON(y) + if err != nil { + err := fmt.Errorf("unable to convert patch to json %s, err: %v", string(y), err) + errs = multierror.Append(errs, err) + continue + } + switch p.Type { + case string(resourcev3.ListenerType): + temp := &listenerv3.Listener{} + if err = protojson.Unmarshal(jsonBytes, temp); err != nil { + err := fmt.Errorf("unable to unmarshal xds resource %+v, err:%v", p.Operation.Value, err) + errs = multierror.Append(errs, err) + continue + } + if err = temp.Validate(); err != nil { + err := fmt.Errorf("validation failed for xds resource %+v, err:%v", p.Operation.Value, err) + errs = multierror.Append(errs, err) + continue + } + + tCtx.AddXdsResource(resourcev3.ListenerType, temp) + case string(resourcev3.RouteType): + temp := &routev3.RouteConfiguration{} + if err = protojson.Unmarshal(jsonBytes, temp); err != nil { + err := fmt.Errorf("unable to unmarshal xds resource %+v, err:%v", p.Operation.Value, err) + errs = multierror.Append(errs, err) + continue + } + if err = temp.Validate(); err != nil { + err := fmt.Errorf("validation failed for xds resource %+v, err:%v", p.Operation.Value, err) + errs = multierror.Append(errs, err) + continue + } + tCtx.AddXdsResource(resourcev3.RouteType, temp) + case string(resourcev3.ClusterType): + temp := &clusterv3.Cluster{} + if err = protojson.Unmarshal(jsonBytes, temp); err != nil { + err := fmt.Errorf("unable to unmarshal xds resource %+v, err:%v", p.Operation.Value, err) + errs = multierror.Append(errs, err) + continue + } + if err = temp.Validate(); err != nil { + err := fmt.Errorf("validation failed for xds resource %+v, err:%v", p.Operation.Value, err) + errs = multierror.Append(errs, err) + continue + } + tCtx.AddXdsResource(resourcev3.ClusterType, temp) + case string(resourcev3.EndpointType): + temp := &endpointv3.ClusterLoadAssignment{} + if err = protojson.Unmarshal(jsonBytes, temp); err != nil { + err := fmt.Errorf("unable to unmarshal xds resource %+v, err:%v", p.Operation.Value, err) + errs = multierror.Append(errs, err) + continue + } + if err = temp.Validate(); err != nil { + err := fmt.Errorf("validation failed for xds resource %+v, err:%v", p.Operation.Value, err) + errs = multierror.Append(errs, err) + continue + } + tCtx.AddXdsResource(resourcev3.EndpointType, temp) + } + + // Skip further processing + continue + } // Find the resource to patch and convert it to JSON switch p.Type { case string(resourcev3.ListenerType): diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml index 77a9be23f1..04470fb309 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml @@ -19,11 +19,37 @@ jsonPatches: - type: "type.googleapis.com/envoy.config.route.v3.RouteConfiguration" name: "first-listener" operation: - op: add + op: "add" path: "/virtual_hosts/0/rate_limits" value: - actions: - remote_address: {} +- type: "type.googleapis.com/envoy.config.cluster.v3.Cluster" + name: rate-limit-cluster + operation: + op: add + path: "" + value: + name: rate-limit-cluster + type: STRICT_DNS + connect_timeout: 10s + lb_policy: ROUND_ROBIN + http2_protocol_options: {} + load_assignment: + cluster_name: rate-limit-cluster + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: ratelimit.svc.cluster.local + port_value: 8081 +- type: "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment" + name: "first-route" + operation: + op: "replace" + path: "/endpoints/0/load_balancing_weight" + value: "50" http: - name: "first-listener" address: "0.0.0.0" diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.clusters.yaml index 2c908f7ef6..5a0733fda8 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.clusters.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.clusters.yaml @@ -11,3 +11,16 @@ outlierDetection: {} perConnectionBufferLimitBytes: 32768 type: EDS +- connectTimeout: 10s + http2ProtocolOptions: {} + loadAssignment: + clusterName: rate-limit-cluster + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: ratelimit.svc.cluster.local + portValue: 8081 + name: rate-limit-cluster + type: STRICT_DNS diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml index f5f137f937..8717f271c2 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml @@ -6,5 +6,5 @@ socketAddress: address: 1.2.3.4 portValue: 50000 - loadBalancingWeight: 1 + loadBalancingWeight: 50 locality: {} From ce795b726127440f5174550755e2863ea2979099 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Fri, 30 Jun 2023 12:09:08 -0700 Subject: [PATCH 08/10] lint Signed-off-by: Arko Dasgupta --- .../testdata/in/xds-ir/jsonpatch.yaml | 18 +++++++++--------- .../out/xds-ir/jsonpatch.endpoints.yaml | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml index 04470fb309..baeec6e056 100644 --- a/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/jsonpatch.yaml @@ -22,9 +22,9 @@ jsonPatches: op: "add" path: "/virtual_hosts/0/rate_limits" value: - - actions: - - remote_address: {} -- type: "type.googleapis.com/envoy.config.cluster.v3.Cluster" + - actions: + - remote_address: {} +- type: "type.googleapis.com/envoy.config.cluster.v3.Cluster" name: rate-limit-cluster operation: op: add @@ -38,12 +38,12 @@ jsonPatches: load_assignment: cluster_name: rate-limit-cluster endpoints: - - lb_endpoints: - - endpoint: - address: - socket_address: - address: ratelimit.svc.cluster.local - port_value: 8081 + - lb_endpoints: + - endpoint: + address: + socket_address: + address: ratelimit.svc.cluster.local + port_value: 8081 - type: "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment" name: "first-route" operation: diff --git a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml index 8717f271c2..ae7ea189da 100644 --- a/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/jsonpatch.endpoints.yaml @@ -6,5 +6,5 @@ socketAddress: address: 1.2.3.4 portValue: 50000 - loadBalancingWeight: 50 + loadBalancingWeight: 50 locality: {} From 2918ea115ada0c6f3018619b5135e15738eb6e6a Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Fri, 30 Jun 2023 17:28:44 -0700 Subject: [PATCH 09/10] move marshaller out of for loop Signed-off-by: Arko Dasgupta --- internal/xds/translator/jsonpatch.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go index 5a98fd0db4..a712ac4731 100644 --- a/internal/xds/translator/jsonpatch.go +++ b/internal/xds/translator/jsonpatch.go @@ -26,6 +26,10 @@ import ( // processJSONPatches applies each JSONPatch to the Xds Resources for a specific type. func processJSONPatches(tCtx *types.ResourceVersionTable, jsonPatches []*ir.JSONPatchConfig) error { var errs error + m := protojson.MarshalOptions{ + UseProtoNames: true, + } + for _, p := range jsonPatches { var ( listener *listenerv3.Listener @@ -35,9 +39,6 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, jsonPatches []*ir.JSON resourceJSON []byte err error ) - m := protojson.MarshalOptions{ - UseProtoNames: true, - } // If Path is "" and op is "add", unmarshal and add the patch as a complete // resource From b9335297118b6fb858c6b86576d58511c5fe125c Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Mon, 3 Jul 2023 09:32:07 -0700 Subject: [PATCH 10/10] address comments Signed-off-by: Arko Dasgupta --- internal/xds/translator/jsonpatch.go | 7 ++++++- internal/xds/translator/translator.go | 8 ++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go index a712ac4731..d5588ac50f 100644 --- a/internal/xds/translator/jsonpatch.go +++ b/internal/xds/translator/jsonpatch.go @@ -23,6 +23,11 @@ import ( "github.com/envoyproxy/gateway/internal/xds/types" ) +const ( + AddOperation = "add" + EmptyPath = "" +) + // processJSONPatches applies each JSONPatch to the Xds Resources for a specific type. func processJSONPatches(tCtx *types.ResourceVersionTable, jsonPatches []*ir.JSONPatchConfig) error { var errs error @@ -42,7 +47,7 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, jsonPatches []*ir.JSON // If Path is "" and op is "add", unmarshal and add the patch as a complete // resource - if p.Operation.Op == "add" && p.Operation.Path == "" { + if p.Operation.Op == AddOperation && p.Operation.Path == EmptyPath { // Convert patch to JSON // The patch library expects an array so convert it into one y, err := yaml.Marshal(p.Operation.Value) diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go index 185e2aa9e2..40f6f603e5 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -80,7 +80,7 @@ func (t *Translator) processHTTPListenerXdsTranslation(tCtx *types.ResourceVersi var xdsRouteCfg *routev3.RouteConfiguration // Search for an existing listener, if it does not exist, create one. - xdsListener := findXdsListenerByAddress(tCtx, httpListener.Address, httpListener.Port, corev3.SocketAddress_TCP) + xdsListener := findXdsListenerByHostPort(tCtx, httpListener.Address, httpListener.Port, corev3.SocketAddress_TCP) if xdsListener == nil { xdsListener = buildXdsTCPListener(httpListener.Name, httpListener.Address, httpListener.Port, accesslog) tCtx.AddXdsResource(resourcev3.ListenerType, xdsListener) @@ -222,7 +222,7 @@ func processTCPListenerXdsTranslation(tCtx *types.ResourceVersionTable, tcpListe } } // Search for an existing listener, if it does not exist, create one. - xdsListener := findXdsListenerByAddress(tCtx, tcpListener.Address, tcpListener.Port, corev3.SocketAddress_TCP) + xdsListener := findXdsListenerByHostPort(tCtx, tcpListener.Address, tcpListener.Port, corev3.SocketAddress_TCP) if xdsListener == nil { xdsListener = buildXdsTCPListener(tcpListener.Name, tcpListener.Address, tcpListener.Port, accesslog) tCtx.AddXdsResource(resourcev3.ListenerType, xdsListener) @@ -258,8 +258,8 @@ func processUDPListenerXdsTranslation(tCtx *types.ResourceVersionTable, udpListe } -// findXdsListenerByAddress finds a xds listener with the same address, port and protocol, and returns nil if there is no match. -func findXdsListenerByAddress(tCtx *types.ResourceVersionTable, address string, port uint32, +// findXdsListenerByHostPort finds a xds listener with the same address, port and protocol, and returns nil if there is no match. +func findXdsListenerByHostPort(tCtx *types.ResourceVersionTable, address string, port uint32, protocol corev3.SocketAddress_Protocol) *listenerv3.Listener { if tCtx == nil || tCtx.XdsResources == nil || tCtx.XdsResources[resourcev3.ListenerType] == nil { return nil