Merge pull request #2047 from Adirio/config-interface

⚠️ Decouple plugin requirements from Config interface

Author: k8s-ci-robot

pkg/config/interface.go

@@ -89,10 +89,10 @@ type Config interface {
 
 	// HasGroup checks if the provided group is the same as any of the tracked resources
 	HasGroup(group string) bool
-	// IsCRDVersionCompatible returns true if crdVersion can be added to the existing set of CRD versions.
-	IsCRDVersionCompatible(crdVersion string) bool
-	// IsWebhookVersionCompatible returns true if webhookVersion can be added to the existing set of Webhook versions.
-	IsWebhookVersionCompatible(webhookVersion string) bool
+	// ListCRDVersions returns a list of the CRD versions in use by the tracked resources
+	ListCRDVersions() []string
+	// ListWebhookVersions returns a list of the webhook versions in use by the tracked resources
+	ListWebhookVersions() []string
 
 	/* Plugins */
 

pkg/config/v2/config.go

@@ -222,14 +222,14 @@ func (c cfg) HasGroup(group string) bool {
 	return false
 }
 
-// IsCRDVersionCompatible implements config.Config
-func (c cfg) IsCRDVersionCompatible(crdVersion string) bool {
-	return crdVersion == apiVersion
+// ListCRDVersions implements config.Config
+func (c cfg) ListCRDVersions() []string {
+	return make([]string, 0)
 }
 
-// IsWebhookVersionCompatible implements config.Config
-func (c cfg) IsWebhookVersionCompatible(webhookVersion string) bool {
-	return webhookVersion == apiVersion
+// ListWebhookVersions implements config.Config
+func (c cfg) ListWebhookVersions() []string {
+	return make([]string, 0)
 }
 
 // DecodePluginConfig implements config.Config

pkg/config/v2/config_test.go

@@ -225,22 +225,12 @@ var _ = Describe("cfg", func() {
 			Expect(c.HasGroup("other-group")).To(BeFalse())
 		})
 
-		It("IsCRDVersionCompatible should return true for `v1beta1`", func() {
-			Expect(c.IsCRDVersionCompatible("v1beta1")).To(BeTrue())
+		It("ListCRDVersions should return an empty list", func() {
+			Expect(c.ListCRDVersions()).To(BeEmpty())
 		})
 
-		It("IsCRDVersionCompatible should return false for any other than `v1beta1`", func() {
-			Expect(c.IsCRDVersionCompatible("v1")).To(BeFalse())
-			Expect(c.IsCRDVersionCompatible("v2")).To(BeFalse())
-		})
-
-		It("IsWebhookVersionCompatible should return true for `v1beta1`", func() {
-			Expect(c.IsWebhookVersionCompatible("v1beta1")).To(BeTrue())
-		})
-
-		It("IsWebhookVersionCompatible should return false for any other than `v1beta1`", func() {
-			Expect(c.IsWebhookVersionCompatible("v1")).To(BeFalse())
-			Expect(c.IsWebhookVersionCompatible("v2")).To(BeFalse())
+		It("ListWebhookVersions should return an empty list", func() {
+			Expect(c.ListWebhookVersions()).To(BeEmpty())
 		})
 	})
 

pkg/config/v3/config.go

@@ -249,35 +249,40 @@ func (c cfg) HasGroup(group string) bool {
 	return false
 }
 
-// IsCRDVersionCompatible implements config.Config
-func (c cfg) IsCRDVersionCompatible(crdVersion string) bool {
-	return c.resourceAPIVersionCompatible("crd", crdVersion)
-}
+// ListCRDVersions implements config.Config
+func (c cfg) ListCRDVersions() []string {
+	// Make a map to remove duplicates
+	versionSet := make(map[string]struct{})
+	for _, r := range c.Resources {
+		if r.API != nil && r.API.CRDVersion != "" {
+			versionSet[r.API.CRDVersion] = struct{}{}
+		}
+	}
 
-// IsWebhookVersionCompatible implements config.Config
-func (c cfg) IsWebhookVersionCompatible(webhookVersion string) bool {
-	return c.resourceAPIVersionCompatible("webhook", webhookVersion)
+	// Convert the map into a slice
+	versions := make([]string, 0, len(versionSet))
+	for version := range versionSet {
+		versions = append(versions, version)
+	}
+	return versions
 }
 
-func (c cfg) resourceAPIVersionCompatible(verType, version string) bool {
-	for _, res := range c.Resources {
-		var currVersion string
-		switch verType {
-		case "crd":
-			if res.API != nil {
-				currVersion = res.API.CRDVersion
-			}
-		case "webhook":
-			if res.Webhooks != nil {
-				currVersion = res.Webhooks.WebhookVersion
-			}
-		}
-		if currVersion != "" && version != currVersion {
-			return false
+// ListWebhookVersions implements config.Config
+func (c cfg) ListWebhookVersions() []string {
+	// Make a map to remove duplicates
+	versionSet := make(map[string]struct{})
+	for _, r := range c.Resources {
+		if r.Webhooks != nil && r.Webhooks.WebhookVersion != "" {
+			versionSet[r.Webhooks.WebhookVersion] = struct{}{}
 		}
 	}
 
-	return true
+	// Convert the map into a slice
+	versions := make([]string, 0, len(versionSet))
+	for version := range versionSet {
+		versions = append(versions, version)
+	}
+	return versions
 }
 
 // DecodePluginConfig implements config.Config

pkg/config/v3/config_test.go

@@ -18,6 +18,7 @@ package v3
 
 import (
 	"errors"
+	"sort"
 	"testing"
 
 	. "github.com/onsi/ginkgo"
@@ -302,42 +303,60 @@ var _ = Describe("cfg", func() {
 			Expect(c.HasGroup("other-group")).To(BeFalse())
 		})
 
-		It("IsCRDVersionCompatible should return true with no tracked resources", func() {
-			Expect(c.IsCRDVersionCompatible("v1beta1")).To(BeTrue())
-			Expect(c.IsCRDVersionCompatible("v1")).To(BeTrue())
+		It("ListCRDVersions should return an empty list with no tracked resources", func() {
+			Expect(c.ListCRDVersions()).To(BeEmpty())
 		})
 
-		It("IsCRDVersionCompatible should return true only for matching CRD versions of tracked resources", func() {
-			c.Resources = append(c.Resources, resource.Resource{
-				GVK: resource.GVK{
-					Group:   res.Group,
-					Version: res.Version,
-					Kind:    res.Kind,
+		It("ListCRDVersions should return a list of tracked resources CRD versions", func() {
+			c.Resources = append(c.Resources,
+				resource.Resource{
+					GVK: resource.GVK{
+						Group:   res.Group,
+						Version: res.Version,
+						Kind:    res.Kind,
+					},
+					API: &resource.API{CRDVersion: "v1beta1"},
+				},
+				resource.Resource{
+					GVK: resource.GVK{
+						Group:   res.Group,
+						Version: res.Version,
+						Kind:    "OtherKind",
+					},
+					API: &resource.API{CRDVersion: "v1"},
 				},
-				API: &resource.API{CRDVersion: "v1beta1"},
-			})
-			Expect(c.IsCRDVersionCompatible("v1beta1")).To(BeTrue())
-			Expect(c.IsCRDVersionCompatible("v1")).To(BeFalse())
-			Expect(c.IsCRDVersionCompatible("v2")).To(BeFalse())
+			)
+			versions := c.ListCRDVersions()
+			sort.Strings(versions) // ListCRDVersions has no order guarantee so sorting for reproducibility
+			Expect(versions).To(Equal([]string{"v1", "v1beta1"}))
 		})
 
-		It("IsWebhookVersionCompatible should return true with no tracked resources", func() {
-			Expect(c.IsWebhookVersionCompatible("v1beta1")).To(BeTrue())
-			Expect(c.IsWebhookVersionCompatible("v1")).To(BeTrue())
+		It("ListWebhookVersions should return an empty list with no tracked resources", func() {
+			Expect(c.ListWebhookVersions()).To(BeEmpty())
 		})
 
-		It("IsWebhookVersionCompatible should return true only for matching webhook versions of tracked resources", func() {
-			c.Resources = append(c.Resources, resource.Resource{
-				GVK: resource.GVK{
-					Group:   res.Group,
-					Version: res.Version,
-					Kind:    res.Kind,
+		It("ListWebhookVersions should return a list of tracked resources webhook versions", func() {
+			c.Resources = append(c.Resources,
+				resource.Resource{
+					GVK: resource.GVK{
+						Group:   res.Group,
+						Version: res.Version,
+						Kind:    res.Kind,
+					},
+					Webhooks: &resource.Webhooks{WebhookVersion: "v1beta1"},
+				},
+				resource.Resource{
+					GVK: resource.GVK{
+						Group:   res.Group,
+						Version: res.Version,
+						Kind:    "OtherKind",
+					},
+					Webhooks: &resource.Webhooks{WebhookVersion: "v1"},
 				},
-				Webhooks: &resource.Webhooks{WebhookVersion: "v1beta1"},
-			})
-			Expect(c.IsWebhookVersionCompatible("v1beta1")).To(BeTrue())
-			Expect(c.IsWebhookVersionCompatible("v1")).To(BeFalse())
-			Expect(c.IsWebhookVersionCompatible("v2")).To(BeFalse())
+			)
+			versions := c.ListWebhookVersions()
+			sort.Strings(versions) // ListWebhookVersions has no order guarantee so sorting for reproducibility
+			Expect(versions).To(Equal([]string{"v1", "v1beta1"}))
 		})
 	})
 

pkg/plugin/util/helpers.go

@@ -0,0 +1,35 @@
+/*
+Copyright 2021 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 util
+
+import (
+	"sigs.k8s.io/kubebuilder/v3/pkg/config"
+)
+
+// HasDifferentCRDVersion returns true if any other CRD version is tracked in the project configuration.
+func HasDifferentCRDVersion(config config.Config, crdVersion string) bool {
+	return hasDifferentAPIVersion(config.ListCRDVersions(), crdVersion)
+}
+
+// HasDifferentWebhookVersion returns true if any other webhook version is tracked in the project configuration.
+func HasDifferentWebhookVersion(config config.Config, webhookVersion string) bool {
+	return hasDifferentAPIVersion(config.ListWebhookVersions(), webhookVersion)
+}
+
+func hasDifferentAPIVersion(versions []string, version string) bool {
+	return !(len(versions) == 0 || (len(versions) == 1 && versions[0] == version))
+}

pkg/plugin/util/helpers_test.go

@@ -0,0 +1,45 @@
+/*
+Copyright 2021 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 util
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+)
+
+func TestPlugin(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Plugin Util Suite")
+}
+
+var _ = Describe("hasDifferentAPIVersion", func() {
+	DescribeTable("should return false",
+		func(versions []string) { Expect(hasDifferentAPIVersion(versions, "v1")).To(BeFalse()) },
+		Entry("for an empty list of versions", []string{}),
+		Entry("for a list of only that version", []string{"v1"}),
+	)
+
+	DescribeTable("should return true",
+		func(versions []string) { Expect(hasDifferentAPIVersion(versions, "v1")).To(BeTrue()) },
+		Entry("for a list of only a different version", []string{"v2"}),
+		Entry("for a list of several different versions", []string{"v2", "v3"}),
+		Entry("for a list of several versions containing that version", []string{"v1", "v2"}),
+	)
+})

pkg/plugins/golang/v3/api.go

@@ -31,6 +31,7 @@ import (
 	"sigs.k8s.io/kubebuilder/v3/pkg/model"
 	"sigs.k8s.io/kubebuilder/v3/pkg/model/resource"
 	"sigs.k8s.io/kubebuilder/v3/pkg/plugin"
+	pluginutil "sigs.k8s.io/kubebuilder/v3/pkg/plugin/util"
 	goPlugin "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang"
 	"sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v3/scaffolds"
 	"sigs.k8s.io/kubebuilder/v3/pkg/plugins/internal/cmdutil"
@@ -189,7 +190,7 @@ func (p *createAPISubcommand) Validate() error {
 		}
 
 		// Check CRDVersion against all other CRDVersions in p.config for compatibility.
-		if !p.config.IsCRDVersionCompatible(p.resource.API.CRDVersion) {
+		if pluginutil.HasDifferentCRDVersion(p.config, p.resource.API.CRDVersion) {
 			return fmt.Errorf("only one CRD version can be used for all resources, cannot add %q",
 				p.resource.API.CRDVersion)
 		}

pkg/plugins/golang/v3/webhook.go

@@ -26,6 +26,7 @@ import (
 	"sigs.k8s.io/kubebuilder/v3/pkg/config"
 	"sigs.k8s.io/kubebuilder/v3/pkg/model/resource"
 	"sigs.k8s.io/kubebuilder/v3/pkg/plugin"
+	pluginutil "sigs.k8s.io/kubebuilder/v3/pkg/plugin/util"
 	goPlugin "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang"
 	"sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v3/scaffolds"
 	"sigs.k8s.io/kubebuilder/v3/pkg/plugins/internal/cmdutil"
@@ -121,7 +122,7 @@ func (p *createWebhookSubcommand) Validate() error {
 		return fmt.Errorf("webhook resource already exists")
 	}
 
-	if !p.config.IsWebhookVersionCompatible(p.resource.Webhooks.WebhookVersion) {
+	if pluginutil.HasDifferentWebhookVersion(p.config, p.resource.Webhooks.WebhookVersion) {
 		return fmt.Errorf("only one webhook version can be used for all resources, cannot add %q",
 			p.resource.Webhooks.WebhookVersion)
 	}