Add validation rule tests for transition rules

This commit is contained in:
Joe Betz 2022-03-16 00:10:08 -04:00
parent fe38a414f8
commit f71c4d4cf4
10 changed files with 437 additions and 252 deletions

View File

@ -787,8 +787,7 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
for property, jsonSchema := range schema.Properties {
subSsv := ssv
// defensively assumes that a future map type is uncorrelatable
if schema.XMapType != nil && (*schema.XMapType != "granular" && *schema.XMapType != "atomic") {
if !cel.MapIsCorrelatable(schema.XMapType) {
subSsv = subSsv.withForbidOldSelfValidations(fldPath)
}

View File

@ -1098,7 +1098,7 @@ func TestCelCostStability(t *testing.T) {
t.Fatal("expected non nil validator")
}
ctx := context.TODO()
errs, remainingBudegt := celValidator.Validate(ctx, field.NewPath("root"), &s, tt.obj, RuntimeCELCostBudget)
errs, remainingBudegt := celValidator.Validate(ctx, field.NewPath("root"), &s, tt.obj, nil, RuntimeCELCostBudget)
for _, err := range errs {
t.Errorf("unexpected error: %v", err)
}

View File

@ -18,16 +18,16 @@ package cel
import (
"fmt"
"hash/maphash"
"strings"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
)
// mapList provides a "lookup by key" operation for lists (arrays) with x-kubernetes-list-type=map.
type mapList interface {
// get returns the unique element having identical values, for all
// x-kubernetes-list-map-keys, to the provided object. If no such unique element exists, or
// if the provided object isn't itself a valid mapList element, get returns nil.
// get returns the first element having given key, for all
// x-kubernetes-list-map-keys, to the provided object. If the provided object isn't itself a valid mapList element,
// get returns nil.
get(interface{}) interface{}
}
@ -39,16 +39,15 @@ type keyStrategy interface {
// singleKeyStrategy is a cheaper strategy for associative lists that have exactly one key.
type singleKeyStrategy struct {
key string
defawlt interface{} // default is a keyword
key string
}
// CompositeKeyFor directly returns the value of the single key (or its default value, if absent) to
// CompositeKeyFor directly returns the value of the single key to
// use as a composite key.
func (ks *singleKeyStrategy) CompositeKeyFor(obj map[string]interface{}) (interface{}, bool) {
v, ok := obj[ks.key]
if !ok {
v = ks.defawlt // substitute default value
return nil, false
}
switch v.(type) {
@ -59,38 +58,37 @@ func (ks *singleKeyStrategy) CompositeKeyFor(obj map[string]interface{}) (interf
}
}
// hashKeyStrategy computes a hash of all key values.
type hashKeyStrategy struct {
sts *schema.Structural
hasher maphash.Hash
// multiKeyStrategy computes a composite key of all key values.
type multiKeyStrategy struct {
sts *schema.Structural
}
// CompositeKeyFor returns a hash computed from the values (or default values, if absent) of all
// CompositeKeyFor returns a composite key computed from the values of all
// keys.
func (ks *hashKeyStrategy) CompositeKeyFor(obj map[string]interface{}) (interface{}, bool) {
const keyDelimiter = "\x00" // 0 byte should never appear in the hash input except as delimiter
func (ks *multiKeyStrategy) CompositeKeyFor(obj map[string]interface{}) (interface{}, bool) {
const keyDelimiter = "\x00" // 0 byte should never appear in the composite key except as delimiter
ks.hasher.Reset()
var delimited strings.Builder
for _, key := range ks.sts.XListMapKeys {
v, ok := obj[key]
if !ok {
v = ks.sts.Properties[key].Default.Object
return nil, false
}
switch v.(type) {
case bool:
fmt.Fprintf(&ks.hasher, keyDelimiter+"%t", v)
fmt.Fprintf(&delimited, keyDelimiter+"%t", v)
case float64:
fmt.Fprintf(&ks.hasher, keyDelimiter+"%f", v)
fmt.Fprintf(&delimited, keyDelimiter+"%f", v)
case int64:
fmt.Fprintf(&ks.hasher, keyDelimiter+"%d", v)
fmt.Fprintf(&delimited, keyDelimiter+"%d", v)
case string:
fmt.Fprintf(&ks.hasher, keyDelimiter+"%q", v)
fmt.Fprintf(&delimited, keyDelimiter+"%q", v)
default:
return nil, false // values must be scalars
}
}
return ks.hasher.Sum64(), true
return delimited.String(), true
}
// emptyMapList is a mapList containing no elements.
@ -101,9 +99,12 @@ func (emptyMapList) get(interface{}) interface{} {
}
type mapListImpl struct {
sts *schema.Structural
ks keyStrategy
elements map[interface{}][]interface{} // composite key -> bucket
sts *schema.Structural
ks keyStrategy
// keyedItems contains all lazily keyed map items
keyedItems map[interface{}]interface{}
// unkeyedItems contains all map items that have not yet been keyed
unkeyedItems []interface{}
}
func (a *mapListImpl) get(obj interface{}) interface{} {
@ -116,76 +117,62 @@ func (a *mapListImpl) get(obj interface{}) interface{} {
if !ok {
return nil
}
if match, ok := a.keyedItems[key]; ok {
return match
}
// keep keying items until we either find a match or run out of unkeyed items
for len(a.unkeyedItems) > 0 {
// dequeue an unkeyed item
item := a.unkeyedItems[0]
a.unkeyedItems = a.unkeyedItems[1:]
// Scan bucket to handle key collisions and duplicate key sets:
var match interface{}
for _, element := range a.elements[key] {
all := true
for _, key := range a.sts.XListMapKeys {
va, ok := element.(map[string]interface{})[key]
if !ok {
va = a.sts.Properties[key].Default.Object
}
vb, ok := mobj[key]
if !ok {
vb = a.sts.Properties[key].Default.Object
}
all = all && (va == vb)
}
if !all {
// key the item
mitem, ok := item.(map[string]interface{})
if !ok {
continue
}
if match != nil {
// Duplicate key set / more than one element matches. This condition should
// have generated a validation error elsewhere.
return nil
itemKey, ok := a.ks.CompositeKeyFor(mitem)
if !ok {
continue
}
if _, exists := a.keyedItems[itemKey]; !exists {
a.keyedItems[itemKey] = mitem
}
// if it matches, short-circuit
if itemKey == key {
return mitem
}
match = element
}
return match // can be nil
return nil
}
func makeKeyStrategy(sts *schema.Structural) keyStrategy {
if len(sts.XListMapKeys) == 1 {
key := sts.XListMapKeys[0]
return &singleKeyStrategy{
key: key,
defawlt: sts.Properties[key].Default.Object,
key: key,
}
}
return &hashKeyStrategy{
return &multiKeyStrategy{
sts: sts,
}
}
// makeMapList returns a queryable interface over the provided x-kubernetes-list-type=map
// elements. If the provided schema is _not_ an array with x-kubernetes-list-type=map, returns an
// keyedItems. If the provided schema is _not_ an array with x-kubernetes-list-type=map, returns an
// empty mapList.
func makeMapList(sts *schema.Structural, ks keyStrategy, items []interface{}) (rv mapList) {
func makeMapList(sts *schema.Structural, items []interface{}) (rv mapList) {
if sts.Type != "array" || sts.XListType == nil || *sts.XListType != "map" || len(sts.XListMapKeys) == 0 || len(items) == 0 {
return emptyMapList{}
}
elements := make(map[interface{}][]interface{}, len(items))
for _, item := range items {
mitem, ok := item.(map[string]interface{})
if !ok {
continue
}
if key, ok := ks.CompositeKeyFor(mitem); ok {
elements[key] = append(elements[key], mitem)
}
}
ks := makeKeyStrategy(sts)
return &mapListImpl{
sts: sts,
ks: ks,
elements: elements,
sts: sts,
ks: ks,
keyedItems: map[interface{}]interface{}{},
unkeyedItems: items,
}
}

View File

@ -25,12 +25,12 @@ import (
func TestMapList(t *testing.T) {
for _, tc := range []struct {
name string
sts schema.Structural
keyStrategy keyStrategy
items []interface{}
query interface{}
expected interface{}
name string
sts schema.Structural
items []interface{}
warmUpQueries []interface{}
query interface{}
expected interface{}
}{
{
name: "default list type",
@ -108,102 +108,6 @@ func TestMapList(t *testing.T) {
"v1": "b",
},
},
{
name: "single key with faked composite key collision",
sts: schema.Structural{
Generic: schema.Generic{
Type: "array",
},
Extensions: schema.Extensions{
XListType: &listTypeMap,
XListMapKeys: []string{"k"},
},
},
keyStrategy: collisionfulKeyStrategy{},
items: []interface{}{
map[string]interface{}{
"k": "a",
"v1": "a",
},
map[string]interface{}{
"k": "b",
"v1": "b",
},
},
query: map[string]interface{}{
"k": "b",
"v1": "B",
},
expected: map[string]interface{}{
"k": "b",
"v1": "b",
},
},
{
name: "single key with default",
sts: schema.Structural{
Generic: schema.Generic{
Type: "array",
},
Extensions: schema.Extensions{
XListType: &listTypeMap,
XListMapKeys: []string{"k"},
},
Properties: map[string]schema.Structural{
"k": {
Generic: schema.Generic{
Default: schema.JSON{Object: "a"},
},
},
},
},
items: []interface{}{
map[string]interface{}{
"v1": "a",
},
map[string]interface{}{
"k": "b",
"v1": "b",
},
},
query: map[string]interface{}{
"k": "a",
"v1": "A",
},
expected: map[string]interface{}{
"v1": "a",
},
},
{
name: "single key with defaulted key missing from query",
sts: schema.Structural{
Generic: schema.Generic{
Type: "array",
},
Extensions: schema.Extensions{
XListType: &listTypeMap,
XListMapKeys: []string{"k"},
},
Properties: map[string]schema.Structural{
"k": {
Generic: schema.Generic{
Default: schema.JSON{Object: "a"},
},
},
},
},
items: []interface{}{
map[string]interface{}{
"v1": "a",
},
},
query: map[string]interface{}{
"v1": "A",
},
expected: map[string]interface{}{
"v1": "a",
},
},
{
name: "single key ignoring non-map query",
sts: schema.Structural{
@ -278,7 +182,7 @@ func TestMapList(t *testing.T) {
},
},
{
name: "ignores items with duplicated key",
name: "keep first entry when duplicated keys are encountered",
sts: schema.Structural{
Generic: schema.Generic{
Type: "array",
@ -302,50 +206,52 @@ func TestMapList(t *testing.T) {
"k": "a",
"v1": "A",
},
expected: nil,
expected: map[string]interface{}{
"k": "a",
"v1": "a",
},
},
{
name: "multiple keys with defaults missing from query",
name: "keep first entry when duplicated multi-keys are encountered",
sts: schema.Structural{
Generic: schema.Generic{
Type: "array",
},
Extensions: schema.Extensions{
XListType: &listTypeMap,
XListMapKeys: []string{"kb", "kf", "ki", "ks"},
},
Properties: map[string]schema.Structural{
"kb": {
Generic: schema.Generic{
Default: schema.JSON{Object: true},
},
},
"kf": {
Generic: schema.Generic{
Default: schema.JSON{Object: float64(2.0)},
},
},
"ki": {
Generic: schema.Generic{
Default: schema.JSON{Object: int64(42)},
},
},
"ks": {
Generic: schema.Generic{
Default: schema.JSON{Object: "hello"},
},
},
XListMapKeys: []string{"k1", "k2"},
},
},
items: []interface{}{
map[string]interface{}{
"k1": "a",
"k2": "b",
"v1": "a",
},
map[string]interface{}{
"k1": "a",
"k2": "b",
"v1": "b",
},
map[string]interface{}{
"k1": "x",
"k2": "y",
"v1": "z",
},
},
warmUpQueries: []interface{}{
map[string]interface{}{
"k1": "x",
"k2": "y",
},
},
query: map[string]interface{}{
"v1": "A",
"k1": "a",
"k2": "b",
},
expected: map[string]interface{}{
"k1": "a",
"k2": "b",
"v1": "a",
},
},
@ -415,20 +321,14 @@ func TestMapList(t *testing.T) {
},
} {
t.Run(tc.name, func(t *testing.T) {
ks := tc.keyStrategy
if ks == nil {
ks = makeKeyStrategy(&tc.sts)
mapList := makeMapList(&tc.sts, tc.items)
for _, warmUp := range tc.warmUpQueries {
mapList.get(warmUp)
}
actual := makeMapList(&tc.sts, ks, tc.items).get(tc.query)
actual := mapList.get(tc.query)
if !reflect.DeepEqual(tc.expected, actual) {
t.Errorf("got: %v, expected %v", actual, tc.expected)
}
})
}
}
type collisionfulKeyStrategy struct{}
func (collisionfulKeyStrategy) CompositeKeyFor(obj map[string]interface{}) (interface{}, bool) {
return 7, true
}

View File

@ -20,6 +20,7 @@ import (
"context"
"fmt"
"math"
"reflect"
"strings"
"github.com/google/cel-go/common/types"
@ -126,6 +127,17 @@ func (s *Validator) Validate(ctx context.Context, fldPath *field.Path, sts *sche
}
func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path, sts *schema.Structural, obj, oldObj interface{}, costBudget int64) (errs field.ErrorList, remainingBudget int64) {
// guard against oldObj being a non-nil interface with a nil value
if oldObj != nil {
v := reflect.ValueOf(oldObj)
switch v.Kind() {
case reflect.Map, reflect.Ptr, reflect.Interface, reflect.Slice:
if v.IsNil() {
oldObj = nil // +k8s:verify-mutation:reason=clone
}
}
}
remainingBudget = costBudget
if obj == nil {
// We only validate non-null values. Rules that need to check for the state of a nullable value or the presence of an optional
@ -147,10 +159,7 @@ func (s *Validator) validateExpressions(ctx context.Context, fldPath *field.Path
if s.isResourceRoot {
sts = model.WithTypeAndObjectMeta(sts)
}
var activation interpreter.Activation = NewValidationActivation(ScopedVarName, obj, sts)
if oldObj != nil {
activation = interpreter.NewHierarchicalActivation(activation, NewValidationActivation(OldScopedVarName, oldObj, sts))
}
var activation interpreter.Activation = NewValidationActivation(obj, oldObj, sts)
for i, compiled := range s.compiledRules {
rule := sts.XValidations[i]
if compiled.Error != nil {
@ -218,23 +227,30 @@ func ruleErrorString(rule apiextensions.ValidationRule) string {
}
type validationActivation struct {
name string
val ref.Val
self, oldSelf ref.Val
hasOldSelf bool
}
func NewValidationActivation(name string, obj interface{}, structural *schema.Structural) *validationActivation {
func NewValidationActivation(obj, oldObj interface{}, structural *schema.Structural) *validationActivation {
va := &validationActivation{
name: name,
val: UnstructuredToVal(obj, structural),
self: UnstructuredToVal(obj, structural),
}
if oldObj != nil {
va.oldSelf = UnstructuredToVal(oldObj, structural) // +k8s:verify-mutation:reason=clone
va.hasOldSelf = true // +k8s:verify-mutation:reason=clone
}
return va
}
func (a *validationActivation) ResolveName(name string) (interface{}, bool) {
if name == a.name {
return a.val, true
switch name {
case ScopedVarName:
return a.self, true
case OldScopedVarName:
return a.oldSelf, a.hasOldSelf
default:
return nil, false
}
return nil, false
}
func (a *validationActivation) Parent() interpreter.Activation {
@ -250,14 +266,13 @@ func (s *Validator) validateMap(ctx context.Context, fldPath *field.Path, sts *s
return nil, remainingBudget
}
// if a third map type is introduced, assume it's not correlatable. granular is the default if unspecified.
correlatable := sts.XMapType == nil || *sts.XMapType == "granular" || *sts.XMapType == "atomic"
correlatable := MapIsCorrelatable(sts.XMapType)
if s.AdditionalProperties != nil && sts.AdditionalProperties != nil && sts.AdditionalProperties.Structural != nil {
for k, v := range obj {
var oldV interface{}
if correlatable {
oldV = oldObj[k]
oldV = oldObj[k] // +k8s:verify-mutation:reason=clone
}
var err field.ErrorList
@ -275,7 +290,7 @@ func (s *Validator) validateMap(ctx context.Context, fldPath *field.Path, sts *s
if ok && stsOk {
var oldV interface{}
if correlatable {
oldV = oldObj[k]
oldV = oldObj[k] // +k8s:verify-mutation:reason=clone
}
var err field.ErrorList
@ -297,11 +312,10 @@ func (s *Validator) validateArray(ctx context.Context, fldPath *field.Path, sts
return errs, remainingBudget
}
// only map-type lists support self-oldSelf correlation for cel rules. if this isn't a
// map-type list, then makeMapList returns an implementation that always returns nil
correlatableOldItems := makeMapList(sts, makeKeyStrategy(sts), oldObj)
if s.Items != nil && sts.Items != nil {
// only map-type lists support self-oldSelf correlation for cel rules. if this isn't a
// map-type list, then makeMapList returns an implementation that always returns nil
correlatableOldItems := makeMapList(sts, oldObj)
for i := range obj {
var err field.ErrorList
err, remainingBudget = s.Items.Validate(ctx, fldPath.Index(i), sts.Items, obj[i], correlatableOldItems.get(obj[i]), remainingBudget)
@ -314,3 +328,10 @@ func (s *Validator) validateArray(ctx context.Context, fldPath *field.Path, sts
return errs, remainingBudget
}
// MapIsCorrelatable returns true if the mapType can be used to correlate the data elements of a map after an update
// with the data elements of the map from before the updated.
func MapIsCorrelatable(mapType *string) bool {
// if a third map type is introduced, assume it's not correlatable. granular is the default if unspecified.
return mapType == nil || *mapType == "granular" || *mapType == "atomic"
}

View File

@ -32,13 +32,15 @@ import (
// TestValidationExpressions tests CEL integration with custom resource values and OpenAPIv3.
func TestValidationExpressions(t *testing.T) {
tests := []struct {
name string
schema *schema.Structural
obj map[string]interface{}
oldObj map[string]interface{}
valid []string
errors map[string]string // rule -> string that error message must contain
costBudget int64
name string
schema *schema.Structural
obj interface{}
oldObj interface{}
valid []string
errors map[string]string // rule -> string that error message must contain
costBudget int64
isRoot bool
expectSkipped bool
}{
// tests where val1 and val2 are equal but val3 is different
// equality, comparisons and type specific functions
@ -627,6 +629,7 @@ func TestValidationExpressions(t *testing.T) {
},
},
{name: "typemeta and objectmeta access not specified",
isRoot: true,
obj: map[string]interface{}{
"apiVersion": "v1",
"kind": "Pod",
@ -1708,6 +1711,44 @@ func TestValidationExpressions(t *testing.T) {
"oldSelf.v == 'old' && self.v == 'new'",
},
},
{name: "skipped transition rule for nil old primitive",
expectSkipped: true,
obj: "exists",
oldObj: nil,
schema: &stringType,
valid: []string{
"oldSelf == self",
},
},
{name: "skipped transition rule for nil old array",
expectSkipped: true,
obj: []interface{}{},
oldObj: nil,
schema: listTypePtr(&stringType),
valid: []string{
"oldSelf == self",
},
},
{name: "skipped transition rule for nil old object",
expectSkipped: true,
obj: map[string]interface{}{"f": "exists"},
oldObj: nil,
schema: objectTypePtr(map[string]schema.Structural{
"f": stringType,
}),
valid: []string{
"oldSelf.f == self.f",
},
},
{name: "skipped transition rule for old with non-nil interface but nil value",
expectSkipped: true,
obj: []interface{}{},
oldObj: nilInterfaceOfStringSlice(),
schema: listTypePtr(&stringType),
valid: []string{
"oldSelf == self",
},
},
}
for i := range tests {
@ -1722,14 +1763,21 @@ func TestValidationExpressions(t *testing.T) {
t.Run(validRule, func(t *testing.T) {
t.Parallel()
s := withRule(*tt.schema, validRule)
celValidator := NewValidator(&s, PerCallLimit)
celValidator := validator(&s, tt.isRoot, PerCallLimit)
if celValidator == nil {
t.Fatal("expected non nil validator")
}
errs, _ := celValidator.Validate(ctx, field.NewPath("root"), &s, tt.obj, tt.oldObj, tt.costBudget)
errs, remainingBudget := celValidator.Validate(ctx, field.NewPath("root"), &s, tt.obj, tt.oldObj, tt.costBudget)
for _, err := range errs {
t.Errorf("unexpected error: %v", err)
}
if tt.expectSkipped {
// Skipped validations should have no cost. The only possible false positive here would be the CEL expression 'true'.
if remainingBudget != tt.costBudget {
t.Errorf("expected no cost expended for skipped validation, but got %d remaining from %d budget", remainingBudget, tt.costBudget)
}
return
}
// test with cost budget exceeded
errs, _ = celValidator.Validate(ctx, field.NewPath("root"), &s, tt.obj, tt.oldObj, 0)
@ -2107,3 +2155,8 @@ func withNullablePtr(nullable bool, s schema.Structural) *schema.Structural {
s.Generic.Nullable = nullable
return &s
}
func nilInterfaceOfStringSlice() []interface{} {
var slice []interface{} = nil
return slice
}

View File

@ -90,7 +90,7 @@ func validate(ctx context.Context, pth *field.Path, s *structuralschema.Structur
} else if errs := apiservervalidation.ValidateCustomResource(pth.Child("default"), s.Default.Object, validator); len(errs) > 0 {
allErrs = append(allErrs, errs...)
} else if celValidator := cel.NewValidator(s, cel.PerCallLimit); celValidator != nil {
celErrs, rmCost := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, nil, remainingCost)
celErrs, rmCost := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, s.Default.Object, remainingCost)
remainingCost = rmCost
allErrs = append(allErrs, celErrs...)
if remainingCost < 0 {
@ -115,7 +115,7 @@ func validate(ctx context.Context, pth *field.Path, s *structuralschema.Structur
} else if errs := apiservervalidation.ValidateCustomResource(pth.Child("default"), s.Default.Object, validator); len(errs) > 0 {
allErrs = append(allErrs, errs...)
} else if celValidator := cel.NewValidator(s, cel.PerCallLimit); celValidator != nil {
celErrs, rmCost := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, nil, remainingCost)
celErrs, rmCost := celValidator.Validate(ctx, pth.Child("default"), s, s.Default.Object, s.Default.Object, remainingCost)
remainingCost = rmCost
allErrs = append(allErrs, celErrs...)
if remainingCost < 0 {

View File

@ -17,6 +17,7 @@ limitations under the License.
package validation
import (
"context"
"math/rand"
"os"
"strconv"
@ -27,16 +28,19 @@ import (
kjson "sigs.k8s.io/json"
kubeopenapispec "k8s.io/kube-openapi/pkg/validation/spec"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
apiextensionsfuzzer "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel"
"k8s.io/apimachinery/pkg/api/apitesting/fuzzer"
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apimachinery/pkg/util/json"
"k8s.io/apimachinery/pkg/util/sets"
kubeopenapispec "k8s.io/kube-openapi/pkg/validation/spec"
)
// TestRoundTrip checks the conversion to go-openapi types.
@ -145,6 +149,7 @@ func stripIntOrStringType(x interface{}) interface{} {
type failingObject struct {
object interface{}
oldObject interface{}
expectErrs []string
}
@ -153,6 +158,7 @@ func TestValidateCustomResource(t *testing.T) {
name string
schema apiextensions.JSONSchemaProps
objects []interface{}
oldObjects []interface{}
failingObjects []failingObject
}{
{name: "!nullable",
@ -416,6 +422,119 @@ func TestValidateCustomResource(t *testing.T) {
}},
},
},
{name: "immutability transition rule",
schema: apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"field": {
Type: "string",
XValidations: []apiextensions.ValidationRule{
{
Rule: "self == oldSelf",
},
},
},
},
},
objects: []interface{}{
map[string]interface{}{"field": "x"},
},
oldObjects: []interface{}{
map[string]interface{}{"field": "x"},
},
failingObjects: []failingObject{
{
object: map[string]interface{}{"field": "y"},
oldObject: map[string]interface{}{"field": "x"},
expectErrs: []string{
`field: Invalid value: "string": failed rule: self == oldSelf`,
}},
},
},
{name: "correlatable transition rule",
// Ensures a transition rule under a "listMap" is supported.
schema: apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"field": {
Type: "array",
XListType: &listMapType,
XListMapKeys: []string{"k1", "k2"},
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"k1": {
Type: "string",
},
"k2": {
Type: "string",
},
"v1": {
Type: "number",
XValidations: []apiextensions.ValidationRule{
{
Rule: "self >= oldSelf",
},
},
},
},
},
},
},
},
},
objects: []interface{}{
map[string]interface{}{"field": []interface{}{map[string]interface{}{"k1": "a", "k2": "b", "v1": 1.2}}},
},
oldObjects: []interface{}{
map[string]interface{}{"field": []interface{}{map[string]interface{}{"k1": "a", "k2": "b", "v1": 1.0}}},
},
failingObjects: []failingObject{
{
object: map[string]interface{}{"field": []interface{}{map[string]interface{}{"k1": "a", "k2": "b", "v1": 0.9}}},
oldObject: map[string]interface{}{"field": []interface{}{map[string]interface{}{"k1": "a", "k2": "b", "v1": 1.0}}},
expectErrs: []string{
`field[0].v1: Invalid value: "number": failed rule: self >= oldSelf`,
}},
},
},
{name: "validation rule under non-correlatable field",
// The array makes the rule on the nested string non-correlatable
// for transition rule purposes. This test ensures that a rule that
// does NOT use oldSelf (is not a transition rule), still behaves
// as expected under a non-correlatable field.
schema: apiextensions.JSONSchemaProps{
Properties: map[string]apiextensions.JSONSchemaProps{
"field": {
Type: "array",
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"x": {
Type: "string",
XValidations: []apiextensions.ValidationRule{
{
Rule: "self == 'x'",
},
},
},
},
},
},
},
},
},
objects: []interface{}{
map[string]interface{}{"field": []interface{}{map[string]interface{}{"x": "x"}}},
},
failingObjects: []failingObject{
{
object: map[string]interface{}{"field": []interface{}{map[string]interface{}{"x": "y"}}},
expectErrs: []string{
`field[0].x: Invalid value: "string": failed rule: self == 'x'`,
}},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -423,13 +542,29 @@ func TestValidateCustomResource(t *testing.T) {
if err != nil {
t.Fatal(err)
}
for _, obj := range tt.objects {
structural, err := structuralschema.NewStructural(&tt.schema)
if err != nil {
t.Fatal(err)
}
celValidator := cel.NewValidator(structural, cel.PerCallLimit)
for i, obj := range tt.objects {
var oldObject interface{}
if len(tt.oldObjects) == len(tt.objects) {
oldObject = tt.oldObjects[i]
}
if errs := ValidateCustomResource(nil, obj, validator); len(errs) > 0 {
t.Errorf("unexpected validation error for %v: %v", obj, errs)
}
errs, _ := celValidator.Validate(context.TODO(), nil, structural, obj, oldObject, cel.RuntimeCELCostBudget)
if len(errs) > 0 {
t.Errorf(errs.ToAggregate().Error())
}
}
for i, failingObject := range tt.failingObjects {
if errs := ValidateCustomResource(nil, failingObject.object, validator); len(errs) == 0 {
errs := ValidateCustomResource(nil, failingObject.object, validator)
celErrs, _ := celValidator.Validate(context.TODO(), nil, structural, failingObject.object, failingObject.oldObject, cel.RuntimeCELCostBudget)
errs = append(errs, celErrs...)
if len(errs) == 0 {
t.Errorf("missing error for %v", failingObject.object)
} else {
sawErrors := sets.NewString()
@ -505,3 +640,5 @@ func TestItemsProperty(t *testing.T) {
})
}
}
var listMapType = "map"

View File

@ -19,11 +19,12 @@ package customresource
import (
"context"
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
"k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/structured-merge-diff/v4/fieldpath"
)
type statusStrategy struct {
@ -91,7 +92,7 @@ func (a statusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Obj
}
uOld, ok := old.(*unstructured.Unstructured)
if !ok {
return errs
uOld = nil // as a safety precaution, continue with validation if uOld self cannot be cast
}
v := obj.GetObjectKind().GroupVersionKind().Version

View File

@ -35,6 +35,7 @@ import (
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/dynamic"
featuregatetesting "k8s.io/component-base/featuregate/testing"
apiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
"k8s.io/kubernetes/test/integration/framework"
)
@ -301,7 +302,7 @@ func TestCustomResourceValidators(t *testing.T) {
}
})
t.Run("Schema with valid transition rule", func(t *testing.T) {
structuralWithValidators := crdWithSchema(t, "Structural", structuralSchemaWithValidTransitionRule)
structuralWithValidators := crdWithSchema(t, "ValidTransitionRule", structuralSchemaWithValidTransitionRule)
crd, err := fixtures.CreateNewV1CustomResourceDefinition(structuralWithValidators, apiExtensionClient, dynamicClient)
if err != nil {
t.Fatal(err)
@ -364,7 +365,7 @@ func TestCustomResourceValidators(t *testing.T) {
})
t.Run("CRD creation MUST fail if a x-kubernetes-validations rule contains invalid transition rule", func(t *testing.T) {
structuralWithValidators := crdWithSchema(t, "InvalidStructuralMetadata", structuralSchemaWithInvalidTransitionRule)
structuralWithValidators := crdWithSchema(t, "InvalidTransitionRule", structuralSchemaWithInvalidTransitionRule)
_, err := fixtures.CreateNewV1CustomResourceDefinition(structuralWithValidators, apiExtensionClient, dynamicClient)
if err == nil {
t.Error("Expected error creating custom resource but got none")
@ -372,6 +373,51 @@ func TestCustomResourceValidators(t *testing.T) {
t.Errorf("Expected error to contain %s but got %v", "oldSelf cannot be used on the uncorrelatable portion of the schema", err.Error())
}
})
t.Run("Schema with default map key transition rule", func(t *testing.T) {
structuralWithValidators := crdWithSchema(t, "DefaultMapKeyTransitionRule", structuralSchemaWithDefaultMapKeyTransitionRule)
crd, err := fixtures.CreateNewV1CustomResourceDefinition(structuralWithValidators, apiExtensionClient, dynamicClient)
if err != nil {
t.Fatal(err)
}
gvr := schema.GroupVersionResource{
Group: crd.Spec.Group,
Version: crd.Spec.Versions[0].Name,
Resource: crd.Spec.Names.Plural,
}
crClient := dynamicClient.Resource(gvr)
t.Run("custom resource update MUST fail if a x-kubernetes-validations if a transition rule contained in a mapList with default map keys fails validation", func(t *testing.T) {
name1 := names.SimpleNameGenerator.GenerateName("cr-1")
cr := &unstructured.Unstructured{Object: map[string]interface{}{
"apiVersion": gvr.Group + "/" + gvr.Version,
"kind": crd.Spec.Names.Kind,
"metadata": map[string]interface{}{
"name": name1,
},
"spec": map[string]interface{}{
"list": []interface{}{
map[string]interface{}{
"k1": "x",
"v": "value",
},
},
},
}}
cr, err = crClient.Create(context.TODO(), cr, metav1.CreateOptions{})
if err != nil {
t.Fatalf("Unexpected error creating custom resource: %v", err)
}
item := cr.Object["spec"].(map[string]interface{})["list"].([]interface{})[0].(map[string]interface{})
item["k2"] = "DEFAULT"
item["v"] = "new value"
_, err = crClient.Update(context.TODO(), cr, metav1.UpdateOptions{})
if err == nil {
t.Fatalf("Expected error updating custom resource: %v", err)
} else if !strings.Contains(err.Error(), "failed rule: self.v == oldSelf.v") {
t.Errorf("Expected error to contain %s but got %v", "failed rule: self.v == oldSelf.v", err.Error())
}
})
})
}
func nonStructuralCrdWithValidations() *apiextensionsv1beta1.CustomResourceDefinition {
@ -623,3 +669,44 @@ var structuralSchemaWithInvalidTransitionRule = []byte(`
}
}
}`)
var structuralSchemaWithDefaultMapKeyTransitionRule = []byte(`
{
"openAPIV3Schema": {
"description": "CRD with CEL validators",
"type": "object",
"properties": {
"spec": {
"type": "object",
"properties": {
"list": {
"type": "array",
"x-kubernetes-list-map-keys": [
"k1",
"k2"
],
"x-kubernetes-list-type": "map",
"items": {
"type": "object",
"properties": {
"k1": { "type": "string" },
"k2": { "type": "string", "default": "DEFAULT" },
"v": { "type": "string" }
},
"required": ["k1"],
"x-kubernetes-validations": [
{
"rule": "self.v == oldSelf.v"
}
]
}
}
}
},
"status": {
"type": "object",
"properties": {}
}
}
}
}`)