1
0
Fork 0
mirror of https://github.com/kyverno/kyverno.git synced 2025-03-05 07:26:55 +00:00

feat(mutate): minimize unmarshals (#10702)

* feat(mutate): minimize unmarshals

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* test(mutate): test type assertion

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* chore(codegen): remove unused import

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

---------

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>
This commit is contained in:
Khaled Emara 2024-08-09 14:12:20 +03:00 committed by GitHub
parent 6e73e8514b
commit 65a43d2059
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 61 additions and 35 deletions

View file

@ -418,7 +418,9 @@ type ForEachMutation struct {
// See https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/
// and https://kubectl.docs.kubernetes.io/references/kustomize/patchesstrategicmerge/.
// +optional
RawPatchStrategicMerge *apiextv1.JSON `json:"patchStrategicMerge,omitempty" yaml:"patchStrategicMerge,omitempty"`
// +kubebuilder:validation:Schemaless
// +kubebuilder:pruning:PreserveUnknownFields
RawPatchStrategicMerge *kyverno.Any `json:"patchStrategicMerge,omitempty" yaml:"patchStrategicMerge,omitempty"`
// PatchesJSON6902 is a list of RFC 6902 JSON Patch declarations used to modify resources.
// See https://tools.ietf.org/html/rfc6902 and https://kubectl.docs.kubernetes.io/references/kustomize/patchesjson6902/.
@ -439,12 +441,12 @@ func (m *ForEachMutation) GetForEachMutation() []ForEachMutation {
return m.ForEachMutation.Items
}
func (m *ForEachMutation) GetPatchStrategicMerge() apiextensions.JSON {
return FromJSON(m.RawPatchStrategicMerge)
func (m *ForEachMutation) GetPatchStrategicMerge() any {
return kyverno.FromAny(m.RawPatchStrategicMerge)
}
func (m *ForEachMutation) SetPatchStrategicMerge(in apiextensions.JSON) {
m.RawPatchStrategicMerge = ToJSON(in)
func (m *ForEachMutation) SetPatchStrategicMerge(in any) {
m.RawPatchStrategicMerge = kyverno.ToAny(in)
}
// Validation defines checks to be performed on matching resources.

View file

@ -559,8 +559,7 @@ func (in *ForEachMutation) DeepCopyInto(out *ForEachMutation) {
}
if in.RawPatchStrategicMerge != nil {
in, out := &in.RawPatchStrategicMerge, &out.RawPatchStrategicMerge
*out = new(apiextensionsv1.JSON)
(*in).DeepCopyInto(*out)
*out = (*in).DeepCopy()
}
if in.ForEachMutation != nil {
in, out := &in.ForEachMutation, &out.ForEachMutation

View file

@ -1724,9 +1724,7 @@ See: <a href="https://kyverno.io/docs/writing-policies/preconditions/">https://k
<td>
<code>patchStrategicMerge</code><br/>
<em>
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#json-v1-apiextensions">
Kubernetes apiextensions/v1.JSON
</a>
github.com/kyverno/kyverno/api/kyverno.Any
</em>
</td>
<td>

View file

@ -3540,7 +3540,7 @@ See: https://kyverno.io/docs/writing-policies/preconditions/</p>
<span style="font-family: monospace">k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSON</span>
<span style="font-family: monospace">github.com/kyverno/kyverno/api/kyverno.Any</span>
</td>

1
go.mod
View file

@ -331,6 +331,7 @@ require (
github.com/spf13/viper v1.19.0 // indirect
github.com/spiffe/go-spiffe/v2 v2.2.0 // indirect
github.com/stoewer/go-strcase v1.3.0 // indirect
github.com/stretchr/objx v0.5.2 // indirect
github.com/subosito/gotenv v1.6.0 // indirect
github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect
github.com/tchap/go-patricia/v2 v2.3.1 // indirect

View file

@ -19,8 +19,8 @@ limitations under the License.
package v1
import (
kyverno "github.com/kyverno/kyverno/api/kyverno"
v1 "github.com/kyverno/kyverno/api/kyverno/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
)
// ForEachMutationApplyConfiguration represents an declarative configuration of the ForEachMutation type for use
@ -30,7 +30,7 @@ type ForEachMutationApplyConfiguration struct {
Order *v1.ForeachOrder `json:"order,omitempty"`
Context []ContextEntryApplyConfiguration `json:"context,omitempty"`
AnyAllConditions *AnyAllConditionsApplyConfiguration `json:"preconditions,omitempty"`
RawPatchStrategicMerge *apiextensionsv1.JSON `json:"patchStrategicMerge,omitempty"`
RawPatchStrategicMerge *kyverno.Any `json:"patchStrategicMerge,omitempty"`
PatchesJSON6902 *string `json:"patchesJson6902,omitempty"`
ForEachMutation *v1.ForEachMutationWrapper `json:"foreach,omitempty"`
}
@ -81,7 +81,7 @@ func (b *ForEachMutationApplyConfiguration) WithAnyAllConditions(value *AnyAllCo
// WithRawPatchStrategicMerge sets the RawPatchStrategicMerge 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 RawPatchStrategicMerge field is set to the value of the last call.
func (b *ForEachMutationApplyConfiguration) WithRawPatchStrategicMerge(value apiextensionsv1.JSON) *ForEachMutationApplyConfiguration {
func (b *ForEachMutationApplyConfiguration) WithRawPatchStrategicMerge(value kyverno.Any) *ForEachMutationApplyConfiguration {
b.RawPatchStrategicMerge = &value
return b
}

View file

@ -69,7 +69,7 @@ func (f *forEachMutator) mutateElements(ctx context.Context, foreach kyvernov1.F
reverse := false
// if it's a patch strategic merge, reverse by default
if foreach.RawPatchStrategicMerge != nil {
if foreach.GetPatchStrategicMerge() != nil {
reverse = true
}
if foreach.Order != nil {

View file

@ -1,7 +1,6 @@
package mutate
import (
"encoding/json"
"fmt"
"strings"
@ -11,7 +10,6 @@ import (
"github.com/kyverno/kyverno/pkg/engine/context"
"github.com/kyverno/kyverno/pkg/engine/mutate/patch"
"github.com/kyverno/kyverno/pkg/engine/variables"
datautils "github.com/kyverno/kyverno/pkg/utils/data"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)
@ -77,7 +75,7 @@ func ForEach(name string, foreach kyvernov1.ForEachMutation, policyContext engin
if err != nil {
return NewErrorResponse("variable substitution failed", err)
}
patcher := NewPatcher(fe.GetPatchStrategicMerge(), fe.PatchesJSON6902)
patcher := NewPatcher(fe["patchStrategicMerge"], fe["patchesJson6902"].(string))
if patcher == nil {
return NewErrorResponse("empty mutate rule", nil)
}
@ -101,28 +99,22 @@ func ForEach(name string, foreach kyvernov1.ForEachMutation, policyContext engin
return NewResponse(engineapi.RuleStatusPass, *patchedResource, "resource patched")
}
func substituteAllInForEach(fe kyvernov1.ForEachMutation, ctx context.Interface, logger logr.Logger) (*kyvernov1.ForEachMutation, error) {
jsonObj, err := datautils.ToMap(fe)
func substituteAllInForEach(fe kyvernov1.ForEachMutation, ctx context.Interface, logger logr.Logger) (map[string]interface{}, error) {
patchesMap := make(map[string]interface{})
patchesMap["patchStrategicMerge"] = fe.GetPatchStrategicMerge()
patchesMap["patchesJson6902"] = fe.PatchesJSON6902
subedPatchesMap, err := variables.SubstituteAll(logger, ctx, patchesMap)
if err != nil {
return nil, err
}
data, err := variables.SubstituteAll(logger, ctx, jsonObj)
if err != nil {
return nil, err
typedMap, ok := subedPatchesMap.(map[string]interface{})
if !ok {
return nil, fmt.Errorf("failed to convert patched map to map[string]interface{}")
}
bytes, err := json.Marshal(data)
if err != nil {
return nil, err
}
var updatedForEach kyvernov1.ForEachMutation
if err := json.Unmarshal(bytes, &updatedForEach); err != nil {
return nil, err
}
return &updatedForEach, nil
return typedMap, nil
}
func NewPatcher(strategicMergePatch apiextensions.JSON, jsonPatch string) patch.Patcher {

View file

@ -7,10 +7,13 @@ import (
"github.com/go-logr/logr"
types "github.com/kyverno/kyverno/api/kyverno/v1"
v1 "github.com/kyverno/kyverno/api/kyverno/v1"
"github.com/kyverno/kyverno/pkg/config"
engineapi "github.com/kyverno/kyverno/pkg/engine/api"
"github.com/kyverno/kyverno/pkg/engine/context"
"github.com/kyverno/kyverno/pkg/engine/jmespath"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@ -235,3 +238,34 @@ func TestProcessPatches_RemovePathDoesntExist_NotEmptyResult(t *testing.T) {
unstructured.SetNestedField(resource.UnstructuredContent(), "label2Value", "metadata", "labels", "label2")
require.Equal(t, resource, patched)
}
type MockContext struct {
context.Interface
mock.Mock
}
func (m *MockContext) Query(query string) (interface{}, error) {
args := m.Called(query)
return args.Get(0), args.Error(1)
}
func (m *MockContext) QueryOperation() string {
args := m.Called()
return args.Get(0).(string)
}
func TestSubstituteAllInForEach_InvalidTypeConversion(t *testing.T) {
ctx := &MockContext{}
// Simulate a scenario where the substitution returns an unexpected type
ctx.On("Query", mock.Anything).Return(true, nil)
ctx.On("QueryOperation").Return("CREATE")
foreach := v1.ForEachMutation{
PatchesJSON6902: "string",
}
fe, err := substituteAllInForEach(foreach, ctx, logr.Discard())
assert.NoError(t, err)
assert.IsType(t, "string", fe["patchesJson6902"])
}

View file

@ -55,7 +55,7 @@ func (m *Mutate) validateForEach(tag string, foreach []kyvernov1.ForEachMutation
tag = tag + fmt.Sprintf("foreach[%d]", i)
fem := fe.GetForEachMutation()
if len(fem) > 0 {
if fe.Context != nil || fe.AnyAllConditions != nil || fe.PatchesJSON6902 != "" || fe.RawPatchStrategicMerge != nil {
if fe.Context != nil || fe.AnyAllConditions != nil || fe.PatchesJSON6902 != "" || fe.GetPatchStrategicMerge() != nil {
return tag, fmt.Errorf("a nested foreach cannot contain other declarations")
}