From 4f890fc2dd6c610ffbd456d6b6c0781418daf129 Mon Sep 17 00:00:00 2001 From: Eric Stroczynski Date: Wed, 3 Jun 2020 12:09:11 -0700 Subject: [PATCH] Plugin versions are now considered single numbers with optional "v" prefix and stability "alpha" or "beta" suffix, ex. "v2-alpha". Minor and patch version changes are poorly defined and may constitute breaking changes; having a single version number makes versioning simpler and permits breaking changes to be grouped into one "major" release for a plugin. *: remove references to minor and patch versions for plugins pkg/plugin: implement a Version type returned by Base.Version() that is easily parsed and validated pkg/cli: simplify plugin resolver given the minor/patch number drop --- pkg/cli/cli.go | 44 +------ pkg/cli/cli_suite_test.go | 11 +- pkg/cli/cli_test.go | 15 ++- pkg/cli/plugins.go | 173 ++++++++++--------------- pkg/cli/plugins_test.go | 71 +++++----- pkg/plugin/interfaces.go | 97 ++++++++++++++ pkg/plugin/plugin.go | 169 +++++++++++++----------- pkg/plugin/plugin_test.go | 156 ++++++++++++++++++++++ pkg/plugin/v2/plugin.go | 12 +- pkg/plugin/validation.go | 48 ------- testdata/project-v3-addon/PROJECT | 2 +- testdata/project-v3-multigroup/PROJECT | 2 +- testdata/project-v3/PROJECT | 2 +- 13 files changed, 481 insertions(+), 321 deletions(-) create mode 100644 pkg/plugin/interfaces.go create mode 100644 pkg/plugin/plugin_test.go delete mode 100644 pkg/plugin/validation.go diff --git a/pkg/cli/cli.go b/pkg/cli/cli.go index c273bb213bf..ad6ffaeb62c 100644 --- a/pkg/cli/cli.go +++ b/pkg/cli/cli.go @@ -213,7 +213,7 @@ func (c *cli) initialize() error { // In case 1, default plugins will be used to determine which plugin to use. // In case 2, the value passed to --plugins is used. // For all other commands, a config's 'layout' key is used. Since both - // layout and --plugins values can be short (ex. "go/v2.0.0") or unversioned + // layout and --plugins values can be short (ex. "go/v2") or unversioned // (ex. "go.kubebuilder.io") keys or both, their values may need to be // resolved to known plugins by key. // Default plugins are checked first so any input key that has more than one @@ -323,9 +323,8 @@ func (c cli) validate() error { } // CLI-set plugins do not have to contain a version. if pluginVersion != "" { - if err := plugin.ValidateVersion(pluginVersion); err != nil { - return fmt.Errorf("invalid plugin %q version %q: %v", - pluginName, pluginVersion, err) + if _, err := plugin.ParseVersion(pluginVersion); err != nil { + return fmt.Errorf("invalid plugin version %q: %v", pluginVersion, err) } } } @@ -333,43 +332,6 @@ func (c cli) validate() error { return nil } -// validatePlugins validates the name and versions of a list of plugins. -func validatePlugins(plugins ...plugin.Base) error { - pluginKeySet := make(map[string]struct{}, len(plugins)) - for _, p := range plugins { - if err := validatePlugin(p); err != nil { - return err - } - // Check for duplicate plugin keys. - pluginKey := plugin.KeyFor(p) - if _, seen := pluginKeySet[pluginKey]; seen { - return fmt.Errorf("two plugins have the same key: %q", pluginKey) - } - pluginKeySet[pluginKey] = struct{}{} - } - return nil -} - -// validatePlugin validates the name and versions of a plugin. -func validatePlugin(p plugin.Base) error { - pluginName := p.Name() - if err := plugin.ValidateName(pluginName); err != nil { - return fmt.Errorf("invalid plugin name %q: %v", pluginName, err) - } - pluginVersion := p.Version() - if err := plugin.ValidateVersion(pluginVersion); err != nil { - return fmt.Errorf("invalid plugin %q version %q: %v", - pluginName, pluginVersion, err) - } - for _, projectVersion := range p.SupportedProjectVersions() { - if err := validation.ValidateProjectVersion(projectVersion); err != nil { - return fmt.Errorf("invalid plugin %q supported project version %q: %v", - pluginName, projectVersion, err) - } - } - return nil -} - // buildRootCmd returns a root command with a subcommand tree reflecting the // current project's state. func (c cli) buildRootCmd() *cobra.Command { diff --git a/pkg/cli/cli_suite_test.go b/pkg/cli/cli_suite_test.go index 51505d85e44..0e520dcc15d 100644 --- a/pkg/cli/cli_suite_test.go +++ b/pkg/cli/cli_suite_test.go @@ -36,12 +36,13 @@ func TestCLI(t *testing.T) { // Test plugin types and constructors. type mockPlugin struct { - name, version string + name string + version plugin.Version projectVersions []string } func (p mockPlugin) Name() string { return p.name } -func (p mockPlugin) Version() string { return p.version } +func (p mockPlugin) Version() plugin.Version { return p.version } func (p mockPlugin) SupportedProjectVersions() []string { return p.projectVersions } func (mockPlugin) UpdateContext(*plugin.Context) {} @@ -50,7 +51,11 @@ func (mockPlugin) InjectConfig(*config.Config) {} func (mockPlugin) Run() error { return nil } func makeBasePlugin(name, version string, projVers ...string) plugin.Base { - return mockPlugin{name, version, projVers} + v, err := plugin.ParseVersion(version) + if err != nil { + panic(err) + } + return mockPlugin{name, v, projVers} } func makePluginsForKeys(keys ...string) (plugins []plugin.Base) { diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go index 67e274f86b5..0a39bd52e61 100644 --- a/pkg/cli/cli_test.go +++ b/pkg/cli/cli_test.go @@ -34,10 +34,10 @@ var _ = Describe("CLI", func() { pluginNameA = "go.example.com" pluginNameB = "go.test.com" projectVersions = []string{config.Version2, config.Version3Alpha} - pluginAV1 = makeAllPlugin(pluginNameA, "v1.0", projectVersions...) - pluginAV2 = makeAllPlugin(pluginNameA, "v2.0", projectVersions...) - pluginBV1 = makeAllPlugin(pluginNameB, "v1.0", projectVersions...) - pluginBV2 = makeAllPlugin(pluginNameB, "v2.0", projectVersions...) + pluginAV1 = makeAllPlugin(pluginNameA, "v1", projectVersions...) + pluginAV2 = makeAllPlugin(pluginNameA, "v2", projectVersions...) + pluginBV1 = makeAllPlugin(pluginNameB, "v1", projectVersions...) + pluginBV2 = makeAllPlugin(pluginNameB, "v2", projectVersions...) allPlugins = []plugin.Base{pluginAV1, pluginAV2, pluginBV1, pluginBV2} ) @@ -89,7 +89,7 @@ var _ = Describe("CLI", func() { By("setting two plugins of the same name and version") _, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginAV1)) - Expect(err).To(MatchError(`broken pre-set plugins: two plugins have the same key: "go.example.com/v1.0"`)) + Expect(err).To(MatchError(`broken pre-set plugins: two plugins have the same key: "go.example.com/v1"`)) }) }) @@ -145,7 +145,10 @@ var _ = Describe("CLI", func() { By(`setting cliPluginKey to an non-existent key "foo"`) setPluginsFlag("foo") _, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginAV2)) - Expect(err).To(MatchError(errAmbiguousPlugin{"foo", "no names match"})) + Expect(err).To(MatchError(errAmbiguousPlugin{ + key: "foo", + msg: `no names match, possible plugins: ["go.example.com/v1" "go.example.com/v2"]`, + })) }) }) diff --git a/pkg/cli/plugins.go b/pkg/cli/plugins.go index f39640676e6..05c96870509 100644 --- a/pkg/cli/plugins.go +++ b/pkg/cli/plugins.go @@ -20,8 +20,7 @@ import ( "fmt" "sort" - "github.com/blang/semver" - + "sigs.k8s.io/kubebuilder/pkg/internal/validation" "sigs.k8s.io/kubebuilder/pkg/plugin" ) @@ -37,12 +36,12 @@ func (e errAmbiguousPlugin) Error() string { // resolvePluginsByKey resolves versionedPlugins to a subset of plugins by // matching keys to some form of pluginKey. Those forms can be a: -// - Fully qualified key: "go.kubebuilder.io/v2.0" -// - Short key: "go/v2.0" +// - Fully qualified key: "go.kubebuilder.io/v2" +// - Short key: "go/v2" // - 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" and "go.kubebuilder.io/v2.0" have ambiguous +// "go.kubebuilder.io/v1" and "go.kubebuilder.io/v2" 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. // @@ -52,82 +51,53 @@ func resolvePluginsByKey(versionedPlugins []plugin.Base, pluginKey string) (reso name, version := plugin.SplitKey(pluginKey) - // Compare versions first to narrow the list of name comparisons. - 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, errAmbiguousPlugin{pluginKey, "no versions match"} - } - // Compare names, taking into account whether name is fully-qualified or not. shortName := plugin.GetShortName(name) if name == shortName { // Case: if plugin name is short, find matching short names. - resolved = findPluginsMatchingShortName(resolved, shortName) + resolved = findPluginsMatchingShortName(versionedPlugins, shortName) } else { // Case: if plugin name is fully-qualified, match only fully-qualified names. - resolved = findPluginsMatchingName(resolved, name) + resolved = findPluginsMatchingName(versionedPlugins, name) } if len(resolved) == 0 { - return nil, errAmbiguousPlugin{pluginKey, "no names match"} - } - - // Since plugins has already been resolved by matching names and versions, - // it should only contain one matching value for a versionless pluginKey if - // it isn't ambiguous. - if version == "" { - if len(resolved) == 1 { - return resolved, nil + return nil, errAmbiguousPlugin{ + key: pluginKey, + msg: fmt.Sprintf("no names match, possible plugins: %+q", makePluginKeySlice(versionedPlugins...)), } - return nil, errAmbiguousPlugin{pluginKey, fmt.Sprintf("possible keys: %+q", makePluginKeySlice(resolved...))} - } - - rp, err := resolveToPlugin(resolved) - if err != nil { - return nil, errAmbiguousPlugin{pluginKey, err.Error()} } - return []plugin.Base{rp}, nil -} - -// findPluginsMatchingVersion returns a set of plugins with Version() matching -// version. The set will contain plugins with either major and minor versions -// matching exactly or major versions matching exactly and greater minor versions, -// but not a mix of the two match types. -func findPluginsMatchingVersion(plugins []plugin.Base, version string) []plugin.Base { - // Assume versions have been validated already. - v := must(semver.ParseTolerant(version)) - var equal, matchingMajor []plugin.Base - for _, p := range plugins { - pv := must(semver.ParseTolerant(p.Version())) - if v.Major == pv.Major { - if v.Minor == pv.Minor { - equal = append(equal, p) - } else if v.Minor < pv.Minor { - matchingMajor = append(matchingMajor, p) + if version != "" { + // Case: if plugin key has version, filter by version. + v, err := plugin.ParseVersion(version) + if err != nil { + return nil, err + } + keys := makePluginKeySlice(resolved...) + for i := 0; i < len(resolved); i++ { + if v.Compare(resolved[i].Version()) != 0 { + resolved = append(resolved[:i], resolved[i+1:]...) + i-- + } + } + if len(resolved) == 0 { + return nil, errAmbiguousPlugin{ + key: pluginKey, + msg: fmt.Sprintf("no versions match, possible plugins: %+q", keys), } } } - if len(equal) != 0 { - return equal - } - return matchingMajor -} - -// must wraps semver.Parse and panics if err is non-nil. -func must(v semver.Version, err error) semver.Version { - if err != nil { - panic(err) + // Since plugins has already been resolved by matching names and versions, + // it should only contain one matching value if it isn't ambiguous. + if len(resolved) != 1 { + return nil, errAmbiguousPlugin{ + key: pluginKey, + msg: fmt.Sprintf("matching plugins: %+q", makePluginKeySlice(resolved...)), + } } - return v + return resolved, nil } // findPluginsMatchingName returns a set of plugins with Name() exactly @@ -152,56 +122,45 @@ func findPluginsMatchingShortName(plugins []plugin.Base, shortName string) (equa return equal } -// resolveToPlugin returns a single plugin from plugins given the following -// conditions about plugins: -// 1. len(plugins) > 0. -// 2. No two plugin names are different. -// An error is returned if either condition is invalidated. -func resolveToPlugin(plugins []plugin.Base) (rp plugin.Base, err error) { - // Versions are either an exact match or have greater minor versions, so - // we choose the last in a sorted list of versions to get the correct one. - versions := make([]semver.Version, len(plugins)) - for i, p := range plugins { - versions[i] = must(semver.ParseTolerant(p.Version())) - } - if len(versions) == 0 { - return nil, fmt.Errorf("possible versions: %+q", versions) +// makePluginKeySlice returns a slice of all keys for each plugin in plugins. +func makePluginKeySlice(plugins ...plugin.Base) (keys []string) { + for _, p := range plugins { + keys = append(keys, plugin.KeyFor(p)) } - semver.Sort(versions) - useVersion := versions[len(versions)-1] + sort.Strings(keys) + return +} - // If more than one name in plugins exists, the name portion of pluginKey - // needs to be more specific. - nameSet := make(map[string]struct{}) +// validatePlugins validates the name and versions of a list of plugins. +func validatePlugins(plugins ...plugin.Base) error { + pluginKeySet := make(map[string]struct{}, len(plugins)) for _, p := range plugins { - nameSet[p.Name()] = struct{}{} - // This condition will only be true once for an unambiguous plugin name, - // since plugin keys have been checked for duplicates already. - if must(semver.ParseTolerant(p.Version())).Equals(useVersion) { - rp = p + if err := validatePlugin(p); err != nil { + return err } + // Check for duplicate plugin keys. + pluginKey := plugin.KeyFor(p) + if _, seen := pluginKeySet[pluginKey]; seen { + return fmt.Errorf("two plugins have the same key: %q", pluginKey) + } + pluginKeySet[pluginKey] = struct{}{} } - if len(nameSet) != 1 { - return nil, fmt.Errorf("possible names: %+q", makeKeySlice(nameSet)) - } - - return rp, nil + return nil } -// makeKeySlice returns a slice of all map keys in set. -func makeKeySlice(set map[string]struct{}) (keys []string) { - for key := range set { - keys = append(keys, key) +// validatePlugin validates the name and versions of a plugin. +func validatePlugin(p plugin.Base) error { + pluginName := p.Name() + if err := plugin.ValidateName(pluginName); err != nil { + return fmt.Errorf("invalid plugin name %q: %v", pluginName, err) } - sort.Strings(keys) - return -} - -// makePluginKeySlice returns a slice of all keys for each plugin in plugins. -func makePluginKeySlice(plugins ...plugin.Base) (keys []string) { - for _, p := range plugins { - keys = append(keys, plugin.KeyFor(p)) + if err := p.Version().Validate(); err != nil { + return fmt.Errorf("invalid plugin version %q: %v", p.Version(), err) } - sort.Strings(keys) - return + for _, projectVersion := range p.SupportedProjectVersions() { + if err := validation.ValidateProjectVersion(projectVersion); err != nil { + return fmt.Errorf("invalid project version %q: %v", projectVersion, err) + } + } + return nil } diff --git a/pkg/cli/plugins_test.go b/pkg/cli/plugins_test.go index efbb895cb45..21d7e9e81dd 100644 --- a/pkg/cli/plugins_test.go +++ b/pkg/cli/plugins_test.go @@ -17,6 +17,8 @@ limitations under the License. package cli import ( + "fmt" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -27,38 +29,38 @@ var _ = Describe("resolvePluginsByKey", func() { var ( 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", + "foo.example.com/v1", + "bar.example.com/v1", + "baz.example.com/v1", + "foo.kubebuilder.io/v1", + "foo.kubebuilder.io/v2", + "bar.kubebuilder.io/v1", + "bar.kubebuilder.io/v2", ) resolvedPlugins []plugin.Base err error ) It("should resolve keys correctly", func() { - By("resolving foo.example.com/v1.0") - resolvedPlugins, err = resolvePluginsByKey(plugins, "foo.example.com/v1.0") + By("resolving foo.example.com/v1") + resolvedPlugins, err = resolvePluginsByKey(plugins, "foo.example.com/v1") Expect(err).NotTo(HaveOccurred()) - Expect(makePluginKeySlice(resolvedPlugins...)).To(Equal([]string{"foo.example.com/v1.0"})) + Expect(makePluginKeySlice(resolvedPlugins...)).To(Equal([]string{"foo.example.com/v1"})) By("resolving foo.example.com") resolvedPlugins, err = resolvePluginsByKey(plugins, "foo.example.com") Expect(err).NotTo(HaveOccurred()) - Expect(makePluginKeySlice(resolvedPlugins...)).To(Equal([]string{"foo.example.com/v1.0"})) + Expect(makePluginKeySlice(resolvedPlugins...)).To(Equal([]string{"foo.example.com/v1"})) By("resolving baz") resolvedPlugins, err = resolvePluginsByKey(plugins, "baz") Expect(err).NotTo(HaveOccurred()) - Expect(makePluginKeySlice(resolvedPlugins...)).To(Equal([]string{"baz.example.com/v1.0"})) + Expect(makePluginKeySlice(resolvedPlugins...)).To(Equal([]string{"baz.example.com/v1"})) - By("resolving foo/v2.0") - resolvedPlugins, err = resolvePluginsByKey(plugins, "foo/v2.0") + By("resolving foo/v2") + resolvedPlugins, err = resolvePluginsByKey(plugins, "foo/v2") Expect(err).NotTo(HaveOccurred()) - Expect(makePluginKeySlice(resolvedPlugins...)).To(Equal([]string{"foo.kubebuilder.io/v2.0"})) + Expect(makePluginKeySlice(resolvedPlugins...)).To(Equal([]string{"foo.kubebuilder.io/v2"})) }) It("should return an error", func() { @@ -66,49 +68,50 @@ var _ = Describe("resolvePluginsByKey", func() { _, err = resolvePluginsByKey(plugins, "foo.kubebuilder.io") Expect(err).To(MatchError(errAmbiguousPlugin{ key: "foo.kubebuilder.io", - msg: `possible keys: ["foo.kubebuilder.io/v1.0" "foo.kubebuilder.io/v2.0"]`, + msg: `matching plugins: ["foo.kubebuilder.io/v1" "foo.kubebuilder.io/v2"]`, })) - By("resolving foo/v1.0") - _, err = resolvePluginsByKey(plugins, "foo/v1.0") + By("resolving foo/v1") + _, err = resolvePluginsByKey(plugins, "foo/v1") Expect(err).To(MatchError(errAmbiguousPlugin{ - key: "foo/v1.0", - msg: `possible names: ["foo.example.com" "foo.kubebuilder.io"]`, + key: "foo/v1", + msg: `matching plugins: ["foo.example.com/v1" "foo.kubebuilder.io/v1"]`, })) By("resolving foo") _, err = resolvePluginsByKey(plugins, "foo") Expect(err).To(MatchError(errAmbiguousPlugin{ key: "foo", - msg: `possible keys: ["foo.example.com/v1.0" "foo.kubebuilder.io/v1.0" "foo.kubebuilder.io/v2.0"]`, + msg: `matching plugins: ["foo.example.com/v1" "foo.kubebuilder.io/v1" "foo.kubebuilder.io/v2"]`, })) By("resolving blah") _, err = resolvePluginsByKey(plugins, "blah") Expect(err).To(MatchError(errAmbiguousPlugin{ key: "blah", - msg: "no names match", + msg: fmt.Sprintf("no names match, possible plugins: %+q", makePluginKeySlice(plugins...)), })) - By("resolving foo.example.com/v2.0") - _, err = resolvePluginsByKey(plugins, "foo.example.com/v2.0") + By("resolving foo.example.com/v2") + _, err = resolvePluginsByKey(plugins, "foo.example.com/v2") Expect(err).To(MatchError(errAmbiguousPlugin{ - key: "foo.example.com/v2.0", - msg: "no names match", + key: "foo.example.com/v2", + msg: fmt.Sprintf(`no versions match, possible plugins: ["foo.example.com/v1"]`), })) - By("resolving foo/v3.0") - _, err = resolvePluginsByKey(plugins, "foo/v3.0") + By("resolving foo/v3") + _, err = resolvePluginsByKey(plugins, "foo/v3") Expect(err).To(MatchError(errAmbiguousPlugin{ - key: "foo/v3.0", - msg: "no versions match", + key: "foo/v3", + msg: "no versions match, possible plugins: " + + `["foo.example.com/v1" "foo.kubebuilder.io/v1" "foo.kubebuilder.io/v2"]`, })) - By("resolving foo.example.com/v3.0") - _, err = resolvePluginsByKey(plugins, "foo.example.com/v3.0") + By("resolving foo.example.com/v3") + _, err = resolvePluginsByKey(plugins, "foo.example.com/v3") Expect(err).To(MatchError(errAmbiguousPlugin{ - key: "foo.example.com/v3.0", - msg: "no versions match", + key: "foo.example.com/v3", + msg: `no versions match, possible plugins: ["foo.example.com/v1"]`, })) }) }) diff --git a/pkg/plugin/interfaces.go b/pkg/plugin/interfaces.go new file mode 100644 index 00000000000..acfb5780d94 --- /dev/null +++ b/pkg/plugin/interfaces.go @@ -0,0 +1,97 @@ +/* +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 plugin + +import ( + "github.com/spf13/pflag" + + "sigs.k8s.io/kubebuilder/pkg/model/config" +) + +type Base interface { + // Name returns a DNS1123 label string identifying the plugin uniquely. This name should be fully-qualified, + // i.e. have a short prefix describing the plugin type (like a language) followed by a domain. + // For example, Kubebuilder's main plugin would return "go.kubebuilder.io". + Name() string + // Version returns the plugin's version, which contains an integer and an optional stability of "alpha" or "beta". + // + // Note: this version is different from config version. + Version() Version + // SupportedProjectVersions lists all project configuration versions this + // plugin supports, ex. []string{"2", "3"}. The returned slice cannot be empty. + SupportedProjectVersions() []string +} + +type Deprecated interface { + // DeprecationWarning returns a string indicating a plugin is deprecated. + DeprecationWarning() string +} + +type GenericSubcommand interface { + // UpdateContext updates a Context with command-specific help text, like description and examples. + // Can be a no-op if default help text is desired. + UpdateContext(*Context) + // BindFlags binds the plugin's flags to the CLI. This allows each plugin to define its own + // command line flags for the kubebuilder subcommand. + BindFlags(*pflag.FlagSet) + // Run runs the subcommand. + Run() error + // InjectConfig passes a config to a plugin. The plugin may modify the + // config. Initializing, loading, and saving the config is managed by the + // cli package. + InjectConfig(*config.Config) +} + +type Context struct { + // CommandName sets the command name for a plugin. + CommandName string + // Description is a description of what this subcommand does. It is used to display help. + Description string + // Examples are one or more examples of the command-line usage + // of this plugin's project subcommand support. It is used to display help. + Examples string +} + +type InitPluginGetter interface { + Base + // GetInitPlugin returns the underlying Init interface. + GetInitPlugin() Init +} + +type Init interface { + GenericSubcommand +} + +type CreateAPIPluginGetter interface { + Base + // GetCreateAPIPlugin returns the underlying CreateAPI interface. + GetCreateAPIPlugin() CreateAPI +} + +type CreateAPI interface { + GenericSubcommand +} + +type CreateWebhookPluginGetter interface { + Base + // GetCreateWebhookPlugin returns the underlying CreateWebhook interface. + GetCreateWebhookPlugin() CreateWebhook +} + +type CreateWebhook interface { + GenericSubcommand +} diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 397b1a6d4ee..0caaf214bc6 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -17,27 +17,102 @@ limitations under the License. package plugin import ( + "errors" + "fmt" "path" + "regexp" + "strconv" "strings" - "github.com/spf13/pflag" - - "sigs.k8s.io/kubebuilder/pkg/model/config" + "sigs.k8s.io/kubebuilder/pkg/internal/validation" ) +// DefaultNameQualifier is the suffix appended to all kubebuilder plugin names. const DefaultNameQualifier = ".kubebuilder.io" -type Base interface { - // Name returns a DNS1123 label string defining the plugin type. - // For example, Kubebuilder's main plugin would return "go". - Name() string - // Version returns the plugin's semantic version, ex. "v1.2.3". - // - // Note: this version is different from config version. - Version() string - // SupportedProjectVersions lists all project configuration versions this - // plugin supports, ex. []string{"2", "3"}. The returned slice cannot be empty. - SupportedProjectVersions() []string +// Valid stage values. +const ( + // AlphaStage should be used for plugins that are frequently changed and may break between uses. + AlphaStage = "alpha" + // BetaStage should be used for plugins that are only changed in minor ways, ex. bug fixes. + BetaStage = "beta" +) + +// verRe defines the string format of a version. +var verRe = regexp.MustCompile("^(v)?([1-9][0-9]*)(-alpha|-beta)?$") + +// Version is a plugin version containing a non-zero integer and an optional stage value +// that if present identifies a version as not stable to some degree. +type Version struct { + // Number denotes the current version of a plugin. Two different numbers between versions + // indicate that they are incompatible. + Number int64 + // Stage indicates stability, and must be "alpha" or "beta". + Stage string +} + +func (v Version) String() string { + if v.Stage != "" { + return fmt.Sprintf("v%v-%s", v.Number, v.Stage) + } + return fmt.Sprintf("v%v", v.Number) +} + +// Validate ensures v contains a number and if it has a stage suffix that it is either "alpha" or "beta". +func (v Version) Validate() error { + if v.Number < 1 { + return errors.New("integer value cannot be or be less than 0") + } + if v.Stage != "" && v.Stage != AlphaStage && v.Stage != BetaStage { + return errors.New(`suffix must be "alpha" or "beta"`) + } + return nil +} + +// ParseVersion parses version into a Version, assuming it adheres to format: (v)?[1-9][0-9]*(-(alpha|beta))? +func ParseVersion(version string) (v Version, err error) { + if version == "" { + return v, errors.New("plugin version is empty") + } + + // A valid version string will have 4 submatches, each of which may be empty: the full string, "v", + // the integer, and the stage suffix. Invalid version strings do not have 4 submatches. + submatches := verRe.FindStringSubmatch(version) + if len(submatches) != 4 { + return v, fmt.Errorf("version format must match %s", verRe.String()) + } + + // Parse version number. + versionNumStr := submatches[2] + if versionNumStr == "" { + return v, errors.New("version must contain an integer") + } + if v.Number, err = strconv.ParseInt(versionNumStr, 10, 64); err != nil { + return v, err + } + + // Parse stage suffix, if any. + v.Stage = strings.TrimPrefix(submatches[3], "-") + + return v, v.Validate() +} + +// Compare returns -1 if v < vp, 0 if v == vp, and 1 if v > vp. +func (v Version) Compare(vp Version) int { + if v.Number == vp.Number { + s, sp := v.Stage, vp.Stage + if s == sp { + return 0 + } + // Since stages are not equal, if s is greater it must either be: beta when sp is alpha + // or "", or alpha when sp is "", otherwise it is lesser. + if s == BetaStage || (s == AlphaStage && sp == "") { + return 1 + } + } else if v.Number > vp.Number { + return 1 + } + return -1 } // Key returns a unique identifying string for a plugin's name and version. @@ -50,7 +125,7 @@ func Key(name, version string) string { // KeyFor returns a Base plugin's unique identifying string. func KeyFor(p Base) string { - return Key(p.Name(), p.Version()) + return Key(p.Name(), p.Version().String()) } // SplitKey returns a name and version for a plugin key. @@ -68,62 +143,10 @@ func GetShortName(name string) string { return strings.SplitN(name, ".", 2)[0] } -type Deprecated interface { - // DeprecationWarning returns a string indicating a plugin is deprecated. - DeprecationWarning() string -} - -type GenericSubcommand interface { - // UpdateContext updates a Context with command-specific help text, like description and examples. - // Can be a no-op if default help text is desired. - UpdateContext(*Context) - // BindFlags binds the plugin's flags to the CLI. This allows each plugin to define its own - // command line flags for the kubebuilder subcommand. - BindFlags(*pflag.FlagSet) - // Run runs the subcommand. - Run() error - // InjectConfig passes a config to a plugin. The plugin may modify the - // config. Initializing, loading, and saving the config is managed by the - // cli package. - InjectConfig(*config.Config) -} - -type Context struct { - // CommandName sets the command name for a plugin. - CommandName string - // Description is a description of what this subcommand does. It is used to display help. - Description string - // Examples are one or more examples of the command-line usage - // of this plugin's project subcommand support. It is used to display help. - Examples string -} - -type InitPluginGetter interface { - Base - // GetInitPlugin returns the underlying Init interface. - GetInitPlugin() Init -} - -type Init interface { - GenericSubcommand -} - -type CreateAPIPluginGetter interface { - Base - // GetCreateAPIPlugin returns the underlying CreateAPI interface. - GetCreateAPIPlugin() CreateAPI -} - -type CreateAPI interface { - GenericSubcommand -} - -type CreateWebhookPluginGetter interface { - Base - // GetCreateWebhookPlugin returns the underlying CreateWebhook interface. - GetCreateWebhookPlugin() CreateWebhook -} - -type CreateWebhook interface { - GenericSubcommand +// ValidateName ensures name is a valid DNS 1123 subdomain. +func ValidateName(name string) error { + if errs := validation.IsDNS1123Subdomain(name); len(errs) != 0 { + return fmt.Errorf("invalid plugin name %q: %v", name, errs) + } + return nil } diff --git a/pkg/plugin/plugin_test.go b/pkg/plugin/plugin_test.go new file mode 100644 index 00000000000..a99e6a74fd0 --- /dev/null +++ b/pkg/plugin/plugin_test.go @@ -0,0 +1,156 @@ +/* +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 plugin + +import ( + "sort" + "testing" + + g "github.com/onsi/ginkgo" // An alias is required because Context is defined elsewhere in this package. + . "github.com/onsi/gomega" +) + +func TestCLI(t *testing.T) { + RegisterFailHandler(g.Fail) + g.RunSpecs(t, "Plugin Suite") +} + +var _ = g.Describe("ParseVersion", func() { + + var ( + v Version + err error + ) + + g.It("should pass validation when a version is positive without a stage", func() { + g.By("passing version string 1") + v, err = ParseVersion("1") + Expect(err).NotTo(HaveOccurred()) + Expect(v.Number).To(BeNumerically("==", int64(1))) + Expect(v.Stage).To(Equal("")) + + g.By("passing version string 22") + v, err = ParseVersion("22") + Expect(err).NotTo(HaveOccurred()) + Expect(v.Number).To(BeNumerically("==", int64(22))) + Expect(v.Stage).To(Equal("")) + + g.By("passing version string v1") + v, err = ParseVersion("v1") + Expect(err).NotTo(HaveOccurred()) + Expect(v.Number).To(BeNumerically("==", int64(1))) + Expect(v.Stage).To(Equal("")) + }) + + g.It("should pass validation when a version is positive with a stage", func() { + g.By("passing version string 1-alpha") + v, err = ParseVersion("1-alpha") + Expect(err).NotTo(HaveOccurred()) + Expect(v.Number).To(BeNumerically("==", int64(1))) + Expect(v.Stage).To(Equal("alpha")) + + g.By("passing version string 1-beta") + v, err = ParseVersion("1-beta") + Expect(err).NotTo(HaveOccurred()) + Expect(v.Number).To(BeNumerically("==", int64(1))) + Expect(v.Stage).To(Equal("beta")) + + g.By("passing version string v1-alpha") + v, err = ParseVersion("v1-alpha") + Expect(err).NotTo(HaveOccurred()) + Expect(v.Number).To(BeNumerically("==", int64(1))) + Expect(v.Stage).To(Equal("alpha")) + + g.By("passing version string v22-alpha") + v, err = ParseVersion("v22-alpha") + Expect(err).NotTo(HaveOccurred()) + Expect(v.Number).To(BeNumerically("==", int64(22))) + Expect(v.Stage).To(Equal("alpha")) + }) + + g.It("should fail validation", func() { + g.By("passing version string an empty string") + _, err = ParseVersion("") + Expect(err).To(HaveOccurred()) + + g.By("passing version string 0") + _, err = ParseVersion("0") + Expect(err).To(HaveOccurred()) + + g.By("passing a negative number version string") + _, err = ParseVersion("-1") + Expect(err).To(HaveOccurred()) + }) + + g.It("should fail validation when a version string is semver", func() { + g.By("passing version string 1.0") + _, err = ParseVersion("1.0") + Expect(err).To(HaveOccurred()) + + g.By("passing version string v1.0") + _, err = ParseVersion("v1.0") + Expect(err).To(HaveOccurred()) + + g.By("passing version string v1.0-alpha") + _, err = ParseVersion("v1.0-alpha") + Expect(err).To(HaveOccurred()) + + g.By("passing version string 1.0.0") + _, err = ParseVersion("1.0.0") + Expect(err).To(HaveOccurred()) + }) +}) + +var _ = g.Describe("Compare", func() { + + // Test Compare() by sorting a list. + var ( + versions = []Version{ + {Number: 2, Stage: AlphaStage}, + {Number: 44, Stage: AlphaStage}, + {Number: 1}, + {Number: 2, Stage: BetaStage}, + {Number: 4, Stage: BetaStage}, + {Number: 1, Stage: AlphaStage}, + {Number: 4}, + {Number: 44, Stage: AlphaStage}, + {Number: 30}, + {Number: 4, Stage: AlphaStage}, + } + + sortedVersions = []Version{ + {Number: 1}, + {Number: 1, Stage: AlphaStage}, + {Number: 2, Stage: AlphaStage}, + {Number: 2, Stage: BetaStage}, + {Number: 4}, + {Number: 4, Stage: AlphaStage}, + {Number: 4, Stage: BetaStage}, + {Number: 30}, + {Number: 44, Stage: AlphaStage}, + {Number: 44, Stage: AlphaStage}, + } + ) + + g.It("sorts a valid list of versions correctly", func() { + sort.Slice(versions, func(i int, j int) bool { + return versions[i].Compare(versions[j]) == -1 + }) + Expect(versions).To(Equal(sortedVersions)) + }) + +}) diff --git a/pkg/plugin/v2/plugin.go b/pkg/plugin/v2/plugin.go index de4ff49e112..3eb7a3bcace 100644 --- a/pkg/plugin/v2/plugin.go +++ b/pkg/plugin/v2/plugin.go @@ -21,12 +21,12 @@ import ( "sigs.k8s.io/kubebuilder/pkg/plugin" ) -const ( - pluginName = "go" + plugin.DefaultNameQualifier - pluginVersion = "v2.0.0" -) +const pluginName = "go" + plugin.DefaultNameQualifier -var supportedProjectVersions = []string{config.Version2, config.Version3Alpha} +var ( + supportedProjectVersions = []string{config.Version2, config.Version3Alpha} + pluginVersion = plugin.Version{Number: 2} +) var ( _ plugin.Base = Plugin{} @@ -42,7 +42,7 @@ type Plugin struct { } func (Plugin) Name() string { return pluginName } -func (Plugin) Version() string { return pluginVersion } +func (Plugin) Version() plugin.Version { return pluginVersion } func (Plugin) SupportedProjectVersions() []string { return supportedProjectVersions } func (p Plugin) GetInitPlugin() plugin.Init { return &p.initPlugin } func (p Plugin) GetCreateAPIPlugin() plugin.CreateAPI { return &p.createAPIPlugin } diff --git a/pkg/plugin/validation.go b/pkg/plugin/validation.go deleted file mode 100644 index 8503ca88469..00000000000 --- a/pkg/plugin/validation.go +++ /dev/null @@ -1,48 +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 plugin - -import ( - "errors" - "fmt" - - "github.com/blang/semver" - - "sigs.k8s.io/kubebuilder/pkg/internal/validation" -) - -// ValidateVersion ensures version adheres to the plugin version format, -// which is tolerant semver. -func ValidateVersion(version string) error { - if version == "" { - return errors.New("plugin version is empty") - } - // ParseTolerant allows versions with a "v" prefix or shortened versions, - // ex. "3" or "v3.0". - if _, err := semver.ParseTolerant(version); err != nil { - return fmt.Errorf("failed to validate plugin version %q: %v", version, err) - } - return nil -} - -// ValidateName ensures name is a valid DNS 1123 subdomain. -func ValidateName(name string) error { - if errs := validation.IsDNS1123Subdomain(name); len(errs) != 0 { - return fmt.Errorf("plugin name %q is invalid: %v", name, errs) - } - return nil -} diff --git a/testdata/project-v3-addon/PROJECT b/testdata/project-v3-addon/PROJECT index b7665b1d25e..14d827d5709 100644 --- a/testdata/project-v3-addon/PROJECT +++ b/testdata/project-v3-addon/PROJECT @@ -1,5 +1,5 @@ domain: testproject.org -layout: go.kubebuilder.io/v2.0.0 +layout: go.kubebuilder.io/v2 repo: sigs.k8s.io/kubebuilder/testdata/project-v3-addon resources: - group: crew diff --git a/testdata/project-v3-multigroup/PROJECT b/testdata/project-v3-multigroup/PROJECT index 81cdaa20878..ec2d164be44 100644 --- a/testdata/project-v3-multigroup/PROJECT +++ b/testdata/project-v3-multigroup/PROJECT @@ -1,5 +1,5 @@ domain: testproject.org -layout: go.kubebuilder.io/v2.0.0 +layout: go.kubebuilder.io/v2 multigroup: true repo: sigs.k8s.io/kubebuilder/testdata/project-v3-multigroup resources: diff --git a/testdata/project-v3/PROJECT b/testdata/project-v3/PROJECT index 6e999292d27..52263f80c0b 100644 --- a/testdata/project-v3/PROJECT +++ b/testdata/project-v3/PROJECT @@ -1,5 +1,5 @@ domain: testproject.org -layout: go.kubebuilder.io/v2.0.0 +layout: go.kubebuilder.io/v2 repo: sigs.k8s.io/kubebuilder/testdata/project-v3 resources: - group: crew