Merge pull request #499 from kevindelgado/pr/follow-up-481

✨ Add test and warning for v1beta1 default removal

pkg/crd/gen.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/types"
+	"os"
 
 	apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextlegacy "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
@@ -201,9 +202,15 @@ func removeDefaultsFromSchemaProps(v *apiextlegacy.JSONSchemaProps) {
 		return
 	}
 
+	if v.Default != nil {
+		fmt.Fprintln(os.Stderr, "Warning: default unsupported in CRD version v1beta1, v1 required. Removing defaults.")
+	}
+
 	// nil-out the default field
 	v.Default = nil
 	for name, prop := range v.Properties {
+		// iter var reference is fine -- we handle the persistence of the modfications on the line below
+		//nolint:gosec
 		removeDefaultsFromSchemaProps(&prop)
 		v.Properties[name] = prop
 	}

pkg/crd/gen_integration_test.go

@@ -0,0 +1,110 @@
+/*
+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_test
+
+import (
+	"bytes"
+	"io"
+	"io/ioutil"
+	"os"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"sigs.k8s.io/controller-tools/pkg/crd"
+	crdmarkers "sigs.k8s.io/controller-tools/pkg/crd/markers"
+	"sigs.k8s.io/controller-tools/pkg/genall"
+	"sigs.k8s.io/controller-tools/pkg/loader"
+	"sigs.k8s.io/controller-tools/pkg/markers"
+)
+
+var _ = Describe("CRD Generation proper defaulting", func() {
+	var ctx *genall.GenerationContext
+	var out *outputRule
+	BeforeEach(func() {
+		By("switching into testdata to appease go modules")
+		cwd, err := os.Getwd()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(os.Chdir("./testdata/gen")).To(Succeed()) // go modules are directory-sensitive
+		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))
+
+		By("settup up the context")
+		reg := &markers.Registry{}
+		Expect(crdmarkers.Register(reg)).To(Succeed())
+		out = &outputRule{
+			buf: &bytes.Buffer{},
+		}
+		ctx = &genall.GenerationContext{
+			Collector:  &markers.Collector{Registry: reg},
+			Roots:      pkgs,
+			Checker:    &loader.TypeChecker{},
+			OutputRule: out,
+		}
+	})
+
+	It("should strip v1beta1 CRDs of default fields", func() {
+		By("calling Generate")
+		gen := &crd.Generator{
+			CRDVersions: []string{"v1beta1"},
+		}
+		Expect(gen.Generate(ctx)).NotTo(HaveOccurred())
+
+		By("loading the desired YAML")
+		expectedFile, err := ioutil.ReadFile("./testdata/gen/foo_crd_v1beta1.yaml")
+		Expect(err).NotTo(HaveOccurred())
+
+		By("comparing the two")
+		Expect(out.buf.Bytes()).To(Equal(expectedFile))
+
+	})
+
+	It("should not strip v1 CRDs of default fields", func() {
+		By("calling Generate")
+		gen := &crd.Generator{
+			CRDVersions: []string{"v1"},
+		}
+		Expect(gen.Generate(ctx)).NotTo(HaveOccurred())
+
+		By("loading the desired YAML")
+		expectedFile, err := ioutil.ReadFile("./testdata/gen/foo_crd_v1.yaml")
+		Expect(err).NotTo(HaveOccurred())
+
+		By("comparing the two")
+		Expect(out.buf.Bytes()).To(Equal(expectedFile))
+
+	})
+})
+
+type outputRule struct {
+	buf *bytes.Buffer
+}
+
+func (o *outputRule) Open(_ *loader.Package, itemPath string) (io.WriteCloser, error) {
+	return nopCloser{o.buf}, nil
+}
+
+type nopCloser struct {
+	io.Writer
+}
+
+func (n nopCloser) Close() error {
+	return nil
+}

pkg/crd/testdata/gen/foo_crd_v1.yaml

@@ -0,0 +1,55 @@
+
+---
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: (unknown)
+  creationTimestamp: null
+  name: foos.bar.example.com
+spec:
+  group: bar.example.com
+  names:
+    kind: Foo
+    listKind: FooList
+    plural: foos
+    singular: foo
+  scope: Namespaced
+  versions:
+  - name: foo
+    schema:
+      openAPIV3Schema:
+        properties:
+          apiVersion:
+            description: 'APIVersion defines the versioned schema of this representation
+              of an object. Servers should convert recognized schemas to the latest
+              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#resources'
+            type: string
+          kind:
+            description: 'Kind is a string value representing the REST resource this
+              object represents. Servers may infer this from the endpoint the client
+              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds'
+            type: string
+          metadata:
+            type: object
+          spec:
+            properties:
+              defaultedString:
+                default: fooDefaultString
+                description: This tests that defaulted fields are stripped for v1beta1,
+                  but not for v1
+                type: string
+            required:
+            - defaultedString
+            type: object
+          status:
+            type: object
+        type: object
+    served: true
+    storage: true
+status:
+  acceptedNames:
+    kind: ""
+    plural: ""
+  conditions: []
+  storedVersions: []

pkg/crd/testdata/gen/foo_crd_v1beta1.yaml

@@ -0,0 +1,55 @@
+
+---
+apiVersion: apiextensions.k8s.io/v1beta1
+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    controller-gen.kubebuilder.io/version: (unknown)
+  creationTimestamp: null
+  name: foos.bar.example.com
+spec:
+  group: bar.example.com
+  names:
+    kind: Foo
+    listKind: FooList
+    plural: foos
+    singular: foo
+  scope: Namespaced
+  validation:
+    openAPIV3Schema:
+      properties:
+        apiVersion:
+          description: 'APIVersion defines the versioned schema of this representation
+            of an object. Servers should convert recognized schemas to the latest
+            internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#resources'
+          type: string
+        kind:
+          description: 'Kind is a string value representing the REST resource this
+            object represents. Servers may infer this from the endpoint the client
+            submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#types-kinds'
+          type: string
+        metadata:
+          type: object
+        spec:
+          properties:
+            defaultedString:
+              description: This tests that defaulted fields are stripped for v1beta1,
+                but not for v1
+              type: string
+          required:
+          - defaultedString
+          type: object
+        status:
+          type: object
+      type: object
+  version: foo
+  versions:
+  - name: foo
+    served: true
+    storage: true
+status:
+  acceptedNames:
+    kind: ""
+    plural: ""
+  conditions: []
+  storedVersions: []

pkg/crd/testdata/gen/foo_types.go

@@ -0,0 +1,38 @@
+/*
+
+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 paths=. output:dir=.
+// +groupName=bar.example.com
+package foo
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+type FooSpec struct {
+	// This tests that defaulted fields are stripped for v1beta1,
+	// but not for v1
+	// +kubebuilder:default=fooDefaultString
+	DefaultedString string `json:"defaultedString"`
+}
+type FooStatus struct{}
+
+type Foo struct {
+	metav1.TypeMeta   `json:",inline"`
+	metav1.ObjectMeta `json:"metadata,omitempty"`
+
+	Spec   FooSpec   `json:"spec,omitempty"`
+	Status FooStatus `json:"status,omitempty"`
+}