✨ Allow maps of arrays of non-string types (#617)

* Allow maps of arrays of non-string types

* Correct golden file test

* Fix golangci linter error

Author: Porges

pkg/crd/schema.go

@@ -39,12 +39,10 @@ const (
 	defPrefix = "#/definitions/"
 )
 
-var (
-	// byteType is the types.Type for byte (see the types documention
-	// for why we need to look this up in the Universe), saved
-	// for quick comparison.
-	byteType = types.Universe.Lookup("byte").Type()
-)
+// byteType is the types.Type for byte (see the types documention
+// for why we need to look this up in the Universe), saved
+// for quick comparison.
+var byteType = types.Universe.Lookup("byte").Type()
 
 // SchemaMarker is any marker that needs to modify the schema of the underlying type or field.
 type SchemaMarker interface {
@@ -309,10 +307,6 @@ func mapToSchema(ctx *schemaContext, mapType *ast.MapType) *apiext.JSONSchemaPro
 		valSchema = namedToSchema(ctx.ForInfo(&markers.TypeInfo{}), val)
 	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("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:

pkg/crd/schema_test.go

@@ -0,0 +1,112 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+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.
+*/
+
+package crd
+
+import (
+	"go/ast"
+	"strings"
+	"testing"
+
+	"github.com/onsi/gomega"
+	"golang.org/x/tools/go/packages"
+	pkgstest "golang.org/x/tools/go/packages/packagestest"
+	apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	testloader "sigs.k8s.io/controller-tools/pkg/loader/testutils"
+	"sigs.k8s.io/controller-tools/pkg/markers"
+)
+
+func transform(t *testing.T, expr string) *apiext.JSONSchemaProps {
+	// this is *very* hacky but I haven’t found a simple way
+	// to get an ast.Expr with all the associated metadata required
+	// to run typeToSchema upon it:
+
+	moduleName := "sigs.k8s.io/controller-tools/pkg/crd"
+	modules := []pkgstest.Module{
+		{
+			Name: moduleName,
+			Files: map[string]interface{}{
+				"test.go": `
+				package crd 
+				type Test ` + expr,
+			},
+		},
+	}
+
+	pkgs, exported, err := testloader.LoadFakeRoots(pkgstest.Modules, modules, moduleName)
+	if exported != nil {
+		t.Cleanup(exported.Cleanup)
+	}
+
+	if err != nil {
+		t.Fatalf("unable to load fake package: %s", err)
+	}
+
+	if len(pkgs) != 1 {
+		t.Fatal("expected to parse only one package")
+	}
+
+	pkg := pkgs[0]
+	pkg.NeedTypesInfo()
+	failIfErrors(t, pkg.Errors)
+
+	schemaContext := newSchemaContext(pkg, nil, true).ForInfo(&markers.TypeInfo{})
+	// yick: grab the only type definition
+	definedType := pkg.Syntax[0].Decls[0].(*ast.GenDecl).Specs[0].(*ast.TypeSpec).Type
+	result := typeToSchema(schemaContext, definedType)
+	failIfErrors(t, pkg.Errors)
+	return result
+}
+
+func failIfErrors(t *testing.T, errs []packages.Error) {
+	if len(errs) > 0 {
+		var msgs []string
+		for _, e := range errs {
+			msgs = append(msgs, e.Msg)
+		}
+
+		t.Fatalf("error loading fake package: %s", strings.Join(msgs, "; "))
+	}
+}
+
+var arrayOfNumbersSchema *apiext.JSONSchemaProps = &apiext.JSONSchemaProps{
+	Type: "array",
+	Items: &apiext.JSONSchemaPropsOrArray{
+		Schema: &apiext.JSONSchemaProps{
+			Type: "number",
+		},
+	},
+}
+
+func Test_Schema_ArrayOfFloat32(t *testing.T) {
+	g := gomega.NewWithT(t)
+
+	output := transform(t, "[]float32")
+	g.Expect(output).To(gomega.Equal(arrayOfNumbersSchema))
+}
+
+func Test_Schema_MapOfStringToArrayOfFloat32(t *testing.T) {
+	g := gomega.NewWithT(t)
+
+	output := transform(t, "map[string][]float32")
+	g.Expect(output).To(gomega.Equal(&apiext.JSONSchemaProps{
+		Type: "object",
+		AdditionalProperties: &apiext.JSONSchemaPropsOrBool{
+			Allows: true,
+			Schema: arrayOfNumbersSchema,
+		},
+	}))
+}

pkg/crd/testdata/cronjob_types.go

@@ -178,6 +178,9 @@ type CronJobSpec struct {
 
 	// Checks that maps containing types that contain maps work
 	ContainsNestedMapMap map[string]ContainsNestedMap `json:"nestedMapInStruct,omitempty"`
+
+	// Maps of arrays of things-that-aren’t-strings are permitted
+	MapOfArraysOfFloats map[string][]bool `json:"mapOfArraysOfFloats,omitempty"`
 }
 
 type ContainsNestedMap struct {

pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml

@@ -7176,6 +7176,13 @@ spec:
                 - bar
                 - foo
                 type: object
+              mapOfArraysOfFloats:
+                additionalProperties:
+                  items:
+                    type: boolean
+                  type: array
+                description: Maps of arrays of things-that-aren’t-strings are permitted
+                type: object
               mapOfInfo:
                 additionalProperties:
                   format: byte