Skip to content

Commit 038e5fa

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#81870 from sttts/sttts-crd-ratcheting-pruned-defaults
apiextension: ratcheting validation of unpruned defaults
2 parents 18d7f88 + 91bab45 commit 038e5fa

File tree

3 files changed

+339
-10
lines changed

3 files changed

+339
-10
lines changed

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ func ValidateCustomResourceDefinition(obj *apiextensions.CustomResourceDefinitio
6363
requireOpenAPISchema: requireOpenAPISchema(requestGV, nil),
6464
requireValidPropertyType: requireValidPropertyType(requestGV, nil),
6565
requireStructuralSchema: requireStructuralSchema(requestGV, nil),
66+
requirePrunedDefaults: true,
6667
}
6768

6869
allErrs := genericvalidation.ValidateObjectMeta(&obj.ObjectMeta, false, nameValidationFn, field.NewPath("metadata"))
@@ -88,6 +89,8 @@ type validationOptions struct {
8889
requireValidPropertyType bool
8990
// requireStructuralSchema indicates that any schemas present must be structural
9091
requireStructuralSchema bool
92+
// requirePrunedDefaults indicates that defaults must be pruned
93+
requirePrunedDefaults bool
9194
}
9295

9396
// ValidateCustomResourceDefinitionUpdate statically validates
@@ -99,6 +102,7 @@ func ValidateCustomResourceDefinitionUpdate(obj, oldObj *apiextensions.CustomRes
99102
requireOpenAPISchema: requireOpenAPISchema(requestGV, &oldObj.Spec),
100103
requireValidPropertyType: requireValidPropertyType(requestGV, &oldObj.Spec),
101104
requireStructuralSchema: requireStructuralSchema(requestGV, &oldObj.Spec),
105+
requirePrunedDefaults: requirePrunedDefaults(&oldObj.Spec),
102106
}
103107

104108
allErrs := genericvalidation.ValidateObjectMetaUpdate(&obj.ObjectMeta, &oldObj.ObjectMeta, field.NewPath("metadata"))
@@ -665,7 +669,7 @@ func validateCustomResourceDefinitionValidation(customResourceValidation *apiext
665669
}
666670
} else if validationErrors := structuralschema.ValidateStructural(fldPath.Child("openAPIV3Schema"), ss); len(validationErrors) > 0 {
667671
allErrs = append(allErrs, validationErrors...)
668-
} else if validationErrors, err := structuraldefaulting.ValidateDefaults(fldPath.Child("openAPIV3Schema"), ss, true); err != nil {
672+
} else if validationErrors, err := structuraldefaulting.ValidateDefaults(fldPath.Child("openAPIV3Schema"), ss, true, opts.requirePrunedDefaults); err != nil {
669673
// this should never happen
670674
allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), "", err.Error()))
671675
} else {
@@ -1186,6 +1190,41 @@ func schemaIsNonStructural(schema *apiextensions.JSONSchemaProps) bool {
11861190
return len(structuralschema.ValidateStructural(nil, ss)) > 0
11871191
}
11881192

1193+
// requirePrunedDefaults returns false if there are any unpruned default in oldCRDSpec, and true otherwise.
1194+
func requirePrunedDefaults(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
1195+
if oldCRDSpec.Validation != nil {
1196+
if has, err := schemaHasUnprunedDefaults(oldCRDSpec.Validation.OpenAPIV3Schema); err == nil && has {
1197+
return false
1198+
}
1199+
}
1200+
for _, v := range oldCRDSpec.Versions {
1201+
if v.Schema == nil {
1202+
continue
1203+
}
1204+
if has, err := schemaHasUnprunedDefaults(v.Schema.OpenAPIV3Schema); err == nil && has {
1205+
return false
1206+
}
1207+
}
1208+
return true
1209+
}
1210+
func schemaHasUnprunedDefaults(schema *apiextensions.JSONSchemaProps) (bool, error) {
1211+
if schema == nil || !schemaHasDefaults(schema) {
1212+
return false, nil
1213+
}
1214+
ss, err := structuralschema.NewStructural(schema)
1215+
if err != nil {
1216+
return false, err
1217+
}
1218+
if errs := structuralschema.ValidateStructural(nil, ss); len(errs) > 0 {
1219+
return false, errs.ToAggregate()
1220+
}
1221+
pruned := ss.DeepCopy()
1222+
if err := structuraldefaulting.PruneDefaults(pruned); err != nil {
1223+
return false, err
1224+
}
1225+
return !reflect.DeepEqual(ss, pruned), nil
1226+
}
1227+
11891228
// requireValidPropertyType returns true if valid openapi v3 types should be required for the given API version
11901229
func requireValidPropertyType(requestGV schema.GroupVersion, oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool {
11911230
if requestGV == v1beta1.SchemeGroupVersion {

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go‎

Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5993,6 +5993,294 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) {
59935993
requestGV: apiextensionsv1.SchemeGroupVersion,
59945994
errors: []validationMatch{},
59955995
},
5996+
{
5997+
name: "ratcheting validation of defaults with disabled feature gate via v1, non-structural, no defaults before",
5998+
old: &apiextensions.CustomResourceDefinition{
5999+
ObjectMeta: metav1.ObjectMeta{
6000+
Name: "plural.group.com",
6001+
ResourceVersion: "42",
6002+
},
6003+
Spec: apiextensions.CustomResourceDefinitionSpec{
6004+
Group: "group.com",
6005+
Version: "version",
6006+
Versions: []apiextensions.CustomResourceDefinitionVersion{
6007+
{
6008+
Name: "version",
6009+
Served: true,
6010+
Storage: true,
6011+
},
6012+
},
6013+
Scope: apiextensions.NamespaceScoped,
6014+
Names: apiextensions.CustomResourceDefinitionNames{
6015+
Plural: "plural",
6016+
Singular: "singular",
6017+
Kind: "Plural",
6018+
ListKind: "PluralList",
6019+
},
6020+
Validation: &apiextensions.CustomResourceValidation{
6021+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
6022+
Type: "object",
6023+
Properties: map[string]apiextensions.JSONSchemaProps{
6024+
"a": {},
6025+
},
6026+
},
6027+
},
6028+
PreserveUnknownFields: pointer.BoolPtr(false),
6029+
},
6030+
Status: apiextensions.CustomResourceDefinitionStatus{
6031+
StoredVersions: []string{"version"},
6032+
},
6033+
},
6034+
resource: &apiextensions.CustomResourceDefinition{
6035+
ObjectMeta: metav1.ObjectMeta{
6036+
Name: "plural.group.com",
6037+
ResourceVersion: "42",
6038+
},
6039+
Spec: apiextensions.CustomResourceDefinitionSpec{
6040+
Group: "group.com",
6041+
Version: "version",
6042+
Versions: []apiextensions.CustomResourceDefinitionVersion{
6043+
{
6044+
Name: "version",
6045+
Served: true,
6046+
Storage: true,
6047+
},
6048+
},
6049+
Scope: apiextensions.NamespaceScoped,
6050+
Names: apiextensions.CustomResourceDefinitionNames{
6051+
Plural: "plural",
6052+
Singular: "singular",
6053+
Kind: "Plural",
6054+
ListKind: "PluralList",
6055+
},
6056+
Validation: &apiextensions.CustomResourceValidation{
6057+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
6058+
Type: "object",
6059+
Properties: map[string]apiextensions.JSONSchemaProps{
6060+
"a": {
6061+
Type: "number",
6062+
Default: jsonPtr(42.0),
6063+
},
6064+
},
6065+
},
6066+
},
6067+
PreserveUnknownFields: pointer.BoolPtr(false),
6068+
},
6069+
Status: apiextensions.CustomResourceDefinitionStatus{
6070+
StoredVersions: []string{"version"},
6071+
},
6072+
},
6073+
requestGV: apiextensionsv1.SchemeGroupVersion,
6074+
errors: []validationMatch{
6075+
forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "default"),
6076+
},
6077+
},
6078+
{
6079+
name: "ratcheting validation of defaults with disabled feature gate via v1, unpruned => unpruned",
6080+
old: &apiextensions.CustomResourceDefinition{
6081+
ObjectMeta: metav1.ObjectMeta{
6082+
Name: "plural.group.com",
6083+
ResourceVersion: "42",
6084+
},
6085+
Spec: apiextensions.CustomResourceDefinitionSpec{
6086+
Group: "group.com",
6087+
Version: "version",
6088+
Versions: []apiextensions.CustomResourceDefinitionVersion{
6089+
{
6090+
Name: "version",
6091+
Served: true,
6092+
Storage: true,
6093+
},
6094+
},
6095+
Scope: apiextensions.NamespaceScoped,
6096+
Names: apiextensions.CustomResourceDefinitionNames{
6097+
Plural: "plural",
6098+
Singular: "singular",
6099+
Kind: "Plural",
6100+
ListKind: "PluralList",
6101+
},
6102+
Validation: &apiextensions.CustomResourceValidation{
6103+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
6104+
Type: "object",
6105+
Properties: map[string]apiextensions.JSONSchemaProps{
6106+
"a": {
6107+
Type: "object",
6108+
Properties: map[string]apiextensions.JSONSchemaProps{
6109+
"foo": {Type: "string"},
6110+
},
6111+
Default: jsonPtr(map[string]interface{}{
6112+
"unknown": "unknown",
6113+
}),
6114+
},
6115+
},
6116+
},
6117+
},
6118+
PreserveUnknownFields: pointer.BoolPtr(false),
6119+
},
6120+
Status: apiextensions.CustomResourceDefinitionStatus{
6121+
StoredVersions: []string{"version"},
6122+
},
6123+
},
6124+
resource: &apiextensions.CustomResourceDefinition{
6125+
ObjectMeta: metav1.ObjectMeta{
6126+
Name: "plural.group.com",
6127+
ResourceVersion: "42",
6128+
},
6129+
Spec: apiextensions.CustomResourceDefinitionSpec{
6130+
Group: "group.com",
6131+
Version: "version",
6132+
Versions: []apiextensions.CustomResourceDefinitionVersion{
6133+
{
6134+
Name: "version",
6135+
Served: true,
6136+
Storage: true,
6137+
},
6138+
},
6139+
Scope: apiextensions.NamespaceScoped,
6140+
Names: apiextensions.CustomResourceDefinitionNames{
6141+
Plural: "plural",
6142+
Singular: "singular",
6143+
Kind: "Plural",
6144+
ListKind: "PluralList",
6145+
},
6146+
Validation: &apiextensions.CustomResourceValidation{
6147+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
6148+
Type: "object",
6149+
Properties: map[string]apiextensions.JSONSchemaProps{
6150+
"a": {
6151+
Type: "object",
6152+
Properties: map[string]apiextensions.JSONSchemaProps{
6153+
"foo": {Type: "string"},
6154+
},
6155+
Default: jsonPtr(map[string]interface{}{
6156+
"unknown": "unknown",
6157+
}),
6158+
},
6159+
"b": {
6160+
Type: "object",
6161+
Properties: map[string]apiextensions.JSONSchemaProps{
6162+
"foo": {Type: "string"},
6163+
},
6164+
Default: jsonPtr(map[string]interface{}{
6165+
"unknown": "unknown",
6166+
}),
6167+
},
6168+
},
6169+
},
6170+
},
6171+
PreserveUnknownFields: pointer.BoolPtr(false),
6172+
},
6173+
Status: apiextensions.CustomResourceDefinitionStatus{
6174+
StoredVersions: []string{"version"},
6175+
},
6176+
},
6177+
requestGV: apiextensionsv1.SchemeGroupVersion,
6178+
errors: []validationMatch{},
6179+
},
6180+
{
6181+
name: "ratcheting validation of defaults with disabled feature gate via v1, pruned => unpruned",
6182+
old: &apiextensions.CustomResourceDefinition{
6183+
ObjectMeta: metav1.ObjectMeta{
6184+
Name: "plural.group.com",
6185+
ResourceVersion: "42",
6186+
},
6187+
Spec: apiextensions.CustomResourceDefinitionSpec{
6188+
Group: "group.com",
6189+
Version: "version",
6190+
Versions: []apiextensions.CustomResourceDefinitionVersion{
6191+
{
6192+
Name: "version",
6193+
Served: true,
6194+
Storage: true,
6195+
},
6196+
},
6197+
Scope: apiextensions.NamespaceScoped,
6198+
Names: apiextensions.CustomResourceDefinitionNames{
6199+
Plural: "plural",
6200+
Singular: "singular",
6201+
Kind: "Plural",
6202+
ListKind: "PluralList",
6203+
},
6204+
Validation: &apiextensions.CustomResourceValidation{
6205+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
6206+
Type: "object",
6207+
Properties: map[string]apiextensions.JSONSchemaProps{
6208+
"a": {
6209+
Type: "object",
6210+
Properties: map[string]apiextensions.JSONSchemaProps{
6211+
"foo": {Type: "string"},
6212+
},
6213+
Default: jsonPtr(map[string]interface{}{
6214+
"foo": "foo",
6215+
}),
6216+
},
6217+
},
6218+
},
6219+
},
6220+
PreserveUnknownFields: pointer.BoolPtr(false),
6221+
},
6222+
Status: apiextensions.CustomResourceDefinitionStatus{
6223+
StoredVersions: []string{"version"},
6224+
},
6225+
},
6226+
resource: &apiextensions.CustomResourceDefinition{
6227+
ObjectMeta: metav1.ObjectMeta{
6228+
Name: "plural.group.com",
6229+
ResourceVersion: "42",
6230+
},
6231+
Spec: apiextensions.CustomResourceDefinitionSpec{
6232+
Group: "group.com",
6233+
Version: "version",
6234+
Versions: []apiextensions.CustomResourceDefinitionVersion{
6235+
{
6236+
Name: "version",
6237+
Served: true,
6238+
Storage: true,
6239+
},
6240+
},
6241+
Scope: apiextensions.NamespaceScoped,
6242+
Names: apiextensions.CustomResourceDefinitionNames{
6243+
Plural: "plural",
6244+
Singular: "singular",
6245+
Kind: "Plural",
6246+
ListKind: "PluralList",
6247+
},
6248+
Validation: &apiextensions.CustomResourceValidation{
6249+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
6250+
Type: "object",
6251+
Properties: map[string]apiextensions.JSONSchemaProps{
6252+
"a": {
6253+
Type: "object",
6254+
Properties: map[string]apiextensions.JSONSchemaProps{
6255+
"foo": {Type: "string"},
6256+
},
6257+
Default: jsonPtr(map[string]interface{}{
6258+
"foo": "foo",
6259+
}),
6260+
},
6261+
"b": {
6262+
Type: "object",
6263+
Properties: map[string]apiextensions.JSONSchemaProps{
6264+
"foo": {Type: "string"},
6265+
},
6266+
Default: jsonPtr(map[string]interface{}{
6267+
"unknown": "unknown",
6268+
}),
6269+
},
6270+
},
6271+
},
6272+
},
6273+
PreserveUnknownFields: pointer.BoolPtr(false),
6274+
},
6275+
Status: apiextensions.CustomResourceDefinitionStatus{
6276+
StoredVersions: []string{"version"},
6277+
},
6278+
},
6279+
requestGV: apiextensionsv1.SchemeGroupVersion,
6280+
errors: []validationMatch{
6281+
invalid("spec", "validation", "openAPIV3Schema", "properties[b]", "default"),
6282+
},
6283+
},
59966284
{
59976285
name: "add default with enabled feature gate, structural schema, without pruning",
59986286
old: &apiextensions.CustomResourceDefinition{

0 commit comments

Comments
 (0)