From 1586c2d740b297576776fb4f390d7656689863b4 Mon Sep 17 00:00:00 2001 From: Adrian Orive Date: Fri, 26 Feb 2021 10:26:59 +0100 Subject: [PATCH] Generalize CRD and webhook version related methods in Config Signed-off-by: Adrian Orive --- pkg/config/interface.go | 8 ++-- pkg/config/v2/config.go | 12 ++--- pkg/config/v2/config_test.go | 18 ++------ pkg/config/v3/config.go | 51 ++++++++++++---------- pkg/config/v3/config_test.go | 75 ++++++++++++++++++++------------ pkg/plugin/util/helpers.go | 35 +++++++++++++++ pkg/plugin/util/helpers_test.go | 45 +++++++++++++++++++ pkg/plugins/golang/v3/api.go | 3 +- pkg/plugins/golang/v3/webhook.go | 3 +- 9 files changed, 173 insertions(+), 77 deletions(-) create mode 100644 pkg/plugin/util/helpers.go create mode 100644 pkg/plugin/util/helpers_test.go diff --git a/pkg/config/interface.go b/pkg/config/interface.go index 8621d5f8afd..e6aacf38ec0 100644 --- a/pkg/config/interface.go +++ b/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 */ diff --git a/pkg/config/v2/config.go b/pkg/config/v2/config.go index b9ee4b57f5d..8c2326f0825 100644 --- a/pkg/config/v2/config.go +++ b/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 diff --git a/pkg/config/v2/config_test.go b/pkg/config/v2/config_test.go index 8c5359550dc..4773b0e9a3f 100644 --- a/pkg/config/v2/config_test.go +++ b/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()) }) }) diff --git a/pkg/config/v3/config.go b/pkg/config/v3/config.go index 144e9580d96..a462bbd5249 100644 --- a/pkg/config/v3/config.go +++ b/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 diff --git a/pkg/config/v3/config_test.go b/pkg/config/v3/config_test.go index cdd49628846..cbcb5890f99 100644 --- a/pkg/config/v3/config_test.go +++ b/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"})) }) }) diff --git a/pkg/plugin/util/helpers.go b/pkg/plugin/util/helpers.go new file mode 100644 index 00000000000..2bd5a2de52e --- /dev/null +++ b/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)) +} diff --git a/pkg/plugin/util/helpers_test.go b/pkg/plugin/util/helpers_test.go new file mode 100644 index 00000000000..342d0f07f9d --- /dev/null +++ b/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"}), + ) +}) diff --git a/pkg/plugins/golang/v3/api.go b/pkg/plugins/golang/v3/api.go index f71bc7e5e4f..8abcbd9d1b1 100644 --- a/pkg/plugins/golang/v3/api.go +++ b/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) } diff --git a/pkg/plugins/golang/v3/webhook.go b/pkg/plugins/golang/v3/webhook.go index 7c81b017fae..6e12fa4c4be 100644 --- a/pkg/plugins/golang/v3/webhook.go +++ b/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) }