From 7b2a3090681656bc4e71cb9e5058104b4f924123 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Fri, 29 May 2020 18:29:31 -0700 Subject: [PATCH] Multiple plugins with the same name are now allowed by resolvePluginsByKey to permit a CLI to understand multiple plugin versions. For example, a config with "layout: go/v2.0" and another config with "layout: go/v2.1" should both be understood by a particular CLI. pkg/cli: remove validations requiring plugin names be unique within a CLI, and resolve all matching plugins such that they can contain multiple plugins with the same name. As long as there is only one plugin type, ex. Init, per set of resolved plugins, no errors will occur. --- pkg/cli/cli.go | 73 +---------------------- pkg/cli/cli_test.go | 107 ---------------------------------- pkg/cli/version.go | 116 ++++++++++++++++++++++++++++++++++++ pkg/cli/version_test.go | 126 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 244 insertions(+), 178 deletions(-) delete mode 100644 pkg/cli/cli_test.go create mode 100644 pkg/cli/version.go create mode 100644 pkg/cli/version_test.go diff --git a/pkg/cli/cli.go b/pkg/cli/cli.go index 1d9dacfd567..228b3b91e54 100644 --- a/pkg/cli/cli.go +++ b/pkg/cli/cli.go @@ -131,9 +131,6 @@ func WithPlugins(plugins ...plugin.Base) Option { return func(c *cli) error { for _, p := range plugins { for _, version := range p.SupportedProjectVersions() { - if _, ok := c.pluginsFromOptions[version]; !ok { - c.pluginsFromOptions[version] = []plugin.Base{} - } c.pluginsFromOptions[version] = append(c.pluginsFromOptions[version], p) } } @@ -151,9 +148,6 @@ func WithDefaultPlugins(plugins ...plugin.Base) Option { return func(c *cli) error { for _, p := range plugins { for _, version := range p.SupportedProjectVersions() { - if _, ok := c.defaultPluginsFromOptions[version]; !ok { - c.defaultPluginsFromOptions[version] = []plugin.Base{} - } c.defaultPluginsFromOptions[version] = append(c.defaultPluginsFromOptions[version], p) } } @@ -196,8 +190,8 @@ func (c *cli) initialize() error { c.projectVersion = projectConfig.Version if projectConfig.IsV1() { - return fmt.Errorf(noticeColor, "The v1 projects are no longer supported.\n"+ - "See how to upgrade your project to v2: https://book.kubebuilder.io/migration/guide.html\n") + return fmt.Errorf(noticeColor, "project version 1 is no longer supported.\n"+ + "See how to upgrade your project: https://book.kubebuilder.io/migration/guide.html\n") } } else { return fmt.Errorf("failed to read config: %v", err) @@ -334,7 +328,6 @@ func (c cli) validate() error { // validatePlugins validates the name and versions of a list of plugins. func validatePlugins(plugins ...plugin.Base) error { - pluginNameSet := make(map[string]struct{}, len(plugins)) for _, p := range plugins { pluginName := p.Name() if err := plugin.ValidateName(pluginName); err != nil { @@ -351,12 +344,6 @@ func validatePlugins(plugins ...plugin.Base) error { pluginName, projectVersion, err) } } - // Check for duplicate plugin names. Names outside of a version can - // conflict because multiple project versions of a plugin may exist. - if _, seen := pluginNameSet[pluginName]; seen { - return fmt.Errorf("two plugins have the same name: %q", pluginName) - } - pluginNameSet[pluginName] = struct{}{} } return nil } @@ -389,62 +376,6 @@ func (c cli) buildRootCmd() *cobra.Command { return rootCmd } -// resolvePluginsByKey finds a plugin for pluginKey if it exactly matches -// some form of a known plugin's key. Those forms can be a: -// - Fully qualified key: "go.kubebuilder.io/v2.0.0" -// - Short key: "go/v2.0.0" -// - Fully qualified name: "go.kubebuilder.io" -// - Short name: "go" -// Some of these keys may conflict, ex. the fully-qualified and short names of -// "go.kubebuilder.io/v1.0.0" and "go.kubebuilder.io/v2.0.0" have ambiguous -// unversioned names "go.kubernetes.io" and "go". If pluginKey is ambiguous -// or does not match any known plugin's key, an error is returned. -// -// Note: resolvePluginsByKey returns a slice so initialize() can generalize -// setting default plugins if no pluginKey is set. -func resolvePluginsByKey(versionedPlugins []plugin.Base, pluginKey string) ([]plugin.Base, error) { - // Make a set of all possible key combinations to check pluginKey against. - // If the key is not ambiguous, set a valid pointer to the plugin for that - // key, otherwise set a tombstone so we know it is ambiguous. There will - // always be at least one key per plugin if their names are fully-qualified. - // - // Note: this isn't actually that inefficient compared to a memory-efficient - // solution since we're working with very small N's; it is also very simple. - allPluginKeyCombos := make(map[string]*plugin.Base) - for i, p := range versionedPlugins { - key := plugin.KeyFor(p) - // Short-circuit if we have an exact match. - if key == pluginKey { - return []plugin.Base{p}, nil - } - name := p.Name() - keys := []string{key, name} - if shortName := plugin.GetShortName(name); name != shortName { - keys = append(keys, shortName) - keys = append(keys, plugin.Key(shortName, p.Version())) - } - - pp := &versionedPlugins[i] - for _, k := range keys { - if _, hasKey := allPluginKeyCombos[k]; hasKey { - allPluginKeyCombos[k] = nil - } else { - allPluginKeyCombos[k] = pp - } - } - } - - pp, hasKey := allPluginKeyCombos[pluginKey] - if !hasKey { - return nil, fmt.Errorf("plugin key %q does not match a known plugin", pluginKey) - } - if pp == nil { - return nil, fmt.Errorf("plugin key %q matches more than one known plugin", pluginKey) - } - - return []plugin.Base{*pp}, nil -} - // defaultCommand returns the root command without its subcommands. func (c cli) defaultCommand() *cobra.Command { return &cobra.Command{ diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go deleted file mode 100644 index ef4fd86b923..00000000000 --- a/pkg/cli/cli_test.go +++ /dev/null @@ -1,107 +0,0 @@ -/* -Copyright 2020 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 cli - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "sigs.k8s.io/kubebuilder/pkg/plugin" -) - -var _ = Describe("CLI", func() { - Describe("resolvePluginsByKey", func() { - plugins := makePluginsForKeys( - "foo.example.com/v1.0.0", - "bar.example.com/v1.0.0", - "baz.example.com/v1.0.0", - "foo.kubebuilder.io/v1.0.0", - "foo.kubebuilder.io/v2.0.0", - "bar.kubebuilder.io/v1.0.0", - "bar.kubebuilder.io/v2.0.0", - ) - - It("should check key correctly", func() { - - By("Resolving foo.example.com/v1.0.0") - resolvedPlugins, err := resolvePluginsByKey(plugins, "foo.example.com/v1.0.0") - Expect(err).NotTo(HaveOccurred()) - Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{"foo.example.com/v1.0.0"})) - - By("Resolving foo.example.com") - resolvedPlugins, err = resolvePluginsByKey(plugins, "foo.example.com") - Expect(err).NotTo(HaveOccurred()) - Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{"foo.example.com/v1.0.0"})) - - By("Resolving baz") - resolvedPlugins, err = resolvePluginsByKey(plugins, "baz") - Expect(err).NotTo(HaveOccurred()) - Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{"baz.example.com/v1.0.0"})) - - By("Resolving foo/v2.0.0") - resolvedPlugins, err = resolvePluginsByKey(plugins, "foo/v2.0.0") - Expect(err).NotTo(HaveOccurred()) - Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{"foo.kubebuilder.io/v2.0.0"})) - - By("Resolving blah") - _, err = resolvePluginsByKey(plugins, "blah") - Expect(err).To(MatchError(`plugin key "blah" does not match a known plugin`)) - - By("Resolving foo.example.com/v2.0.0") - _, err = resolvePluginsByKey(plugins, "foo.example.com/v2.0.0") - Expect(err).To(MatchError(`plugin key "foo.example.com/v2.0.0" does not match a known plugin`)) - - By("Resolving foo.kubebuilder.io") - _, err = resolvePluginsByKey(plugins, "foo.kubebuilder.io") - Expect(err).To(MatchError(`plugin key "foo.kubebuilder.io" matches more than one known plugin`)) - - By("Resolving foo/v1.0.0") - _, err = resolvePluginsByKey(plugins, "foo/v1.0.0") - Expect(err).To(MatchError(`plugin key "foo/v1.0.0" matches more than one known plugin`)) - - By("Resolving foo") - _, err = resolvePluginsByKey(plugins, "foo") - Expect(err).To(MatchError(`plugin key "foo" matches more than one known plugin`)) - }) - }) -}) - -type mockPlugin struct { - name, version string -} - -func (p mockPlugin) Name() string { return p.name } -func (p mockPlugin) Version() string { return p.version } -func (mockPlugin) SupportedProjectVersions() []string { return []string{"2"} } - -func makeBasePlugin(name, version string) plugin.Base { - return mockPlugin{name, version} -} - -func makePluginsForKeys(keys ...string) (plugins []plugin.Base) { - for _, key := range keys { - plugins = append(plugins, makeBasePlugin(plugin.SplitKey(key))) - } - return -} - -func getPluginKeys(plugins ...plugin.Base) (keys []string) { - for _, p := range plugins { - keys = append(keys, plugin.KeyFor(p)) - } - return -} diff --git a/pkg/cli/version.go b/pkg/cli/version.go new file mode 100644 index 00000000000..1d43a7cf915 --- /dev/null +++ b/pkg/cli/version.go @@ -0,0 +1,116 @@ +/* +Copyright 2020 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 cli + +import ( + "fmt" + + "github.com/blang/semver" + + "sigs.k8s.io/kubebuilder/pkg/plugin" +) + +// resolvePluginsByKey finds a plugin for pluginKey if it exactly matches +// some form of a known plugin's key. Those forms can be a: +// - Fully qualified key: "go.kubebuilder.io/v2.0.0" +// - Short key: "go/v2.0.0" +// - Fully qualified name: "go.kubebuilder.io" +// - Short name: "go" +// Some of these keys may conflict, ex. the fully-qualified and short names of +// "go.kubebuilder.io/v1.0.0" and "go.kubebuilder.io/v2.0.0" have ambiguous +// unversioned names "go.kubernetes.io" and "go". If pluginKey is ambiguous +// or does not match any known plugin's key, an error is returned. +// +// Note: resolvePluginsByKey returns a slice so initialize() can generalize +// setting default plugins if no pluginKey is set. +func resolvePluginsByKey(versionedPlugins []plugin.Base, pluginKey string) (resolved []plugin.Base, err error) { + name, version := plugin.SplitKey(pluginKey) + shortName := plugin.GetShortName(name) + + if version == "" { + // Case: if plugin key has no version, check all plugin names. + resolved = versionedPlugins + } else { + // Case: if plugin key has version, filter by version. + resolved = findPluginsMatchingVersion(versionedPlugins, version) + } + + if len(resolved) == 0 { + return nil, fmt.Errorf("ambiguous plugin version %q: no versions match", version) + } + + if name == shortName { + // Case: if plugin name is short, find matching short names. + resolved = findPluginsMatchingShortName(resolved, shortName) + } else { + // Case: if plugin name is fully-qualified, match only fully-qualified names. + resolved = findPluginsMatchingName(resolved, name) + } + + if len(resolved) == 0 { + return nil, fmt.Errorf("ambiguous plugin name %q: no names match", name) + } + + return resolved, nil +} + +func findPluginsMatchingName(ps []plugin.Base, name string) (equal []plugin.Base) { + for _, p := range ps { + if p.Name() == name { + equal = append(equal, p) + } + } + return equal +} + +func findPluginsMatchingShortName(ps []plugin.Base, shortName string) (equal []plugin.Base) { + for _, p := range ps { + if plugin.GetShortName(p.Name()) == shortName { + equal = append(equal, p) + } + } + return equal +} + +func findPluginsMatchingVersion(ps []plugin.Base, version string) []plugin.Base { + // Assume versions have been validated already. + sv := must(semver.ParseTolerant(version)) + + var equal, matchingMajor []plugin.Base + for _, p := range ps { + pv := must(semver.ParseTolerant(p.Version())) + if sv.Major == pv.Major { + if sv.Minor == pv.Minor { + equal = append(equal, p) + } else { + matchingMajor = append(matchingMajor, p) + } + } + } + + if len(equal) != 0 { + return equal + } + return matchingMajor +} + +func must(v semver.Version, err error) semver.Version { + if err != nil { + panic(err) + } + return v +} diff --git a/pkg/cli/version_test.go b/pkg/cli/version_test.go new file mode 100644 index 00000000000..90a14a4714f --- /dev/null +++ b/pkg/cli/version_test.go @@ -0,0 +1,126 @@ +/* +Copyright 2020 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 cli + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "sigs.k8s.io/kubebuilder/pkg/plugin" +) + +var _ = Describe("resolvePluginsByKey", func() { + plugins := makePluginsForKeys( + "foo.example.com/v1.0", + "bar.example.com/v1.0", + "baz.example.com/v1.0", + "foo.kubebuilder.io/v1.0", + "foo.kubebuilder.io/v2.0", + "bar.kubebuilder.io/v1.0", + "bar.kubebuilder.io/v2.0", + ) + + It("should check key correctly", func() { + + By("Resolving foo.example.com/v1.0") + resolvedPlugins, err := resolvePluginsByKey(plugins, "foo.example.com/v1.0") + Expect(err).NotTo(HaveOccurred()) + Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{"foo.example.com/v1.0"})) + + By("Resolving foo.example.com") + resolvedPlugins, err = resolvePluginsByKey(plugins, "foo.example.com") + Expect(err).NotTo(HaveOccurred()) + Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{"foo.example.com/v1.0"})) + + By("Resolving baz") + resolvedPlugins, err = resolvePluginsByKey(plugins, "baz") + Expect(err).NotTo(HaveOccurred()) + Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{"baz.example.com/v1.0"})) + + By("Resolving foo/v2.0") + resolvedPlugins, err = resolvePluginsByKey(plugins, "foo/v2.0") + Expect(err).NotTo(HaveOccurred()) + Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{"foo.kubebuilder.io/v2.0"})) + + By("Resolving foo.kubebuilder.io") + resolvedPlugins, err = resolvePluginsByKey(plugins, "foo.kubebuilder.io") + Expect(err).NotTo(HaveOccurred()) + Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{ + "foo.kubebuilder.io/v1.0", + "foo.kubebuilder.io/v2.0", + })) + + By("Resolving foo/v1.0") + resolvedPlugins, err = resolvePluginsByKey(plugins, "foo/v1.0") + Expect(err).NotTo(HaveOccurred()) + Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{ + "foo.example.com/v1.0", + "foo.kubebuilder.io/v1.0", + })) + + By("Resolving foo") + resolvedPlugins, err = resolvePluginsByKey(plugins, "foo") + Expect(err).NotTo(HaveOccurred()) + Expect(getPluginKeys(resolvedPlugins...)).To(Equal([]string{ + "foo.example.com/v1.0", + "foo.kubebuilder.io/v1.0", + "foo.kubebuilder.io/v2.0", + })) + + By("Resolving blah") + _, err = resolvePluginsByKey(plugins, "blah") + Expect(err).To(MatchError(`ambiguous plugin name "blah": no names match`)) + + By("Resolving foo.example.com/v2.0") + _, err = resolvePluginsByKey(plugins, "foo.example.com/v2.0") + Expect(err).To(MatchError(`ambiguous plugin name "foo.example.com": no names match`)) + + By("Resolving foo/v3.0") + _, err = resolvePluginsByKey(plugins, "foo/v3.0") + Expect(err).To(MatchError(`ambiguous plugin version "v3.0": no versions match`)) + + By("Resolving foo.example.com/v3.0") + _, err = resolvePluginsByKey(plugins, "foo.example.com/v3.0") + Expect(err).To(MatchError(`ambiguous plugin version "v3.0": no versions match`)) + }) +}) + +type mockPlugin struct { + name, version string +} + +func (p mockPlugin) Name() string { return p.name } +func (p mockPlugin) Version() string { return p.version } +func (mockPlugin) SupportedProjectVersions() []string { return []string{"2"} } + +func makeBasePlugin(name, version string) plugin.Base { + return mockPlugin{name, version} +} + +func makePluginsForKeys(keys ...string) (plugins []plugin.Base) { + for _, key := range keys { + plugins = append(plugins, makeBasePlugin(plugin.SplitKey(key))) + } + return +} + +func getPluginKeys(plugins ...plugin.Base) (keys []string) { + for _, p := range plugins { + keys = append(keys, plugin.KeyFor(p)) + } + return +}