diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go index 6011f48c802..1705f3abcf8 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go @@ -24,6 +24,7 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation" + apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" apiequality "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -31,10 +32,13 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apimachinery/pkg/util/version" + "k8s.io/apiserver/pkg/cel/environment" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/kube-openapi/pkg/validation/spec" ) // strategy implements behavior for CustomResources. @@ -118,7 +122,24 @@ func (strategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorLis } // WarningsOnCreate returns warnings for the creation of the given object. -func (strategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { return nil } +func (strategy) WarningsOnCreate(ctx context.Context, obj runtime.Object) []string { + newCRD := obj.(*apiextensions.CustomResourceDefinition) + + var warnings []string + + // Use the system's default compatibility version for format checking + compatibilityVersion := environment.DefaultCompatibilityVersion() + + // Get unrecognized formats from the new CRD + unrecognizedFormats := getUnrecognizedFormatsInCRD(&newCRD.Spec, compatibilityVersion) + + // Create warnings for unrecognized formats + for _, format := range unrecognizedFormats { + warnings = append(warnings, fmt.Sprintf("unrecognized format %q", format)) + } + + return warnings +} // AllowCreateOnUpdate is false for CustomResourceDefinition; this means a POST is // needed to create one. @@ -142,7 +163,75 @@ func (strategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) fie // WarningsOnUpdate returns warnings for the given update. func (strategy) WarningsOnUpdate(ctx context.Context, obj, old runtime.Object) []string { - return nil + newCRD := obj.(*apiextensions.CustomResourceDefinition) + oldCRD := old.(*apiextensions.CustomResourceDefinition) + + var warnings []string + + // Use the system's default compatibility version for format checking + compatibilityVersion := environment.DefaultCompatibilityVersion() + + // Get unrecognized formats from both old and new CRDs + oldUnrecognizedFormats := getUnrecognizedFormatsInCRD(&oldCRD.Spec, compatibilityVersion) + newUnrecognizedFormats := getUnrecognizedFormatsInCRD(&newCRD.Spec, compatibilityVersion) + + // Find newly introduced unrecognized formats (ratcheting) + oldFormatsSet := make(map[string]bool) + for _, format := range oldUnrecognizedFormats { + oldFormatsSet[format] = true + } + + var newlyIntroducedFormats []string + for _, format := range newUnrecognizedFormats { + if !oldFormatsSet[format] { + newlyIntroducedFormats = append(newlyIntroducedFormats, format) + } + } + + // Create warnings for newly introduced unrecognized formats + for _, format := range newlyIntroducedFormats { + warnings = append(warnings, fmt.Sprintf("unrecognized format %q", format)) + } + + return warnings +} + +// getUnrecognizedFormatsInCRD returns a list of unrecognized formats found in the CRD schema. +func getUnrecognizedFormatsInCRD(spec *apiextensions.CustomResourceDefinitionSpec, compatibilityVersion *version.Version) []string { + var unrecognizedFormats []string + + // Check top-level validation schema (deprecated in v1, but still supported for backward compatibility) + if spec.Validation != nil && spec.Validation.OpenAPIV3Schema != nil { + unrecognizedFormats = append(unrecognizedFormats, getUnrecognizedFormatsInSchema(spec.Validation.OpenAPIV3Schema, compatibilityVersion)...) + } + + // Check per-version schemas + for _, v := range spec.Versions { + if v.Schema != nil && v.Schema.OpenAPIV3Schema != nil { + unrecognizedFormats = append(unrecognizedFormats, getUnrecognizedFormatsInSchema(v.Schema.OpenAPIV3Schema, compatibilityVersion)...) + } + } + + return unrecognizedFormats +} + +// getUnrecognizedFormatsInSchema recursively traverses the schema and collects unrecognized formats. +func getUnrecognizedFormatsInSchema(schema *apiextensions.JSONSchemaProps, compatibilityVersion *version.Version) []string { + var unrecognizedFormats []string + + // Use the existing SchemaHas function to traverse the schema + validation.SchemaHas(schema, func(s *apiextensions.JSONSchemaProps) bool { + if len(s.Format) > 0 { + // Convert to spec.Schema for format validation + specSchema := &spec.Schema{SchemaProps: spec.SchemaProps{Format: s.Format}} + if formats := apiservervalidation.GetUnrecognizedFormats(specSchema, compatibilityVersion); len(formats) > 0 { + unrecognizedFormats = append(unrecognizedFormats, formats...) + } + } + return false // Continue traversing + }) + + return unrecognizedFormats } type statusStrategy struct { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go index 549c62c1299..17abd2374eb 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go @@ -18,6 +18,7 @@ package customresourcedefinition import ( "context" + "sort" "testing" "github.com/google/go-cmp/cmp" @@ -34,6 +35,22 @@ import ( "k8s.io/utils/ptr" ) +// mkCRD helps construct CustomResourceDefinition objects for testing more +// legibly and tersely than a Go struct definition. +func mkCRD(tweaks ...func(*apiextensions.CustomResourceDefinitionSpec)) *apiextensions.CustomResourceDefinition { + crd := &apiextensions.CustomResourceDefinition{ + Spec: apiextensions.CustomResourceDefinitionSpec{ + Versions: []apiextensions.CustomResourceDefinitionVersion{}, + }, + } + + for _, tweak := range tweaks { + tweak(&crd.Spec) + } + + return crd +} + func TestValidateAPIApproval(t *testing.T) { okFn := func(t *testing.T, errors field.ErrorList) { t.Helper() @@ -1313,3 +1330,357 @@ func TestDropDisabledFields(t *testing.T) { }) } } + +func TestWarningsOnCreate(t *testing.T) { + ctx := context.Background() + strategy := NewStrategy(nil) + + testcases := map[string]struct { + crd *apiextensions.CustomResourceDefinition + wantWarningMessages []string + }{ + "no unrecognized formats": { + wantWarningMessages: []string{}, + crd: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "date-time", + }, + }, + }, + }, + }) + }), + }, + "unrecognized format in version schema": { + wantWarningMessages: []string{ + `unrecognized format "invalidformat"`, + }, + crd: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "invalidformat", + }, + }, + }, + }, + }) + }), + }, + "unrecognized format in embedded schema": { + wantWarningMessages: []string{ + `unrecognized format "invalidformat"`, + }, + crd: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "nested": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "embeddedField": { + Type: "string", + Format: "invalidformat", + }, + }, + }, + }, + }, + }, + }) + }), + }, + "unrecognized format in top-level validation schema": { + wantWarningMessages: []string{ + `unrecognized format "invalidformat"`, + }, + crd: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Validation = &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "invalidformat", + }, + }, + }, + } + }), + }, + "multiple unrecognized formats": { + wantWarningMessages: []string{ + `unrecognized format "unknownformat1"`, + `unrecognized format "unknownformat2"`, + `unrecognized format "unknownformat3"`, + }, + crd: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "unknownformat1", + }, + "field2": { + Type: "string", + Format: "unknownformat2", + }, + "nested": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field3": { + Type: "string", + Format: "unknownformat3", + }, + }, + }, + }, + }, + }, + }) + }), + }, + } + + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + gotWarnings := strategy.WarningsOnCreate(ctx, tc.crd) + if len(gotWarnings) != len(tc.wantWarningMessages) { + t.Errorf("got %d warnings but expected %d", len(gotWarnings), len(tc.wantWarningMessages)) + return + } + + // Sort gotWarnings to match expected order + sort.Strings(gotWarnings) + + for i, expectedMessage := range tc.wantWarningMessages { + if gotWarnings[i] != expectedMessage { + t.Errorf("warning %d: got %s, expected %s", i, gotWarnings[i], expectedMessage) + } + } + }) + } +} + +func TestWarningsOnUpdate(t *testing.T) { + ctx := context.Background() + strategy := NewStrategy(nil) + + testcases := map[string]struct { + oldCRD *apiextensions.CustomResourceDefinition + newCRD *apiextensions.CustomResourceDefinition + wantWarningMessages []string + }{ + "no unrecognized formats": { + wantWarningMessages: []string{}, + oldCRD: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "date-time", + }, + }, + }, + }, + }) + }), + newCRD: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "date-time", + }, + }, + }, + }, + }) + }), + }, + "newly introduced unrecognized format": { + wantWarningMessages: []string{ + `unrecognized format "invalidformat"`, + }, + oldCRD: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "date-time", + }, + }, + }, + }, + }) + }), + newCRD: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "invalidformat", + }, + }, + }, + }, + }) + }), + }, + "existing unrecognized format - no warning (ratcheting)": { + wantWarningMessages: []string{}, + oldCRD: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "invalidformat", + }, + }, + }, + }, + }) + }), + newCRD: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "invalidformat", + }, + }, + }, + }, + }) + }), + }, + "multiple newly introduced unrecognized formats": { + wantWarningMessages: []string{ + `unrecognized format "unknownformat1"`, + `unrecognized format "unknownformat2"`, + }, + oldCRD: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "date-time", + }, + }, + }, + }, + }) + }), + newCRD: mkCRD(func(spec *apiextensions.CustomResourceDefinitionSpec) { + spec.Versions = append(spec.Versions, apiextensions.CustomResourceDefinitionVersion{ + Name: "v1", + Served: true, + Storage: true, + Schema: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "string", + Format: "unknownformat1", + }, + "field2": { + Type: "string", + Format: "unknownformat2", + }, + }, + }, + }, + }) + }), + }, + } + + for name, tc := range testcases { + t.Run(name, func(t *testing.T) { + gotWarnings := strategy.WarningsOnUpdate(ctx, tc.newCRD, tc.oldCRD) + if len(gotWarnings) != len(tc.wantWarningMessages) { + t.Errorf("got %d warnings but expected %d", len(gotWarnings), len(tc.wantWarningMessages)) + return + } + + // Sort gotWarnings to match expected order + sort.Strings(gotWarnings) + + for i, expectedMessage := range tc.wantWarningMessages { + if gotWarnings[i] != expectedMessage { + t.Errorf("warning %d: got %s, expected %s", i, gotWarnings[i], expectedMessage) + } + } + }) + } +}