diff --git a/pkg/crd/gen.go b/pkg/crd/gen.go index dd154c549..8aac877c6 100644 --- a/pkg/crd/gen.go +++ b/pkg/crd/gen.go @@ -64,6 +64,7 @@ type Generator struct { // Currently the following additional types are allowed when this is true: // float32 // float64 + // nested maps: Ex: map[string]map[string]string . Also see crd/testdata/dangerous_types/types.go // // Left unspecified, the default is false AllowDangerousTypes *bool `marker:",optional"` diff --git a/pkg/crd/parser.go b/pkg/crd/parser.go index 164e4756d..d81759266 100644 --- a/pkg/crd/parser.go +++ b/pkg/crd/parser.go @@ -73,7 +73,7 @@ type Parser struct { flattener *Flattener - // AllowDangerousTypes controls the handling of non-recommended types such as float. If + // AllowDangerousTypes controls the handling of non-recommended types such as float, nested map. If // false (the default), these types are not supported. // There is a continuum here: // 1. Types that are always supported. diff --git a/pkg/crd/parser_integration_test.go b/pkg/crd/parser_integration_test.go index 0903ec0cf..601975a7e 100644 --- a/pkg/crd/parser_integration_test.go +++ b/pkg/crd/parser_integration_test.go @@ -190,4 +190,88 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func Expect(packageErrors(cronJobPkg, packages.TypeError)).NotTo(HaveOccurred()) }) + + It("should not allow dangerous types by default", func() { + By("switching into testdata to appease go modules") + cwd, err := os.Getwd() + Expect(err).NotTo(HaveOccurred()) + Expect(os.Chdir("./testdata/dangerous_types")).To(Succeed()) + defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }() + + By("loading the roots") + pkgs, err := loader.LoadRoots(".") + Expect(err).NotTo(HaveOccurred()) + Expect(pkgs).To(HaveLen(1)) + pkg := pkgs[0] + + By("setting up the parser") + reg := &markers.Registry{} + Expect(crdmarkers.Register(reg)).To(Succeed()) + parser := &crd.Parser{ + Collector: &markers.Collector{Registry: reg}, + Checker: &loader.TypeChecker{}, + } + crd.AddKnownTypes(parser) + + By("requesting that the package be parsed") + parser.NeedPackage(pkg) + + By("requesting that the CRD be generated") + groupKind := schema.GroupKind{Kind: "DangerousType", Group: "dangerous.example.com"} + parser.NeedCRDFor(groupKind, nil) + + By("checking that no errors occurred along the way (expect for type errors)") + Expect(packageErrors(pkg, packages.TypeError)).To(HaveOccurred()) + }) + + It("should allow dangerous types with 'AllowDangerousTypes' flag", func() { + By("switching into testdata to appease go modules") + cwd, err := os.Getwd() + Expect(err).NotTo(HaveOccurred()) + Expect(os.Chdir("./testdata/dangerous_types")).To(Succeed()) + defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }() + + By("loading the roots") + pkgs, err := loader.LoadRoots(".") + Expect(err).NotTo(HaveOccurred()) + Expect(pkgs).To(HaveLen(1)) + pkg := pkgs[0] + + By("setting up the parser") + reg := &markers.Registry{} + Expect(crdmarkers.Register(reg)).To(Succeed()) + // parser with AllowDangerousTypes as true. + parser := &crd.Parser{ + Collector: &markers.Collector{Registry: reg}, + Checker: &loader.TypeChecker{}, + AllowDangerousTypes: true, + } + crd.AddKnownTypes(parser) + + By("requesting that the package be parsed") + parser.NeedPackage(pkg) + + By("requesting that the CRD be generated") + groupKind := schema.GroupKind{Kind: "DangerousType", Group: "dangerous.example.com"} + parser.NeedCRDFor(groupKind, nil) + + By("checking that no errors occurred along the way (expect for type errors)") + Expect(packageErrors(pkg, packages.TypeError)).ToNot(HaveOccurred()) + + By("checking that the CRD is present") + Expect(parser.CustomResourceDefinitions).To(HaveKey(groupKind)) + + By("loading the desired YAML") + expectedFile, err := ioutil.ReadFile("dangerous.example.com_dangeroustypes.yaml") + Expect(err).NotTo(HaveOccurred()) + + By("parsing the desired YAML") + var crd apiext.CustomResourceDefinition + Expect(yaml.Unmarshal(expectedFile, &crd)).To(Succeed()) + // clear the annotations -- we don't care about the attribution annotation + crd.Annotations = nil + + By("comparing the two") + Expect(parser.CustomResourceDefinitions[groupKind]).To(Equal(crd), "type not as expected, check pkg/crd/testdata/README.md for more details.\n\nDiff:\n\n%s", cmp.Diff(parser.CustomResourceDefinitions[groupKind], crd)) + }) }) diff --git a/pkg/crd/schema.go b/pkg/crd/schema.go index 0f682808b..86c2ee011 100644 --- a/pkg/crd/schema.go +++ b/pkg/crd/schema.go @@ -299,13 +299,19 @@ func mapToSchema(ctx *schemaContext, mapType *ast.MapType) *apiext.JSONSchemaPro case *ast.ArrayType: valSchema = arrayToSchema(ctx.ForInfo(&markers.TypeInfo{}), val) if valSchema.Type == "array" && valSchema.Items.Schema.Type != "string" { - ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("map values must be a named type, not %T", mapType.Value), mapType.Value)) + ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("not a supported map value type: %T", mapType.Value), mapType.Value)) return &apiext.JSONSchemaProps{} } case *ast.StarExpr: valSchema = typeToSchema(ctx.ForInfo(&markers.TypeInfo{}), val) + case *ast.MapType: + if ctx.allowDangerousTypes { + valSchema = typeToSchema(ctx.ForInfo(&markers.TypeInfo{}), val) + } else { + ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("nested maps are not supported by default. They are only supported with 'allowDangerousTypes' flag"), mapType.Value)) + } default: - ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("map values must be a named type, not %T", mapType.Value), mapType.Value)) + ctx.pkg.AddError(loader.ErrFromNode(fmt.Errorf("not a supported map value type: %T", mapType.Value), mapType.Value)) return &apiext.JSONSchemaProps{} } diff --git a/pkg/crd/testdata/dangerous_types/dangerous.example.com_dangeroustypes.yaml b/pkg/crd/testdata/dangerous_types/dangerous.example.com_dangeroustypes.yaml new file mode 100644 index 000000000..6520b3247 --- /dev/null +++ b/pkg/crd/testdata/dangerous_types/dangerous.example.com_dangeroustypes.yaml @@ -0,0 +1,59 @@ + +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + creationTimestamp: null + name: dangeroustypes.dangerous.example.com +spec: + group: dangerous.example.com + names: + kind: DangerousType + listKind: DangerousTypeList + plural: dangeroustypes + singular: dangeroustype + scope: Namespaced + versions: + - name: v1 + schema: + openAPIV3Schema: + properties: + f32: + type: number + f64: + type: number + nestedMap: + additionalProperties: + additionalProperties: + type: string + type: object + description: Checks that nested maps work + type: object + nestedMapInStruct: + additionalProperties: + properties: + innerMap: + additionalProperties: + type: string + type: object + type: object + description: Checks that maps containing types that contain maps work + type: object + nestedNestedMap: + additionalProperties: + additionalProperties: + additionalProperties: + type: string + type: object + type: object + description: Checks that multiply-nested maps work + type: object + type: object + served: true + storage: true +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] \ No newline at end of file diff --git a/pkg/crd/testdata/dangerous_types/types.go b/pkg/crd/testdata/dangerous_types/types.go new file mode 100644 index 000000000..5502ff83b --- /dev/null +++ b/pkg/crd/testdata/dangerous_types/types.go @@ -0,0 +1,39 @@ +/* + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +//go:generate ../../../../.run-controller-gen.sh crd:crdVersions=v1 paths=. output:dir=. + +// +groupName=dangerous.example.com +// +versionName=v1 +package dangerous_types + +type DangerousType struct { + + f32 float32 `json:"f32,omitempty"` + f64 float64 `json:"f64,omitempty"` + + // Checks that nested maps work + NestedMap map[string]map[string]string `json:"nestedMap,omitempty"` + + // Checks that multiply-nested maps work + NestedNestedMap map[string]map[string]map[string]string `json:"nestedNestedMap,omitempty"` + + // Checks that maps containing types that contain maps work + ContainsNestedMapMap map[string]ContainsNestedMap `json:"nestedMapInStruct,omitempty"` +} + +type ContainsNestedMap struct { + InnerMap map[string]string `json:"innerMap,omitempty"` +}