From ddfa00fb343b1de8b3d130a5115f99dc8a748c9d Mon Sep 17 00:00:00 2001 From: Max Smythe Date: Wed, 25 Nov 2020 19:24:37 -0800 Subject: [PATCH 1/2] Add crd:validation:Schemaless marker This marker will avoid trying to do any type detection on any struct field on which it is set. This gives users a safety valve when they hit an edge case where type inference does the wrong thing for them. Signed-off-by: Max Smythe --- .golangci.yml | 2 -- pkg/crd/markers/validation.go | 17 +++++++++++++++++ pkg/crd/markers/zz_generated.markerhelp.go | 11 +++++++++++ pkg/crd/schema.go | 8 +++++++- pkg/crd/testdata/cronjob_types.go | 9 +++++++-- .../testdata.kubebuilder.io_cronjobs.yaml | 2 ++ test.sh | 4 ++-- 7 files changed, 46 insertions(+), 7 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 9b77af3f1..301b194e4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,4 @@ run: - modules-download-mode: readonly - # Increase the default deadline from 1m as some module operations can take a # while if uncached! deadline: 5m diff --git a/pkg/crd/markers/validation.go b/pkg/crd/markers/validation.go index 5bf8942d9..5a038e4e6 100644 --- a/pkg/crd/markers/validation.go +++ b/pkg/crd/markers/validation.go @@ -26,6 +26,10 @@ import ( "sigs.k8s.io/controller-tools/pkg/markers" ) +const ( + SchemalessName = "kubebuilder:validation:Schemaless" +) + // ValidationMarkers lists all available markers that affect CRD schema generation, // except for the few that don't make sense as type-level markers (see FieldOnlyMarkers). // All markers start with `+kubebuilder:validation:`, and continue with their type name. @@ -82,6 +86,9 @@ var FieldOnlyMarkers = []*definitionWithHelp{ must(markers.MakeDefinition("kubebuilder:validation:EmbeddedResource", markers.DescribesField, XEmbeddedResource{})). WithHelp(XEmbeddedResource{}.Help()), + + must(markers.MakeDefinition(SchemalessName, markers.DescribesField, Schemaless{})). + WithHelp(Schemaless{}.Help()), } // ValidationIshMarkers are field-and-type markers that don't fall under the @@ -225,6 +232,16 @@ type XPreserveUnknownFields struct{} // field, yet it is possible. This can be combined with PreserveUnknownFields. type XEmbeddedResource struct{} +// +controllertools:marker:generateHelp:category="CRD validation" +// Schemaless marks a field as being a schemaless object. +// +// Schemaless objects are not introspected, so you must provide +// any type and validation information yourself. One use for this +// tag is for embedding fields that hold JSONSchema typed objects. +// Because this field disables all type checking, it is recommended +// to be used only as a last resort. +type Schemaless struct{} + func (m Maximum) ApplyToSchema(schema *apiext.JSONSchemaProps) error { if schema.Type != "integer" { return fmt.Errorf("must apply maximum to an integer") diff --git a/pkg/crd/markers/zz_generated.markerhelp.go b/pkg/crd/markers/zz_generated.markerhelp.go index c1e02729e..7c6aca4e1 100644 --- a/pkg/crd/markers/zz_generated.markerhelp.go +++ b/pkg/crd/markers/zz_generated.markerhelp.go @@ -306,6 +306,17 @@ func (Resource) Help() *markers.DefinitionHelp { } } +func (Schemaless) Help() *markers.DefinitionHelp { + return &markers.DefinitionHelp{ + Category: "CRD validation", + DetailedHelp: markers.DetailedHelp{ + Summary: "marks a field as being a schemaless object. ", + Details: "Schemaless objects are not introspected, so you must provide any type and validation information yourself. One use for this tag is for embedding fields that hold JSONSchema typed objects. Because this field disables all type checking, it is recommended to be used only as a last resort.", + }, + FieldHelp: map[string]markers.DetailedHelp{}, + } +} + func (SkipVersion) Help() *markers.DefinitionHelp { return &markers.DefinitionHelp{ Category: "CRD", diff --git a/pkg/crd/schema.go b/pkg/crd/schema.go index 8e44af593..0f682808b 100644 --- a/pkg/crd/schema.go +++ b/pkg/crd/schema.go @@ -23,6 +23,7 @@ import ( "strings" apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers" "sigs.k8s.io/controller-tools/pkg/loader" "sigs.k8s.io/controller-tools/pkg/markers" @@ -378,7 +379,12 @@ func structToSchema(ctx *schemaContext, structType *ast.StructType) *apiext.JSON } } - propSchema := typeToSchema(ctx.ForInfo(&markers.TypeInfo{}), field.RawField.Type) + var propSchema *apiext.JSONSchemaProps + if field.Markers.Get(crdmarkers.SchemalessName) != nil { + propSchema = &apiext.JSONSchemaProps{} + } else { + propSchema = typeToSchema(ctx.ForInfo(&markers.TypeInfo{}), field.RawField.Type) + } propSchema.Description = field.Doc applyMarkers(ctx, field.Markers, propSchema, field.RawField) diff --git a/pkg/crd/testdata/cronjob_types.go b/pkg/crd/testdata/cronjob_types.go index 7ebb1171e..9d0cbb969 100644 --- a/pkg/crd/testdata/cronjob_types.go +++ b/pkg/crd/testdata/cronjob_types.go @@ -154,14 +154,19 @@ type CronJobSpec struct { // This tests that min/max properties work MinMaxProperties MinMaxObject `json:"minMaxProperties,omitempty"` + + // This tests that the schemaless marker works + // +kubebuilder:validation:Schemaless + Schemaless []byte `json:"schemaless,omitempty"` } // +kubebuilder:validation:Type=object // +kubebuilder:pruning:PreserveUnknownFields type Preserved struct { - ConcreteField string `json:"concreteField"` - Rest map[string]interface{} `json:"-"` + ConcreteField string `json:"concreteField"` + Rest map[string]interface{} `json:"-"` } + func (p *Preserved) UnmarshalJSON(data []byte) error { if err := json.Unmarshal(data, &p.Rest); err != nil { return err diff --git a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml index 3c2949310..3ea8aeb4f 100644 --- a/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml +++ b/pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml @@ -5071,6 +5071,8 @@ spec: schedule: description: The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron. type: string + schemaless: + description: This tests that the schemaless marker works startingDeadlineSeconds: description: Optional deadline in seconds for starting the job if it misses scheduled time for any reason. Missed jobs executions diff --git a/test.sh b/test.sh index c6e702ecc..7a6d65ac2 100755 --- a/test.sh +++ b/test.sh @@ -107,13 +107,13 @@ fetch_kb_tools # setup testing env setup_envs -header_text "running golangci-lint" - header_text "generating marker help" pushd cmd/controller-gen > /dev/null go generate popd > /dev/null +header_text "running golangci-lint" + golangci-lint run --disable-all \ --enable=misspell \ --enable=golint \ From 0489594f50f6a9876e5b54f6b2517abc81e07679 Mon Sep 17 00:00:00 2001 From: Max Smythe Date: Wed, 27 Jan 2021 19:02:04 -0800 Subject: [PATCH 2/2] Revert modules-download-mode removal Signed-off-by: Max Smythe --- .golangci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yml b/.golangci.yml index 301b194e4..3a3f0f04c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,4 +1,5 @@ run: + modules-download-mode: readonly # Increase the default deadline from 1m as some module operations can take a # while if uncached! deadline: 5m