From becf73227bd468e2b68fd8a49e8e7735921f7972 Mon Sep 17 00:00:00 2001 From: Shivansh Yadav Date: Mon, 17 Oct 2022 20:55:03 +0530 Subject: [PATCH] validate patchJSON6902 (#4469) * validate patchJSON6902 Signed-off-by: Shivansh-yadav13 * validate patchJSON6902 Signed-off-by: Shivansh-yadav13 * test: validateJSON6902 tests Signed-off-by: Shivansh-yadav13 * validate patchJSON6902 Signed-off-by: Shivansh-yadav13 * test: validate patchJSON6902 Signed-off-by: Shivansh-yadav13 Signed-off-by: Shivansh-yadav13 Signed-off-by: Shivansh Yadav Co-authored-by: shuting --- pkg/policy/validate.go | 33 +++++++++++++++++++++++++++++++++ pkg/policy/validate_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/pkg/policy/validate.go b/pkg/policy/validate.go index 73c0235d8f..7202287635 100644 --- a/pkg/policy/validate.go +++ b/pkg/policy/validate.go @@ -79,6 +79,36 @@ func validateJSONPatchPathForForwardSlash(patch string) error { return nil } +func validateJSONPatch(patch string, ruleIdx int) error { + patch = variables.ReplaceAllVars(patch, func(s string) string { return "kyvernojsonpatchvariable" }) + + jsonPatch, err := yaml.ToJSON([]byte(patch)) + if err != nil { + return err + } + + decodedPatch, err := jsonpatch.DecodePatch(jsonPatch) + if err != nil { + return err + } + + for _, operation := range decodedPatch { + op := operation.Kind() + if op != "add" && op != "remove" && op != "replace" { + return fmt.Errorf("Unexpected kind: spec.rules[%d]: %s", ruleIdx, op) + } + v, _ := operation.ValueInterface() + if v != nil { + vs := fmt.Sprintf("%v", v) + if strings.ContainsAny(vs, `"`) || strings.ContainsAny(vs, `'`) { + return fmt.Errorf("missing quote around value: spec.rules[%d]: %s", ruleIdx, vs) + } + } + } + + return nil +} + // Validate checks the policy and rules declarations for required configurations func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock bool, openApiManager openapi.Manager) (*admissionv1.AdmissionResponse, error) { namespaced := policy.IsNamespaced() @@ -140,6 +170,9 @@ func Validate(policy kyvernov1.PolicyInterface, client dclient.Interface, mock b if err := validateJSONPatchPathForForwardSlash(rule.Mutation.PatchesJSON6902); err != nil { return nil, fmt.Errorf("path must begin with a forward slash: spec.rules[%d]: %s", i, err) } + if err := validateJSONPatch(rule.Mutation.PatchesJSON6902, i); err != nil { + return nil, fmt.Errorf("%s", err) + } if jsonPatchOnPod(rule) { msg := "Pods managed by workload controllers should not be directly mutated using policies. " + diff --git a/pkg/policy/validate_test.go b/pkg/policy/validate_test.go index 0bd1402b89..e5587e2ebe 100644 --- a/pkg/policy/validate_test.go +++ b/pkg/policy/validate_test.go @@ -1599,6 +1599,38 @@ func Test_PodControllerAutoGenExclusion_None_Policy(t *testing.T) { assert.NilError(t, err) } +func Test_ValidateJSON6902(t *testing.T) { + var patch string = `- path: "/metadata/labels/img" + op: addition + value: "nginx"` + err := validateJSONPatch(patch, 0) + assert.Error(t, err, "Unexpected kind: spec.rules[0]: addition") + + patch = `- path: "/metadata/labels/img" + op: add + value: "nginx"` + err = validateJSONPatch(patch, 0) + assert.NilError(t, err) + + patch = `- path: "/metadata/labels/img" + op: add + value: nginx"` + err = validateJSONPatch(patch, 0) + assert.Error(t, err, `missing quote around value: spec.rules[0]: nginx"`) + + patch = `- path: "/metadata/labels/img" + op: add + value: {"node.kubernetes.io/role": test"}` + err = validateJSONPatch(patch, 0) + assert.Error(t, err, `missing quote around value: spec.rules[0]: map[node.kubernetes.io/role:test"]`) + + patch = `- path: "/metadata/labels/img" + op: add + value: "nginx"` + err = validateJSONPatch(patch, 0) + assert.NilError(t, err) +} + func Test_ValidateNamespace(t *testing.T) { testcases := []struct { description string