Skip to content

Commit 5edb27a

Browse files
author
Alexander Zielenski
committed
ratcheting-cel: add optionalOldSelf field
1 parent 18adc30 commit 5edb27a

File tree

7 files changed

+829
-0
lines changed

7 files changed

+829
-0
lines changed

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,19 @@ type ValidationRule struct {
210210
// - 'map': `X + Y` performs a merge where the array positions of all keys in `X` are preserved but the values
211211
// are overwritten by values in `Y` when the key sets of `X` and `Y` intersect. Elements in `Y` with
212212
// non-intersecting keys are appended, retaining their partial order.
213+
//
214+
// If `rule` makes use of the `oldSelf` variable it is implicitly a
215+
// `transition rule`.
216+
//
217+
// By default, the `oldSelf` variable is the same type as `self`.
218+
// When `optionalOldSelf` is true, the `oldSelf` variable is a CEL optional
219+
// variable whose value() is the same type as `self`.
220+
// See the documentation for the `optionalOldSelf` field for details.
221+
//
222+
// Transition rules by default are applied only on UPDATE requests and are
223+
// skipped if an old value could not be found. You can opt a transition
224+
// rule into unconditional evaluation by setting `optionalOldSelf` to true.
225+
//
213226
Rule string
214227
// Message represents the message displayed when validation fails. The message is required if the Rule contains
215228
// line breaks. The message must not contain line breaks.
@@ -246,6 +259,24 @@ type ValidationRule struct {
246259
// e.g. for attribute `foo.34$` appears in a list `testList`, the fieldPath could be set to `.testList['foo.34$']`
247260
// +optional
248261
FieldPath string
262+
263+
// optionalOldSelf is used to opt a transition rule into evaluation
264+
// even when the object is first created, or if the old object is
265+
// missing the value.
266+
//
267+
// When enabled `oldSelf` will be a CEL optional whose value will be
268+
// `None` if there is no old value, or when the object is initially created.
269+
//
270+
// You may check for presence of oldSelf using `oldSelf.hasValue()` and
271+
// unwrap it after checking using `oldSelf.value()`. Check the CEL
272+
// documentation for Optional types for more information:
273+
// https://pkg.go.dev/github.com/google/cel-go/cel#OptionalTypes
274+
//
275+
// May not be set unless `oldSelf` is used in `rule`.
276+
//
277+
// +featureGate=CRDValidationRatcheting
278+
// +optional
279+
OptionalOldSelf *bool
249280
}
250281

251282
// JSON represents any valid JSON value.

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,19 @@ type ValidationRule struct {
249249
// - 'map': `X + Y` performs a merge where the array positions of all keys in `X` are preserved but the values
250250
// are overwritten by values in `Y` when the key sets of `X` and `Y` intersect. Elements in `Y` with
251251
// non-intersecting keys are appended, retaining their partial order.
252+
//
253+
// If `rule` makes use of the `oldSelf` variable it is implicitly a
254+
// `transition rule`.
255+
//
256+
// By default, the `oldSelf` variable is the same type as `self`.
257+
// When `optionalOldSelf` is true, the `oldSelf` variable is a CEL optional
258+
// variable whose value() is the same type as `self`.
259+
// See the documentation for the `optionalOldSelf` field for details.
260+
//
261+
// Transition rules by default are applied only on UPDATE requests and are
262+
// skipped if an old value could not be found. You can opt a transition
263+
// rule into unconditional evaluation by setting `optionalOldSelf` to true.
264+
//
252265
Rule string `json:"rule" protobuf:"bytes,1,opt,name=rule"`
253266
// Message represents the message displayed when validation fails. The message is required if the Rule contains
254267
// line breaks. The message must not contain line breaks.
@@ -285,6 +298,24 @@ type ValidationRule struct {
285298
// e.g. for attribute `foo.34$` appears in a list `testList`, the fieldPath could be set to `.testList['foo.34$']`
286299
// +optional
287300
FieldPath string `json:"fieldPath,omitempty" protobuf:"bytes,5,opt,name=fieldPath"`
301+
302+
// optionalOldSelf is used to opt a transition rule into evaluation
303+
// even when the object is first created, or if the old object is
304+
// missing the value.
305+
//
306+
// When enabled `oldSelf` will be a CEL optional whose value will be
307+
// `None` if there is no old value, or when the object is initially created.
308+
//
309+
// You may check for presence of oldSelf using `oldSelf.hasValue()` and
310+
// unwrap it after checking using `oldSelf.value()`. Check the CEL
311+
// documentation for Optional types for more information:
312+
// https://pkg.go.dev/github.com/google/cel-go/cel#OptionalTypes
313+
//
314+
// May not be set unless `oldSelf` is used in `rule`.
315+
//
316+
// +featureGate=CRDValidationRatcheting
317+
// +optional
318+
OptionalOldSelf *bool `json:"optionalOldSelf,omitempty" protobuf:"bytes,6,opt,name=optionalOldSelf"`
288319
}
289320

290321
// JSON represents any valid JSON value.

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,19 @@ type ValidationRule struct {
249249
// - 'map': `X + Y` performs a merge where the array positions of all keys in `X` are preserved but the values
250250
// are overwritten by values in `Y` when the key sets of `X` and `Y` intersect. Elements in `Y` with
251251
// non-intersecting keys are appended, retaining their partial order.
252+
//
253+
// If `rule` makes use of the `oldSelf` variable it is implicitly a
254+
// `transition rule`.
255+
//
256+
// By default, the `oldSelf` variable is the same type as `self`.
257+
// When `optionalOldSelf` is true, the `oldSelf` variable is a CEL optional
258+
// variable whose value() is the same type as `self`.
259+
// See the documentation for the `optionalOldSelf` field for details.
260+
//
261+
// Transition rules by default are applied only on UPDATE requests and are
262+
// skipped if an old value could not be found. You can opt a transition
263+
// rule into unconditional evaluation by setting `optionalOldSelf` to true.
264+
//
252265
Rule string `json:"rule" protobuf:"bytes,1,opt,name=rule"`
253266
// Message represents the message displayed when validation fails. The message is required if the Rule contains
254267
// line breaks. The message must not contain line breaks.
@@ -285,6 +298,24 @@ type ValidationRule struct {
285298
// e.g. for attribute `foo.34$` appears in a list `testList`, the fieldPath could be set to `.testList['foo.34$']`
286299
// +optional
287300
FieldPath string `json:"fieldPath,omitempty" protobuf:"bytes,5,opt,name=fieldPath"`
301+
302+
// optionalOldSelf is used to opt a transition rule into evaluation
303+
// even when the object is first created, or if the old object is
304+
// missing the value.
305+
//
306+
// When enabled `oldSelf` will be a CEL optional whose value will be
307+
// `None` if there is no old value, or when the object is initially created.
308+
//
309+
// You may check for presence of oldSelf using `oldSelf.hasValue()` and
310+
// unwrap it after checking using `oldSelf.value()`. Check the CEL
311+
// documentation for Optional types for more information:
312+
// https://pkg.go.dev/github.com/google/cel-go/cel#OptionalTypes
313+
//
314+
// May not be set unless `oldSelf` is used in `rule`.
315+
//
316+
// +featureGate=CRDValidationRatcheting
317+
// +optional
318+
OptionalOldSelf *bool `json:"optionalOldSelf,omitempty" protobuf:"bytes,6,opt,name=optionalOldSelf"`
288319
}
289320

290321
// JSON represents any valid JSON value.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,8 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
11861186
if uncorrelatablePath := ssv.forbidOldSelfValidations(); uncorrelatablePath != nil {
11871187
allErrs.CELErrors = append(allErrs.CELErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i].Rule, fmt.Sprintf("oldSelf cannot be used on the uncorrelatable portion of the schema within %v", uncorrelatablePath)))
11881188
}
1189+
} else if schema.XValidations[i].OptionalOldSelf != nil {
1190+
allErrs.CELErrors = append(allErrs.CELErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("optionalOldSelf"), *schema.XValidations[i].OptionalOldSelf, "may not be set if oldSelf is not used in rule"))
11891191
}
11901192
}
11911193
}

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

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
"k8s.io/apiserver/pkg/cel/environment"
4242
"k8s.io/apiserver/pkg/cel/library"
4343
"k8s.io/utils/pointer"
44+
"k8s.io/utils/ptr"
4445
)
4546

4647
type validationMatch struct {
@@ -9650,6 +9651,157 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
96509651
required("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].messageExpression"),
96519652
},
96529653
},
9654+
{
9655+
name: "forbid transition rule on element of list of type atomic when optionalOldSelf is set",
9656+
opts: validationOptions{requireStructuralSchema: true},
9657+
input: apiextensions.CustomResourceValidation{
9658+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
9659+
Type: "object",
9660+
Properties: map[string]apiextensions.JSONSchemaProps{
9661+
"value": {
9662+
Type: "array",
9663+
XListType: strPtr("atomic"),
9664+
Items: &apiextensions.JSONSchemaPropsOrArray{
9665+
Schema: &apiextensions.JSONSchemaProps{
9666+
Type: "string",
9667+
MaxLength: int64ptr(10),
9668+
XValidations: apiextensions.ValidationRules{
9669+
{Rule: `self == oldSelf.orValue("")`, OptionalOldSelf: ptr.To(true)},
9670+
},
9671+
},
9672+
},
9673+
},
9674+
},
9675+
},
9676+
},
9677+
expectedErrors: []validationMatch{
9678+
invalid("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
9679+
},
9680+
},
9681+
{
9682+
name: "forbid transition rule on element of list defaulting to type atomic when optionalOldSelf is set",
9683+
opts: validationOptions{requireStructuralSchema: true},
9684+
input: apiextensions.CustomResourceValidation{
9685+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
9686+
Type: "object",
9687+
Properties: map[string]apiextensions.JSONSchemaProps{
9688+
"value": {
9689+
Type: "array",
9690+
Items: &apiextensions.JSONSchemaPropsOrArray{
9691+
Schema: &apiextensions.JSONSchemaProps{
9692+
Type: "string",
9693+
MaxLength: int64ptr(10),
9694+
XValidations: apiextensions.ValidationRules{
9695+
{Rule: `self == oldSelf.orValue("")`, OptionalOldSelf: ptr.To(true)},
9696+
},
9697+
},
9698+
},
9699+
},
9700+
},
9701+
},
9702+
},
9703+
expectedErrors: []validationMatch{
9704+
invalid("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
9705+
},
9706+
},
9707+
{
9708+
name: "forbid transition rule on element of list of type set when optionalOldSelf is set",
9709+
opts: validationOptions{requireStructuralSchema: true},
9710+
input: apiextensions.CustomResourceValidation{
9711+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
9712+
Type: "object",
9713+
Properties: map[string]apiextensions.JSONSchemaProps{
9714+
"value": {
9715+
Type: "array",
9716+
MaxItems: int64ptr(10),
9717+
XListType: strPtr("set"),
9718+
Items: &apiextensions.JSONSchemaPropsOrArray{
9719+
Schema: &apiextensions.JSONSchemaProps{
9720+
Type: "string",
9721+
MaxLength: int64ptr(10),
9722+
XValidations: apiextensions.ValidationRules{
9723+
{Rule: `self == oldSelf.orValue("")`, OptionalOldSelf: ptr.To(true)},
9724+
},
9725+
},
9726+
},
9727+
},
9728+
},
9729+
},
9730+
},
9731+
expectedErrors: []validationMatch{
9732+
invalid("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
9733+
},
9734+
},
9735+
{
9736+
name: "forbid transition rule on element of map of unrecognized type when optionalOldSelf is set",
9737+
opts: validationOptions{requireStructuralSchema: true},
9738+
input: apiextensions.CustomResourceValidation{
9739+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
9740+
Type: "object",
9741+
Properties: map[string]apiextensions.JSONSchemaProps{
9742+
"value": {
9743+
Type: "object",
9744+
XMapType: strPtr("future"),
9745+
Properties: map[string]apiextensions.JSONSchemaProps{
9746+
"subfield": {
9747+
Type: "string",
9748+
MaxLength: int64ptr(10),
9749+
XValidations: apiextensions.ValidationRules{
9750+
{Rule: `self == oldSelf.orValue("")`, OptionalOldSelf: ptr.To(true)},
9751+
},
9752+
},
9753+
},
9754+
},
9755+
},
9756+
},
9757+
},
9758+
expectedErrors: []validationMatch{
9759+
invalid("spec.validation.openAPIV3Schema.properties[value].properties[subfield].x-kubernetes-validations[0].rule"),
9760+
unsupported("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-map-type"),
9761+
},
9762+
},
9763+
{
9764+
name: "forbid setting optionalOldSelf to true if oldSelf is not used",
9765+
opts: validationOptions{requireStructuralSchema: true},
9766+
input: apiextensions.CustomResourceValidation{
9767+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
9768+
Type: "object",
9769+
Properties: map[string]apiextensions.JSONSchemaProps{
9770+
"value": {
9771+
Type: "string",
9772+
MaxLength: int64ptr(10),
9773+
XValidations: apiextensions.ValidationRules{
9774+
{Rule: `self == "foo"`, OptionalOldSelf: ptr.To(true)},
9775+
},
9776+
},
9777+
},
9778+
},
9779+
},
9780+
expectedErrors: []validationMatch{
9781+
invalid("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].optionalOldSelf"),
9782+
},
9783+
},
9784+
{
9785+
name: "forbid setting optionalOldSelf to false if oldSelf is not used",
9786+
opts: validationOptions{requireStructuralSchema: true},
9787+
input: apiextensions.CustomResourceValidation{
9788+
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
9789+
Type: "object",
9790+
Properties: map[string]apiextensions.JSONSchemaProps{
9791+
"value": {
9792+
Type: "string",
9793+
MaxLength: int64ptr(10),
9794+
XValidations: apiextensions.ValidationRules{
9795+
{Rule: `self == "foo"`, OptionalOldSelf: ptr.To(false)},
9796+
},
9797+
},
9798+
},
9799+
},
9800+
},
9801+
expectedErrors: []validationMatch{
9802+
invalid("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].optionalOldSelf"),
9803+
},
9804+
},
96539805
}
96549806
for _, tt := range tests {
96559807
t.Run(tt.name, func(t *testing.T) {

‎staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
2626
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation"
27+
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
2728
apiequality "k8s.io/apimachinery/pkg/api/equality"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
"k8s.io/apimachinery/pkg/fields"
@@ -33,6 +34,7 @@ import (
3334
"k8s.io/apiserver/pkg/registry/generic"
3435
"k8s.io/apiserver/pkg/storage"
3536
"k8s.io/apiserver/pkg/storage/names"
37+
utilfeature "k8s.io/apiserver/pkg/util/feature"
3638
)
3739

3840
// strategy implements behavior for CustomResources.
@@ -223,3 +225,60 @@ func MatchCustomResourceDefinition(label labels.Selector, field fields.Selector)
223225
func CustomResourceDefinitionToSelectableFields(obj *apiextensions.CustomResourceDefinition) fields.Set {
224226
return generic.ObjectMetaFieldsSet(&obj.ObjectMeta, true)
225227
}
228+
229+
// dropDisabledFields drops disabled fields that are not used if their associated feature gates
230+
// are not enabled.
231+
func dropDisabledFields(newCRD *apiextensions.CustomResourceDefinition, oldCRD *apiextensions.CustomResourceDefinition) {
232+
if !utilfeature.DefaultFeatureGate.Enabled(apiextensionsfeatures.CRDValidationRatcheting) && (oldCRD == nil || (oldCRD != nil && !specHasOptionalOldSelf(&oldCRD.Spec))) {
233+
if newCRD.Spec.Validation != nil {
234+
dropOptionalOldSelfField(newCRD.Spec.Validation.OpenAPIV3Schema)
235+
}
236+
237+
for _, v := range newCRD.Spec.Versions {
238+
if v.Schema != nil {
239+
dropOptionalOldSelfField(v.Schema.OpenAPIV3Schema)
240+
}
241+
}
242+
}
243+
}
244+
245+
// dropOptionalOldSelfField drops field optionalOldSelf from CRD schema
246+
func dropOptionalOldSelfField(schema *apiextensions.JSONSchemaProps) {
247+
if schema == nil {
248+
return
249+
}
250+
for i := range schema.XValidations {
251+
schema.XValidations[i].OptionalOldSelf = nil
252+
}
253+
254+
if schema.AdditionalProperties != nil {
255+
dropOptionalOldSelfField(schema.AdditionalProperties.Schema)
256+
}
257+
for def, jsonSchema := range schema.Properties {
258+
dropOptionalOldSelfField(&jsonSchema)
259+
schema.Properties[def] = jsonSchema
260+
}
261+
if schema.Items != nil {
262+
dropOptionalOldSelfField(schema.Items.Schema)
263+
for i, jsonSchema := range schema.Items.JSONSchemas {
264+
dropOptionalOldSelfField(&jsonSchema)
265+
schema.Items.JSONSchemas[i] = jsonSchema
266+
}
267+
}
268+
}
269+
270+
func specHasOptionalOldSelf(spec *apiextensions.CustomResourceDefinitionSpec) bool {
271+
return validation.HasSchemaWith(spec, schemaHasOptionalOldSelf)
272+
}
273+
274+
func schemaHasOptionalOldSelf(s *apiextensions.JSONSchemaProps) bool {
275+
return validation.SchemaHas(s, func(s *apiextensions.JSONSchemaProps) bool {
276+
for _, v := range s.XValidations {
277+
if v.OptionalOldSelf != nil {
278+
return true
279+
}
280+
281+
}
282+
return false
283+
})
284+
}

0 commit comments

Comments
 (0)