validate defaults across an update from nil to ensure create ratcheting rules work

This commit is contained in:
Alexander Zielenski 2023-11-03 14:10:27 -07:00
parent eef1515815
commit bba0c9a81e
2 changed files with 154 additions and 4 deletions

View File

@ -26,10 +26,12 @@ import (
schemaobjectmeta "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning"
apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
celconfig "k8s.io/apiserver/pkg/apis/cel"
utilfeature "k8s.io/apiserver/pkg/util/feature"
)
// ValidateDefaults checks that default values validate and are properly pruned.
@ -91,8 +93,27 @@ func validate(ctx context.Context, pth *field.Path, s *structuralschema.Structur
allErrs = append(allErrs, errs...)
} else if celValidator := cel.NewValidator(s, isResourceRoot, celconfig.PerCallLimit); celValidator != nil {
celErrs, rmCost := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, s.Default.Object, remainingCost)
remainingCost = rmCost
allErrs = append(allErrs, celErrs...)
if len(celErrs) == 0 && utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CRDValidationRatcheting) {
// If ratcheting is enabled some CEL rules may use optionalOldSelf
// For such rules the above validation is not sufficient for
// determining if the default value is a valid value to introduce
// via create or uncorrelated update.
//
// Validate an update from nil to the default value to ensure
// that the default value pass
celErrs, rmCostWithoutOldObject := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, nil, remainingCost)
allErrs = append(allErrs, celErrs...)
// capture the cost of both types of runs and take whichever
// leaves less remaining cost
if rmCostWithoutOldObject < rmCost {
rmCost = rmCostWithoutOldObject
}
}
remainingCost = rmCost
if remainingCost < 0 {
return allErrs, nil, remainingCost
}
@ -116,8 +137,27 @@ func validate(ctx context.Context, pth *field.Path, s *structuralschema.Structur
allErrs = append(allErrs, errs...)
} else if celValidator := cel.NewValidator(s, isResourceRoot, celconfig.PerCallLimit); celValidator != nil {
celErrs, rmCost := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, s.Default.Object, remainingCost)
remainingCost = rmCost
allErrs = append(allErrs, celErrs...)
if len(celErrs) == 0 && utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CRDValidationRatcheting) {
// If ratcheting is enabled some CEL rules may use optionalOldSelf
// For such rules the above validation is not sufficient for
// determining if the default value is a valid value to introduce
// via create or uncorrelated update.
//
// Validate an update from nil to the default value to ensure
// that the default value pass
celErrs, rmCostWithoutOldObject := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, nil, remainingCost)
allErrs = append(allErrs, celErrs...)
// capture the cost of both types of runs and take whichever
// leaves less remaining cost
if rmCostWithoutOldObject < rmCost {
rmCost = rmCostWithoutOldObject
}
}
remainingCost = rmCost
if remainingCost < 0 {
return allErrs, nil, remainingCost
}

View File

@ -23,8 +23,13 @@ import (
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/component-base/featuregate"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/utils/ptr"
)
func jsonPtr(x interface{}) *apiextensions.JSON {
@ -34,8 +39,9 @@ func jsonPtr(x interface{}) *apiextensions.JSON {
func TestDefaultValidationWithCostBudget(t *testing.T) {
tests := []struct {
name string
input apiextensions.CustomResourceValidation
name string
input apiextensions.CustomResourceValidation
features []featuregate.Feature
}{
{
name: "default cel validation",
@ -98,6 +104,10 @@ func TestDefaultValidationWithCostBudget(t *testing.T) {
for _, tt := range tests {
ctx := context.TODO()
t.Run(tt.name, func(t *testing.T) {
for _, f := range tt.features {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, f, true)()
}
schema := tt.input.OpenAPIV3Schema
ss, err := structuralschema.NewStructural(schema)
if err != nil {
@ -148,3 +158,103 @@ func TestDefaultValidationWithCostBudget(t *testing.T) {
})
}
}
func TestDefaultValidationWithOptionalOldSelf(t *testing.T) {
tests := []struct {
name string
input apiextensions.CustomResourceValidation
errors []string
}{
{
name: "invalid default",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"defaultFailsRatcheting": {
Type: "string",
Default: jsonPtr("default"),
XValidations: apiextensions.ValidationRules{
{
Rule: "oldSelf.hasValue()",
OptionalOldSelf: ptr.To(true),
Message: "foobarErrorMessage",
},
},
},
},
},
},
errors: []string{"foobarErrorMessage"},
},
{
name: "valid default",
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"defaultFailsRatcheting": {
Type: "string",
Default: jsonPtr("default"),
XValidations: apiextensions.ValidationRules{
{
Rule: "oldSelf.orValue(self) == self",
OptionalOldSelf: ptr.To(true),
Message: "foobarErrorMessage",
},
},
},
},
},
},
errors: []string{},
},
}
for _, tt := range tests {
ctx := context.TODO()
t.Run(tt.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CRDValidationRatcheting, true)()
schema := tt.input.OpenAPIV3Schema
ss, err := structuralschema.NewStructural(schema)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
f := NewRootObjectFunc().WithTypeMeta(metav1.TypeMeta{APIVersion: "validation/v1", Kind: "Validation"})
// cost budget is large enough to pass all validation rules
allErrs, err, _ := validate(ctx, field.NewPath("test"), ss, ss, f, false, false, 10)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
for _, err := range allErrs {
found := false
for _, expected := range tt.errors {
if strings.Contains(err.Error(), expected) {
found = true
break
}
}
if !found {
t.Errorf("unexpected error: %v", err)
}
}
for _, expected := range tt.errors {
found := false
for _, err := range allErrs {
if strings.Contains(err.Error(), expected) {
found = true
break
}
}
if !found {
t.Errorf("expected error: %v", expected)
}
}
})
}
}