Skip to content

Implement JSON Patch in Xds Translator #1606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zirain merged 11 commits into from
Jul 5, 2023
Merged

Implement JSON Patch in Xds Translator #1606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b9baf83
Implement JSON Patch in Xds Translator
Jun 29, 2023
019bf45
use temp variable to unmarshal into
Jun 29, 2023
74ea031
lint
Jun 30, 2023
631dace
fix test
Jun 30, 2023
5fca2d9
use apiextensionsv1.JSON
Jun 30, 2023
b323f06
routeConfig test
Jun 30, 2023
5b91f56
add entire resource and more test cases
Jun 30, 2023
ce795b7
lint
Jun 30, 2023
2918ea1
move marshaller out of for loop
Jul 1, 2023
b933529
address comments
Jul 3, 2023
a85c6af
Merge branch 'main' into xds-jsonpatch
zirain Jul 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,6 +26,7 @@ require (
google.golang.org/protobuf v1.31.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
Expand All @@ -45,7 +47,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
Expand Down Expand Up @@ -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
Expand Down
29 changes: 29 additions & 0 deletions internal/ir/xds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -54,6 +55,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.
Expand Down Expand Up @@ -792,3 +796,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"`
Copy link
Copy Markdown
Member

@zirain zirain Jun 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use full name here?

Copy link
Copy Markdown
Member

@zirain zirain Jun 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or rename to Type, and change to Enum?

Copy link
Copy Markdown
Contributor Author

@arkodg arkodg Jul 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this naming is tied to JSONPatch spec, since we marshal this and directly pass it to https://github.com/evanphx/json-patch#create-and-apply-a-json-patch

Copy link
Copy Markdown
Member

@zirain zirain Jul 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operation.Op that's odd.

Copy link
Copy Markdown
Member

@qicz qicz Jul 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Copy Markdown
Contributor Author

@arkodg arkodg Jul 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its part of the spec :)
https://datatracker.ietf.org/doc/html/rfc6902/#section-4

Copy link
Copy Markdown
Member

@zhaohuabing zhaohuabing Jul 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it looks a bit ackward to have an "op" member inside the "Operation" object, but as Arko pointed out, it aligns with the RFC standard.

Below description is extracted from the RFC:

Operation objects MUST have exactly one "op" member, whose value indicates the operation to perform. Its value MUST be one of "add", "remove", "replace", "move", "copy", or "test"; other values are errors.

The following examples are from the same RFC. The Operation structure is the same as the JSONPatchOperation in this PR.

   { "op": "add", "path": "/a/b/c", "value": "foo" }
   { "path": "/a/b/c", "op": "add", "value": "foo" }
   { "value": "foo", "path": "/a/b/c", "op": "add" }

I think we can settle with the current implementation so we won't block user's custom extensions replying on the EnvoyPatchPolicy. It's internal to EG anyway, we can always go back and change it once we figure out a better way to handle it.

// 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 apiextensionsv1.JSON `json:"value"`
}
43 changes: 43 additions & 0 deletions internal/ir/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading