From f45aedb7161f0fcf94c60be5f3828a5201a70c28 Mon Sep 17 00:00:00 2001 From: chaunceyjiang Date: Fri, 11 Oct 2024 14:46:54 +0800 Subject: [PATCH] feat: validate fieldOverrider operation Signed-off-by: chaunceyjiang --- pkg/util/validation/validation.go | 18 ++++ pkg/webhook/overridepolicy/validating_test.go | 94 +++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/pkg/util/validation/validation.go b/pkg/util/validation/validation.go index 59d1b42899b3..0fdb9861d6a8 100644 --- a/pkg/util/validation/validation.go +++ b/pkg/util/validation/validation.go @@ -21,6 +21,7 @@ import ( "github.com/go-openapi/jsonpointer" corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apivalidation "k8s.io/apimachinery/pkg/api/validation" metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/util/validation" @@ -340,6 +341,7 @@ func validateJSONPatchSubPaths(patches []policyv1alpha1.JSONPatchOperation, fiel if _, err := jsonpointer.New(patch.SubPath); err != nil { allErrs = append(allErrs, field.Invalid(patchPath.Child("subPath"), patch.SubPath, err.Error())) } + allErrs = append(allErrs, validateOverrideOperator(patch.Operator, patch.Value, patchPath.Child("value"))...) } return allErrs } @@ -351,6 +353,22 @@ func validateYAMLPatchSubPaths(patches []policyv1alpha1.YAMLPatchOperation, fiel if _, err := jsonpointer.New(patch.SubPath); err != nil { allErrs = append(allErrs, field.Invalid(patchPath.Child("subPath"), patch.SubPath, err.Error())) } + allErrs = append(allErrs, validateOverrideOperator(patch.Operator, patch.Value, patchPath.Child("value"))...) + } + return allErrs +} + +func validateOverrideOperator(operator policyv1alpha1.OverriderOperator, value apiextensionsv1.JSON, fldPath *field.Path) field.ErrorList { + var allErrs field.ErrorList + switch operator { + case policyv1alpha1.OverriderOpAdd, policyv1alpha1.OverriderOpReplace: + if value.Size() == 0 { + allErrs = append(allErrs, field.Invalid(fldPath, value, "value is required for add or replace operation")) + } + case policyv1alpha1.OverriderOpRemove: + if value.Size() != 0 { + allErrs = append(allErrs, field.Invalid(fldPath, value, "value is not allowed for remove operation")) + } } return allErrs } diff --git a/pkg/webhook/overridepolicy/validating_test.go b/pkg/webhook/overridepolicy/validating_test.go index 0c97867931d0..43a9c6bc3c48 100644 --- a/pkg/webhook/overridepolicy/validating_test.go +++ b/pkg/webhook/overridepolicy/validating_test.go @@ -296,6 +296,100 @@ func TestValidatingAdmission_Handle(t *testing.T) { Message: "spec.overrideRules[0].overriders.fieldOverrider[0].yaml[0].subPath: Invalid value: \"invalidSubPath\": JSON pointer must be empty or start with a \"/", }, }, + { + name: "Handle_InvalidJSONValue_DeniesAdmission_OverriderOpReplace", + decoder: &fakeValidationDecoder{ + obj: &policyv1alpha1.OverridePolicy{ + Spec: policyv1alpha1.OverrideSpec{ + OverrideRules: []policyv1alpha1.RuleWithCluster{ + { + Overriders: policyv1alpha1.Overriders{ + FieldOverrider: []policyv1alpha1.FieldOverrider{ + { + FieldPath: "/data/config", + JSON: []policyv1alpha1.JSONPatchOperation{ + { + SubPath: "/db-config", + Operator: policyv1alpha1.OverriderOpReplace, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + req: admission.Request{}, + want: TestResponse{ + Type: Denied, + Message: "spec.overrideRules[0].overriders.fieldOverrider[0].json[0].value: Invalid value: v1.JSON{Raw:[]uint8(nil)}: value is required for add or replace operation", + }, + }, + { + name: "Handle_InvalidJSONValue_DeniesAdmission_OverriderOpAdd", + decoder: &fakeValidationDecoder{ + obj: &policyv1alpha1.OverridePolicy{ + Spec: policyv1alpha1.OverrideSpec{ + OverrideRules: []policyv1alpha1.RuleWithCluster{ + { + Overriders: policyv1alpha1.Overriders{ + FieldOverrider: []policyv1alpha1.FieldOverrider{ + { + FieldPath: "/data/config", + JSON: []policyv1alpha1.JSONPatchOperation{ + { + SubPath: "/db-config", + Operator: policyv1alpha1.OverriderOpAdd, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + req: admission.Request{}, + want: TestResponse{ + Type: Denied, + Message: "spec.overrideRules[0].overriders.fieldOverrider[0].json[0].value: Invalid value: v1.JSON{Raw:[]uint8(nil)}: value is required for add or replace operation", + }, + }, + { + name: "Handle_InvalidJSONValue_DeniesAdmission_OverriderOpRemove", + decoder: &fakeValidationDecoder{ + obj: &policyv1alpha1.OverridePolicy{ + Spec: policyv1alpha1.OverrideSpec{ + OverrideRules: []policyv1alpha1.RuleWithCluster{ + { + Overriders: policyv1alpha1.Overriders{ + FieldOverrider: []policyv1alpha1.FieldOverrider{ + { + FieldPath: "/data/config", + JSON: []policyv1alpha1.JSONPatchOperation{ + { + SubPath: "/db-config", + Operator: policyv1alpha1.OverriderOpRemove, + Value: apiextensionsv1.JSON{Raw: []byte(`{"db": "new"}`)}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + req: admission.Request{}, + want: TestResponse{ + Type: Denied, + Message: "spec.overrideRules[0].overriders.fieldOverrider[0].json[0].value: Invalid value: v1.JSON{Raw:[]uint8{0x7b, 0x22, 0x64, 0x62, 0x22, 0x3a, 0x20, 0x22, 0x6e, 0x65, 0x77, 0x22, 0x7d}}: value is not allowed for remove operation", + }, + }, } for _, tt := range tests {