diff --git a/pkg/crd/markers/validation.go b/pkg/crd/markers/validation.go index 14a85c00f..d8d53a1f3 100644 --- a/pkg/crd/markers/validation.go +++ b/pkg/crd/markers/validation.go @@ -35,6 +35,8 @@ const ( ValidationExactlyOneOfPrefix = validationPrefix + "ExactlyOneOf" ValidationAtMostOneOfPrefix = validationPrefix + "AtMostOneOf" ValidationAtLeastOneOfPrefix = validationPrefix + "AtLeastOneOf" + + OpaqueFieldName = "k8s:opaque" ) // ValidationMarkers lists all available markers that affect CRD schema generation, @@ -123,6 +125,9 @@ var FieldOnlyMarkers = []*definitionWithHelp{ must(markers.MakeDefinition(SchemalessName, markers.DescribesField, Schemaless{})). WithHelp(Schemaless{}.Help()), + + must(markers.MakeDefinition(OpaqueFieldName, markers.DescribesField, Opaque{})). + WithHelp(markers.SimpleHelp("CRD validation", "suppresses type-level validation inheritance for this field; field-level markers still apply.")), } // ValidationIshMarkers are field-and-type markers that don't fall under the @@ -568,6 +573,10 @@ type XIntOrString struct{} // +controllertools:marker:generateHelp:category="CRD validation" type Schemaless struct{} +// Opaque instructs the CRD generator to suppress inheritance of type-level +// validation for this field. Field-level markers still apply. +type Opaque struct{} + func hasNumericType(schema *apiextensionsv1.JSONSchemaProps) bool { return schema.Type == string(Integer) || schema.Type == string(Number) } diff --git a/pkg/crd/schema.go b/pkg/crd/schema.go index 4b20bbdf0..3c6ded0b7 100644 --- a/pkg/crd/schema.go +++ b/pkg/crd/schema.go @@ -73,6 +73,7 @@ type schemaContext struct { allowDangerousTypes bool ignoreUnexportedFields bool + opaque bool } // newSchemaContext constructs a new schemaContext for the given package and schema requester. @@ -99,6 +100,16 @@ func (c *schemaContext) ForInfo(info *markers.TypeInfo) *schemaContext { } } +// ForInfoOpaque produces a new schemaContext with containing the same information +// as this one, except with the given type information and marked as opaque. +// An opaque context prevents the emission of $ref to the type's schema, suppressing +// inherited type-level validations. +func (c *schemaContext) ForInfoOpaque(info *markers.TypeInfo) *schemaContext { + ctx := c.ForInfo(info) + ctx.opaque = true + return ctx +} + // requestSchema asks for the schema for a type in the package with the // given import path. func (c *schemaContext) requestSchema(pkgPath, typeName string) { @@ -285,6 +296,12 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J // > Otherwise, the alias information is only in the type name, which // > points directly to the actual (aliased) type. if typeInfo.Name() != ident.Name { + if ctx.opaque { + return &apiextensionsv1.JSONSchemaProps{ + Type: typ, + Format: fmt, + } + } ctx.requestSchema("", ident.Name) link := TypeRefLink("", ident.Name) return &apiextensionsv1.JSONSchemaProps{ @@ -309,8 +326,6 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J if pkg == ctx.pkg.Types { pkgPath = "" } - ctx.requestSchema(pkgPath, typeNameInfo.Name()) - link := TypeRefLink(pkgPath, typeNameInfo.Name()) // In cases where we have a named type, apply the type and format from the named schema // to allow markers that need this information to apply correctly. @@ -327,11 +342,18 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J } } - return &apiextensionsv1.JSONSchemaProps{ + props := &apiextensionsv1.JSONSchemaProps{ Type: typ, Format: fmt, - Ref: &link, } + + if !ctx.opaque { + ctx.requestSchema(pkgPath, typeNameInfo.Name()) + link := TypeRefLink(pkgPath, typeNameInfo.Name()) + props.Ref = &link + } + + return props default: ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("unsupported type %T for identifier %s", typeInfo, ident.Name), ident)) return &apiextensionsv1.JSONSchemaProps{} @@ -537,9 +559,12 @@ func structToSchema(ctx *schemaContext, structType *ast.StructType) *apiextensio } var propSchema *apiextensionsv1.JSONSchemaProps - if field.Markers.Get(crdmarkers.SchemalessName) != nil { + switch { + case field.Markers.Get(crdmarkers.SchemalessName) != nil: propSchema = &apiextensionsv1.JSONSchemaProps{} - } else { + case field.Markers.Get(crdmarkers.OpaqueFieldName) != nil: + propSchema = typeToSchema(ctx.ForInfoOpaque(&markers.TypeInfo{}), field.RawField.Type) + default: propSchema = typeToSchema(ctx.ForInfo(&markers.TypeInfo{}), field.RawField.Type) } propSchema.Description = field.Doc diff --git a/pkg/crd/testdata/crd_test.go b/pkg/crd/testdata/crd_test.go index b08952e56..72396f15f 100644 --- a/pkg/crd/testdata/crd_test.go +++ b/pkg/crd/testdata/crd_test.go @@ -16,11 +16,15 @@ limitations under the License. package cronjob import ( + "context" "os" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/yaml" ) @@ -36,4 +40,109 @@ var _ = Describe("CronJob CRD", func() { err = k8sClient.Create(ctx, crd) Expect(err).NotTo(HaveOccurred()) }) + + Context("validating opaque markers", func() { + applyCronJob := func(ctx context.Context, name, opaqueVal, nonOpaqueVal, opaqueMinLengthVal string) error { + obj := &unstructured.Unstructured{} + obj.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "testdata.kubebuilder.io", + Version: "v1", + Kind: "CronJob", + }) + obj.SetName(name) + obj.SetNamespace("default") + spec := map[string]interface{}{ + "schedule": "*/5 * * * *", // required field + "foo": "bar", + "baz": "baz", + "binaryName": "YmluYXJ5", // base64 for "binary" + "canBeNull": "ok", + "defaultedEmptyMap": map[string]interface{}{}, + "defaultedEmptyObject": map[string]interface{}{}, + "defaultedEmptySlice": []interface{}{}, + "defaultedObject": []interface{}{}, + "defaultedSlice": []interface{}{}, + "defaultedString": "some string", + "doubleDefaultedString": "some string", + "embeddedResource": map[string]interface{}{"kind": "Pod", "apiVersion": "v1"}, + "explicitlyRequiredK8s": "required", + "explicitlyRequiredKubebuilder": "required", + "explicitlyRequiredKubernetes": "required", + "float64WithValidations": 1.5, + "floatWithValidations": 1.5, + "int32WithValidations": 2, + "intWithValidations": 2, + "jobTemplate": map[string]interface{}{ + "template": map[string]interface{}{}, + }, + "kubernetesDefaultedEmptyMap": map[string]interface{}{}, + "kubernetesDefaultedEmptyObject": map[string]interface{}{}, + "kubernetesDefaultedEmptySlice": []interface{}{}, + "kubernetesDefaultedObject": []interface{}{}, + "kubernetesDefaultedSlice": []interface{}{}, + "kubernetesDefaultedString": "string", + "mapOfInfo": map[string]interface{}{}, + "nestedMapOfInfo": map[string]interface{}{}, + "nestedStructWithSeveralFields": map[string]interface{}{"foo": "str", "bar": true}, + "nestedStructWithSeveralFieldsDoubleMarked": map[string]interface{}{"foo": "str", "bar": true}, + "nestedassociativeList": []interface{}{}, + "patternObject": "https://example.com", + "stringPair": []interface{}{"a", "b"}, + "structWithSeveralFields": map[string]interface{}{"foo": "str", "bar": true}, + "twoOfAKindPart0": "longenough", + "twoOfAKindPart1": "longenough", + "unprunedEmbeddedResource": map[string]interface{}{"kind": "Pod", "apiVersion": "v1"}, + "unprunedFomType": map[string]interface{}{}, + "unprunedFomTypeAndField": map[string]interface{}{}, + "unprunedJSON": map[string]interface{}{"foo": "str", "bar": true}, + "associativeList": []interface{}{}, + } + if opaqueVal != "" { + spec["opaqueField"] = opaqueVal + } + if nonOpaqueVal != "" { + spec["nonOpaqueField"] = nonOpaqueVal + } + if opaqueMinLengthVal != "" { + spec["opaqueMinLengthField"] = opaqueMinLengthVal + } + obj.Object["spec"] = spec + + return k8sClient.Create(ctx, obj) + } + + It("should suppress type-level validation for fields with +k8s:opaque", func(ctx SpecContext) { + // type-level validation is MinLength=4 + // field-level validation is MaxLength=5 + + By("allowing opaqueField with length 3 (suppresses type-level MinLength=4)") + Eventually(func() error { + return applyCronJob(ctx, "test-opaque-short", "abc", "", "") + }, 5*time.Second, 1*time.Second).Should(Succeed()) + + By("rejecting nonOpaqueField with length 3 (inherits type-level MinLength=4)") + err := applyCronJob(ctx, "test-non-opaque-short", "", "abc", "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("should be at least 4 chars long")) + + By("rejecting opaqueField with length 6 (applies field-level MaxLength=5)") + err = applyCronJob(ctx, "test-opaque-long", "abcdef", "", "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Too long")) + + By("rejecting nonOpaqueField with length 6 (applies field-level MaxLength=5)") + err = applyCronJob(ctx, "test-non-opaque-long", "", "abcdef", "") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Too long")) + + By("allowing opaqueMinLengthField with length 2 (suppresses type-level MinLength=4, uses field-level MinLength=2)") + err = applyCronJob(ctx, "test-opaque-min-2", "", "", "ab") + Expect(err).ToNot(HaveOccurred()) + + By("rejecting opaqueMinLengthField with length 1 (uses field-level MinLength=2)") + err = applyCronJob(ctx, "test-opaque-min-1", "", "", "a") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("should be at least 2 chars long")) + }) + }) }) diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go index 64bba99b9..50c24e207 100644 --- a/pkg/crd/testdata/cronjob_types.go +++ b/pkg/crd/testdata/cronjob_types.go @@ -430,6 +430,21 @@ type CronJobSpec struct { // +kubebuilder:validation:MinLength=10 // +kubebuilder:validation:MaxLength=10 FieldLevelLocalDeclarationOverride LongerString `json:"fieldLevelLocalDeclarationOverride,omitempty"` + + // This tests that +k8s:opaque suppresses type-level validation from LongerString (MinLength=4). + // +k8s:opaque + // +kubebuilder:validation:MaxLength=5 + OpaqueField LongerString `json:"opaqueField,omitempty"` + + // This tests that without +k8s:opaque, type-level validations from LongerString are inherited. + // +kubebuilder:validation:MaxLength=5 + NonOpaqueField LongerString `json:"nonOpaqueField,omitempty"` + + // This tests that +k8s:opaque allows a field to completely replace a type-level validation. + // LongerString has MinLength=4. This field should have only MinLength=2. + // +k8s:opaque + // +kubebuilder:validation:MinLength=2 + OpaqueMinLengthField LongerString `json:"opaqueMinLengthField,omitempty"` } type InlineAlias = EmbeddedStruct diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml index b01322a1e..e5a547a9e 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml @@ -9542,6 +9542,12 @@ spec: This flag is like suspend, but for when you really mean it. It helps test the +kubebuilder:validation:Type marker. type: string + nonOpaqueField: + description: This tests that without +k8s:opaque, type-level validations + from LongerString are inherited. + maxLength: 5 + minLength: 4 + type: string onlyAllowSettingOnUpdate: description: Test that we can add a field that can only be set to a non-default value on updates using XValidation OptionalOldSelf. @@ -9552,6 +9558,17 @@ spec: an update. optionalOldSelf: true rule: oldSelf.hasValue() || self == 0 + opaqueField: + description: This tests that +k8s:opaque suppresses type-level validation + from LongerString (MinLength=4). + maxLength: 5 + type: string + opaqueMinLengthField: + description: |- + This tests that +k8s:opaque allows a field to completely replace a type-level validation. + LongerString has MinLength=4. This field should have only MinLength=2. + minLength: 2 + type: string patternObject: description: This tests that pattern validator is properly applied. pattern: ^$|^((https):\/\/?)[^\s()<>]+(?:\([\w\d]+\)|([^[:punct:]\s]|\/?))$