From d17375204111a88f6425ce09d98b9fb9d4be0af2 Mon Sep 17 00:00:00 2001 From: Khaled Emara Date: Tue, 30 Jul 2024 13:52:41 +0300 Subject: [PATCH] feat(json): unmarshal once per policy (#10701) Signed-off-by: Khaled Emara Co-authored-by: Mariam Fahmy Co-authored-by: shuting --- api/kyverno/v1/common_types.go | 22 ++- api/kyverno/v1/wrappers.go | 79 ++++++++++ api/kyverno/v1/zz_generated.deepcopy.go | 6 +- docs/user/crd/index.html | 78 +++++++++- docs/user/crd/kyverno.v1.html | 140 +++++++++++++++++- .../kyverno/v1/foreachmutation.go | 4 +- .../kyverno/v1/foreachvalidation.go | 5 +- pkg/engine/forceMutate.go | 13 +- pkg/engine/handlers/mutation/common.go | 11 +- .../handlers/validation/validate_resource.go | 12 +- pkg/policy/mutate/validate.go | 16 +- pkg/policy/validate/validate.go | 2 +- pkg/utils/api/json.go | 16 -- pkg/validation/policy/validate.go | 18 +-- 14 files changed, 345 insertions(+), 77 deletions(-) create mode 100644 api/kyverno/v1/wrappers.go diff --git a/api/kyverno/v1/common_types.go b/api/kyverno/v1/common_types.go index 15b4a2662e..5592dabb30 100644 --- a/api/kyverno/v1/common_types.go +++ b/api/kyverno/v1/common_types.go @@ -427,7 +427,16 @@ type ForEachMutation struct { // Foreach declares a nested foreach iterator // +optional - ForEachMutation *apiextv1.JSON `json:"foreach,omitempty" yaml:"foreach,omitempty"` + // +kubebuilder:validation:Schemaless + // +kubebuilder:pruning:PreserveUnknownFields + ForEachMutation *ForEachMutationWrapper `json:"foreach,omitempty" yaml:"foreach,omitempty"` +} + +func (m *ForEachMutation) GetForEachMutation() []ForEachMutation { + if m.ForEachMutation == nil { + return nil + } + return m.ForEachMutation.Items } func (m *ForEachMutation) GetPatchStrategicMerge() apiextensions.JSON { @@ -690,7 +699,16 @@ type ForEachValidation struct { // Foreach declares a nested foreach iterator // +optional - ForEachValidation *apiextv1.JSON `json:"foreach,omitempty" yaml:"foreach,omitempty"` + // +kubebuilder:validation:Schemaless + // +kubebuilder:pruning:PreserveUnknownFields + ForEachValidation *ForEachValidationWrapper `json:"foreach,omitempty" yaml:"foreach,omitempty"` +} + +func (v *ForEachValidation) GetForEachValidation() []ForEachValidation { + if v.ForEachValidation == nil { + return nil + } + return v.ForEachValidation.Items } func (v *ForEachValidation) GetPattern() apiextensions.JSON { diff --git a/api/kyverno/v1/wrappers.go b/api/kyverno/v1/wrappers.go new file mode 100644 index 0000000000..710bfd0dcc --- /dev/null +++ b/api/kyverno/v1/wrappers.go @@ -0,0 +1,79 @@ +package v1 + +import ( + "encoding/json" + + "github.com/jinzhu/copier" +) + +// ForEachValidationWrapper contains a list of ForEach descriptors. +// +k8s:deepcopy-gen=false +type ForEachValidationWrapper struct { + // Item is a descriptor on how to iterate over the list of items. + // +optional + Items []ForEachValidation `json:"-"` +} + +func (in *ForEachValidationWrapper) DeepCopyInto(out *ForEachValidationWrapper) { + if err := copier.Copy(out, in); err != nil { + panic("deep copy failed") + } +} + +func (in *ForEachValidationWrapper) DeepCopy() *ForEachValidationWrapper { + if in == nil { + return nil + } + out := new(ForEachValidationWrapper) + in.DeepCopyInto(out) + return out +} + +func (a *ForEachValidationWrapper) MarshalJSON() ([]byte, error) { + return json.Marshal(a.Items) +} + +func (a *ForEachValidationWrapper) UnmarshalJSON(data []byte) error { + var res []ForEachValidation + if err := json.Unmarshal(data, &res); err != nil { + return err + } + a.Items = res + return nil +} + +// ForEachMutationWrapper contains a list of ForEach descriptors. +// +k8s:deepcopy-gen=false +type ForEachMutationWrapper struct { + // Item is a descriptor on how to iterate over the list of items. + // +optional + Items []ForEachMutation `json:"-"` +} + +func (in *ForEachMutationWrapper) DeepCopyInto(out *ForEachMutationWrapper) { + if err := copier.Copy(out, in); err != nil { + panic("deep copy failed") + } +} + +func (in *ForEachMutationWrapper) DeepCopy() *ForEachMutationWrapper { + if in == nil { + return nil + } + out := new(ForEachMutationWrapper) + in.DeepCopyInto(out) + return out +} + +func (a *ForEachMutationWrapper) MarshalJSON() ([]byte, error) { + return json.Marshal(a.Items) +} + +func (a *ForEachMutationWrapper) UnmarshalJSON(data []byte) error { + var res []ForEachMutation + if err := json.Unmarshal(data, &res); err != nil { + return err + } + a.Items = res + return nil +} diff --git a/api/kyverno/v1/zz_generated.deepcopy.go b/api/kyverno/v1/zz_generated.deepcopy.go index 478a57f7e7..a2e7401891 100755 --- a/api/kyverno/v1/zz_generated.deepcopy.go +++ b/api/kyverno/v1/zz_generated.deepcopy.go @@ -565,8 +565,7 @@ func (in *ForEachMutation) DeepCopyInto(out *ForEachMutation) { } if in.ForEachMutation != nil { in, out := &in.ForEachMutation, &out.ForEachMutation - *out = new(apiextensionsv1.JSON) - (*in).DeepCopyInto(*out) + *out = (*in).DeepCopy() } return } @@ -618,8 +617,7 @@ func (in *ForEachValidation) DeepCopyInto(out *ForEachValidation) { } if in.ForEachValidation != nil { in, out := &in.ForEachValidation, &out.ForEachValidation - *out = new(apiextensionsv1.JSON) - (*in).DeepCopyInto(*out) + *out = (*in).DeepCopy() } return } diff --git a/docs/user/crd/index.html b/docs/user/crd/index.html index f87b0056b5..13d4807a30 100644 --- a/docs/user/crd/index.html +++ b/docs/user/crd/index.html @@ -1615,6 +1615,7 @@ string

(Appears on: +ForEachMutationWrapper, Mutation)

@@ -1718,8 +1719,8 @@ See https://tools.ietf.org/html/rf foreach
- -Kubernetes apiextensions/v1.JSON + +ForEachMutationWrapper @@ -1731,10 +1732,45 @@ Kubernetes apiextensions/v1.JSON

+

ForEachMutationWrapper +

+

+(Appears on: +ForEachMutation) +

+

+

ForEachMutationWrapper contains a list of ForEach descriptors.

+

+ + + + + + + + + + + + + +
FieldDescription
+-
+ + +[]ForEachMutation + + +		 +(Optional) +

Item is a descriptor on how to iterate over the list of items.

+
+

ForEachValidation

(Appears on: +ForEachValidationWrapper, Validation, Validation)

@@ -1852,8 +1888,8 @@ Deny foreach
- -Kubernetes apiextensions/v1.JSON + +ForEachValidationWrapper @@ -1865,6 +1901,40 @@ Kubernetes apiextensions/v1.JSON
+

ForEachValidationWrapper +

+

+(Appears on: +ForEachValidation) +

+

+

ForEachValidationWrapper contains a list of ForEach descriptors.

+

+ + + + + + + + + + + + + +
FieldDescription
+-
+ + +[]ForEachValidation + + +		 +(Optional) +

Item is a descriptor on how to iterate over the list of items.

+
+

ForeachOrder (string alias)

diff --git a/docs/user/crd/kyverno.v1.html b/docs/user/crd/kyverno.v1.html index 2ef4588bd3..dc349e3865 100644 --- a/docs/user/crd/kyverno.v1.html +++ b/docs/user/crd/kyverno.v1.html @@ -3318,6 +3318,7 @@ Dryrun requires additional permissions. See config/dryrun/dryrun_rbac.yaml

(Appears in: + ForEachMutationWrapper, Mutation)

@@ -3529,7 +3530,9 @@ See https://tools.ietf.org/html/rfc6902 and https://kubectl.docs.kubernetes.io/r - k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSON + + ForEachMutationWrapper + @@ -3548,6 +3551,71 @@ See https://tools.ietf.org/html/rfc6902 and https://kubectl.docs.kubernetes.io/r + + + + +

ForEachMutationWrapper +

+ + +

+ (Appears in: + ForEachMutation) +

+ + +

ForEachMutationWrapper contains a list of ForEach descriptors.

+

+ + + + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
- + +
+ + + + + + []ForEachMutation + + + + 		+ + +

Item is a descriptor on how to iterate over the list of items.

+ + + + + +
@@ -3558,6 +3626,7 @@ See https://tools.ietf.org/html/rfc6902 and https://kubectl.docs.kubernetes.io/r

(Appears in: + ForEachValidationWrapper, Validation)

@@ -3795,7 +3864,9 @@ must be satisfied for the validation rule to succeed.

- k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSON + + ForEachValidationWrapper + @@ -3814,6 +3885,71 @@ must be satisfied for the validation rule to succeed.

+ + + + +

ForEachValidationWrapper +

+ + +

+ (Appears in: + ForEachValidation) +

+ + +

ForEachValidationWrapper contains a list of ForEach descriptors.

+

+ + + + + + + + + + + + + + + + + + + + + + + + + + + +
FieldDescription
- + +
+ + + + + + []ForEachValidation + + + + 		+ + +

Item is a descriptor on how to iterate over the list of items.

+ + + + + +
diff --git a/pkg/client/applyconfigurations/kyverno/v1/foreachmutation.go b/pkg/client/applyconfigurations/kyverno/v1/foreachmutation.go index 96df14f479..d6f56a6696 100644 --- a/pkg/client/applyconfigurations/kyverno/v1/foreachmutation.go +++ b/pkg/client/applyconfigurations/kyverno/v1/foreachmutation.go @@ -32,7 +32,7 @@ type ForEachMutationApplyConfiguration struct { AnyAllConditions *AnyAllConditionsApplyConfiguration `json:"preconditions,omitempty"` RawPatchStrategicMerge *apiextensionsv1.JSON `json:"patchStrategicMerge,omitempty"` PatchesJSON6902 *string `json:"patchesJson6902,omitempty"` - ForEachMutation *apiextensionsv1.JSON `json:"foreach,omitempty"` + ForEachMutation *v1.ForEachMutationWrapper `json:"foreach,omitempty"` } // ForEachMutationApplyConfiguration constructs an declarative configuration of the ForEachMutation type for use with @@ -97,7 +97,7 @@ func (b *ForEachMutationApplyConfiguration) WithPatchesJSON6902(value string) *F // WithForEachMutation sets the ForEachMutation field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the ForEachMutation field is set to the value of the last call. -func (b *ForEachMutationApplyConfiguration) WithForEachMutation(value apiextensionsv1.JSON) *ForEachMutationApplyConfiguration { +func (b *ForEachMutationApplyConfiguration) WithForEachMutation(value v1.ForEachMutationWrapper) *ForEachMutationApplyConfiguration { b.ForEachMutation = &value return b } diff --git a/pkg/client/applyconfigurations/kyverno/v1/foreachvalidation.go b/pkg/client/applyconfigurations/kyverno/v1/foreachvalidation.go index c18cd3240c..04bf1f4f8b 100644 --- a/pkg/client/applyconfigurations/kyverno/v1/foreachvalidation.go +++ b/pkg/client/applyconfigurations/kyverno/v1/foreachvalidation.go @@ -19,6 +19,7 @@ limitations under the License. package v1 import ( + kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) @@ -32,7 +33,7 @@ type ForEachValidationApplyConfiguration struct { RawPattern *apiextensionsv1.JSON `json:"pattern,omitempty"` RawAnyPattern *apiextensionsv1.JSON `json:"anyPattern,omitempty"` Deny *DenyApplyConfiguration `json:"deny,omitempty"` - ForEachValidation *apiextensionsv1.JSON `json:"foreach,omitempty"` + ForEachValidation *kyvernov1.ForEachValidationWrapper `json:"foreach,omitempty"` } // ForEachValidationApplyConfiguration constructs an declarative configuration of the ForEachValidation type for use with @@ -105,7 +106,7 @@ func (b *ForEachValidationApplyConfiguration) WithDeny(value *DenyApplyConfigura // WithForEachValidation sets the ForEachValidation field in the declarative configuration to the given value // and returns the receiver, so that objects can be built by chaining "With" function invocations. // If called multiple times, the ForEachValidation field is set to the value of the last call. -func (b *ForEachValidationApplyConfiguration) WithForEachValidation(value apiextensionsv1.JSON) *ForEachValidationApplyConfiguration { +func (b *ForEachValidationApplyConfiguration) WithForEachValidation(value kyvernov1.ForEachValidationWrapper) *ForEachValidationApplyConfiguration { b.ForEachValidation = &value return b } diff --git a/pkg/engine/forceMutate.go b/pkg/engine/forceMutate.go index e6ca6edd11..741de18b11 100644 --- a/pkg/engine/forceMutate.go +++ b/pkg/engine/forceMutate.go @@ -1,15 +1,12 @@ package engine import ( - "fmt" - "github.com/go-logr/logr" kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine/context" "github.com/kyverno/kyverno/pkg/engine/internal" "github.com/kyverno/kyverno/pkg/engine/mutate" "github.com/kyverno/kyverno/pkg/engine/variables" - "github.com/kyverno/kyverno/pkg/utils/api" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -64,13 +61,9 @@ func ForceMutate( func applyForEachMutate(name string, foreach []kyvernov1.ForEachMutation, resource unstructured.Unstructured, logger logr.Logger) (patchedResource unstructured.Unstructured, err error) { patchedResource = resource for _, fe := range foreach { - if fe.ForEachMutation != nil { - nestedForEach, err := api.DeserializeJSONArray[kyvernov1.ForEachMutation](fe.ForEachMutation) - if err != nil { - return patchedResource, fmt.Errorf("failed to deserialize foreach: %w", err) - } - - return applyForEachMutate(name, nestedForEach, patchedResource, logger) + fem := fe.GetForEachMutation() + if len(fem) > 0 { + return applyForEachMutate(name, fem, patchedResource, logger) } patchedResource, err = applyPatches(fe.GetPatchStrategicMerge(), fe.PatchesJSON6902, patchedResource, logger) diff --git a/pkg/engine/handlers/mutation/common.go b/pkg/engine/handlers/mutation/common.go index 4ba5609068..5ac473daca 100644 --- a/pkg/engine/handlers/mutation/common.go +++ b/pkg/engine/handlers/mutation/common.go @@ -11,7 +11,6 @@ import ( "github.com/kyverno/kyverno/pkg/engine/internal" "github.com/kyverno/kyverno/pkg/engine/mutate" engineutils "github.com/kyverno/kyverno/pkg/engine/utils" - "github.com/kyverno/kyverno/pkg/utils/api" datautils "github.com/kyverno/kyverno/pkg/utils/data" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -110,18 +109,14 @@ func (f *forEachMutator) mutateElements(ctx context.Context, foreach kyvernov1.F } var mutateResp *mutate.Response - if foreach.ForEachMutation != nil { - nestedForEach, err := api.DeserializeJSONArray[kyvernov1.ForEachMutation](foreach.ForEachMutation) - if err != nil { - return mutate.NewErrorResponse("failed to deserialize foreach", err) - } - + fem := foreach.GetForEachMutation() + if len(fem) > 0 { m := &forEachMutator{ rule: f.rule, policyContext: f.policyContext, resource: patchedResource, logger: f.logger, - foreach: nestedForEach, + foreach: fem, nesting: f.nesting + 1, contextLoader: f.contextLoader, } diff --git a/pkg/engine/handlers/validation/validate_resource.go b/pkg/engine/handlers/validation/validate_resource.go index f181fcbc67..bcb5299c91 100644 --- a/pkg/engine/handlers/validation/validate_resource.go +++ b/pkg/engine/handlers/validation/validate_resource.go @@ -16,7 +16,6 @@ import ( engineutils "github.com/kyverno/kyverno/pkg/engine/utils" "github.com/kyverno/kyverno/pkg/engine/validate" "github.com/kyverno/kyverno/pkg/engine/variables" - "github.com/kyverno/kyverno/pkg/utils/api" datautils "github.com/kyverno/kyverno/pkg/utils/data" stringutils "github.com/kyverno/kyverno/pkg/utils/strings" "github.com/pkg/errors" @@ -103,9 +102,12 @@ func newForEachValidator( if err != nil { return nil, fmt.Errorf("failed to convert ruleCopy.Validation.ForEachValidation.AnyAllConditions: %w", err) } - nestedForEach, err := api.DeserializeJSONArray[kyvernov1.ForEachValidation](foreach.ForEachValidation) - if err != nil { - return nil, fmt.Errorf("failed to convert ruleCopy.Validation.ForEachValidation.AnyAllConditions: %w", err) + var loopItems []kyvernov1.ForEachValidation + fev := foreach.GetForEachValidation() + if len(fev) > 0 { + loopItems = fev + } else { + loopItems = make([]kyvernov1.ForEachValidation, 0) } return &validator{ log: log, @@ -117,7 +119,7 @@ func newForEachValidator( pattern: foreach.GetPattern(), anyPattern: foreach.GetAnyPattern(), deny: foreach.Deny, - forEach: nestedForEach, + forEach: loopItems, nesting: nesting, }, nil } diff --git a/pkg/policy/mutate/validate.go b/pkg/policy/mutate/validate.go index 320916f196..4d7f221a29 100644 --- a/pkg/policy/mutate/validate.go +++ b/pkg/policy/mutate/validate.go @@ -8,10 +8,8 @@ import ( kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" "github.com/kyverno/kyverno/pkg/engine/variables/regex" "github.com/kyverno/kyverno/pkg/policy/auth" - "github.com/kyverno/kyverno/pkg/utils/api" kubeutils "github.com/kyverno/kyverno/pkg/utils/kube" "go.uber.org/multierr" - v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) // Mutate provides implementation to validate 'mutate' rule @@ -55,12 +53,13 @@ func (m *Mutate) Validate(ctx context.Context) (string, error) { func (m *Mutate) validateForEach(tag string, foreach []kyvernov1.ForEachMutation) (string, error) { for i, fe := range foreach { tag = tag + fmt.Sprintf("foreach[%d]", i) - if fe.ForEachMutation != nil { + fem := fe.GetForEachMutation() + if len(fem) > 0 { if fe.Context != nil || fe.AnyAllConditions != nil || fe.PatchesJSON6902 != "" || fe.RawPatchStrategicMerge != nil { return tag, fmt.Errorf("a nested foreach cannot contain other declarations") } - return m.validateNestedForEach(tag, fe.ForEachMutation) + return m.validateNestedForEach(tag, fem) } psm := fe.GetPatchStrategicMerge() @@ -72,13 +71,12 @@ func (m *Mutate) validateForEach(tag string, foreach []kyvernov1.ForEachMutation return "", nil } -func (m *Mutate) validateNestedForEach(tag string, j *v1.JSON) (string, error) { - nestedForeach, err := api.DeserializeJSONArray[kyvernov1.ForEachMutation](j) - if err != nil { - return tag, fmt.Errorf("invalid foreach syntax: %w", err) +func (m *Mutate) validateNestedForEach(tag string, j []kyvernov1.ForEachMutation) (string, error) { + if j != nil { + return m.validateForEach(tag, j) } - return m.validateForEach(tag, nestedForeach) + return "", nil } func (m *Mutate) hasForEach() bool { diff --git a/pkg/policy/validate/validate.go b/pkg/policy/validate/validate.go index fa0a079fc2..49e6282df3 100644 --- a/pkg/policy/validate/validate.go +++ b/pkg/policy/validate/validate.go @@ -204,7 +204,7 @@ func foreachElemCount(foreach kyvernov1.ForEachValidation) int { count++ } - if foreach.ForEachValidation != nil { + if foreach.GetForEachValidation() != nil && len(foreach.GetForEachValidation()) > 0 { count++ } diff --git a/pkg/utils/api/json.go b/pkg/utils/api/json.go index 3f3d3f9703..9fdc9fa6c7 100644 --- a/pkg/utils/api/json.go +++ b/pkg/utils/api/json.go @@ -8,22 +8,6 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" ) -// Deserialize "apiextensions.JSON" to a typed array -func DeserializeJSONArray[T any](in apiextensions.JSON) ([]T, error) { - if in == nil { - return nil, nil - } - data, err := json.Marshal(in) - if err != nil { - return nil, err - } - var res []T - if err := json.Unmarshal(data, &res); err != nil { - return nil, err - } - return res, nil -} - // ApiextensionsJsonToKyvernoConditions takes in user-provided conditions in abstract apiextensions.JSON form // and converts it into []kyverno.Condition or kyverno.AnyAllConditions according to its content. // it also helps in validating the condtions as it returns an error when the conditions are provided wrongfully by the user. diff --git a/pkg/validation/policy/validate.go b/pkg/validation/policy/validate.go index 359e013e96..ead41c3d52 100644 --- a/pkg/validation/policy/validate.go +++ b/pkg/validation/policy/validate.go @@ -1002,12 +1002,9 @@ func validateValidationForEach(foreach []kyvernov1.ForEachValidation, schemaKey } } } - if fe.ForEachValidation != nil { - nestedForEach, err := apiutils.DeserializeJSONArray[kyvernov1.ForEachValidation](fe.ForEachValidation) - if err != nil { - return schemaKey, err - } - if path, err := validateValidationForEach(nestedForEach, schemaKey); err != nil { + fev := fe.GetForEachValidation() + if len(fev) > 0 { + if path, err := validateValidationForEach(fev, schemaKey); err != nil { return fmt.Sprintf("%s.%s", schemaKey, path), err } } @@ -1022,12 +1019,9 @@ func validateMutationForEach(foreach []kyvernov1.ForEachMutation, schemaKey stri return fmt.Sprintf("%s.%s", schemaKey, path), err } } - if fe.ForEachMutation != nil { - nestedForEach, err := apiutils.DeserializeJSONArray[kyvernov1.ForEachMutation](fe.ForEachMutation) - if err != nil { - return schemaKey, err - } - if path, err := validateMutationForEach(nestedForEach, schemaKey); err != nil { + fem := fe.GetForEachMutation() + if len(fem) > 0 { + if path, err := validateMutationForEach(fem, schemaKey); err != nil { return fmt.Sprintf("%s.%s", schemaKey, path), err } }