Merge pull request #1644 from estroz/feature/crd-webhook-v1

⚠️ (go/v3-alpha) default to v1 CRDs and webhooks

Author: k8s-ci-robot

docs/book/src/reference/makefile-helpers.md

@@ -1,6 +1,6 @@
 # Makefile Helpers
 
-By default, the projects are scaffolded with a `Makefile`. You can customize and update this file as please you. Here, you will find some helpers that can be useful. 
+By default, the projects are scaffolded with a `Makefile`. You can customize and update this file as please you. Here, you will find some helpers that can be useful.
 
 ## To debug with go-delve
 
@@ -14,28 +14,24 @@ run-delve: generate fmt vet manifests
     dlv --listen=:2345 --headless=true --api-version=2 --accept-multiclient exec ./bin/manager
 ```
 
-## To change the version of CRDs 
+## To change the version of CRDs
 
-The tool generate the CRDs by using [controller-tools](https://github.com/kubernetes-sigs/controller-tools), see in the manifests target:
+The `controller-gen` program (from [controller-tools](https://github.com/kubernetes-sigs/controller-tools))
+generates CRDs for kubebuilder projects, wrapped in the following `make` rule:
 
 ```sh
-# Generate manifests e.g. CRD, RBAC etc.
 manifests: controller-gen
 	$(CONTROLLER_GEN) $(CRD_OPTIONS) rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
 ```
 
-In this way, update `CRD_OPTIONS` to define the version of the CRDs manifests which will be generated in the `config/crd/bases` directory:
+`controller-gen` lets you specify what CRD API version to generate (either "v1", the default, or "v1beta1").
+You can direct it to generate a specific version by adding `crd:crdVersions={<version>}` to your `CRD_OPTIONS`,
+found at the top of your Makefile:
 
 ```sh
-# Produce CRDs that work back to Kubernetes 1.11 (no version conversion)
-CRD_OPTIONS ?= "crd:trivialVersions=true"
+CRD_OPTIONS ?= "crd:crdVersions={v1beta1},trivialVersions=true,preserveUnknownFields=false"
 ```
 
-|   CRD_OPTIONS	|   API version	|  
-|---	|---	|
-| `"crd:trivialVersions=true"` |  `apiextensions.k8s.io/v1beta1` |
-| `"crd:crdVersions=v1"` | `apiextensions.k8s.io/v1`	|  
-
 ## To get all the manifests without deploying
 
 By adding `make dry-run` you can get the patched manifests in the dry-run folder, unlike `make depĺoy` which runs `kustomize` and `kubectl apply`.

pkg/model/config/config.go

@@ -87,17 +87,19 @@ func (c Config) HasResource(target GVK) bool {
 	return false
 }
 
-// AddResource appends the provided resource to the tracked ones
-// It returns if the configuration was modified
-func (c *Config) AddResource(gvk GVK) bool {
-	// No-op if the resource was already tracked, return false
-	if c.HasResource(gvk) {
-		return false
+// UpdateResources either adds gvk to the tracked set or, if the resource already exists,
+// updates the the equivalent resource in the set.
+func (c *Config) UpdateResources(gvk GVK) {
+	// If the resource already exists, update it.
+	for i, r := range c.Resources {
+		if r.isEqualTo(gvk) {
+			c.Resources[i].merge(gvk)
+			return
+		}
 	}
 
-	// Append the resource to the tracked ones, return true
+	// The resource does not exist, append the resource to the tracked ones.
 	c.Resources = append(c.Resources, gvk)
-	return true
 }
 
 // HasGroup returns true if group is already tracked
@@ -113,11 +115,44 @@ func (c Config) HasGroup(group string) bool {
 	return false
 }
 
+// IsCRDVersionCompatible returns true if crdVersion can be added to the existing set of CRD versions.
+func (c Config) IsCRDVersionCompatible(crdVersion string) bool {
+	return c.resourceAPIVersionCompatible("crd", crdVersion)
+}
+
+// IsWebhookVersionCompatible returns true if webhookVersion can be added to the existing set of Webhook versions.
+func (c Config) IsWebhookVersionCompatible(webhookVersion string) bool {
+	return c.resourceAPIVersionCompatible("webhook", webhookVersion)
+}
+
+// resourceAPIVersionCompatible returns true if version can be added to the existing set of versions
+// for a given verType.
+func (c Config) resourceAPIVersionCompatible(verType, version string) bool {
+	for _, res := range c.Resources {
+		var currVersion string
+		switch verType {
+		case "crd":
+			currVersion = res.CRDVersion
+		case "webhook":
+			currVersion = res.WebhookVersion
+		}
+		if currVersion != "" && version != currVersion {
+			return false
+		}
+	}
+	return true
+}
+
 // GVK contains information about scaffolded resources
 type GVK struct {
 	Group   string `json:"group,omitempty"`
 	Version string `json:"version,omitempty"`
 	Kind    string `json:"kind,omitempty"`
+
+	// CRDVersion holds the CustomResourceDefinition API version used for the GVK.
+	CRDVersion string `json:"crdVersion,omitempty"`
+	// WebhookVersion holds the {Validating,Mutating}WebhookConfiguration API version used for the GVK.
+	WebhookVersion string `json:"webhookVersion,omitempty"`
 }
 
 // isEqualTo compares it with another resource
@@ -127,6 +162,17 @@ func (r GVK) isEqualTo(other GVK) bool {
 		r.Kind == other.Kind
 }
 
+// merge combines fields of two GVKs that have matching group, version, and kind,
+// favoring the receiver's values.
+func (r *GVK) merge(other GVK) {
+	if r.CRDVersion == "" && other.CRDVersion != "" {
+		r.CRDVersion = other.CRDVersion
+	}
+	if r.WebhookVersion == "" && other.WebhookVersion != "" {
+		r.WebhookVersion = other.WebhookVersion
+	}
+}
+
 // Marshal returns the bytes of c.
 func (c Config) Marshal() ([]byte, error) {
 	// Ignore extra fields at first.

pkg/model/config/config_test.go

@@ -21,7 +21,9 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-var _ = Describe("Config", func() {
+const v1beta1 = "v1beta1"
+
+var _ = Describe("PluginConfig", func() {
 	// Test plugin config. Don't want to export this config, but need it to
 	// be accessible by test.
 	type PluginConfig struct {
@@ -146,3 +148,94 @@ var _ = Describe("Config", func() {
 		Expect(pluginConfig).To(Equal(expectedPluginConfig))
 	})
 })
+
+var _ = Describe("Resource Version Compatibility", func() {
+
+	var (
+		c          *Config
+		gvk1, gvk2 GVK
+
+		defaultVersion = "v1"
+	)
+
+	BeforeEach(func() {
+		c = &Config{}
+		gvk1 = GVK{Group: "example", Version: "v1", Kind: "TestKind"}
+		gvk2 = GVK{Group: "example", Version: "v1", Kind: "TestKind2"}
+	})
+
+	Context("resourceAPIVersionCompatible", func() {
+		It("returns true for a list of empty resources", func() {
+			Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeTrue())
+		})
+		It("returns true for one resource with an empty version", func() {
+			c.Resources = []GVK{gvk1}
+			Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeTrue())
+		})
+		It("returns true for one resource with matching version", func() {
+			gvk1.CRDVersion = defaultVersion
+			c.Resources = []GVK{gvk1}
+			Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeTrue())
+		})
+		It("returns true for two resources with matching versions", func() {
+			gvk1.CRDVersion = defaultVersion
+			gvk2.CRDVersion = defaultVersion
+			c.Resources = []GVK{gvk1, gvk2}
+			Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeTrue())
+		})
+		It("returns false for one resource with a non-matching version", func() {
+			gvk1.CRDVersion = v1beta1
+			c.Resources = []GVK{gvk1}
+			Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeFalse())
+		})
+		It("returns false for two resources containing a non-matching version", func() {
+			gvk1.CRDVersion = v1beta1
+			gvk2.CRDVersion = defaultVersion
+			c.Resources = []GVK{gvk1, gvk2}
+			Expect(c.resourceAPIVersionCompatible("crd", defaultVersion)).To(BeFalse())
+		})
+
+		It("returns false for two resources containing a non-matching version (webhooks)", func() {
+			gvk1.WebhookVersion = v1beta1
+			gvk2.WebhookVersion = defaultVersion
+			c.Resources = []GVK{gvk1, gvk2}
+			Expect(c.resourceAPIVersionCompatible("webhook", defaultVersion)).To(BeFalse())
+		})
+	})
+})
+
+var _ = Describe("Config", func() {
+	var (
+		c          *Config
+		gvk1, gvk2 GVK
+	)
+
+	BeforeEach(func() {
+		c = &Config{}
+		gvk1 = GVK{Group: "example", Version: "v1", Kind: "TestKind"}
+		gvk2 = GVK{Group: "example", Version: "v1", Kind: "TestKind2"}
+	})
+
+	Context("UpdateResource", func() {
+		It("Adds a non-existing resource", func() {
+			c.UpdateResources(gvk1)
+			Expect(c.Resources).To(Equal([]GVK{gvk1}))
+			// Update again to ensure idempotency.
+			c.UpdateResources(gvk1)
+			Expect(c.Resources).To(Equal([]GVK{gvk1}))
+		})
+		It("Updates an existing resource", func() {
+			c.UpdateResources(gvk1)
+			gvk := GVK{Group: gvk1.Group, Version: gvk1.Version, Kind: gvk1.Kind, CRDVersion: "v1"}
+			c.UpdateResources(gvk)
+			Expect(c.Resources).To(Equal([]GVK{gvk}))
+		})
+		It("Updates an existing resource with more than one resource present", func() {
+			c.UpdateResources(gvk1)
+			c.UpdateResources(gvk2)
+			gvk := GVK{Group: gvk1.Group, Version: gvk1.Version, Kind: gvk1.Kind, CRDVersion: "v1"}
+			c.UpdateResources(gvk)
+			Expect(c.Resources).To(Equal([]GVK{gvk, gvk2}))
+		})
+	})
+})

pkg/model/resource/options.go

@@ -82,6 +82,11 @@ type Options struct {
 
 	// Namespaced is true if the resource is namespaced.
 	Namespaced bool
+
+	// CRDVersion holds the CustomResourceDefinition API version used for the Options.
+	CRDVersion string
+	// WebhookVersion holds the {Validating,Mutating}WebhookConfiguration API version used for the Options.
+	WebhookVersion string
 }
 
 // ValidateV2 verifies that V2 project has all the fields have valid values
@@ -183,6 +188,18 @@ func (opts *Options) Validate() error {
 		return fmt.Errorf("invalid Kind: %#v", validationErrors)
 	}
 
+	// Ensure apiVersions for k8s types are empty or valid.
+	for typ, apiVersion := range map[string]string{
+		"CRD":     opts.CRDVersion,
+		"Webhook": opts.WebhookVersion,
+	} {
+		switch apiVersion {
+		case "", "v1", "v1beta1":
+		default:
+			return fmt.Errorf("%s version must be one of: v1, v1beta1", typ)
+		}
+	}
+
 	// TODO: validate plural strings if provided
 
 	return nil
@@ -191,9 +208,11 @@ func (opts *Options) Validate() error {
 // GVK returns the group-version-kind information to check against tracked resources in the configuration file
 func (opts *Options) GVK() config.GVK {
 	return config.GVK{
-		Group:   opts.Group,
-		Version: opts.Version,
-		Kind:    opts.Kind,
+		Group:          opts.Group,
+		Version:        opts.Version,
+		Kind:           opts.Kind,
+		CRDVersion:     opts.CRDVersion,
+		WebhookVersion: opts.WebhookVersion,
 	}
 }
 
@@ -269,5 +288,7 @@ func (opts *Options) newResource() *Resource {
 		Kind:             opts.Kind,
 		Plural:           plural,
 		ImportAlias:      opts.safeImport(opts.Group + opts.Version),
+		CRDVersion:       opts.CRDVersion,
+		WebhookVersion:   opts.WebhookVersion,
 	}
 }

pkg/model/resource/resource.go

@@ -51,14 +51,21 @@ type Resource struct {
 
 	// Namespaced is true if the resource is namespaced.
 	Namespaced bool `json:"namespaced,omitempty"`
+
+	// CRDVersion holds the CustomResourceDefinition API version used for the Resource.
+	CRDVersion string `json:"crdVersion,omitempty"`
+	// WebhookVersion holds the {Validating,Mutating}WebhookConfiguration API version used for the Resource.
+	WebhookVersion string `json:"webhookVersion,omitempty"`
 }
 
 // GVK returns the group-version-kind information to check against tracked resources in the configuration file
 func (r *Resource) GVK() config.GVK {
 	return config.GVK{
-		Group:   r.Group,
-		Version: r.Version,
-		Kind:    r.Kind,
+		Group:          r.Group,
+		Version:        r.Version,
+		Kind:           r.Kind,
+		CRDVersion:     r.CRDVersion,
+		WebhookVersion: r.WebhookVersion,
 	}
 }
 

pkg/plugin/v2/scaffolds/api.go

@@ -93,7 +93,7 @@ func (s *apiScaffolder) newUniverse() *model.Universe {
 
 func (s *apiScaffolder) scaffold() error {
 	if s.doResource {
-		s.config.AddResource(s.resource.GVK())
+		s.config.UpdateResources(s.resource.GVK())
 
 		if err := machinery.NewScaffold(s.plugins...).Execute(
 			s.newUniverse(),

pkg/plugin/v3/api.go

@@ -37,10 +37,15 @@ import (
 	"sigs.k8s.io/kubebuilder/v2/plugins/addon"
 )
 
-// KbDeclarativePatternVersion is the sigs.k8s.io/kubebuilder-declarative-pattern version
-// (used only to gen api with --pattern=addon)
-// TODO: remove this when a better solution for using addons is implemented.
-const KbDeclarativePatternVersion = "1cbf859290cab81ae8e73fc5caebe792280175d1"
+const (
+	// KbDeclarativePatternVersion is the sigs.k8s.io/kubebuilder-declarative-pattern version
+	// (used only to gen api with --pattern=addon)
+	// TODO: remove this when a better solution for using addons is implemented.
+	KbDeclarativePatternVersion = "1cbf859290cab81ae8e73fc5caebe792280175d1"
+
+	// defaultCRDVersion is the default CRD API version to scaffold.
+	defaultCRDVersion = "v1"
+)
 
 // DefaultMainPath is default file path of main.go
 const DefaultMainPath = "main.go"
@@ -124,6 +129,8 @@ func (p *createAPISubcommand) BindFlags(fs *pflag.FlagSet) {
 	fs.StringVar(&p.resource.Group, "group", "", "resource Group")
 	fs.StringVar(&p.resource.Version, "version", "", "resource Version")
 	fs.BoolVar(&p.resource.Namespaced, "namespaced", true, "resource is namespaced")
+	fs.StringVar(&p.resource.CRDVersion, "crd-version", defaultCRDVersion,
+		"version of CustomResourceDefinition to scaffold. Options: [v1, v1beta1]")
 }
 
 func (p *createAPISubcommand) InjectConfig(c *config.Config) {
@@ -172,6 +179,12 @@ func (p *createAPISubcommand) Validate() error {
 			return fmt.Errorf("multiple groups are not allowed by default, " +
 				"to enable multi-group visit kubebuilder.io/migration/multi-group.html")
 		}
+
+		// Check CRDVersion against all other CRDVersions in p.config for compatibility.
+		if !p.config.IsCRDVersionCompatible(p.resource.CRDVersion) {
+			return fmt.Errorf("only one CRD version can be used for all resources, cannot add %q",
+				p.resource.CRDVersion)
+		}
 	}
 
 	return nil