From 2b0ee4b473cafede5ad26bccd2a6911543a63934 Mon Sep 17 00:00:00 2001 From: Adrian Orive Date: Fri, 26 Feb 2021 11:04:26 +0100 Subject: [PATCH] Use a single set of default plugins instead of one per project version Signed-off-by: Adrian Orive --- cmd/main.go | 4 +--- pkg/cli/cli.go | 21 ++++++++++++++----- pkg/cli/cli_test.go | 33 +++++++++--------------------- pkg/cli/options.go | 12 +++-------- pkg/cli/options_test.go | 28 +++++++------------------ pkg/cli/root.go | 4 ++-- test/e2e/v2/plugin_cluster_test.go | 1 + test/testdata/generate.sh | 8 ++++---- 8 files changed, 44 insertions(+), 67 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index c3527528372..56e89280afe 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -20,7 +20,6 @@ import ( "log" "sigs.k8s.io/kubebuilder/v3/pkg/cli" - cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2" cfgv3 "sigs.k8s.io/kubebuilder/v3/pkg/config/v3" pluginv2 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v2" pluginv3 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v3" @@ -35,8 +34,7 @@ func main() { &pluginv2.Plugin{}, &pluginv3.Plugin{}, ), - cli.WithDefaultPlugins(cfgv2.Version, &pluginv2.Plugin{}), - cli.WithDefaultPlugins(cfgv3.Version, &pluginv3.Plugin{}), + cli.WithDefaultPlugins(&pluginv3.Plugin{}), cli.WithCompletion(), ) if err != nil { diff --git a/pkg/cli/cli.go b/pkg/cli/cli.go index 8c12384676b..d3d92e5cd30 100644 --- a/pkg/cli/cli.go +++ b/pkg/cli/cli.go @@ -26,8 +26,10 @@ import ( internalconfig "sigs.k8s.io/kubebuilder/v3/pkg/cli/internal/config" "sigs.k8s.io/kubebuilder/v3/pkg/config" + cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2" cfgv3 "sigs.k8s.io/kubebuilder/v3/pkg/config/v3" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" + pluginv2 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v2" ) const ( @@ -76,7 +78,7 @@ type cli struct { //nolint:maligned // Default project version in case none is provided and a config file can't be found. defaultProjectVersion config.Version // Default plugins in case none is provided and a config file can't be found. - defaultPlugins map[config.Version][]string + defaultPlugins []string // Plugins registered in the cli. plugins map[string]plugin.Plugin // Commands injected by options. @@ -132,7 +134,7 @@ func newCLI(opts ...Option) (*cli, error) { c := &cli{ commandName: "kubebuilder", defaultProjectVersion: cfgv3.Version, - defaultPlugins: make(map[config.Version][]string), + defaultPlugins: make([]string, 0), plugins: make(map[string]plugin.Plugin), } @@ -187,6 +189,12 @@ func (c *cli) getInfoFromFlags() (string, []string, error) { plugins[i] = strings.TrimSpace(key) } + // For backwards compatibility reasons, defining the project version flag and not defining + // the plugins flag should be interpreted as if the plugins flag was set to go/v2. + if projectVersion == "2" && len(plugins) == 0 { + plugins = append(plugins, plugin.KeyFor(pluginv2.Plugin{})) + } + return projectVersion, plugins, nil } @@ -208,6 +216,11 @@ func getInfoFromConfigFile() (config.Version, []string, error) { // getInfoFromConfig obtains the project version and plugin keys from the project config. // It is extracted from getInfoFromConfigFile for testing purposes. func getInfoFromConfig(projectConfig config.Config) (config.Version, []string, error) { + // Project v2 did not store the layout field, so we set it to the only available plugin + if projectConfig.GetVersion().Compare(cfgv2.Version) == 0 { + return cfgv2.Version, []string{plugin.KeyFor(pluginv2.Plugin{})}, nil + } + // Split the comma-separated plugins var pluginSet []string if projectConfig.GetLayout() != "" { @@ -263,9 +276,7 @@ func (c cli) resolveFlagsAndConfigFileConflicts( switch { // If they are both empty, use the default case isFlagPluginsEmpty && isCfgPluginsEmpty: - if defaults, hasDefaults := c.defaultPlugins[projectVersion]; hasDefaults { - plugins = defaults - } + plugins = c.defaultPlugins // If any is empty, choose the other case isCfgPluginsEmpty: plugins = flagPlugins diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go index 56080a87bb7..0a72e179d2e 100644 --- a/pkg/cli/cli_test.go +++ b/pkg/cli/cli_test.go @@ -108,7 +108,7 @@ var _ = Describe("CLI", func() { projectVersion, plugins, err = c.getInfoFromFlags() Expect(err).NotTo(HaveOccurred()) Expect(projectVersion).To(Equal("2")) - Expect(len(plugins)).To(Equal(0)) + Expect(plugins).To(Equal([]string{"go.kubebuilder.io/v2"})) }) }) @@ -197,14 +197,14 @@ var _ = Describe("CLI", func() { projectVersion, plugins, err = getInfoFromConfig(projectConfig) Expect(err).NotTo(HaveOccurred()) Expect(projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0)) - Expect(len(plugins)).To(Equal(0)) + Expect(plugins).To(Equal([]string{"go.kubebuilder.io/v2"})) }) }) When("having layout field", func() { It("should succeed", func() { projectConfig = cfgv3.New() - Expect(projectConfig.SetLayout("go.kubebuilder.io/v2")).To(Succeed()) + Expect(projectConfig.SetLayout("go.kubebuilder.io/v3")).To(Succeed()) projectVersion, plugins, err = getInfoFromConfig(projectConfig) Expect(err).NotTo(HaveOccurred()) Expect(projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0)) @@ -411,10 +411,7 @@ var _ = Describe("CLI", func() { It("should succeed", func() { c = &cli{ defaultProjectVersion: projectVersion1, - defaultPlugins: map[config.Version][]string{ - projectVersion1: {pluginKey1}, - projectVersion2: {pluginKey2}, - }, + defaultPlugins: []string{pluginKey1}, } _, plugins, err = c.resolveFlagsAndConfigFileConflicts( "", @@ -463,9 +460,7 @@ var _ = Describe("CLI", func() { When("having default plugin keys set and from flags", func() { It("should succeed", func() { c = &cli{ - defaultPlugins: map[config.Version][]string{ - {}: {pluginKey1}, - }, + defaultPlugins: []string{pluginKey1}, } _, plugins, err = c.resolveFlagsAndConfigFileConflicts( "", @@ -482,9 +477,7 @@ var _ = Describe("CLI", func() { When("having default plugin keys set and from config file", func() { It("should succeed", func() { c = &cli{ - defaultPlugins: map[config.Version][]string{ - {}: {pluginKey1}, - }, + defaultPlugins: []string{pluginKey1}, } _, plugins, err = c.resolveFlagsAndConfigFileConflicts( "", @@ -528,9 +521,7 @@ var _ = Describe("CLI", func() { When("having three plugin keys sources", func() { It("should succeed if plugin keys from flags and config file are the same", func() { c = &cli{ - defaultPlugins: map[config.Version][]string{ - {}: {pluginKey1}, - }, + defaultPlugins: []string{pluginKey1}, } _, plugins, err = c.resolveFlagsAndConfigFileConflicts( "", @@ -545,9 +536,7 @@ var _ = Describe("CLI", func() { It("should fail if plugin keys from flags and config file are different", func() { c = &cli{ - defaultPlugins: map[config.Version][]string{ - {}: {pluginKey1}, - }, + defaultPlugins: []string{pluginKey1}, } _, _, err = c.resolveFlagsAndConfigFileConflicts( "", @@ -582,9 +571,7 @@ var _ = Describe("CLI", func() { pluginKeys := []string{"go.kubebuilder.io/v2"} c := &cli{ defaultProjectVersion: projectVersion, - defaultPlugins: map[config.Version][]string{ - projectVersion: pluginKeys, - }, + defaultPlugins: pluginKeys, } c.cmd = c.newRootCmd() Expect(c.getInfo()).To(Succeed()) @@ -745,7 +732,7 @@ var _ = Describe("CLI", func() { c, err = New( WithDefaultProjectVersion(projectVersion), - WithDefaultPlugins(projectVersion, deprecatedPlugin), + WithDefaultPlugins(deprecatedPlugin), WithPlugins(deprecatedPlugin), ) _ = w.Close() diff --git a/pkg/cli/options.go b/pkg/cli/options.go index 36b2a8fdc80..98649d0f001 100644 --- a/pkg/cli/options.go +++ b/pkg/cli/options.go @@ -57,22 +57,16 @@ func WithDefaultProjectVersion(version config.Version) Option { } // WithDefaultPlugins is an Option that sets the cli's default plugins. -func WithDefaultPlugins(projectVersion config.Version, plugins ...plugin.Plugin) Option { +func WithDefaultPlugins(plugins ...plugin.Plugin) Option { return func(c *cli) error { - if err := projectVersion.Validate(); err != nil { - return fmt.Errorf("broken pre-set project version %q for default plugins: %v", projectVersion, err) - } if len(plugins) == 0 { - return fmt.Errorf("empty set of plugins provided for project version %q", projectVersion) + return fmt.Errorf("empty set of plugins provided") } for _, p := range plugins { if err := plugin.Validate(p); err != nil { return fmt.Errorf("broken pre-set default plugin %q: %v", plugin.KeyFor(p), err) } - if !plugin.SupportsVersion(p, projectVersion) { - return fmt.Errorf("default plugin %q doesn't support version %q", plugin.KeyFor(p), projectVersion) - } - c.defaultPlugins[projectVersion] = append(c.defaultPlugins[projectVersion], plugin.KeyFor(p)) + c.defaultPlugins = append(c.defaultPlugins, plugin.KeyFor(p)) } return nil } diff --git a/pkg/cli/options_test.go b/pkg/cli/options_test.go index 9bf4b0153f8..430f7636379 100644 --- a/pkg/cli/options_test.go +++ b/pkg/cli/options_test.go @@ -99,57 +99,43 @@ var _ = Describe("CLI options", func() { Context("WithDefaultPlugins", func() { It("should return a valid CLI", func() { - c, err = newCLI(WithDefaultPlugins(projectVersion, p)) + c, err = newCLI(WithDefaultPlugins(p)) Expect(err).NotTo(HaveOccurred()) Expect(c).NotTo(BeNil()) - Expect(c.defaultPlugins).To(Equal(map[config.Version][]string{projectVersion: {plugin.KeyFor(p)}})) - }) - - When("providing an invalid project version", func() { - It("should return an error", func() { - _, err = newCLI(WithDefaultPlugins(config.Version{}, p)) - Expect(err).To(HaveOccurred()) - }) + Expect(c.defaultPlugins).To(Equal([]string{plugin.KeyFor(p)})) }) When("providing an empty set of plugins", func() { It("should return an error", func() { - _, err = newCLI(WithDefaultPlugins(projectVersion)) + _, err = newCLI(WithDefaultPlugins()) Expect(err).To(HaveOccurred()) }) }) When("providing a plugin with an invalid name", func() { It("should return an error", func() { - _, err = newCLI(WithDefaultPlugins(projectVersion, np1)) + _, err = newCLI(WithDefaultPlugins(np1)) Expect(err).To(HaveOccurred()) }) }) When("providing a plugin with an invalid version", func() { It("should return an error", func() { - _, err = newCLI(WithDefaultPlugins(projectVersion, np2)) + _, err = newCLI(WithDefaultPlugins(np2)) Expect(err).To(HaveOccurred()) }) }) When("providing a plugin with an empty list of supported versions", func() { It("should return an error", func() { - _, err = newCLI(WithDefaultPlugins(projectVersion, np3)) + _, err = newCLI(WithDefaultPlugins(np3)) Expect(err).To(HaveOccurred()) }) }) When("providing a plugin with an invalid list of supported versions", func() { It("should return an error", func() { - _, err = newCLI(WithDefaultPlugins(projectVersion, np4)) - Expect(err).To(HaveOccurred()) - }) - }) - - When("providing a default plugin for an unsupported project version", func() { - It("should return an error", func() { - _, err = newCLI(WithDefaultPlugins(config.Version{Number: 2}, p)) + _, err = newCLI(WithDefaultPlugins(np4)) Expect(err).To(HaveOccurred()) }) }) diff --git a/pkg/cli/root.go b/pkg/cli/root.go index e0baa398180..860d6dbb73c 100644 --- a/pkg/cli/root.go +++ b/pkg/cli/root.go @@ -74,8 +74,8 @@ configuration please run: str += fmt.Sprintf("\nDefault project version: %s\n", c.defaultProjectVersion) - if defaultPlugins, hasDefaultPlugins := c.defaultPlugins[c.defaultProjectVersion]; hasDefaultPlugins { - str += fmt.Sprintf("Default plugin keys: %q\n", strings.Join(defaultPlugins, ",")) + if len(c.defaultPlugins) != 0 { + str += fmt.Sprintf("Default plugin keys: %q\n", strings.Join(c.defaultPlugins, ",")) } str += fmt.Sprintf(` diff --git a/test/e2e/v2/plugin_cluster_test.go b/test/e2e/v2/plugin_cluster_test.go index 6832259788a..a0a67c48c1b 100644 --- a/test/e2e/v2/plugin_cluster_test.go +++ b/test/e2e/v2/plugin_cluster_test.go @@ -67,6 +67,7 @@ var _ = Describe("kubebuilder", func() { By("init v2 project") err := kbc.Init( "--project-version", "2", + "--plugins", "go/v2", "--domain", kbc.Domain, "--fetch-deps=false") Expect(err).Should(Succeed()) diff --git a/test/testdata/generate.sh b/test/testdata/generate.sh index 608bc757ce0..2a439d038a0 100755 --- a/test/testdata/generate.sh +++ b/test/testdata/generate.sh @@ -113,10 +113,10 @@ function scaffold_test_project { build_kb -# Project version 2 uses plugin go/v2 (default). -scaffold_test_project project-v2 --project-version=2 -scaffold_test_project project-v2-multigroup --project-version=2 -scaffold_test_project project-v2-addon --project-version=2 +# Project version 2 uses plugin go/v2. +scaffold_test_project project-v2 --plugins=go/v2 --project-version=2 +scaffold_test_project project-v2-multigroup --plugins=go/v2 --project-version=2 +scaffold_test_project project-v2-addon --plugins=go/v2 --project-version=2 # Project version 3 (default) uses plugin go/v3 (default). scaffold_test_project project-v3 scaffold_test_project project-v3-multigroup