From 9bb1601be206fb3983b13af070716ac836fc777c Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Thu, 5 Sep 2024 16:06:51 +0300 Subject: [PATCH] feat: support JSONPatches for proxy bootstrap modifications (#4116) * Modify the API to support expressing JSONPatches Signed-off-by: Lior Okman * Support JSONPatch in the bootstrap proxy flow Signed-off-by: Lior Okman * Accept JSONPatches in a YAML format for unit-tests to be consistent with other unit tests. Signed-off-by: Lior Okman * Reduce the runtime imports, move imports needed for tests into the test code. Signed-off-by: Lior Okman * Moved validation code that requires access to internal/xds out of the api/v1alpha1/validation package. Signed-off-by: Lior Okman * Cleanup and make the linter happy Signed-off-by: Lior Okman * Added another test for the JSONPatch code Signed-off-by: Lior Okman * Make the linter happy Signed-off-by: Lior Okman * More unit-tests. Signed-off-by: Lior Okman --------- Signed-off-by: Lior Okman --- api/v1alpha1/envoyproxy_types.go | 17 +- .../validation/envoyproxy_validate.go | 72 +----- .../validation/envoyproxy_validate_test.go | 92 ------- api/v1alpha1/zz_generated.deepcopy.go | 12 + .../gateway.envoyproxy.io_envoyproxies.yaml | 54 +++- internal/cmd/egctl/translate.go | 7 +- internal/gatewayapi/envoypatchpolicy.go | 2 +- .../proxy/resource_provider_test.go | 6 +- internal/ir/xds.go | 64 ++++- internal/ir/xds_test.go | 121 +++++++++ internal/provider/kubernetes/controller.go | 4 + .../jsonpatch}/jsonpathtopointer.go | 2 +- .../jsonpatch}/jsonpathtopointer_test.go | 2 +- internal/utils/jsonpatch/patch.go | 92 +++++++ internal/utils/jsonpatch/patch_test.go | 124 ++++++++++ .../merge/merge-user-bootstrap.in.yaml | 0 .../merge/merge-user-bootstrap.out.yaml | 178 ++++++++++++++ .../merge/patch-global-config.in.yaml | 6 + .../merge/patch-global-config.out.yaml | 169 +++++++++++++ ...rent-dynamic-resources-user-bootstrap.yaml | 0 ...fferent-xds-cluster-address-bootstrap.yaml | 0 .../missing-admin-address-user-bootstrap.yaml | 0 .../validate}/valid-user-bootstrap.yaml | 0 internal/xds/bootstrap/util.go | 55 ++++- internal/xds/bootstrap/util_test.go | 24 +- internal/xds/bootstrap/validate.go | 89 +++++++ internal/xds/bootstrap/validate_test.go | 76 ++++++ internal/xds/translator/jsonpatch.go | 232 +++++++----------- internal/xds/translator/translator_test.go | 3 +- site/content/en/latest/api/extension_types.md | 7 +- site/content/zh/latest/api/extension_types.md | 7 +- test/cel-validation/envoyproxy_test.go | 59 +++++ 32 files changed, 1243 insertions(+), 333 deletions(-) rename internal/{xds/translator => utils/jsonpatch}/jsonpathtopointer.go (99%) rename internal/{xds/translator => utils/jsonpatch}/jsonpathtopointer_test.go (99%) create mode 100644 internal/utils/jsonpatch/patch.go create mode 100644 internal/utils/jsonpatch/patch_test.go rename api/v1alpha1/validation/testdata/merge-user-bootstrap.yaml => internal/xds/bootstrap/testdata/merge/merge-user-bootstrap.in.yaml (100%) create mode 100644 internal/xds/bootstrap/testdata/merge/merge-user-bootstrap.out.yaml create mode 100644 internal/xds/bootstrap/testdata/merge/patch-global-config.in.yaml create mode 100644 internal/xds/bootstrap/testdata/merge/patch-global-config.out.yaml rename {api/v1alpha1/validation/testdata => internal/xds/bootstrap/testdata/validate}/different-dynamic-resources-user-bootstrap.yaml (100%) rename {api/v1alpha1/validation/testdata => internal/xds/bootstrap/testdata/validate}/different-xds-cluster-address-bootstrap.yaml (100%) rename {api/v1alpha1/validation/testdata => internal/xds/bootstrap/testdata/validate}/missing-admin-address-user-bootstrap.yaml (100%) rename {api/v1alpha1/validation/testdata => internal/xds/bootstrap/testdata/validate}/valid-user-bootstrap.yaml (100%) create mode 100644 internal/xds/bootstrap/validate.go create mode 100644 internal/xds/bootstrap/validate_test.go diff --git a/api/v1alpha1/envoyproxy_types.go b/api/v1alpha1/envoyproxy_types.go index e2ada31c3fc..74218aad20b 100644 --- a/api/v1alpha1/envoyproxy_types.go +++ b/api/v1alpha1/envoyproxy_types.go @@ -358,19 +358,27 @@ const ( ) // ProxyBootstrap defines Envoy Bootstrap configuration. +// +union +// +kubebuilder:validation:XValidation:rule="self.type == 'JSONPatch' ? self.jsonPatches.size() > 0 : has(self.value)", message="provided bootstrap patch doesn't match the configured patch type" type ProxyBootstrap struct { - // Type is the type of the bootstrap configuration, it should be either Replace or Merge. + // Type is the type of the bootstrap configuration, it should be either Replace, Merge, or JSONPatch. // If unspecified, it defaults to Replace. // +optional // +kubebuilder:default=Replace + // +unionDiscriminator Type *BootstrapType `json:"type"` // Value is a YAML string of the bootstrap. - Value string `json:"value"` + // +optional + Value *string `json:"value,omitempty"` + + // JSONPatches is an array of JSONPatches to be applied to the default bootstrap. Patches are + // applied in the order in which they are defined. + JSONPatches []JSONPatchOperation `json:"jsonPatches,omitempty"` } // BootstrapType defines the types of bootstrap supported by Envoy Gateway. -// +kubebuilder:validation:Enum=Merge;Replace +// +kubebuilder:validation:Enum=Merge;Replace;JSONPatch type BootstrapType string const ( @@ -381,6 +389,9 @@ const ( // Replace replaces the default bootstrap with the provided one. BootstrapTypeReplace BootstrapType = "Replace" + + // JSONPatch applies the provided JSONPatches to the default bootstrap. + BootstrapTypeJSONPatch BootstrapType = "JSONPatch" ) // EnvoyProxyStatus defines the observed state of EnvoyProxy. This type is not implemented diff --git a/api/v1alpha1/validation/envoyproxy_validate.go b/api/v1alpha1/validation/envoyproxy_validate.go index b4d8b6b7392..74ce4e0451c 100644 --- a/api/v1alpha1/validation/envoyproxy_validate.go +++ b/api/v1alpha1/validation/envoyproxy_validate.go @@ -12,16 +12,9 @@ import ( "net/netip" "github.com/dominikbraun/graph" - bootstrapv3 "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3" - clusterv3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" - "github.com/google/go-cmp/cmp" - "google.golang.org/protobuf/testing/protocmp" utilerrors "k8s.io/apimachinery/pkg/util/errors" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" - "github.com/envoyproxy/gateway/internal/utils/proto" - "github.com/envoyproxy/gateway/internal/xds/bootstrap" - _ "github.com/envoyproxy/gateway/internal/xds/extensions" // register the generated types to support protojson unmarshalling ) // ValidateEnvoyProxy validates the provided EnvoyProxy. @@ -38,6 +31,8 @@ func ValidateEnvoyProxy(proxy *egv1a1.EnvoyProxy) error { } // validateEnvoyProxySpec validates the provided EnvoyProxy spec. +// This method validates everything except for the bootstrap section, because validating the bootstrap +// section in this method would require calling into the internal apis, and would cause an import cycle. func validateEnvoyProxySpec(spec *egv1a1.EnvoyProxySpec) error { var errs []error @@ -51,13 +46,6 @@ func validateEnvoyProxySpec(spec *egv1a1.EnvoyProxySpec) error { errs = append(errs, validateProviderErrs...) } - // validate bootstrap - if spec != nil && spec.Bootstrap != nil { - if err := validateBootstrap(spec.Bootstrap); err != nil { - errs = append(errs, err) - } - } - validateProxyTelemetryErrs := validateProxyTelemetry(spec) if len(validateProxyTelemetryErrs) != 0 { errs = append(errs, validateProxyTelemetryErrs...) @@ -156,62 +144,6 @@ func validateService(spec *egv1a1.EnvoyProxySpec) []error { return errs } -func validateBootstrap(boostrapConfig *egv1a1.ProxyBootstrap) error { - // Validate user bootstrap config - defaultBootstrap := &bootstrapv3.Bootstrap{} - // TODO: need validate when enable prometheus? - defaultBootstrapStr, err := bootstrap.GetRenderedBootstrapConfig(nil) - if err != nil { - return err - } - if err := proto.FromYAML([]byte(defaultBootstrapStr), defaultBootstrap); err != nil { - return fmt.Errorf("unable to unmarshal default bootstrap: %w", err) - } - if err := defaultBootstrap.Validate(); err != nil { - return fmt.Errorf("default bootstrap validation failed: %w", err) - } - - // Validate user bootstrap config - userBootstrapStr, err := bootstrap.ApplyBootstrapConfig(boostrapConfig, defaultBootstrapStr) - if err != nil { - return err - } - userBootstrap := &bootstrapv3.Bootstrap{} - if err := proto.FromYAML([]byte(userBootstrapStr), userBootstrap); err != nil { - return fmt.Errorf("failed to parse default bootstrap config: %w", err) - } - if err := userBootstrap.Validate(); err != nil { - return fmt.Errorf("validation failed for user bootstrap: %w", err) - } - - // Ensure dynamic resources config is same - if userBootstrap.DynamicResources == nil || - cmp.Diff(userBootstrap.DynamicResources, defaultBootstrap.DynamicResources, protocmp.Transform()) != "" { - return fmt.Errorf("dynamic_resources cannot be modified") - } - - // Ensure that the xds_cluster config is same - var userXdsCluster, defaultXdsCluster *clusterv3.Cluster - for _, cluster := range userBootstrap.StaticResources.Clusters { - if cluster.Name == "xds_cluster" { - userXdsCluster = cluster - break - } - } - for _, cluster := range defaultBootstrap.StaticResources.Clusters { - if cluster.Name == "xds_cluster" { - defaultXdsCluster = cluster - break - } - } - if userXdsCluster == nil || - cmp.Diff(userXdsCluster.LoadAssignment, defaultXdsCluster.LoadAssignment, protocmp.Transform()) != "" { - return fmt.Errorf("xds_cluster's loadAssigntment cannot be modified") - } - - return nil -} - func validateProxyTelemetry(spec *egv1a1.EnvoyProxySpec) []error { var errs []error diff --git a/api/v1alpha1/validation/envoyproxy_validate_test.go b/api/v1alpha1/validation/envoyproxy_validate_test.go index 591c184fdd5..bd7e4bc18e0 100644 --- a/api/v1alpha1/validation/envoyproxy_validate_test.go +++ b/api/v1alpha1/validation/envoyproxy_validate_test.go @@ -6,8 +6,6 @@ package validation import ( - // Register embed - _ "embed" "reflect" "testing" @@ -21,19 +19,6 @@ import ( egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" ) -var ( - //go:embed testdata/valid-user-bootstrap.yaml - validUserBootstrap string - //go:embed testdata/merge-user-bootstrap.yaml - mergeUserBootstrap string - //go:embed testdata/missing-admin-address-user-bootstrap.yaml - missingAdminAddressUserBootstrap string - //go:embed testdata/different-dynamic-resources-user-bootstrap.yaml - differentDynamicResourcesUserBootstrap string - //go:embed testdata/different-xds-cluster-address-bootstrap.yaml - differentXdsClusterAddressBootstrap string -) - func TestValidateEnvoyProxy(t *testing.T) { testCases := []struct { name string @@ -319,83 +304,6 @@ func TestValidateEnvoyProxy(t *testing.T) { }, expected: false, }, - - { - name: "valid user bootstrap replace type", - proxy: &egv1a1.EnvoyProxy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "test", - }, - Spec: egv1a1.EnvoyProxySpec{ - Bootstrap: &egv1a1.ProxyBootstrap{ - Value: validUserBootstrap, - }, - }, - }, - expected: true, - }, - { - name: "valid user bootstrap merge type", - proxy: &egv1a1.EnvoyProxy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "test", - }, - Spec: egv1a1.EnvoyProxySpec{ - Bootstrap: &egv1a1.ProxyBootstrap{ - Type: ptr.To(egv1a1.BootstrapTypeMerge), - Value: mergeUserBootstrap, - }, - }, - }, - expected: true, - }, - { - name: "user bootstrap with missing admin address", - proxy: &egv1a1.EnvoyProxy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "test", - }, - Spec: egv1a1.EnvoyProxySpec{ - Bootstrap: &egv1a1.ProxyBootstrap{ - Value: missingAdminAddressUserBootstrap, - }, - }, - }, - expected: false, - }, - { - name: "user bootstrap with different dynamic resources", - proxy: &egv1a1.EnvoyProxy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "test", - }, - Spec: egv1a1.EnvoyProxySpec{ - Bootstrap: &egv1a1.ProxyBootstrap{ - Value: differentDynamicResourcesUserBootstrap, - }, - }, - }, - expected: false, - }, - { - name: "user bootstrap with different xds_cluster endpoint", - proxy: &egv1a1.EnvoyProxy{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "test", - Name: "test", - }, - Spec: egv1a1.EnvoyProxySpec{ - Bootstrap: &egv1a1.ProxyBootstrap{ - Value: differentXdsClusterAddressBootstrap, - }, - }, - }, - expected: false, - }, { name: "should invalid when accesslog enabled using Text format, but `text` field being empty", proxy: &egv1a1.EnvoyProxy{ diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index c0b8e8c657a..9afe197fc97 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -3899,6 +3899,18 @@ func (in *ProxyBootstrap) DeepCopyInto(out *ProxyBootstrap) { *out = new(BootstrapType) **out = **in } + if in.Value != nil { + in, out := &in.Value, &out.Value + *out = new(string) + **out = **in + } + if in.JSONPatches != nil { + in, out := &in.JSONPatches, &out.JSONPatches + *out = make([]JSONPatchOperation, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProxyBootstrap. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml index 40590f37988..6baf2056ec3 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml @@ -191,21 +191,69 @@ spec: We strongly recommend using `egctl x translate` to generate a `EnvoyProxy` resource with the `Bootstrap` field set to the default Bootstrap configuration used. You can edit this configuration, and rerun `egctl x translate` to ensure there are no validation errors. properties: + jsonPatches: + description: |- + JSONPatches is an array of JSONPatches to be applied to the default bootstrap. Patches are + applied in the order in which they are defined. + items: + description: |- + JSONPatchOperation defines the JSON Patch Operation as defined in + https://datatracker.ietf.org/doc/html/rfc6902 + properties: + from: + description: |- + From is the source location of the value to be copied or moved. Only valid + for move or copy operations + Refer to https://datatracker.ietf.org/doc/html/rfc6901 for more details. + type: string + jsonPath: + description: |- + JSONPath specifies the locations of the target document/field where the operation will be performed + Refer to https://datatracker.ietf.org/doc/rfc9535/ for more details. + type: string + op: + description: Op is the type of operation to perform + enum: + - add + - remove + - replace + - move + - copy + - test + type: string + path: + description: |- + 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. + type: string + value: + description: |- + Value is the new value of the path location. The value is only used by + the `add` and `replace` operations. + x-kubernetes-preserve-unknown-fields: true + required: + - op + type: object + type: array type: default: Replace description: |- - Type is the type of the bootstrap configuration, it should be either Replace or Merge. + Type is the type of the bootstrap configuration, it should be either Replace, Merge, or JSONPatch. If unspecified, it defaults to Replace. enum: - Merge - Replace + - JSONPatch type: string value: description: Value is a YAML string of the bootstrap. type: string - required: - - value type: object + x-kubernetes-validations: + - message: provided bootstrap patch doesn't match the configured patch + type + rule: 'self.type == ''JSONPatch'' ? self.jsonPatches.size() > 0 + : has(self.value)' concurrency: description: |- Concurrency defines the number of worker threads to run. If unset, it defaults to diff --git a/internal/cmd/egctl/translate.go b/internal/cmd/egctl/translate.go index ceb4e9deee3..045d4733e47 100644 --- a/internal/cmd/egctl/translate.go +++ b/internal/cmd/egctl/translate.go @@ -326,6 +326,11 @@ func translateGatewayAPIToGatewayAPI(resources *gatewayapi.Resources) (gatewayap msg := fmt.Sprintf("%s: %v", status.MsgGatewayClassInvalidParams, err) status.SetGatewayClassAccepted(resources.GatewayClass, false, string(gwapiv1.GatewayClassReasonInvalidParameters), msg) } + if err := bootstrap.Validate(resources.EnvoyProxyForGatewayClass.Spec.Bootstrap); err != nil { + epInvalid = true + msg := fmt.Sprintf("%s: %v", status.MsgGatewayClassInvalidParams, err) + status.SetGatewayClassAccepted(resources.GatewayClass, false, string(gwapiv1.GatewayClassReasonInvalidParameters), msg) + } gRes.EnvoyProxyForGatewayClass = resources.EnvoyProxyForGatewayClass } if !epInvalid { @@ -959,7 +964,7 @@ func addDefaultEnvoyProxy(resources *gatewayapi.Resources) error { }, Spec: egv1a1.EnvoyProxySpec{ Bootstrap: &egv1a1.ProxyBootstrap{ - Value: defaultBootstrapStr, + Value: &defaultBootstrapStr, }, }, } diff --git a/internal/gatewayapi/envoypatchpolicy.go b/internal/gatewayapi/envoypatchpolicy.go index 5d2480f5d23..c2dd5480362 100644 --- a/internal/gatewayapi/envoypatchpolicy.go +++ b/internal/gatewayapi/envoypatchpolicy.go @@ -112,7 +112,7 @@ func (t *Translator) ProcessEnvoyPatchPolicies(envoyPatchPolicies []*egv1a1.Envo irPatch := ir.JSONPatchConfig{} irPatch.Type = string(patch.Type) irPatch.Name = patch.Name - irPatch.Operation.Op = string(patch.Operation.Op) + irPatch.Operation.Op = ir.JSONPatchOp(patch.Operation.Op) irPatch.Operation.Path = patch.Operation.Path irPatch.Operation.JSONPath = patch.Operation.JSONPath irPatch.Operation.From = patch.Operation.From diff --git a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go index 16c94d037a4..c92d94d4b42 100644 --- a/internal/infrastructure/kubernetes/proxy/resource_provider_test.go +++ b/internal/infrastructure/kubernetes/proxy/resource_provider_test.go @@ -529,9 +529,10 @@ func TestDeployment(t *testing.T) { replace := egv1a1.BootstrapTypeReplace if tc.bootstrap != "" { + bsValue := tc.bootstrap tc.infra.Proxy.Config.Spec.Bootstrap = &egv1a1.ProxyBootstrap{ Type: &replace, - Value: tc.bootstrap, + Value: &bsValue, } } @@ -963,9 +964,10 @@ func TestDaemonSet(t *testing.T) { replace := egv1a1.BootstrapTypeReplace if tc.bootstrap != "" { + bsValue := tc.bootstrap tc.infra.Proxy.Config.Spec.Bootstrap = &egv1a1.ProxyBootstrap{ Type: &replace, - Value: tc.bootstrap, + Value: &bsValue, } } diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 1b5b5971b8c..9a3af5efefb 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -8,6 +8,7 @@ package ir import ( "cmp" "errors" + "fmt" "net/http" "net/netip" "reflect" @@ -1753,12 +1754,42 @@ type JSONPatchConfig struct { Operation JSONPatchOperation `json:"operation" yaml:"operation"` } +type JSONPatchOp string + +const ( + JSONPatchOpAdd JSONPatchOp = "add" + JSONPatchOpRemove JSONPatchOp = "remove" + JSONPatchOpReplace JSONPatchOp = "replace" + JSONPatchOpCopy JSONPatchOp = "copy" + JSONPatchOpMove JSONPatchOp = "move" + JSONPatchOpTest JSONPatchOp = "test" +) + +func TranslateJSONPatchOp(op egv1a1.JSONPatchOperationType) JSONPatchOp { + switch op { + case "add": + return JSONPatchOpAdd + case "remove": + return JSONPatchOpRemove + case "replace": + return JSONPatchOpReplace + case "move": + return JSONPatchOpMove + case "copy": + return JSONPatchOpCopy + case "test": + return JSONPatchOpTest + default: + return "" + } +} + // 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" yaml:"op"` + Op JSONPatchOp `json:"op" yaml:"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. // +optional @@ -1784,6 +1815,37 @@ func (o *JSONPatchOperation) IsJSONPathNilOrEmpty() bool { return o.JSONPath == nil || *o.JSONPath == EmptyPath } +// Validate ensures that the appropriate fields are set for each operation type according to RFC 6902: +// https://www.rfc-editor.org/rfc/rfc6902.html +func (o *JSONPatchOperation) Validate() error { + if o.Path == nil && o.JSONPath == nil { + return fmt.Errorf("a patch operation must specify a path or jsonPath") + } + switch o.Op { + case JSONPatchOpAdd, JSONPatchOpReplace, JSONPatchOpTest: + if o.Value == nil { + return fmt.Errorf("the %s operation requires a value", o.Op) + } + if o.From != nil { + return fmt.Errorf("the %s operation doesn't support a from attribute", o.Op) + } + case JSONPatchOpRemove: + if o.From != nil || o.Value != nil { + return fmt.Errorf("value and from can't be specified with the remove operation") + } + case JSONPatchOpMove, JSONPatchOpCopy: + if o.From == nil { + return fmt.Errorf("the %s operation requires a valid from attribute", o.Op) + } + if o.Value != nil { + return fmt.Errorf("the %s operation doesn't support a value attribute", o.Op) + } + default: + return fmt.Errorf("unsupported JSONPatch operation") + } + return nil +} + // Tracing defines the configuration for tracing a Envoy xDS Resource // +k8s:deepcopy-gen=true type Tracing struct { diff --git a/internal/ir/xds_test.go b/internal/ir/xds_test.go index 876e37f9e13..882aa090e55 100644 --- a/internal/ir/xds_test.go +++ b/internal/ir/xds_test.go @@ -13,6 +13,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" @@ -1618,3 +1619,123 @@ func TestValidateHealthCheck(t *testing.T) { }) } } + +func TestJSONPatchOperationValidation(t *testing.T) { + tests := []struct { + name string + input JSONPatchOperation + want *string + }{ + { + name: "no path or jsonpath", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("remove")), + }, + want: ptr.To("a patch operation must specify a path or jsonPath"), + }, + { + name: "replace with from", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("replace")), + JSONPath: ptr.To("$.some.json[@?name=='lala'].key"), + Value: &apiextensionsv1.JSON{ + Raw: []byte{}, + }, + From: ptr.To("/some/from"), + }, + want: ptr.To("the replace operation doesn't support a from attribute"), + }, + { + name: "add with no value", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("add")), + JSONPath: ptr.To("$.some.json[@?name=='lala'].key"), + }, + want: ptr.To("the add operation requires a value"), + }, + { + name: "remove with from", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("remove")), + JSONPath: ptr.To("$.some.json[@?name=='lala'].key"), + From: ptr.To("/some/from"), + }, + want: ptr.To("value and from can't be specified with the remove operation"), + }, + { + name: "remove with value", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("remove")), + JSONPath: ptr.To("$.some.json[@?name=='lala'].key"), + Value: &apiextensionsv1.JSON{ + Raw: []byte{}, + }, + }, + want: ptr.To("value and from can't be specified with the remove operation"), + }, + { + name: "move without from", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("move")), + JSONPath: ptr.To("$.some.json[@?name=='lala'].key"), + }, + want: ptr.To("the move operation requires a valid from attribute"), + }, + { + name: "copy with value", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("copy")), + JSONPath: ptr.To("$.some.json[@?name=='lala'].key"), + From: ptr.To("/some/from"), + Value: &apiextensionsv1.JSON{ + Raw: []byte{}, + }, + }, + want: ptr.To("the copy operation doesn't support a value attribute"), + }, + { + name: "invalid operation", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("invalid")), + Path: ptr.To("/some/path"), + }, + want: ptr.To("unsupported JSONPatch operation"), + }, + { + name: "valid test operation", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("test")), + Path: ptr.To("/some/path"), + Value: &apiextensionsv1.JSON{ + Raw: []byte{}, + }, + }, + }, + { + name: "valid remove operation", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("remove")), + Path: ptr.To("/some/path"), + }, + }, + { + name: "valid copy operation", + input: JSONPatchOperation{ + Op: TranslateJSONPatchOp(egv1a1.JSONPatchOperationType("copy")), + Path: ptr.To("/some/path"), + From: ptr.To("/some/other/path"), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := tc.input.Validate() + if tc.want != nil { + require.EqualError(t, err, *tc.want) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/internal/provider/kubernetes/controller.go b/internal/provider/kubernetes/controller.go index 05cb0fa5528..1c0555c0772 100644 --- a/internal/provider/kubernetes/controller.go +++ b/internal/provider/kubernetes/controller.go @@ -43,6 +43,7 @@ import ( "github.com/envoyproxy/gateway/internal/message" "github.com/envoyproxy/gateway/internal/utils" "github.com/envoyproxy/gateway/internal/utils/slice" + "github.com/envoyproxy/gateway/internal/xds/bootstrap" ) var skipNameValidation = func() *bool { @@ -1651,6 +1652,9 @@ func (r *gatewayAPIReconciler) processEnvoyProxy(ep *egv1a1.EnvoyProxy, resource if err := validation.ValidateEnvoyProxy(ep); err != nil { return fmt.Errorf("invalid envoyproxy: %w", err) } + if err := bootstrap.Validate(ep.Spec.Bootstrap); err != nil { + return fmt.Errorf("invalid envoyproxy: %w", err) + } if ep.Spec.Telemetry != nil { var backendRefs []egv1a1.BackendRef diff --git a/internal/xds/translator/jsonpathtopointer.go b/internal/utils/jsonpatch/jsonpathtopointer.go similarity index 99% rename from internal/xds/translator/jsonpathtopointer.go rename to internal/utils/jsonpatch/jsonpathtopointer.go index 89d7fdf1c77..730baa94ee2 100644 --- a/internal/xds/translator/jsonpathtopointer.go +++ b/internal/utils/jsonpatch/jsonpathtopointer.go @@ -3,7 +3,7 @@ // The full text of the Apache license is available in the LICENSE file at // the root of the repo. -package translator +package jsonpatch import ( "reflect" diff --git a/internal/xds/translator/jsonpathtopointer_test.go b/internal/utils/jsonpatch/jsonpathtopointer_test.go similarity index 99% rename from internal/xds/translator/jsonpathtopointer_test.go rename to internal/utils/jsonpatch/jsonpathtopointer_test.go index cefb5925869..4b57424562d 100644 --- a/internal/xds/translator/jsonpathtopointer_test.go +++ b/internal/utils/jsonpatch/jsonpathtopointer_test.go @@ -3,7 +3,7 @@ // The full text of the Apache license is available in the LICENSE file at // the root of the repo. -package translator +package jsonpatch import ( "sort" diff --git a/internal/utils/jsonpatch/patch.go b/internal/utils/jsonpatch/patch.go new file mode 100644 index 00000000000..15cac85a308 --- /dev/null +++ b/internal/utils/jsonpatch/patch.go @@ -0,0 +1,92 @@ +// 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 jsonpatch + +import ( + "encoding/json" + "errors" + "fmt" + + jsonpatchv5 "github.com/evanphx/json-patch/v5" + "sigs.k8s.io/yaml" + + "github.com/envoyproxy/gateway/internal/ir" +) + +// ApplyJSONPatches applies a series of JSONPatches to a provided JSON document. +// Patches are applied in order, and any errors are aggregated into the return value. +// An error with a specific patch just means that this specific patch is skipped, the document +// will still be modified with any other provided patch operation. +// If a patch is applied to a JSONPath, then that JSONPath is first exploded to standard paths +// and the patch is applied to all matching paths. +func ApplyJSONPatches(document json.RawMessage, patches ...ir.JSONPatchOperation) (json.RawMessage, error) { + opts := jsonpatchv5.NewApplyOptions() + opts.EnsurePathExistsOnAdd = true + + var tErrs, err error + for _, p := range patches { + + if err := p.Validate(); err != nil { + tErrs = errors.Join(tErrs, err) + continue + } + + var jsonPointers []string + if p.JSONPath != nil { + path := "" + if p.Path != nil { + path = *p.Path + } + jsonPointers, err = ConvertPathToPointers(document, *p.JSONPath, path) + if err != nil { + tErr := fmt.Errorf("unable to convert jsonPath: '%s' into jsonPointers, err: %s", *p.JSONPath, err.Error()) + tErrs = errors.Join(tErrs, tErr) + continue + } + } else { + jsonPointers = []string{*p.Path} + } + + for _, path := range jsonPointers { + op := ir.JSONPatchOperation{ + Path: &path, + Op: p.Op, + Value: p.Value, + From: p.From, + } + + // Convert patch to JSON + // The patch library expects an array so convert it into one + y, err := yaml.Marshal([]ir.JSONPatchOperation{op}) + if err != nil { + tErr := fmt.Errorf("unable to marshal patch %+v, err: %s", op, err.Error()) + tErrs = errors.Join(tErrs, tErr) + continue + } + jsonBytes, err := yaml.YAMLToJSON(y) + if err != nil { + tErr := fmt.Errorf("unable to convert patch to json %s, err: %s", string(y), err.Error()) + tErrs = errors.Join(tErrs, tErr) + continue + } + patchObj, err := jsonpatchv5.DecodePatch(jsonBytes) + if err != nil { + tErr := fmt.Errorf("unable to decode patch %s, err: %s", string(jsonBytes), err.Error()) + tErrs = errors.Join(tErrs, tErr) + continue + } + + // Apply patch + document, err = patchObj.ApplyWithOptions(document, opts) + if err != nil { + tErr := fmt.Errorf("unable to apply patch:\n%s on resource:\n%s, err: %s", string(jsonBytes), string(document), err.Error()) + tErrs = errors.Join(tErrs, tErr) + continue + } + } + } + return document, tErrs +} diff --git a/internal/utils/jsonpatch/patch_test.go b/internal/utils/jsonpatch/patch_test.go new file mode 100644 index 00000000000..ace677124e0 --- /dev/null +++ b/internal/utils/jsonpatch/patch_test.go @@ -0,0 +1,124 @@ +// 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 jsonpatch + +import ( + "testing" + + "github.com/stretchr/testify/require" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/utils/ptr" + + "github.com/envoyproxy/gateway/internal/ir" +) + +const sourceDocument = ` +{ + "topLevel" : { + "mapContainer" : { + "key": "value", + "other": "key" + }, + "arrayContainer": [ + "str1", + "str2" + ], + "mapArray" : [ + { + "name": "first", + "key" : "value" + }, + { + "name": "second", + "key" : "other value" + } + ] + } +} +` + +func TestApplyJSONPatches(t *testing.T) { + testCases := []struct { + name string + patchOperation []ir.JSONPatchOperation + errorExpected bool + }{ + { + name: "simple add with single patch", + patchOperation: []ir.JSONPatchOperation{ + { + Op: "add", + Path: ptr.To("/topLevel/newKey"), + Value: &apiextensionsv1.JSON{ + Raw: []byte("true"), + }, + }, + }, + errorExpected: false, + }, + { + name: "two operations in a set", + patchOperation: []ir.JSONPatchOperation{ + { + Op: "add", + Path: ptr.To("/topLevel/newKey"), + Value: &apiextensionsv1.JSON{ + Raw: []byte("true"), + }, + }, + { + Op: "remove", + Path: ptr.To("/topLevel/arrayContainer/1"), + }, + }, + errorExpected: false, + }, + { + name: "invalid operation", + patchOperation: []ir.JSONPatchOperation{ + { + Op: "badbadbad", + Path: ptr.To("/topLevel/newKey"), + Value: &apiextensionsv1.JSON{ + Raw: []byte("true"), + }, + }, + }, + errorExpected: true, + }, + { + name: "jsonpath affecting two places", + patchOperation: []ir.JSONPatchOperation{ + { + Op: "remove", + JSONPath: ptr.To("$.topLevel.mapArray[*].key"), + }, + }, + errorExpected: false, + }, + { + name: "invalid jsonpath", + patchOperation: []ir.JSONPatchOperation{ + { + Op: "remove", + JSONPath: ptr.To("i'm not a json path string"), + }, + }, + errorExpected: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := ApplyJSONPatches([]byte(sourceDocument), tc.patchOperation...) + if tc.errorExpected { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/api/v1alpha1/validation/testdata/merge-user-bootstrap.yaml b/internal/xds/bootstrap/testdata/merge/merge-user-bootstrap.in.yaml similarity index 100% rename from api/v1alpha1/validation/testdata/merge-user-bootstrap.yaml rename to internal/xds/bootstrap/testdata/merge/merge-user-bootstrap.in.yaml diff --git a/internal/xds/bootstrap/testdata/merge/merge-user-bootstrap.out.yaml b/internal/xds/bootstrap/testdata/merge/merge-user-bootstrap.out.yaml new file mode 100644 index 00000000000..7fcb292368a --- /dev/null +++ b/internal/xds/bootstrap/testdata/merge/merge-user-bootstrap.out.yaml @@ -0,0 +1,178 @@ +admin: + accessLog: + - name: envoy.access_loggers.file + typedConfig: + '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog + path: /dev/null + address: + socketAddress: + address: 127.0.0.1 + portValue: 8080 +dynamicResources: + adsConfig: + apiType: DELTA_GRPC + grpcServices: + - envoyGrpc: + clusterName: xds_cluster + setNodeOnFirstMessageOnly: true + transportApiVersion: V3 + cdsConfig: + ads: {} + resourceApiVersion: V3 + ldsConfig: + ads: {} + resourceApiVersion: V3 +layeredRuntime: + layers: + - name: global_config + staticLayer: + envoy.restart_features.use_eds_cache_for_ads: true + re2.max_program_size.error_level: 4294967295 + re2.max_program_size.warn_level: 1000 +overloadManager: + refreshInterval: 0.250s + resourceMonitors: + - name: envoy.resource_monitors.global_downstream_max_connections + typedConfig: + '@type': type.googleapis.com/envoy.extensions.resource_monitors.downstream_connections.v3.DownstreamConnectionsConfig + maxActiveDownstreamConnections: "50000" +staticResources: + clusters: + - connectTimeout: 0.250s + loadAssignment: + clusterName: prometheus_stats + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 127.0.0.1 + portValue: 19000 + name: prometheus_stats + type: STATIC + - connectTimeout: 10s + loadAssignment: + clusterName: xds_cluster + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: envoy-gateway + portValue: 18000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + name: xds_cluster + transportSocket: + name: envoy.transport_sockets.tls + typedConfig: + '@type': type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext + commonTlsContext: + tlsCertificateSdsSecretConfigs: + - name: xds_certificate + sdsConfig: + pathConfigSource: + path: /sds/xds-certificate.json + resourceApiVersion: V3 + tlsParams: + tlsMaximumProtocolVersion: TLSv1_3 + validationContextSdsSecretConfig: + name: xds_trusted_ca + sdsConfig: + pathConfigSource: + path: /sds/xds-trusted-ca.json + resourceApiVersion: V3 + type: STRICT_DNS + typedExtensionProtocolOptions: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + '@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + explicitHttpConfig: + http2ProtocolOptions: + connectionKeepalive: + interval: 30s + timeout: 5s + - connectTimeout: 10s + loadAssignment: + clusterName: wasm_cluster + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: envoy-gateway + portValue: 18002 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + name: wasm_cluster + transportSocket: + name: envoy.transport_sockets.tls + typedConfig: + '@type': type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext + commonTlsContext: + tlsCertificateSdsSecretConfigs: + - name: xds_certificate + sdsConfig: + pathConfigSource: + path: /sds/xds-certificate.json + resourceApiVersion: V3 + tlsParams: + tlsMaximumProtocolVersion: TLSv1_3 + validationContextSdsSecretConfig: + name: xds_trusted_ca + sdsConfig: + pathConfigSource: + path: /sds/xds-trusted-ca.json + resourceApiVersion: V3 + type: STRICT_DNS + typedExtensionProtocolOptions: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + '@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + explicitHttpConfig: + http2ProtocolOptions: {} + - connectTimeout: 0.250s + loadAssignment: + clusterName: prometheus_stats + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 127.0.0.1 + portValue: 19000 + name: prometheus_stats + type: STATIC + listeners: + - address: + socketAddress: + address: 0.0.0.0 + portValue: 19001 + filterChains: + - filters: + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + httpFilters: + - name: envoy.filters.http.health_check + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.health_check.v3.HealthCheck + headers: + - name: :path + stringMatch: + exact: /ready + passThroughMode: false + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + routeConfig: + name: local_route + virtualHosts: + - domains: + - '*' + name: prometheus_stats + routes: + - match: + prefix: /stats/prometheus + route: + cluster: prometheus_stats + statPrefix: eg-ready-http + name: envoy-gateway-proxy-ready-0.0.0.0-19001 diff --git a/internal/xds/bootstrap/testdata/merge/patch-global-config.in.yaml b/internal/xds/bootstrap/testdata/merge/patch-global-config.in.yaml new file mode 100644 index 00000000000..2035e5f3433 --- /dev/null +++ b/internal/xds/bootstrap/testdata/merge/patch-global-config.in.yaml @@ -0,0 +1,6 @@ +- op: add + path: /layered_runtime/layers/0/static_layer/envoy.restart_features.use_eds_cache_for_ads + value: false +- op: add + path: /layered_runtime/layers/0/static_layer/envoy.something.completely.made.up + value: arbitrary string diff --git a/internal/xds/bootstrap/testdata/merge/patch-global-config.out.yaml b/internal/xds/bootstrap/testdata/merge/patch-global-config.out.yaml new file mode 100644 index 00000000000..63915cc277a --- /dev/null +++ b/internal/xds/bootstrap/testdata/merge/patch-global-config.out.yaml @@ -0,0 +1,169 @@ +admin: + access_log: + - name: envoy.access_loggers.file + typed_config: + '@type': type.googleapis.com/envoy.extensions.access_loggers.file.v3.FileAccessLog + path: /dev/null + address: + socket_address: + address: 127.0.0.1 + port_value: 19000 +dynamic_resources: + ads_config: + api_type: DELTA_GRPC + grpc_services: + - envoy_grpc: + cluster_name: xds_cluster + set_node_on_first_message_only: true + transport_api_version: V3 + cds_config: + ads: {} + resource_api_version: V3 + lds_config: + ads: {} + resource_api_version: V3 +layered_runtime: + layers: + - name: global_config + static_layer: + envoy.restart_features.use_eds_cache_for_ads: false + envoy.something.completely.made.up: arbitrary string + re2.max_program_size.error_level: 4294967295 + re2.max_program_size.warn_level: 1000 +overload_manager: + refresh_interval: 0.25s + resource_monitors: + - name: envoy.resource_monitors.global_downstream_max_connections + typed_config: + '@type': type.googleapis.com/envoy.extensions.resource_monitors.downstream_connections.v3.DownstreamConnectionsConfig + max_active_downstream_connections: 50000 +static_resources: + clusters: + - connect_timeout: 0.250s + lb_policy: ROUND_ROBIN + load_assignment: + cluster_name: prometheus_stats + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: 127.0.0.1 + port_value: 19000 + name: prometheus_stats + type: STATIC + - connect_timeout: 10s + load_assignment: + cluster_name: xds_cluster + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: envoy-gateway + port_value: 18000 + load_balancing_weight: 1 + load_balancing_weight: 1 + name: xds_cluster + transport_socket: + name: envoy.transport_sockets.tls + typed_config: + '@type': type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext + common_tls_context: + tls_certificate_sds_secret_configs: + - name: xds_certificate + sds_config: + path_config_source: + path: /sds/xds-certificate.json + resource_api_version: V3 + tls_params: + tls_maximum_protocol_version: TLSv1_3 + validation_context_sds_secret_config: + name: xds_trusted_ca + sds_config: + path_config_source: + path: /sds/xds-trusted-ca.json + resource_api_version: V3 + type: STRICT_DNS + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + '@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + explicit_http_config: + http2_protocol_options: + connection_keepalive: + interval: 30s + timeout: 5s + - connect_timeout: 10s + load_assignment: + cluster_name: wasm_cluster + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: envoy-gateway + port_value: 18002 + load_balancing_weight: 1 + load_balancing_weight: 1 + name: wasm_cluster + transport_socket: + name: envoy.transport_sockets.tls + typed_config: + '@type': type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext + common_tls_context: + tls_certificate_sds_secret_configs: + - name: xds_certificate + sds_config: + path_config_source: + path: /sds/xds-certificate.json + resource_api_version: V3 + tls_params: + tls_maximum_protocol_version: TLSv1_3 + validation_context_sds_secret_config: + name: xds_trusted_ca + sds_config: + path_config_source: + path: /sds/xds-trusted-ca.json + resource_api_version: V3 + type: STRICT_DNS + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + '@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + explicit_http_config: + http2_protocol_options: {} + listeners: + - address: + socket_address: + address: 0.0.0.0 + port_value: 19001 + protocol: TCP + filter_chains: + - filters: + - name: envoy.filters.network.http_connection_manager + typed_config: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + http_filters: + - name: envoy.filters.http.health_check + typed_config: + '@type': type.googleapis.com/envoy.extensions.filters.http.health_check.v3.HealthCheck + headers: + - name: :path + string_match: + exact: /ready + pass_through_mode: false + - name: envoy.filters.http.router + typed_config: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + route_config: + name: local_route + virtual_hosts: + - domains: + - '*' + name: prometheus_stats + routes: + - match: + prefix: /stats/prometheus + route: + cluster: prometheus_stats + stat_prefix: eg-ready-http + name: envoy-gateway-proxy-ready-0.0.0.0-19001 diff --git a/api/v1alpha1/validation/testdata/different-dynamic-resources-user-bootstrap.yaml b/internal/xds/bootstrap/testdata/validate/different-dynamic-resources-user-bootstrap.yaml similarity index 100% rename from api/v1alpha1/validation/testdata/different-dynamic-resources-user-bootstrap.yaml rename to internal/xds/bootstrap/testdata/validate/different-dynamic-resources-user-bootstrap.yaml diff --git a/api/v1alpha1/validation/testdata/different-xds-cluster-address-bootstrap.yaml b/internal/xds/bootstrap/testdata/validate/different-xds-cluster-address-bootstrap.yaml similarity index 100% rename from api/v1alpha1/validation/testdata/different-xds-cluster-address-bootstrap.yaml rename to internal/xds/bootstrap/testdata/validate/different-xds-cluster-address-bootstrap.yaml diff --git a/api/v1alpha1/validation/testdata/missing-admin-address-user-bootstrap.yaml b/internal/xds/bootstrap/testdata/validate/missing-admin-address-user-bootstrap.yaml similarity index 100% rename from api/v1alpha1/validation/testdata/missing-admin-address-user-bootstrap.yaml rename to internal/xds/bootstrap/testdata/validate/missing-admin-address-user-bootstrap.yaml diff --git a/api/v1alpha1/validation/testdata/valid-user-bootstrap.yaml b/internal/xds/bootstrap/testdata/validate/valid-user-bootstrap.yaml similarity index 100% rename from api/v1alpha1/validation/testdata/valid-user-bootstrap.yaml rename to internal/xds/bootstrap/testdata/validate/valid-user-bootstrap.yaml diff --git a/internal/xds/bootstrap/util.go b/internal/xds/bootstrap/util.go index e00294e2715..701da1f102a 100644 --- a/internal/xds/bootstrap/util.go +++ b/internal/xds/bootstrap/util.go @@ -9,33 +9,57 @@ import ( "fmt" bootstrapv3 "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3" + "k8s.io/utils/ptr" + "sigs.k8s.io/yaml" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/jsonpatch" "github.com/envoyproxy/gateway/internal/utils/proto" _ "github.com/envoyproxy/gateway/internal/xds/extensions" // DON'T REMOVE: import of all extensions ) // ApplyBootstrapConfig applies the bootstrap config to the default bootstrap config and return the result config. +// The defaultBootstrap is expected to be a YAML string func ApplyBootstrapConfig(boostrapConfig *egv1a1.ProxyBootstrap, defaultBootstrap string) (string, error) { bootstrapType := boostrapConfig.Type - if bootstrapType != nil && *bootstrapType == egv1a1.BootstrapTypeMerge { + if bootstrapType == nil { + // The documentation defines that a nil bootstrapType defaults to the "Replace" operation + bootstrapType = ptr.To(egv1a1.BootstrapTypeReplace) + } + switch *bootstrapType { + case egv1a1.BootstrapTypeMerge: mergedBootstrap, err := mergeBootstrap(defaultBootstrap, boostrapConfig.Value) if err != nil { return "", err } return mergedBootstrap, nil + case egv1a1.BootstrapTypeReplace: + // CEL validates that Value will not be nil + return *boostrapConfig.Value, nil + case egv1a1.BootstrapTypeJSONPatch: + patchedBootstrap, err := jsonPatchBootstrap(defaultBootstrap, boostrapConfig.JSONPatches) + if err != nil { + return "", err + } + return patchedBootstrap, nil + default: + // This is unreachable code due to the CEL validation on egv1a1.ProxyBootstrap + return defaultBootstrap, fmt.Errorf("unsupported bootstrap patch type %s", *bootstrapType) } - return boostrapConfig.Value, nil } -func mergeBootstrap(base, override string) (string, error) { +func mergeBootstrap(base string, override *string) (string, error) { + if override == nil { + return base, nil + } dst := &bootstrapv3.Bootstrap{} if err := proto.FromYAML([]byte(base), dst); err != nil { return "", fmt.Errorf("failed to parse default bootstrap config: %w", err) } src := &bootstrapv3.Bootstrap{} - if err := proto.FromYAML([]byte(override), src); err != nil { + if err := proto.FromYAML([]byte(*override), src); err != nil { return "", fmt.Errorf("failed to parse override bootstrap config: %w", err) } @@ -52,3 +76,26 @@ func mergeBootstrap(base, override string) (string, error) { return string(data), nil } + +func jsonPatchBootstrap(baseYAML string, patches []egv1a1.JSONPatchOperation) (string, error) { + jsonBytes, err := yaml.YAMLToJSON([]byte(baseYAML)) + if err != nil { + return baseYAML, err + } + translatedPatches := []ir.JSONPatchOperation{} + for _, p := range patches { + translatedPatches = append(translatedPatches, ir.JSONPatchOperation{ + Op: ir.TranslateJSONPatchOp(p.Op), + Path: p.Path, + JSONPath: p.JSONPath, + From: p.From, + Value: p.Value, + }) + } + jsonBytes, err = jsonpatch.ApplyJSONPatches(jsonBytes, translatedPatches...) + if err != nil { + return baseYAML, err + } + yamlBytes, err := yaml.JSONToYAML(jsonBytes) + return string(yamlBytes), err +} diff --git a/internal/xds/bootstrap/util_test.go b/internal/xds/bootstrap/util_test.go index 5591d0e4f53..bfa5d191c46 100644 --- a/internal/xds/bootstrap/util_test.go +++ b/internal/xds/bootstrap/util_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/require" "k8s.io/utils/ptr" + "sigs.k8s.io/yaml" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" ) @@ -34,6 +35,13 @@ func TestApplyBootstrapConfig(t *testing.T) { }, defaultBootstrap: str, }, + { + name: "merge-user-bootstrap", + boostrapConfig: &egv1a1.ProxyBootstrap{ + Type: ptr.To(egv1a1.BootstrapTypeMerge), + }, + defaultBootstrap: str, + }, { name: "stats_sinks", boostrapConfig: &egv1a1.ProxyBootstrap{ @@ -41,6 +49,13 @@ func TestApplyBootstrapConfig(t *testing.T) { }, defaultBootstrap: str, }, + { + name: "patch-global-config", + boostrapConfig: &egv1a1.ProxyBootstrap{ + Type: ptr.To(egv1a1.BootstrapTypeJSONPatch), + }, + defaultBootstrap: str, + }, } for _, tc := range cases { @@ -48,7 +63,14 @@ func TestApplyBootstrapConfig(t *testing.T) { in, err := loadData(tc.name, "in") require.NoError(t, err) - tc.boostrapConfig.Value = in + switch *tc.boostrapConfig.Type { + case egv1a1.BootstrapTypeJSONPatch: + err = yaml.Unmarshal([]byte(in), &tc.boostrapConfig.JSONPatches) + require.NoError(t, err) + default: + tc.boostrapConfig.Value = &in + } + data, err := ApplyBootstrapConfig(tc.boostrapConfig, tc.defaultBootstrap) require.NoError(t, err) diff --git a/internal/xds/bootstrap/validate.go b/internal/xds/bootstrap/validate.go new file mode 100644 index 00000000000..5e08db72957 --- /dev/null +++ b/internal/xds/bootstrap/validate.go @@ -0,0 +1,89 @@ +// 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 bootstrap + +import ( + "fmt" + + bootstrapv3 "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3" + clusterv3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" + "github.com/google/go-cmp/cmp" + "google.golang.org/protobuf/testing/protocmp" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" + "github.com/envoyproxy/gateway/internal/utils/proto" + _ "github.com/envoyproxy/gateway/internal/xds/extensions" // DON'T REMOVE: import of all extensions +) + +func fetchAndPatchBootstrap(boostrapConfig *egv1a1.ProxyBootstrap) (*bootstrapv3.Bootstrap, *bootstrapv3.Bootstrap, error) { + defaultBootstrapStr, err := GetRenderedBootstrapConfig(nil) + if err != nil { + return nil, nil, err + } + defaultBootstrap := &bootstrapv3.Bootstrap{} + if err := proto.FromYAML([]byte(defaultBootstrapStr), defaultBootstrap); err != nil { + return nil, nil, fmt.Errorf("unable to unmarshal default bootstrap: %w", err) + } + if err := defaultBootstrap.Validate(); err != nil { + return nil, nil, fmt.Errorf("default bootstrap validation failed: %w", err) + } + // Validate user bootstrap config + patchedYaml, err := ApplyBootstrapConfig(boostrapConfig, defaultBootstrapStr) + if err != nil { + return nil, nil, err + } + patchedBootstrap := &bootstrapv3.Bootstrap{} + if err := proto.FromYAML([]byte(patchedYaml), patchedBootstrap); err != nil { + return nil, nil, fmt.Errorf("unable to unmarshal user bootstrap: %w", err) + } + if err := patchedBootstrap.Validate(); err != nil { + return nil, nil, fmt.Errorf("validation failed for user bootstrap: %w", err) + } + return patchedBootstrap, defaultBootstrap, err +} + +// Validate ensures that after applying the provided bootstrap configuration, the resulting +// bootstrap is still OK. +// This code previously was part of the validate logic in api/v1alpha1/validate, but was moved +// here to prevent code in the api packages from accessing code from the internal packages. +func Validate(boostrapConfig *egv1a1.ProxyBootstrap) error { + if boostrapConfig == nil { + return nil + } + // Validate user bootstrap config + // TODO: need validate when enable prometheus? + userBootstrap, defaultBootstrap, err := fetchAndPatchBootstrap(boostrapConfig) + if err != nil { + return err + } + + // Ensure dynamic resources config is same + if userBootstrap.DynamicResources == nil || + cmp.Diff(userBootstrap.DynamicResources, defaultBootstrap.DynamicResources, protocmp.Transform()) != "" { + return fmt.Errorf("dynamic_resources cannot be modified") + } + + // Ensure that the xds_cluster config is same + var userXdsCluster, defaultXdsCluster *clusterv3.Cluster + for _, cluster := range userBootstrap.StaticResources.Clusters { + if cluster.Name == "xds_cluster" { + userXdsCluster = cluster + break + } + } + for _, cluster := range defaultBootstrap.StaticResources.Clusters { + if cluster.Name == "xds_cluster" { + defaultXdsCluster = cluster + break + } + } + if userXdsCluster == nil || + cmp.Diff(userXdsCluster.LoadAssignment, defaultXdsCluster.LoadAssignment, protocmp.Transform()) != "" { + return fmt.Errorf("xds_cluster's loadAssigntment cannot be modified") + } + + return nil +} diff --git a/internal/xds/bootstrap/validate_test.go b/internal/xds/bootstrap/validate_test.go new file mode 100644 index 00000000000..9c6791fea3b --- /dev/null +++ b/internal/xds/bootstrap/validate_test.go @@ -0,0 +1,76 @@ +// 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 bootstrap + +import ( + // Register embed + _ "embed" + "testing" + + "github.com/stretchr/testify/require" + + egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" +) + +var ( + //go:embed testdata/validate/valid-user-bootstrap.yaml + validUserBootstrap string + //go:embed testdata/validate/missing-admin-address-user-bootstrap.yaml + missingAdminAddressUserBootstrap string + //go:embed testdata/validate/different-dynamic-resources-user-bootstrap.yaml + differentDynamicResourcesUserBootstrap string + //go:embed testdata/validate/different-xds-cluster-address-bootstrap.yaml + differentXdsClusterAddressBootstrap string +) + +func TestValidateBootstrap(t *testing.T) { + testCases := []struct { + name string + bootstrap *egv1a1.ProxyBootstrap + expected bool + }{ + { + name: "valid user bootstrap replace type", + bootstrap: &egv1a1.ProxyBootstrap{ + Value: &validUserBootstrap, + }, + expected: true, + }, + { + name: "user bootstrap with missing admin address", + bootstrap: &egv1a1.ProxyBootstrap{ + Value: &missingAdminAddressUserBootstrap, + }, + expected: false, + }, + { + name: "user bootstrap with different dynamic resources", + bootstrap: &egv1a1.ProxyBootstrap{ + Value: &differentDynamicResourcesUserBootstrap, + }, + expected: false, + }, + { + name: "user bootstrap with different xds_cluster endpoint", + bootstrap: &egv1a1.ProxyBootstrap{ + Value: &differentXdsClusterAddressBootstrap, + }, + expected: false, + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + err := Validate(tc.bootstrap) + if tc.expected { + require.NoError(t, err) + } else { + require.Error(t, err) + } + }) + } +} diff --git a/internal/xds/translator/jsonpatch.go b/internal/xds/translator/jsonpatch.go index bb8ed8bef2b..8a56ed689e4 100644 --- a/internal/xds/translator/jsonpatch.go +++ b/internal/xds/translator/jsonpatch.go @@ -16,23 +16,18 @@ 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" resourcev3 "github.com/envoyproxy/go-control-plane/pkg/resource/v3" - jsonpatchv5 "github.com/evanphx/json-patch/v5" "google.golang.org/protobuf/encoding/protojson" "sigs.k8s.io/yaml" "github.com/envoyproxy/gateway/internal/gatewayapi/status" "github.com/envoyproxy/gateway/internal/ir" + "github.com/envoyproxy/gateway/internal/utils/jsonpatch" _ "github.com/envoyproxy/gateway/internal/xds/extensions" // register the generated types to support protojson unmarshalling "github.com/envoyproxy/gateway/internal/xds/types" ) const ( - AddOperation = "add" - RemoveOperation = "remove" - ReplaceOperation = "replace" - CopyOperation = "copy" - MoveOperation = "move" - EmptyPath = "" + EmptyPath = "" ) type typedName struct { @@ -69,24 +64,14 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, envoyPatchPolicies []* err error ) - switch p.Operation.Op { - case AddOperation, ReplaceOperation: - if p.Operation.Value == nil { - tErr := fmt.Errorf("the %s operation requires a value", p.Operation.Op) - tErrs = errors.Join(tErrs, tErr) - continue - } - default: - if p.Operation.Value != nil { - tErr := fmt.Errorf("the value field can not be set for the %s operation", p.Operation.Op) - tErrs = errors.Join(tErrs, tErr) - continue - } + if err := p.Operation.Validate(); err != nil { + tErrs = errors.Join(tErrs, err) + continue } // If Path and JSONPath is "" and op is "add", unmarshal and add the patch as a complete // resource - if p.Operation.Op == AddOperation && p.Operation.IsPathNilOrEmpty() && p.Operation.IsJSONPathNilOrEmpty() { + if p.Operation.Op == ir.JSONPatchOpAdd && p.Operation.IsPathNilOrEmpty() && p.Operation.IsJSONPathNilOrEmpty() { // Convert patch to JSON // The patch library expects an array so convert it into one y, err := yaml.Marshal(p.Operation.Value) @@ -240,152 +225,103 @@ func processJSONPatches(tCtx *types.ResourceVersionTable, envoyPatchPolicies []* } } - var jsonPointers []string - if p.Operation.JSONPath != nil { - path := "" - if p.Operation.Path != nil { - path = *p.Operation.Path + modifiedJSON, err := jsonpatch.ApplyJSONPatches(resourceJSON, p.Operation) + if err != nil { + tErrs = errors.Join(tErrs, err) + continue + } + + // 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 resourcev3.ListenerType: + temp := &listenerv3.Listener{} + if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { + tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) + tErrs = errors.Join(tErrs, tErr) + continue } - jsonPointers, err = ConvertPathToPointers(resourceJSON, *p.Operation.JSONPath, path) - if err != nil { - tErr := fmt.Errorf("unable to convert jsonPath: '%s' into jsonPointers, err: %s", *p.Operation.JSONPath, err.Error()) + if err = temp.Validate(); err != nil { + tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) tErrs = errors.Join(tErrs, tErr) continue } - } else { - jsonPointers = []string{*p.Operation.Path} - } - - for _, path := range jsonPointers { - op := ir.JSONPatchOperation{ - Path: &path, - Op: p.Operation.Op, - Value: p.Operation.Value, - From: p.Operation.From, + if err = deepCopyPtr(temp, listener); err != nil { + tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) + tErrs = errors.Join(tErrs, tErr) + continue } - - // Convert patch to JSON - // The patch library expects an array so convert it into one - y, err := yaml.Marshal([]ir.JSONPatchOperation{op}) - if err != nil { - tErr := fmt.Errorf("unable to marshal patch %+v, err: %s", op, err.Error()) + case resourcev3.RouteType: + temp := &routev3.RouteConfiguration{} + if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { + tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) tErrs = errors.Join(tErrs, tErr) continue } - jsonBytes, err := yaml.YAMLToJSON(y) - if err != nil { - tErr := fmt.Errorf("unable to convert patch to json %s, err: %s", string(y), err.Error()) + if err = temp.Validate(); err != nil { + tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) tErrs = errors.Join(tErrs, tErr) continue } - patchObj, err := jsonpatchv5.DecodePatch(jsonBytes) - if err != nil { - tErr := fmt.Errorf("unable to decode patch %s, err: %s", string(jsonBytes), err.Error()) + if err = deepCopyPtr(temp, routeConfig); err != nil { + tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) tErrs = errors.Join(tErrs, tErr) continue } - - // Apply patch - opts := jsonpatchv5.NewApplyOptions() - opts.EnsurePathExistsOnAdd = true - modifiedJSON, err := patchObj.ApplyWithOptions(resourceJSON, opts) - if err != nil { - tErr := fmt.Errorf("unable to apply patch:\n%s on resource:\n%s, err: %s", string(jsonBytes), string(resourceJSON), err.Error()) + case resourcev3.ClusterType: + temp := &clusterv3.Cluster{} + if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { + tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) tErrs = errors.Join(tErrs, tErr) continue } - - // 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 resourcev3.ListenerType: - temp := &listenerv3.Listener{} - if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { - tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = temp.Validate(); err != nil { - tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = deepCopyPtr(temp, listener); err != nil { - tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) - tErrs = errors.Join(tErrs, tErr) - continue - } - case resourcev3.RouteType: - temp := &routev3.RouteConfiguration{} - if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { - tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = temp.Validate(); err != nil { - tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = deepCopyPtr(temp, routeConfig); err != nil { - tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) - tErrs = errors.Join(tErrs, tErr) - continue - } - case resourcev3.ClusterType: - temp := &clusterv3.Cluster{} - if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { - tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = temp.Validate(); err != nil { - tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = deepCopyPtr(temp, cluster); err != nil { - tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) - tErrs = errors.Join(tErrs, tErr) - continue - } - case resourcev3.EndpointType: - temp := &endpointv3.ClusterLoadAssignment{} - if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { - tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = temp.Validate(); err != nil { - tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = deepCopyPtr(temp, endpoint); err != nil { - tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) - tErrs = errors.Join(tErrs, tErr) - continue - } - case resourcev3.SecretType: - temp := &tlsv3.Secret{} - if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { - tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = temp.Validate(); err != nil { - tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) - tErrs = errors.Join(tErrs, tErr) - continue - } - if err = deepCopyPtr(temp, secret); err != nil { - tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) - tErrs = errors.Join(tErrs, tErr) - continue - } + if err = temp.Validate(); err != nil { + tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) + tErrs = errors.Join(tErrs, tErr) + continue + } + if err = deepCopyPtr(temp, cluster); err != nil { + tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) + tErrs = errors.Join(tErrs, tErr) + continue + } + case resourcev3.EndpointType: + temp := &endpointv3.ClusterLoadAssignment{} + if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { + tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) + tErrs = errors.Join(tErrs, tErr) + continue + } + if err = temp.Validate(); err != nil { + tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) + tErrs = errors.Join(tErrs, tErr) + continue + } + if err = deepCopyPtr(temp, endpoint); err != nil { + tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) + tErrs = errors.Join(tErrs, tErr) + continue + } + case resourcev3.SecretType: + temp := &tlsv3.Secret{} + if err = protojson.Unmarshal(modifiedJSON, temp); err != nil { + tErr := errors.New(unmarshalErrorMessage(err, string(modifiedJSON))) + tErrs = errors.Join(tErrs, tErr) + continue + } + if err = temp.Validate(); err != nil { + tErr := fmt.Errorf("validation failed for xds resource %s, err:%s", string(modifiedJSON), err.Error()) + tErrs = errors.Join(tErrs, tErr) + continue + } + if err = deepCopyPtr(temp, secret); err != nil { + tErr := fmt.Errorf("unable to copy xds resource %s, err: %w", string(modifiedJSON), err) + tErrs = errors.Join(tErrs, tErr) + continue } } + } // Set translation errors for every policy ancestor references diff --git a/internal/xds/translator/translator_test.go b/internal/xds/translator/translator_test.go index 08ab0d24b3b..06a9a86131b 100644 --- a/internal/xds/translator/translator_test.go +++ b/internal/xds/translator/translator_test.go @@ -60,6 +60,7 @@ func TestTranslateXds(t *testing.T) { }, "jsonpatch-add-op-empty-jsonpath": { requireEnvoyPatchPolicies: true, + errMsg: "a patch operation must specify a path or jsonPath", }, "jsonpatch-missing-resource": { requireEnvoyPatchPolicies: true, @@ -74,7 +75,7 @@ func TestTranslateXds(t *testing.T) { }, "jsonpatch-move-op-with-value": { requireEnvoyPatchPolicies: true, - errMsg: "the value field can not be set for the remove operation", + errMsg: "value and from can't be specified with the remove operation", }, "http-route-invalid": { errMsg: "validation failed for xds resource", diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 1dca75a33db..94fad9c157e 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -507,6 +507,7 @@ _Appears in:_ | ----- | ----------- | | `Merge` | Merge merges the provided bootstrap with the default one. The provided bootstrap can add or override a value
within a map, or add a new value to a list.
Please note that the provided bootstrap can't override a value within a list.
| | `Replace` | Replace replaces the default bootstrap with the provided one.
| +| `JSONPatch` | JSONPatch applies the provided JSONPatches to the default bootstrap.
| #### CIDR @@ -2082,6 +2083,7 @@ https://datatracker.ietf.org/doc/html/rfc6902 _Appears in:_ - [EnvoyJSONPatchConfig](#envoyjsonpatchconfig) +- [ProxyBootstrap](#proxybootstrap) | Field | Type | Required | Description | | --- | --- | --- | --- | @@ -2805,8 +2807,9 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `type` | _[BootstrapType](#bootstraptype)_ | false | Type is the type of the bootstrap configuration, it should be either Replace or Merge.
If unspecified, it defaults to Replace. | -| `value` | _string_ | true | Value is a YAML string of the bootstrap. | +| `type` | _[BootstrapType](#bootstraptype)_ | false | Type is the type of the bootstrap configuration, it should be either Replace, Merge, or JSONPatch.
If unspecified, it defaults to Replace. | +| `value` | _string_ | false | Value is a YAML string of the bootstrap. | +| `jsonPatches` | _[JSONPatchOperation](#jsonpatchoperation) array_ | true | JSONPatches is an array of JSONPatches to be applied to the default bootstrap. Patches are
applied in the order in which they are defined. | #### ProxyLogComponent diff --git a/site/content/zh/latest/api/extension_types.md b/site/content/zh/latest/api/extension_types.md index 1dca75a33db..94fad9c157e 100644 --- a/site/content/zh/latest/api/extension_types.md +++ b/site/content/zh/latest/api/extension_types.md @@ -507,6 +507,7 @@ _Appears in:_ | ----- | ----------- | | `Merge` | Merge merges the provided bootstrap with the default one. The provided bootstrap can add or override a value
within a map, or add a new value to a list.
Please note that the provided bootstrap can't override a value within a list.
| | `Replace` | Replace replaces the default bootstrap with the provided one.
| +| `JSONPatch` | JSONPatch applies the provided JSONPatches to the default bootstrap.
| #### CIDR @@ -2082,6 +2083,7 @@ https://datatracker.ietf.org/doc/html/rfc6902 _Appears in:_ - [EnvoyJSONPatchConfig](#envoyjsonpatchconfig) +- [ProxyBootstrap](#proxybootstrap) | Field | Type | Required | Description | | --- | --- | --- | --- | @@ -2805,8 +2807,9 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `type` | _[BootstrapType](#bootstraptype)_ | false | Type is the type of the bootstrap configuration, it should be either Replace or Merge.
If unspecified, it defaults to Replace. | -| `value` | _string_ | true | Value is a YAML string of the bootstrap. | +| `type` | _[BootstrapType](#bootstraptype)_ | false | Type is the type of the bootstrap configuration, it should be either Replace, Merge, or JSONPatch.
If unspecified, it defaults to Replace. | +| `value` | _string_ | false | Value is a YAML string of the bootstrap. | +| `jsonPatches` | _[JSONPatchOperation](#jsonpatchoperation) array_ | true | JSONPatches is an array of JSONPatches to be applied to the default bootstrap. Patches are
applied in the order in which they are defined. | #### ProxyLogComponent diff --git a/test/cel-validation/envoyproxy_test.go b/test/cel-validation/envoyproxy_test.go index 76ba7434048..865da4ac306 100644 --- a/test/cel-validation/envoyproxy_test.go +++ b/test/cel-validation/envoyproxy_test.go @@ -1346,6 +1346,65 @@ func TestEnvoyProxyProvider(t *testing.T) { }, wantErrors: []string{"cannot use envoyHpa if envoyDaemonSet is used"}, }, + { + desc: "mismatched bootstrap patch configured - one", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Bootstrap: &egv1a1.ProxyBootstrap{ + Type: ptr.To(egv1a1.BootstrapType("Merge")), + JSONPatches: []egv1a1.JSONPatchOperation{ + { + Op: egv1a1.JSONPatchOperationType("remove"), + Path: ptr.To("/some/path"), + }, + }, + }, + } + }, + wantErrors: []string{ + "provided bootstrap patch doesn't match the configured patch type", + }, + }, + { + desc: "mismatched bootstrap patch configured - two", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Bootstrap: &egv1a1.ProxyBootstrap{ + Type: ptr.To(egv1a1.BootstrapType("JSONPatch")), + Value: ptr.To("some value"), + }, + } + }, + wantErrors: []string{ + "provided bootstrap patch doesn't match the configured patch type", + }, + }, + { + desc: "missing bootstrap patch - one", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Bootstrap: &egv1a1.ProxyBootstrap{ + Type: ptr.To(egv1a1.BootstrapType("JSONPatch")), + }, + } + }, + wantErrors: []string{ + "provided bootstrap patch doesn't match the configured patch type", + }, + }, + { + desc: "missing bootstrap patch - two", + mutate: func(envoy *egv1a1.EnvoyProxy) { + envoy.Spec = egv1a1.EnvoyProxySpec{ + Bootstrap: &egv1a1.ProxyBootstrap{ + Type: ptr.To(egv1a1.BootstrapType("Merge")), + }, + } + }, + wantErrors: []string{ + "provided bootstrap patch doesn't match the configured patch type", + }, + }, } for _, tc := range cases {