diff --git a/pkg/autogen/autogen.go b/pkg/autogen/autogen.go index cdfa52b18b..70967259af 100644 --- a/pkg/autogen/autogen.go +++ b/pkg/autogen/autogen.go @@ -178,7 +178,7 @@ func GenerateRulePatches(spec *kyvernov1.Spec, controllers string) (rulePatches operation = "replace" patchPostion = existingIndex } - patch := jsonutils.NewPatch(fmt.Sprintf("/spec/rules/%s", strconv.Itoa(patchPostion)), operation, genRule) + patch := jsonutils.NewPatchOperation(fmt.Sprintf("/spec/rules/%s", strconv.Itoa(patchPostion)), operation, genRule) pbytes, err := patch.Marshal() if err != nil { errs = append(errs, err) diff --git a/pkg/policy/apply.go b/pkg/policy/apply.go index 1161937f94..205938c26f 100644 --- a/pkg/policy/apply.go +++ b/pkg/policy/apply.go @@ -135,7 +135,7 @@ func extractPatchPath(patches [][]byte, log logr.Logger) string { var resultPath []string // extract the patch path and value for _, patch := range patches { - if data, err := jsonutils.UnmarshalPatch(patch); err != nil { + if data, err := jsonutils.UnmarshalPatchOperation(patch); err != nil { log.Error(err, "failed to decode the generate patch", "patch", string(patch)) continue } else { diff --git a/pkg/policymutation/policymutation.go b/pkg/policymutation/policymutation.go index 8788a3f089..8ab47eef4e 100644 --- a/pkg/policymutation/policymutation.go +++ b/pkg/policymutation/policymutation.go @@ -56,7 +56,7 @@ func defaultBackgroundFlag(spec *kyvernov1.Spec, log logr.Logger) ([]byte, strin if spec.Background == nil { defaultVal := true log.V(4).Info("setting default value", "spec.background", true) - patchByte, err := jsonutils.MarshalPatch("/spec/background", "add", &defaultVal) + patchByte, err := jsonutils.MarshalPatchOperation("/spec/background", "add", &defaultVal) if err != nil { log.Error(err, "failed to set default value", "spec.background", true) return nil, "" @@ -72,7 +72,7 @@ func defaultvalidationFailureAction(spec *kyvernov1.Spec, log logr.Logger) ([]by if spec.ValidationFailureAction == "" { audit := kyvernov1.Audit log.V(4).Info("setting default value", "spec.validationFailureAction", audit) - patchByte, err := jsonutils.MarshalPatch("/spec/validationFailureAction", "add", audit) + patchByte, err := jsonutils.MarshalPatchOperation("/spec/validationFailureAction", "add", audit) if err != nil { log.Error(err, "failed to default value", "spec.validationFailureAction", audit) return nil, "" @@ -88,7 +88,7 @@ func defaultFailurePolicy(spec *kyvernov1.Spec, log logr.Logger) ([]byte, string if spec.FailurePolicy == nil { failurePolicy := string(kyvernov1.Fail) log.V(4).Info("setting default value", "spec.failurePolicy", failurePolicy) - patchByte, err := jsonutils.MarshalPatch("/spec/failurePolicy", "add", failurePolicy) + patchByte, err := jsonutils.MarshalPatchOperation("/spec/failurePolicy", "add", failurePolicy) if err != nil { log.Error(err, "failed to set default value", "spec.failurePolicy", failurePolicy) return nil, "" @@ -155,13 +155,13 @@ func defaultPodControllerAnnotation(ann map[string]string, controllers string) ( if ann == nil { ann = make(map[string]string) ann[kyvernov1.PodControllersAnnotation] = controllers - patchByte, err := jsonutils.MarshalPatch("/metadata/annotations", "add", ann) + patchByte, err := jsonutils.MarshalPatchOperation("/metadata/annotations", "add", ann) if err != nil { return nil, err } return patchByte, nil } - patchByte, err := jsonutils.MarshalPatch("/metadata/annotations/pod-policies.kyverno.io~1autogen-controllers", "add", controllers) + patchByte, err := jsonutils.MarshalPatchOperation("/metadata/annotations/pod-policies.kyverno.io~1autogen-controllers", "add", controllers) if err != nil { return nil, err } diff --git a/pkg/utils/annotations.go b/pkg/utils/annotations.go index 14d0662c82..16249bcf98 100644 --- a/pkg/utils/annotations.go +++ b/pkg/utils/annotations.go @@ -43,7 +43,7 @@ func GenerateAnnotationPatches(engineResponses []*response.EngineResponse, log l if annotations == nil { annotations = make(map[string]string) } - var patchResponse jsonutils.Patch + var patchResponse jsonutils.PatchOperation value := annotationFromEngineResponses(engineResponses, log) if value == nil { // no patches or error while processing patches @@ -52,30 +52,30 @@ func GenerateAnnotationPatches(engineResponses []*response.EngineResponse, log l if _, ok := annotations[strings.ReplaceAll(policyAnnotation, "~1", "/")]; ok { // create update patch string if _, ok := annotations["policies.kyverno.io/patches"]; ok { - patchResponse = jsonutils.NewPatch("/metadata/annotations/"+oldAnnotation, "remove", nil) + patchResponse = jsonutils.NewPatchOperation("/metadata/annotations/"+oldAnnotation, "remove", nil) delete(annotations, "policies.kyverno.io/patches") patchByte, _ := json.Marshal(patchResponse) patchBytes = append(patchBytes, patchByte) } - patchResponse = jsonutils.NewPatch("/metadata/annotations/"+policyAnnotation, "replace", string(value)) + patchResponse = jsonutils.NewPatchOperation("/metadata/annotations/"+policyAnnotation, "replace", string(value)) patchByte, _ := json.Marshal(patchResponse) patchBytes = append(patchBytes, patchByte) } else { // mutate rule has annotation patches if len(annotations) > 0 { if _, ok := annotations["policies.kyverno.io/patches"]; ok { - patchResponse = jsonutils.NewPatch("/metadata/annotations/"+oldAnnotation, "remove", nil) + patchResponse = jsonutils.NewPatchOperation("/metadata/annotations/"+oldAnnotation, "remove", nil) delete(annotations, "policies.kyverno.io/patches") patchByte, _ := json.Marshal(patchResponse) patchBytes = append(patchBytes, patchByte) } - patchResponse = jsonutils.NewPatch("/metadata/annotations/"+policyAnnotation, "add", string(value)) + patchResponse = jsonutils.NewPatchOperation("/metadata/annotations/"+policyAnnotation, "add", string(value)) patchByte, _ := json.Marshal(patchResponse) patchBytes = append(patchBytes, patchByte) } else { // insert 'policies.kyverno.patches' entry in annotation map annotations[strings.ReplaceAll(policyAnnotation, "~1", "/")] = string(value) - patchResponse = jsonutils.NewPatch("/metadata/annotations", "add", annotations) + patchResponse = jsonutils.NewPatchOperation("/metadata/annotations", "add", annotations) patchByte, _ := json.Marshal(patchResponse) patchBytes = append(patchBytes, patchByte) } diff --git a/pkg/utils/json/patch.go b/pkg/utils/json/patch.go index 0653a52b66..1fe0ec27ea 100644 --- a/pkg/utils/json/patch.go +++ b/pkg/utils/json/patch.go @@ -6,21 +6,21 @@ import ( jsonpatch "github.com/evanphx/json-patch" ) -type Patch struct { +type PatchOperation struct { Path string `json:"path"` Op string `json:"op"` Value interface{} `json:"value,omitempty"` } -func NewPatch(path, op string, value interface{}) Patch { - return Patch{path, op, value} +func NewPatchOperation(path, op string, value interface{}) PatchOperation { + return PatchOperation{path, op, value} } -func (p *Patch) Marshal() ([]byte, error) { +func (p *PatchOperation) Marshal() ([]byte, error) { return json.Marshal(p) } -func (p *Patch) ToPatchBytes() ([]byte, error) { +func (p *PatchOperation) ToPatchBytes() ([]byte, error) { if patch, err := json.Marshal(p); err != nil { return nil, err } else { @@ -28,8 +28,8 @@ func (p *Patch) ToPatchBytes() ([]byte, error) { } } -func MarshalPatch(path, op string, value interface{}) ([]byte, error) { - p := NewPatch(path, op, value) +func MarshalPatchOperation(path, op string, value interface{}) ([]byte, error) { + p := NewPatchOperation(path, op, value) return p.Marshal() } @@ -38,8 +38,8 @@ func CheckPatch(patch []byte) error { return err } -func UnmarshalPatch(patch []byte) (*Patch, error) { - var p Patch +func UnmarshalPatchOperation(patch []byte) (*PatchOperation, error) { + var p PatchOperation if err := json.Unmarshal(patch, &p); err != nil { return nil, err } diff --git a/pkg/utils/json/utils.go b/pkg/utils/json/utils.go index 688d0113f0..b9aeca5f9e 100644 --- a/pkg/utils/json/utils.go +++ b/pkg/utils/json/utils.go @@ -1,18 +1,37 @@ package json +import ( + "strings" +) + // JoinPatches joins array of serialized JSON patches to the single JSONPatch array +// It accepts patch operations and patches (arrays of patch operations) and returns +// a single combined patch. func JoinPatches(patches ...[]byte) []byte { - var result []byte if len(patches) == 0 { - return result + return nil } - result = append(result, []byte("[\n")...) - for index, patch := range patches { - result = append(result, patch...) - if index != len(patches)-1 { - result = append(result, []byte(",\n")...) + + var patchOperations []string + for _, patch := range patches { + str := strings.TrimSpace(string(patch)) + if len(str) == 0 { + continue } + + if strings.HasPrefix(str, "[") { + str = strings.TrimPrefix(str, "[") + str = strings.TrimSuffix(str, "]") + str = strings.TrimSpace(str) + } + + patchOperations = append(patchOperations, str) } - result = append(result, []byte("\n]")...) - return result + + if len(patchOperations) == 0 { + return nil + } + + result := "[" + strings.Join(patchOperations, ", ") + "]" + return []byte(result) } diff --git a/pkg/utils/json/utils_test.go b/pkg/utils/json/utils_test.go new file mode 100644 index 0000000000..332dff89dc --- /dev/null +++ b/pkg/utils/json/utils_test.go @@ -0,0 +1,49 @@ +package json + +import ( + "testing" + + jsonpatch "github.com/evanphx/json-patch" + "gotest.tools/assert" +) + +func Test_JoinPatches(t *testing.T) { + patches := JoinPatches() + assert.Assert(t, patches == nil, "invalid patch %#v", string(patches)) + + patches = JoinPatches([]byte("")) + assert.Assert(t, patches == nil, "invalid patch %#v", string(patches)) + + patches = JoinPatches([]byte(""), []byte(""), []byte(""), []byte("")) + assert.Assert(t, patches == nil, "invalid patch %#v", string(patches)) + + p1 := `{ "op": "replace", "path": "/baz", "value": "boo" }` + p2 := `{ "op": "add", "path": "/hello", "value": ["world"] }` + p1p2 := `[ + { "op": "replace", "path": "/baz", "value": "boo" }, + { "op": "add", "path": "/hello", "value": ["world"] } + ]` + + patches = JoinPatches([]byte(p1), []byte(p2)) + _, err := jsonpatch.DecodePatch(patches) + assert.NilError(t, err, "failed to decode patch %s", string(patches)) + if !jsonpatch.Equal([]byte(p1p2), patches) { + assert.Assert(t, false, "patches are not equal") + } + + p3 := `{ "op": "remove", "path": "/foo" }` + p1p2p3 := `[ + { "op": "replace", "path": "/baz", "value": "boo" }, + { "op": "add", "path": "/hello", "value": ["world"] }, + { "op": "remove", "path": "/foo" } + ]` + + patches = JoinPatches([]byte(p1p2), []byte(p3)) + assert.NilError(t, err, "failed to join patches %s", string(patches)) + + _, err = jsonpatch.DecodePatch(patches) + assert.NilError(t, err, "failed to decode patch %s", string(patches)) + if !jsonpatch.Equal([]byte(p1p2p3), patches) { + assert.Assert(t, false, "patches are not equal %+v %+v", p1p2p3, string(patches)) + } +} diff --git a/pkg/webhooks/resource/handlers.go b/pkg/webhooks/resource/handlers.go index bae6172a20..0712e7b22e 100644 --- a/pkg/webhooks/resource/handlers.go +++ b/pkg/webhooks/resource/handlers.go @@ -190,6 +190,7 @@ func (h *handlers) Validate(logger logr.Logger, request *admissionv1.AdmissionRe go h.createUpdateRequests(logger, request, policyContext, generatePolicies, mutatePolicies, requestTime) + logger.V(4).Info("completed validating webhook") return admissionutils.ResponseSuccess(true, "") } @@ -234,7 +235,12 @@ func (h *handlers) Mutate(logger logr.Logger, request *admissionv1.AdmissionRequ logger.Error(err, "image verification failed") return admissionutils.ResponseFailure(false, err.Error()) } - return admissionutils.ResponseSuccessWithPatch(true, "", append(mutatePatches, imagePatches...)) + + patch := jsonutils.JoinPatches(mutatePatches, imagePatches) + admissionResponse := admissionutils.ResponseSuccessWithPatch(true, "", patch) + + logger.V(4).Info("completed mutating webhook", "response", admissionResponse) + return admissionResponse } func (h *handlers) buildPolicyContext(request *admissionv1.AdmissionRequest, addRoles bool) (*engine.PolicyContext, error) {