Skip to content

Commit

Permalink
Merge pull request #5671 from chaunceyjiang/fieldoverrider_validate
Browse files Browse the repository at this point in the history
feat: validate fieldOverrider operation
  • Loading branch information
karmada-bot authored Oct 12, 2024
2 parents d295576 + f45aedb commit b52775f
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 0 deletions.
18 changes: 18 additions & 0 deletions pkg/util/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
94 changes: 94 additions & 0 deletions pkg/webhook/overridepolicy/validating_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit b52775f

Please sign in to comment.