Allow nested maps only when used with 'AllowDangerousTypes' flag.

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"`

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.

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))
+	})
 })

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{}
 	}
 

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: []

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"`
+}