diff --git a/common/go.mod b/common/go.mod index 9122e31d6..e3ddded19 100644 --- a/common/go.mod +++ b/common/go.mod @@ -19,6 +19,7 @@ require ( k8s.io/api v0.32.5 k8s.io/apiextensions-apiserver v0.32.5 k8s.io/apimachinery v0.32.5 + k8s.io/apiserver v0.32.5 k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 sigs.k8s.io/cluster-api v1.10.2 sigs.k8s.io/cluster-api/test v1.10.2 @@ -85,7 +86,6 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/apiserver v0.32.5 // indirect k8s.io/client-go v0.32.5 // indirect k8s.io/cluster-bootstrap v0.32.3 // indirect k8s.io/component-base v0.32.5 // indirect diff --git a/common/pkg/testutils/capitest/variables.go b/common/pkg/testutils/capitest/variables.go index 691e714bf..7f4b5dc33 100644 --- a/common/pkg/testutils/capitest/variables.go +++ b/common/pkg/testutils/capitest/variables.go @@ -22,6 +22,7 @@ import ( type VariableTestDef struct { Name string Vals any + OldVals any ExpectError bool } @@ -65,14 +66,34 @@ func ValidateDiscoverVariables[T mutation.DiscoverVariables]( encodedVals, err := json.Marshal(tt.Vals) g.Expect(err).NotTo(gomega.HaveOccurred()) - validateErr := openapi.ValidateClusterVariable( - &clusterv1.ClusterVariable{ - Name: variableName, - Value: apiextensionsv1.JSON{Raw: encodedVals}, - }, - &variable, - field.NewPath(variableName), - ).ToAggregate() + var validateErr error + + switch { + case tt.OldVals != nil: + encodedOldVals, err := json.Marshal(tt.OldVals) + g.Expect(err).NotTo(gomega.HaveOccurred()) + validateErr = openapi.ValidateClusterVariableUpdate( + &clusterv1.ClusterVariable{ + Name: variableName, + Value: apiextensionsv1.JSON{Raw: encodedVals}, + }, + &clusterv1.ClusterVariable{ + Name: variableName, + Value: apiextensionsv1.JSON{Raw: encodedOldVals}, + }, + &variable, + field.NewPath(variableName), + ).ToAggregate() + default: + validateErr = openapi.ValidateClusterVariable( + &clusterv1.ClusterVariable{ + Name: variableName, + Value: apiextensionsv1.JSON{Raw: encodedVals}, + }, + &variable, + field.NewPath(variableName), + ).ToAggregate() + } if tt.ExpectError { g.Expect(validateErr).To(gomega.HaveOccurred()) diff --git a/common/pkg/testutils/openapi/convert.go b/common/pkg/testutils/openapi/convert.go index a7fba70d6..81e29a4f9 100644 --- a/common/pkg/testutils/openapi/convert.go +++ b/common/pkg/testutils/openapi/convert.go @@ -208,5 +208,22 @@ func ConvertJSONSchemaPropsToAPIExtensions( } } + if schema.XValidations != nil { + props.XValidations = make([]apiextensions.ValidationRule, 0, len(schema.XValidations)) + for _, v := range schema.XValidations { + var reason *apiextensions.FieldValueErrorReason + if v.Reason != "" { + reason = ptr.To(apiextensions.FieldValueErrorReason(v.Reason)) + } + props.XValidations = append(props.XValidations, apiextensions.ValidationRule{ + Rule: v.Rule, + Message: v.Message, + MessageExpression: v.MessageExpression, + Reason: reason, + FieldPath: v.FieldPath, + }) + } + } + return props, allErrs } diff --git a/common/pkg/testutils/openapi/validate.go b/common/pkg/testutils/openapi/validate.go index 9c1697ab5..753c78ab8 100644 --- a/common/pkg/testutils/openapi/validate.go +++ b/common/pkg/testutils/openapi/validate.go @@ -4,16 +4,19 @@ package openapi import ( + "context" "encoding/json" "fmt" "strings" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" + "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting" structuralpruning "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning" "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" "k8s.io/apimachinery/pkg/util/validation/field" + celconfig "k8s.io/apiserver/pkg/apis/cel" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) @@ -26,62 +29,133 @@ func ValidateClusterVariable( definition *clusterv1.ClusterClassVariable, fldPath *field.Path, ) field.ErrorList { + validator, apiExtensionsSchema, structuralSchema, err := validatorAndSchemas(fldPath, definition) + if err != nil { + return field.ErrorList{err} + } + + variableValue, err := unmarshalAndDefaultVariableValue(fldPath, value, structuralSchema) + if err != nil { + return field.ErrorList{err} + } + + // Validate variable against the schema. + // NOTE: We're reusing a library func used in CRD validation. + if err := validation.ValidateCustomResource(fldPath, variableValue, validator); err != nil { + return err + } + + // Validate variable against the schema using CEL. + if err := validateCEL(fldPath, variableValue, nil, structuralSchema); err != nil { + return err + } + + return validateUnknownFields(fldPath, value, variableValue, apiExtensionsSchema) +} + +func unmarshalAndDefaultVariableValue( + fldPath *field.Path, + value *clusterv1.ClusterVariable, + s *structuralschema.Structural, +) (any, *field.Error) { // Parse JSON value. - var variableValue interface{} + var variableValue any // Only try to unmarshal the clusterVariable if it is not nil, otherwise the variableValue is nil. // Note: A clusterVariable with a nil value is the result of setting the variable value to "null" via YAML. if value.Value.Raw != nil { if err := json.Unmarshal(value.Value.Raw, &variableValue); err != nil { - return field.ErrorList{field.Invalid(fldPath.Child("value"), string(value.Value.Raw), - fmt.Sprintf("variable %q could not be parsed: %v", value.Name, err))} + return nil, field.Invalid( + fldPath.Child("value"), string(value.Value.Raw), + fmt.Sprintf("variable %q could not be parsed: %v", value.Name, err), + ) } } + defaulting.Default(variableValue, s) + + return variableValue, nil +} + +func validatorAndSchemas( + fldPath *field.Path, definition *clusterv1.ClusterClassVariable, +) (validation.SchemaValidator, *apiextensions.JSONSchemaProps, *structuralschema.Structural, *field.Error) { // Convert schema to Kubernetes APIExtensions Schema. apiExtensionsSchema, allErrs := ConvertJSONSchemaPropsToAPIExtensions( &definition.Schema.OpenAPIV3Schema, field.NewPath("schema"), ) if len(allErrs) > 0 { - return field.ErrorList{field.InternalError(fldPath, + return nil, nil, nil, field.InternalError( + fldPath, fmt.Errorf( "failed to convert schema definition for variable %q; ClusterClass should be checked: %v", definition.Name, allErrs, ), - )} + ) } // Create validator for schema. validator, _, err := validation.NewSchemaValidator(apiExtensionsSchema) if err != nil { - return field.ErrorList{field.InternalError(fldPath, + return nil, nil, nil, field.InternalError( + fldPath, fmt.Errorf( "failed to create schema validator for variable %q; ClusterClass should be checked: %v", - value.Name, + definition.Name, err, ), - )} + ) } s, err := structuralschema.NewStructural(apiExtensionsSchema) if err != nil { - return field.ErrorList{field.InternalError(fldPath, + return nil, nil, nil, field.InternalError( + fldPath, fmt.Errorf( "failed to create structural schema for variable %q; ClusterClass should be checked: %v", - value.Name, + definition.Name, err, ), - )} + ) } - defaulting.Default(variableValue, s) - // Validate variable against the schema. - // NOTE: We're reusing a library func used in CRD validation. - if err := validation.ValidateCustomResource(fldPath, variableValue, validator); err != nil { - return err + return validator, apiExtensionsSchema, s, nil +} + +func validateCEL( + fldPath *field.Path, + variableValue, oldVariableValue any, + structuralSchema *structuralschema.Structural, +) field.ErrorList { + // Note: k/k CR validation also uses celconfig.PerCallLimit when creating the validator for a custom resource. + // The current PerCallLimit gives roughly 0.1 second for each expression validation call. + celValidator := cel.NewValidator(structuralSchema, false, celconfig.PerCallLimit) + // celValidation will be nil if there are no CEL validations specified in the schema + // under `x-kubernetes-validations`. + if celValidator == nil { + return nil } - return validateUnknownFields(fldPath, value, variableValue, apiExtensionsSchema) + // Note: k/k CRD validation also uses celconfig.RuntimeCELCostBudget for the Validate call. + // The current RuntimeCELCostBudget gives roughly 1 second for the validation of a variable value. + if validationErrors, _ := celValidator.Validate( + context.Background(), + fldPath.Child("value"), + structuralSchema, + variableValue, + oldVariableValue, + celconfig.RuntimeCELCostBudget, + ); len(validationErrors) > 0 { + var allErrs field.ErrorList + for _, validationError := range validationErrors { + // Set correct value in the field error. ValidateCustomResource sets the type instead of the value. + validationError.BadValue = variableValue + allErrs = append(allErrs, validationError) + } + return allErrs + } + + return nil } // validateUnknownFields validates the given variableValue for unknown fields. @@ -140,3 +214,38 @@ func validateUnknownFields( return nil } + +// ValidateClusterVariable validates an update to a clusterVariable. +func ValidateClusterVariableUpdate( + value, oldValue *clusterv1.ClusterVariable, + definition *clusterv1.ClusterClassVariable, + fldPath *field.Path, +) field.ErrorList { + validator, apiExtensionsSchema, structuralSchema, err := validatorAndSchemas(fldPath, definition) + if err != nil { + return field.ErrorList{err} + } + + variableValue, err := unmarshalAndDefaultVariableValue(fldPath, value, structuralSchema) + if err != nil { + return field.ErrorList{err} + } + + oldVariableValue, err := unmarshalAndDefaultVariableValue(fldPath, oldValue, structuralSchema) + if err != nil { + return field.ErrorList{err} + } + + // Validate variable against the schema. + // NOTE: We're reusing a library func used in CRD validation. + if err := validation.ValidateCustomResourceUpdate(fldPath, variableValue, oldVariableValue, validator); err != nil { + return err + } + + // Validate variable against the schema using CEL. + if err := validateCEL(fldPath, variableValue, oldVariableValue, structuralSchema); err != nil { + return err + } + + return validateUnknownFields(fldPath, value, variableValue, apiExtensionsSchema) +}