diff --git a/cmd/main.go b/cmd/main.go index ef8393264db..9d8cf1ec5ff 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" declarativev1 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/declarative/v1" pluginv2 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v2" @@ -31,14 +30,13 @@ func main() { c, err := cli.New( cli.WithCommandName("kubebuilder"), cli.WithVersion(versionString()), - cli.WithDefaultProjectVersion(cfgv3.Version), cli.WithPlugins( &pluginv2.Plugin{}, &pluginv3.Plugin{}, &declarativev1.Plugin{}, ), - cli.WithDefaultPlugins(cfgv2.Version, &pluginv2.Plugin{}), - cli.WithDefaultPlugins(cfgv3.Version, &pluginv3.Plugin{}), + cli.WithDefaultPlugins(&pluginv3.Plugin{}), + cli.WithDefaultProjectVersion(cfgv3.Version), cli.WithCompletion, ) if err != nil { diff --git a/pkg/cli/cli.go b/pkg/cli/cli.go index eeda6863869..487fa977be3 100644 --- a/pkg/cli/cli.go +++ b/pkg/cli/cli.go @@ -30,33 +30,17 @@ import ( yamlstore "sigs.k8s.io/kubebuilder/v3/pkg/config/store/yaml" cfgv3 "sigs.k8s.io/kubebuilder/v3/pkg/config/v3" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" + goPluginV2 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v2" ) const ( noticeColor = "\033[1;36m%s\033[0m" deprecationFmt = "[Deprecation Notice] %s\n\n" - projectVersionFlag = "project-version" pluginsFlag = "plugins" + projectVersionFlag = "project-version" ) -// equalStringSlice checks if two string slices are equal. -func equalStringSlice(a, b []string) bool { - // Check lengths - if len(a) != len(b) { - return false - } - - // Check elements - for i, v := range a { - if v != b[i] { - return false - } - } - - return true -} - // CLI interacts with a command line interface. type CLI interface { // Run runs the CLI, usually returning an error if command line configuration @@ -73,12 +57,12 @@ type cli struct { //nolint:maligned commandName string // CLI version string. version string - // 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 // Plugins registered in the cli. plugins map[string]plugin.Plugin + // Default plugins in case none is provided and a config file can't be found. + defaultPlugins []string + // Default project version in case none is provided and a config file can't be found. + defaultProjectVersion config.Version // Commands injected by options. extraCommands []*cobra.Command // Whether to add a completion command to the cli. @@ -86,10 +70,10 @@ type cli struct { //nolint:maligned /* Internal fields */ - // Project version to scaffold. - projectVersion config.Version // Plugin keys to scaffold with. pluginKeys []string + // Project version to scaffold. + projectVersion config.Version // A filtered set of plugins that should be used by command constructors. resolvedPlugins []plugin.Plugin @@ -134,9 +118,9 @@ func newCLI(opts ...Option) (*cli, error) { // Default cli options. c := &cli{ commandName: "kubebuilder", - defaultProjectVersion: cfgv3.Version, - defaultPlugins: make(map[config.Version][]string), plugins: make(map[string]plugin.Plugin), + defaultPlugins: make([]string, 0), + defaultProjectVersion: cfgv3.Version, fs: afero.NewOsFs(), } @@ -150,258 +134,232 @@ func newCLI(opts ...Option) (*cli, error) { return c, nil } -// getInfoFromFlags obtains the project version and plugin keys from flags. -func (c *cli) getInfoFromFlags() (string, []string, error) { - // Partially parse the command line arguments - fs := pflag.NewFlagSet("base", pflag.ContinueOnError) +// buildCmd creates the underlying cobra command and stores it internally. +func (c *cli) buildCmd() error { + c.cmd = c.newRootCmd() - // Load the base command global flags - fs.AddFlagSet(c.cmd.PersistentFlags()) + // Get project version and plugin keys. + if err := c.getInfo(); err != nil { + return err + } - // Omit unknown flags to avoid parsing errors - fs.ParseErrorsWhitelist = pflag.ParseErrorsWhitelist{UnknownFlags: true} + // Resolve plugins for project version and plugin keys. + if err := c.resolvePlugins(); err != nil { + return err + } - // FlagSet special cases --help and -h, so we need to create a dummy flag with these 2 values to prevent the default - // behavior (printing the usage of this FlagSet) as we want to print the usage message of the underlying command. - fs.BoolP("help", "h", false, fmt.Sprintf("help for %s", c.commandName)) + // Add the subcommands + c.addSubcommands() - // Parse the arguments - if err := fs.Parse(os.Args[1:]); err != nil { - return "", []string{}, err - } + return nil +} - // Define the flags needed for plugin resolution - var ( - projectVersion string - plugins []string - err error - ) - // GetXxxxx methods will not yield errors because we know for certain these flags exist and types match. - projectVersion, err = fs.GetString(projectVersionFlag) - if err != nil { - return "", []string{}, err +// getInfo obtains the plugin keys and project version resolving conflicts between the project config file and flags. +func (c *cli) getInfo() error { + // Get plugin keys and project version from project configuration file + // We discard the error if file doesn't exist because not being able to read a project configuration + // file is not fatal for some commands. The ones that require it need to check its existence later. + if err := c.getInfoFromConfigFile(); err != nil && !errors.Is(err, os.ErrNotExist) { + return err } - plugins, err = fs.GetStringSlice(pluginsFlag) - if err != nil { - return "", []string{}, err + + // Get project version and plugin info from flags + if err := c.getInfoFromFlags(); err != nil { + return err } - // Remove leading and trailing spaces - for i, key := range plugins { - plugins[i] = strings.TrimSpace(key) + // By this point, if plugin keys are empty we should use the defaults + if len(c.pluginKeys) == 0 { + c.pluginKeys = c.defaultPlugins } - return projectVersion, plugins, nil + // We don't assign a default value for project version here because we may be able to + // resolve the project version after resolving the plugins. + + return nil } // getInfoFromConfigFile obtains the project version and plugin keys from the project config file. -func (c cli) getInfoFromConfigFile() (config.Version, []string, error) { +func (c *cli) getInfoFromConfigFile() error { // Read the project configuration file cfg := yamlstore.New(c.fs) - err := cfg.Load() - switch { - case err == nil: - case errors.Is(err, os.ErrNotExist): - return config.Version{}, nil, nil - default: - return config.Version{}, nil, err + if err := cfg.Load(); err != nil { + return err } - return getInfoFromConfig(cfg.Config()) + return c.getInfoFromConfig(cfg.Config()) } // 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) { +func (c *cli) getInfoFromConfig(projectConfig config.Config) error { // Split the comma-separated plugins var pluginSet []string if projectConfig.GetLayout() != "" { for _, p := range strings.Split(projectConfig.GetLayout(), ",") { pluginSet = append(pluginSet, strings.TrimSpace(p)) } + for _, pluginKey := range pluginSet { + if err := plugin.ValidateKey(pluginKey); err != nil { + return fmt.Errorf("invalid plugin key found in project configuration file: %w", err) + } + } + } else { + // Project version 2 did not store the layout field, but only the go.kubebuilder.io/v2 plugin was supported + pluginSet = append(pluginSet, plugin.KeyFor(goPluginV2.Plugin{})) } - return projectConfig.GetVersion(), pluginSet, nil + c.pluginKeys = append(c.pluginKeys, pluginSet...) + c.projectVersion = projectConfig.GetVersion() + return nil } -// resolveFlagsAndConfigFileConflicts checks if the provided combined input from flags and -// the config file is valid and uses default values in case some info was not provided. -func (c cli) resolveFlagsAndConfigFileConflicts( - flagProjectVersionString string, - cfgProjectVersion config.Version, - flagPlugins, cfgPlugins []string, -) (config.Version, []string, error) { - // Parse project configuration version from flags - var flagProjectVersion config.Version - if flagProjectVersionString != "" { - if err := flagProjectVersion.Parse(flagProjectVersionString); err != nil { - return config.Version{}, nil, fmt.Errorf("unable to parse project version flag: %w", err) - } +// getInfoFromFlags obtains the project version and plugin keys from flags. +func (c *cli) getInfoFromFlags() error { + // Check if the provided project version is invalid (empty) + // This means that we could not load the information from the config file so we add the project version flag + knowsProjectVersionFromConfig := c.projectVersion.Validate() == nil + + // Partially parse the command line arguments + fs := pflag.NewFlagSet("base", pflag.ContinueOnError) + + // Load the base command global flags + fs.AddFlagSet(c.cmd.PersistentFlags()) + + // If we were unable to load the project configuration, we should also accept the project version flag + var projectVersionStr string + if !knowsProjectVersionFromConfig { + fs.StringVar(&projectVersionStr, projectVersionFlag, "", "project version") } - // Resolve project version - var projectVersion config.Version - isFlagProjectVersionInvalid := flagProjectVersion.Validate() != nil - isCfgProjectVersionInvalid := cfgProjectVersion.Validate() != nil - switch { - // If they are both invalid (empty is invalid), use the default - case isFlagProjectVersionInvalid && isCfgProjectVersionInvalid: - projectVersion = c.defaultProjectVersion - // If any is invalid (empty is invalid), choose the other - case isCfgProjectVersionInvalid: - projectVersion = flagProjectVersion - case isFlagProjectVersionInvalid: - projectVersion = cfgProjectVersion - // If they are equal doesn't matter which we choose - case flagProjectVersion.Compare(cfgProjectVersion) == 0: - projectVersion = flagProjectVersion - // If both are valid (empty is invalid) and they are different error out - default: - return config.Version{}, nil, fmt.Errorf("project version conflict between command line args (%s) "+ - "and project configuration file (%s)", flagProjectVersionString, cfgProjectVersion) + // FlagSet special cases --help and -h, so we need to create a dummy flag with these 2 values to prevent the default + // behavior (printing the usage of this FlagSet) as we want to print the usage message of the underlying command. + fs.BoolP("help", "h", false, fmt.Sprintf("help for %s", c.commandName)) + + // Omit unknown flags to avoid parsing errors + fs.ParseErrorsWhitelist = pflag.ParseErrorsWhitelist{UnknownFlags: true} + + // Parse the arguments + if err := fs.Parse(os.Args[1:]); err != nil { + return err } - // Resolve plugins - var plugins []string - isFlagPluginsEmpty := len(flagPlugins) == 0 - isCfgPluginsEmpty := len(cfgPlugins) == 0 - switch { - // If they are both empty, use the default - case isFlagPluginsEmpty && isCfgPluginsEmpty: - if defaults, hasDefaults := c.defaultPlugins[projectVersion]; hasDefaults { - plugins = defaults + // If any plugin key was provided, replace those from the project configuration file + if pluginKeys, err := fs.GetStringSlice(pluginsFlag); err != nil { + return err + } else if len(pluginKeys) != 0 { + // Remove leading and trailing spaces and validate the plugin keys + for i, key := range pluginKeys { + pluginKeys[i] = strings.TrimSpace(key) + if err := plugin.ValidateKey(pluginKeys[i]); err != nil { + return fmt.Errorf("invalid plugin %q found in flags: %w", pluginKeys[i], err) + } } - // If any is empty, choose the other - case isCfgPluginsEmpty: - plugins = flagPlugins - case isFlagPluginsEmpty: - plugins = cfgPlugins - // If they are equal doesn't matter which we choose - case equalStringSlice(flagPlugins, cfgPlugins): - plugins = flagPlugins - // If none is empty and they are different error out - default: - return config.Version{}, nil, fmt.Errorf("plugins conflict between command line args (%v) "+ - "and project configuration file (%v)", flagPlugins, cfgPlugins) + + c.pluginKeys = pluginKeys } - // Validate the plugins - for _, p := range plugins { - if err := plugin.ValidateKey(p); err != nil { - return config.Version{}, nil, err + + // If the project version flag was accepted but not provided keep the empty version and try to resolvePlugins later + if projectVersionStr != "" { + if err := c.projectVersion.Parse(projectVersionStr); err != nil { + return fmt.Errorf("invalid project version flag: %w", err) } } - return projectVersion, plugins, nil -} - -// getInfo obtains the project version and plugin keys resolving conflicts among flags and the project config file. -func (c *cli) getInfo() error { - // Get project version and plugin info from flags - flagProjectVersion, flagPlugins, err := c.getInfoFromFlags() - if err != nil { - return err - } - // Get project version and plugin info from project configuration file - cfgProjectVersion, cfgPlugins, _ := c.getInfoFromConfigFile() - // We discard the error because not being able to read a project configuration file - // is not fatal for some commands. The ones that require it need to check its existence. - - // Resolve project version and plugin keys - c.projectVersion, c.pluginKeys, err = c.resolveFlagsAndConfigFileConflicts( - flagProjectVersion, cfgProjectVersion, flagPlugins, cfgPlugins, - ) - return err + return nil } const unstablePluginMsg = " (plugin version is unstable, there may be an upgrade available: " + "https://kubebuilder.io/migration/plugin/plugins.html)" -// resolve selects from the available plugins those that match the project version and plugin keys provided. -func (c *cli) resolve() error { - var plugins []plugin.Plugin +// resolvePlugins selects from the available plugins those that match the project version and plugin keys provided. +func (c *cli) resolvePlugins() error { + knownProjectVersion := c.projectVersion.Validate() == nil + for _, pluginKey := range c.pluginKeys { - name, version := plugin.SplitKey(pluginKey) - shortName := plugin.GetShortName(name) + var extraErrMsg string + + plugins := make([]plugin.Plugin, 0, len(c.plugins)) + for _, p := range c.plugins { + plugins = append(plugins, p) + } + // We can omit the error because plugin keys have already been validated + plugins, _ = plugin.FilterPluginsByKey(plugins, pluginKey) + if knownProjectVersion { + plugins = plugin.FilterPluginsByProjectVersion(plugins, c.projectVersion) + extraErrMsg += fmt.Sprintf(" for project version %q", c.projectVersion) + } // Plugins are often released as "unstable" (alpha/beta) versions, then upgraded to "stable". // This upgrade effectively removes a plugin, which is fine because unstable plugins are // under no support contract. However users should be notified _why_ their plugin cannot be found. - var extraErrMsg string - if version != "" { + if _, version := plugin.SplitKey(pluginKey); version != "" { var ver plugin.Version if err := ver.Parse(version); err != nil { return fmt.Errorf("error parsing input plugin version from key %q: %v", pluginKey, err) } if !ver.IsStable() { - extraErrMsg = unstablePluginMsg + extraErrMsg += unstablePluginMsg } } - var resolvedPlugins []plugin.Plugin - isFullName := shortName != name - hasVersion := version != "" - - switch { - // If it is fully qualified search it - case isFullName && hasVersion: - p, isKnown := c.plugins[pluginKey] - if !isKnown { - return fmt.Errorf("unknown fully qualified plugin %q%s", pluginKey, extraErrMsg) - } - if !plugin.SupportsVersion(p, c.projectVersion) { - return fmt.Errorf("plugin %q does not support project version %q", pluginKey, c.projectVersion) - } - plugins = append(plugins, p) - continue - // Shortname with version - case hasVersion: - for _, p := range c.plugins { - // Check that the shortname and version match - if plugin.GetShortName(p.Name()) == name && p.Version().String() == version { - resolvedPlugins = append(resolvedPlugins, p) - } - } - // Full name without version - case isFullName: - for _, p := range c.plugins { - // Check that the name matches - if p.Name() == name { - resolvedPlugins = append(resolvedPlugins, p) - } - } - // Shortname without version + // Only 1 plugin can match + switch len(plugins) { + case 1: + c.resolvedPlugins = append(c.resolvedPlugins, plugins[0]) + case 0: + return fmt.Errorf("no plugin could be resolved with key %q%s", pluginKey, extraErrMsg) default: - for _, p := range c.plugins { - // Check that the shortname matches - if plugin.GetShortName(p.Name()) == name { - resolvedPlugins = append(resolvedPlugins, p) + return fmt.Errorf("ambiguous plugin %q%s", pluginKey, extraErrMsg) + } + } + + // Now we can try to resolve the project version if not known by this point + if !knownProjectVersion { + // Count how many times each supported project version appears + supportedProjectVersionCounter := make(map[config.Version]int) + for _, p := range c.resolvedPlugins { + for _, supportedProjectVersion := range p.SupportedProjectVersions() { + if _, exists := supportedProjectVersionCounter[supportedProjectVersion]; !exists { + supportedProjectVersionCounter[supportedProjectVersion] = 1 + } else { + supportedProjectVersionCounter[supportedProjectVersion]++ } } } - // Filter the ones that do not support the required project version - i := 0 - for _, resolvedPlugin := range resolvedPlugins { - if plugin.SupportsVersion(resolvedPlugin, c.projectVersion) { - resolvedPlugins[i] = resolvedPlugin - i++ + // Extract the common supported project versions + supportedProjectVersions := make([]config.Version, 0, len(supportedProjectVersionCounter)) + expectedTimes := len(c.resolvedPlugins) + for supportedProjectVersion, times := range supportedProjectVersionCounter { + if times == expectedTimes { + supportedProjectVersions = append(supportedProjectVersions, supportedProjectVersion) } } - resolvedPlugins = resolvedPlugins[:i] - // Only 1 plugin can match - switch len(resolvedPlugins) { - case 0: - return fmt.Errorf("no plugin could be resolved with key %q for project version %q%s", - pluginKey, c.projectVersion, extraErrMsg) + // If there is only one common supported project version, resolve to it + ProjectNumberVersionSwitch: + switch len(supportedProjectVersions) { case 1: - plugins = append(plugins, resolvedPlugins[0]) + c.projectVersion = supportedProjectVersions[0] + case 0: + return fmt.Errorf("no project version supported by all the resolved plugins") default: - return fmt.Errorf("ambiguous plugin %q for project version %q", pluginKey, c.projectVersion) + supportedProjectVersionStrings := make([]string, 0, len(supportedProjectVersions)) + for _, supportedProjectVersion := range supportedProjectVersions { + // In case one of the multiple supported versions is the default one, choose that and exit the switch + if supportedProjectVersion.Compare(c.defaultProjectVersion) == 0 { + c.projectVersion = c.defaultProjectVersion + break ProjectNumberVersionSwitch + } + supportedProjectVersionStrings = append(supportedProjectVersionStrings, + fmt.Sprintf("%q", supportedProjectVersion)) + } + return fmt.Errorf("ambiguous project version, resolved plugins support the following project versions: %s", + strings.Join(supportedProjectVersionStrings, ", ")) } } - c.resolvedPlugins = plugins return nil } @@ -436,26 +394,6 @@ func (c *cli) addSubcommands() { } } -// buildCmd creates the underlying cobra command and stores it internally. -func (c *cli) buildCmd() error { - c.cmd = c.newRootCmd() - - // Get project version and plugin keys. - if err := c.getInfo(); err != nil { - return err - } - - // Resolve plugins for project version and plugin keys. - if err := c.resolve(); err != nil { - return err - } - - // Add the subcommands - c.addSubcommands() - - return nil -} - // addExtraCommands adds the additional commands. func (c *cli) addExtraCommands() error { for _, cmd := range c.extraCommands { diff --git a/pkg/cli/cli_test.go b/pkg/cli/cli_test.go index a7fe02cfd02..f6b8b63ba7f 100644 --- a/pkg/cli/cli_test.go +++ b/pkg/cli/cli_test.go @@ -20,8 +20,10 @@ import ( "fmt" "io/ioutil" "os" + "strings" . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" "github.com/spf13/afero" "github.com/spf13/cobra" @@ -30,6 +32,7 @@ import ( 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" + goPluginV3 "sigs.k8s.io/kubebuilder/v3/pkg/plugins/golang/v3" ) func makeMockPluginsFor(projectVersion config.Version, pluginKeys ...string) []plugin.Plugin { @@ -76,531 +79,188 @@ func hasSubCommand(c CLI, name string) bool { } var _ = Describe("CLI", func() { + var ( + c *cli - Context("getInfoFromFlags", func() { - var ( - projectVersion string - plugins []string - err error - c *cli - ) + projectVersion = config.Version{Number: 2} + ) - // Save os.Args and restore it for every test - var args []string - BeforeEach(func() { - c = &cli{} - c.cmd = c.newRootCmd() - args = os.Args - }) - AfterEach(func() { os.Args = args }) + BeforeEach(func() { + c = &cli{ + fs: afero.NewMemMapFs(), + } + }) - When("no flag is set", func() { - It("should succeed", func() { - projectVersion, plugins, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion).To(Equal("")) - Expect(len(plugins)).To(Equal(0)) - }) - }) + // TODO: test cli.getInfoFromConfigFile using a mock filesystem - When(fmt.Sprintf("--%s flag is set", projectVersionFlag), func() { + Context("cli.getInfoFromConfig", func() { + When("not having layout field", func() { It("should succeed", func() { - setProjectVersionFlag("2") - projectVersion, plugins, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion).To(Equal("2")) - Expect(len(plugins)).To(Equal(0)) - }) - }) - - When(fmt.Sprintf("--%s flag is set", pluginsFlag), func() { - It("should succeed using one plugin key", func() { - setPluginsFlag("go/v1") - projectVersion, plugins, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion).To(Equal("")) - Expect(plugins).To(Equal([]string{"go/v1"})) - }) + projectConfig := cfgv2.New() - It("should succeed using more than one plugin key", func() { - setPluginsFlag("go/v1,example/v2,test/v1") - projectVersion, plugins, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion).To(Equal("")) - Expect(plugins).To(Equal([]string{"go/v1", "example/v2", "test/v1"})) - }) - - It("should succeed using more than one plugin key with spaces", func() { - setPluginsFlag("go/v1 , example/v2 , test/v1") - projectVersion, plugins, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion).To(Equal("")) - Expect(plugins).To(Equal([]string{"go/v1", "example/v2", "test/v1"})) + Expect(c.getInfoFromConfig(projectConfig)).To(Succeed()) + Expect(c.pluginKeys).To(Equal([]string{"go.kubebuilder.io/v2"})) + Expect(c.projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0)) }) }) - When(fmt.Sprintf("--%s and --%s flags are set", projectVersionFlag, pluginsFlag), func() { - It("should succeed using one plugin key", func() { - setProjectVersionFlag("2") - setPluginsFlag("go/v1") - projectVersion, plugins, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion).To(Equal("2")) - Expect(plugins).To(Equal([]string{"go/v1"})) - }) - - It("should succeed using more than one plugin keys", func() { - setProjectVersionFlag("2") - setPluginsFlag("go/v1,example/v2,test/v1") - projectVersion, plugins, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion).To(Equal("2")) - Expect(plugins).To(Equal([]string{"go/v1", "example/v2", "test/v1"})) - }) + When("having a single plugin in the layout field", func() { + It("should succeed", func() { + projectConfig := cfgv3.New() + plugins := []string{"go.kubebuilder.io/v2"} + Expect(projectConfig.SetLayout(strings.Join(plugins, ","))).To(Succeed()) - It("should succeed using more than one plugin keys with spaces", func() { - setProjectVersionFlag("2") - setPluginsFlag("go/v1 , example/v2 , test/v1") - projectVersion, plugins, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion).To(Equal("2")) - Expect(plugins).To(Equal([]string{"go/v1", "example/v2", "test/v1"})) + Expect(c.getInfoFromConfig(projectConfig)).To(Succeed()) + Expect(c.pluginKeys).To(Equal(plugins)) + Expect(c.projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0)) }) }) - When("additional flags are set", func() { + When("having multiple plugins in the layout field", func() { It("should succeed", func() { - setFlag("extra-flag", "extra-value") - _, _, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) - }) + projectConfig := cfgv3.New() + plugins := []string{"go.kubebuilder.io/v2", "declarative.kubebuilder.io/v1"} + Expect(projectConfig.SetLayout(strings.Join(plugins, ","))).To(Succeed()) - // `--help` is not captured by the whitelist, so we need to special case it - It("should not fail for `--help`", func() { - setBoolFlag("help") - _, _, err = c.getInfoFromFlags() - Expect(err).NotTo(HaveOccurred()) + Expect(c.getInfoFromConfig(projectConfig)).To(Succeed()) + Expect(c.pluginKeys).To(Equal(plugins)) + Expect(c.projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0)) }) }) - }) - Context("getInfoFromConfig", func() { - var ( - projectConfig config.Config - projectVersion config.Version - plugins []string - err error - ) - - When("not having layout field", func() { - It("should succeed", func() { - projectConfig = cfgv2.New() - projectVersion, plugins, err = getInfoFromConfig(projectConfig) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0)) - Expect(len(plugins)).To(Equal(0)) - }) - }) + When("having invalid plugin keys in the layout field", func() { + It("should fail", func() { + projectConfig := cfgv3.New() + plugins := []string{"go_kubebuilder.io/v2"} + Expect(projectConfig.SetLayout(strings.Join(plugins, ","))).To(Succeed()) - When("having layout field", func() { - It("should succeed", func() { - projectConfig = cfgv3.New() - Expect(projectConfig.SetLayout("go.kubebuilder.io/v2")).To(Succeed()) - projectVersion, plugins, err = getInfoFromConfig(projectConfig) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(projectConfig.GetVersion())).To(Equal(0)) - Expect(plugins).To(Equal([]string{projectConfig.GetLayout()})) + Expect(c.getInfoFromConfig(projectConfig)).NotTo(Succeed()) }) }) }) - Context("cli.resolveFlagsAndConfigFileConflicts", func() { - const ( - pluginKey1 = "go.kubebuilder.io/v1" - pluginKey2 = "go.kubebuilder.io/v2" - pluginKey3 = "go.kubebuilder.io/v3" - ) - var ( - c *cli - - projectVersion config.Version - plugins []string - err error - - projectVersion1 = config.Version{Number: 1} - projectVersion2 = config.Version{Number: 2} - projectVersion3 = config.Version{Number: 3} - ) + Context("cli.getInfoFromFlags", func() { + // Save os.Args and restore it for every test + var args []string + BeforeEach(func() { + c.cmd = c.newRootCmd() - When("having no project version set", func() { - It("should succeed", func() { - c = &cli{} - projectVersion, _, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(config.Version{})).To(Equal(0)) - }) + args = os.Args + }) + AfterEach(func() { + os.Args = args }) - When("having one project version source", func() { - When("having default project version set", func() { - It("should succeed", func() { - c = &cli{ - defaultProjectVersion: projectVersion1, - } - projectVersion, _, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(projectVersion1)).To(Equal(0)) - }) - }) - - When("having project version set from flags", func() { - It("should succeed", func() { - c = &cli{} - projectVersion, _, err = c.resolveFlagsAndConfigFileConflicts( - projectVersion1.String(), - config.Version{}, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(projectVersion1)).To(Equal(0)) - }) - }) - - When("having project version set from config file", func() { - It("should succeed", func() { - c = &cli{} - projectVersion, _, err = c.resolveFlagsAndConfigFileConflicts( - "", - projectVersion1, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(projectVersion1)).To(Equal(0)) - }) + When("no flag is set", func() { + It("should succeed", func() { + Expect(c.getInfoFromFlags()).To(Succeed()) + Expect(len(c.pluginKeys)).To(Equal(0)) + Expect(c.projectVersion.Compare(config.Version{})).To(Equal(0)) }) }) - When("having two project version source", func() { - When("having default project version set and from flags", func() { - It("should succeed", func() { - c = &cli{ - defaultProjectVersion: projectVersion1, - } - projectVersion, _, err = c.resolveFlagsAndConfigFileConflicts( - projectVersion2.String(), - config.Version{}, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(projectVersion2)).To(Equal(0)) - }) - }) + When(fmt.Sprintf("--%s flag is set", pluginsFlag), func() { + It("should succeed using one plugin key", func() { + pluginKeys := []string{"go/v1"} + setPluginsFlag(strings.Join(pluginKeys, ",")) - When("having default project version set and from config file", func() { - It("should succeed", func() { - c = &cli{ - defaultProjectVersion: projectVersion1, - } - projectVersion, _, err = c.resolveFlagsAndConfigFileConflicts( - "", - projectVersion2, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(projectVersion2)).To(Equal(0)) - }) + Expect(c.getInfoFromFlags()).To(Succeed()) + Expect(c.pluginKeys).To(Equal(pluginKeys)) + Expect(c.projectVersion.Compare(config.Version{})).To(Equal(0)) }) - When("having project version set from flags and config file", func() { - It("should succeed if they are the same", func() { - c = &cli{} - projectVersion, _, err = c.resolveFlagsAndConfigFileConflicts( - projectVersion1.String(), - projectVersion1, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(projectVersion1)).To(Equal(0)) - }) - - It("should fail if they are different", func() { - c = &cli{} - _, _, err = c.resolveFlagsAndConfigFileConflicts( - projectVersion1.String(), - projectVersion2, - nil, - nil, - ) - Expect(err).To(HaveOccurred()) - }) - }) - }) + It("should succeed using more than one plugin key", func() { + pluginKeys := []string{"go/v1", "example/v2", "test/v1"} + setPluginsFlag(strings.Join(pluginKeys, ",")) - When("having three project version sources", func() { - It("should succeed if project version from flags and config file are the same", func() { - c = &cli{ - defaultProjectVersion: projectVersion1, - } - projectVersion, _, err = c.resolveFlagsAndConfigFileConflicts( - projectVersion2.String(), - projectVersion2, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(projectVersion.Compare(projectVersion2)).To(Equal(0)) + Expect(c.getInfoFromFlags()).To(Succeed()) + Expect(c.pluginKeys).To(Equal(pluginKeys)) + Expect(c.projectVersion.Compare(config.Version{})).To(Equal(0)) }) - It("should fail if project version from flags and config file are different", func() { - c = &cli{ - defaultProjectVersion: projectVersion1, - } - _, _, err = c.resolveFlagsAndConfigFileConflicts( - projectVersion2.String(), - projectVersion3, - nil, - nil, - ) - Expect(err).To(HaveOccurred()) - }) - }) + It("should succeed using more than one plugin key with spaces", func() { + pluginKeys := []string{"go/v1", "example/v2", "test/v1"} + setPluginsFlag(strings.Join(pluginKeys, ", ")) - When("an invalid project version is set", func() { - It("should fail", func() { - c = &cli{} - projectVersion, _, err = c.resolveFlagsAndConfigFileConflicts( - "0", - config.Version{}, - nil, - nil, - ) - Expect(err).To(HaveOccurred()) + Expect(c.getInfoFromFlags()).To(Succeed()) + Expect(c.pluginKeys).To(Equal(pluginKeys)) + Expect(c.projectVersion.Compare(config.Version{})).To(Equal(0)) }) }) - When("having no plugin keys set", func() { + When(fmt.Sprintf("--%s flag is set", projectVersionFlag), func() { It("should succeed", func() { - c = &cli{} - _, plugins, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(len(plugins)).To(Equal(0)) + setProjectVersionFlag(projectVersion.String()) + + Expect(c.getInfoFromFlags()).To(Succeed()) + Expect(len(c.pluginKeys)).To(Equal(0)) + Expect(c.projectVersion.Compare(projectVersion)).To(Equal(0)) }) }) - When("having one plugin keys source", func() { - When("having default plugin keys set", func() { - It("should succeed", func() { - c = &cli{ - defaultProjectVersion: projectVersion1, - defaultPlugins: map[config.Version][]string{ - projectVersion1: {pluginKey1}, - projectVersion2: {pluginKey2}, - }, - } - _, plugins, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - nil, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(len(plugins)).To(Equal(1)) - Expect(plugins[0]).To(Equal(pluginKey1)) - }) - }) + When(fmt.Sprintf("--%s and --%s flags are set", pluginsFlag, projectVersionFlag), func() { + It("should succeed using one plugin key", func() { + pluginKeys := []string{"go/v1"} + setPluginsFlag(strings.Join(pluginKeys, ",")) + setProjectVersionFlag(projectVersion.String()) - When("having plugin keys set from flags", func() { - It("should succeed", func() { - c = &cli{} - _, plugins, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - []string{pluginKey1}, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(len(plugins)).To(Equal(1)) - Expect(plugins[0]).To(Equal(pluginKey1)) - }) + Expect(c.getInfoFromFlags()).To(Succeed()) + Expect(c.pluginKeys).To(Equal(pluginKeys)) + Expect(c.projectVersion.Compare(projectVersion)).To(Equal(0)) }) - When("having plugin keys set from config file", func() { - It("should succeed", func() { - c = &cli{} - _, plugins, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - nil, - []string{pluginKey1}, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(len(plugins)).To(Equal(1)) - Expect(plugins[0]).To(Equal(pluginKey1)) - }) - }) - }) + It("should succeed using more than one plugin key", func() { + pluginKeys := []string{"go/v1", "example/v2", "test/v1"} + setPluginsFlag(strings.Join(pluginKeys, ",")) + setProjectVersionFlag(projectVersion.String()) - When("having two plugin keys source", func() { - When("having default plugin keys set and from flags", func() { - It("should succeed", func() { - c = &cli{ - defaultPlugins: map[config.Version][]string{ - {}: {pluginKey1}, - }, - } - _, plugins, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - []string{pluginKey2}, - nil, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(len(plugins)).To(Equal(1)) - Expect(plugins[0]).To(Equal(pluginKey2)) - }) + Expect(c.getInfoFromFlags()).To(Succeed()) + Expect(c.pluginKeys).To(Equal(pluginKeys)) + Expect(c.projectVersion.Compare(projectVersion)).To(Equal(0)) }) - When("having default plugin keys set and from config file", func() { - It("should succeed", func() { - c = &cli{ - defaultPlugins: map[config.Version][]string{ - {}: {pluginKey1}, - }, - } - _, plugins, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - nil, - []string{pluginKey2}, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(len(plugins)).To(Equal(1)) - Expect(plugins[0]).To(Equal(pluginKey2)) - }) - }) + It("should succeed using more than one plugin key with spaces", func() { + pluginKeys := []string{"go/v1", "example/v2", "test/v1"} + setPluginsFlag(strings.Join(pluginKeys, ", ")) + setProjectVersionFlag(projectVersion.String()) - When("having plugin keys set from flags and config file", func() { - It("should succeed if they are the same", func() { - c = &cli{} - _, plugins, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - []string{pluginKey1}, - []string{pluginKey1}, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(len(plugins)).To(Equal(1)) - Expect(plugins[0]).To(Equal(pluginKey1)) - }) - - It("should fail if they are different", func() { - c = &cli{} - _, _, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - []string{pluginKey1}, - []string{pluginKey2}, - ) - Expect(err).To(HaveOccurred()) - }) + Expect(c.getInfoFromFlags()).To(Succeed()) + Expect(c.pluginKeys).To(Equal(pluginKeys)) + Expect(c.projectVersion.Compare(projectVersion)).To(Equal(0)) }) }) - 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}, - }, - } - _, plugins, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - []string{pluginKey2}, - []string{pluginKey2}, - ) - Expect(err).NotTo(HaveOccurred()) - Expect(len(plugins)).To(Equal(1)) - Expect(plugins[0]).To(Equal(pluginKey2)) - }) + When("additional flags are set", func() { + It("should succeed", func() { + setFlag("extra-flag", "extra-value") - It("should fail if plugin keys from flags and config file are different", func() { - c = &cli{ - defaultPlugins: map[config.Version][]string{ - {}: {pluginKey1}, - }, - } - _, _, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - []string{pluginKey2}, - []string{pluginKey3}, - ) - Expect(err).To(HaveOccurred()) + Expect(c.getInfoFromFlags()).To(Succeed()) }) - }) - When("an invalid plugin key is set", func() { - It("should fail", func() { - c = &cli{} - _, plugins, err = c.resolveFlagsAndConfigFileConflicts( - "", - config.Version{}, - []string{"A"}, - nil, - ) - Expect(err).To(HaveOccurred()) + // `--help` is not captured by the allowlist, so we need to special case it + It("should not fail for `--help`", func() { + setBoolFlag("help") + + Expect(c.getInfoFromFlags()).To(Succeed()) }) }) }) - // NOTE: only flag info can be tested with cli.getInfo as the config file doesn't exist, - // previous tests ensure that the info from config files is read properly and that - // conflicts are solved appropriately. Context("cli.getInfo", func() { - It("should set project version and plugin keys", func() { - projectVersion := config.Version{Number: 2} + It("should set default plugin keys but not project version", func() { pluginKeys := []string{"go.kubebuilder.io/v2"} - c := &cli{ - defaultProjectVersion: projectVersion, - defaultPlugins: map[config.Version][]string{ - projectVersion: pluginKeys, - }, - fs: afero.NewMemMapFs(), - } + c.defaultPlugins = pluginKeys + c.defaultProjectVersion = projectVersion c.cmd = c.newRootCmd() + Expect(c.getInfo()).To(Succeed()) - Expect(c.projectVersion.Compare(projectVersion)).To(Equal(0)) Expect(c.pluginKeys).To(Equal(pluginKeys)) + Expect(c.projectVersion.Compare(config.Version{})).To(Equal(0)) }) }) - Context("cli.resolve", func() { + Context("cli.resolvePlugins", func() { var ( - c *cli - - projectVersion = config.Version{Number: 2} - pluginKeys = []string{ "foo.example.com/v1", "bar.example.com/v1", @@ -616,45 +276,40 @@ var _ = Describe("CLI", func() { plugins = append(plugins, newMockPlugin("invalid.kubebuilder.io", "v1")) pluginMap := makeMapFor(plugins...) - for key, qualified := range map[string]string{ - "foo.example.com/v1": "foo.example.com/v1", - "foo.example.com": "foo.example.com/v1", - "baz": "baz.example.com/v1", - "foo/v2": "foo.kubebuilder.io/v2", - } { - key, qualified := key, qualified - It(fmt.Sprintf("should resolve %q", key), func() { - c = &cli{ - plugins: pluginMap, - projectVersion: projectVersion, - pluginKeys: []string{key}, - } - Expect(c.resolve()).To(Succeed()) + BeforeEach(func() { + c.plugins = pluginMap + c.projectVersion = projectVersion + }) + + DescribeTable("should resolve", + func(key, qualified string) { + c.pluginKeys = []string{key} + + Expect(c.resolvePlugins()).To(Succeed()) Expect(len(c.resolvedPlugins)).To(Equal(1)) Expect(plugin.KeyFor(c.resolvedPlugins[0])).To(Equal(qualified)) - }) - } + }, + Entry("fully qualified plugin", "foo.example.com/v1", "foo.example.com/v1"), + Entry("plugin without version", "foo.example.com", "foo.example.com/v1"), + Entry("shortname without version", "baz", "baz.example.com/v1"), + Entry("shortname with version", "foo/v2", "foo.kubebuilder.io/v2"), + ) - for _, key := range []string{ - "foo.kubebuilder.io", - "foo/v1", - "foo", - "blah", - "foo.example.com/v2", - "foo/v3", - "foo.example.com/v3", - "invalid.kubebuilder.io/v1", - } { - key := key - It(fmt.Sprintf("should not resolve %q", key), func() { - c = &cli{ - plugins: pluginMap, - projectVersion: projectVersion, - pluginKeys: []string{key}, - } - Expect(c.resolve()).NotTo(Succeed()) - }) - } + DescribeTable("should not resolve", + func(key string) { + c.pluginKeys = []string{key} + + Expect(c.resolvePlugins()).NotTo(Succeed()) + }, + Entry("for an ambiguous version", "foo.kubebuilder.io"), + Entry("for an ambiguous name", "foo/v1"), + Entry("for an ambiguous name and version", "foo"), + Entry("for a non-existent name", "blah"), + Entry("for a non-existent version", "foo.example.com/v2"), + Entry("for a non-existent version", "foo/v3"), + Entry("for a non-existent version", "foo.example.com/v3"), + Entry("for a plugin that doesn't support the project version", "invalid.kubebuilder.io/v1"), + ) }) Context("New", func() { @@ -674,7 +329,11 @@ var _ = Describe("CLI", func() { When("providing a version string", func() { It("should create a valid CLI", func() { const version = "version string" - c, err = New(WithVersion(version)) + c, err = New( + WithPlugins(&goPluginV3.Plugin{}), + WithDefaultPlugins(&goPluginV3.Plugin{}), + WithVersion(version), + ) Expect(err).NotTo(HaveOccurred()) Expect(hasSubCommand(c, "version")).To(BeTrue()) }) @@ -682,7 +341,11 @@ var _ = Describe("CLI", func() { When("enabling completion", func() { It("should create a valid CLI", func() { - c, err = New(WithCompletion) + c, err = New( + WithPlugins(&goPluginV3.Plugin{}), + WithDefaultPlugins(&goPluginV3.Plugin{}), + WithCompletion, + ) Expect(err).NotTo(HaveOccurred()) Expect(hasSubCommand(c, "completion")).To(BeTrue()) }) @@ -704,8 +367,19 @@ var _ = Describe("CLI", func() { It("should return a CLI that returns an error", func() { setPluginsFlag("foo") + c, err = New() Expect(err).NotTo(HaveOccurred()) + + // Overwrite stderr to read the output and reset it afterwards + _, w, _ := os.Pipe() + temp := os.Stderr + defer func() { + os.Stderr = temp + _ = w.Close() + }() + os.Stderr = w + Expect(c.Run()).NotTo(Succeed()) }) }) @@ -715,14 +389,22 @@ var _ = Describe("CLI", func() { It("should create a valid CLI for non-conflicting ones", func() { extraCommand = &cobra.Command{Use: "extra"} - c, err = New(WithExtraCommands(extraCommand)) + c, err = New( + WithPlugins(&goPluginV3.Plugin{}), + WithDefaultPlugins(&goPluginV3.Plugin{}), + WithExtraCommands(extraCommand), + ) Expect(err).NotTo(HaveOccurred()) Expect(hasSubCommand(c, extraCommand.Use)).To(BeTrue()) }) It("should return an error for conflicting ones", func() { extraCommand = &cobra.Command{Use: "init"} - _, err = New(WithExtraCommands(extraCommand)) + c, err = New( + WithPlugins(&goPluginV3.Plugin{}), + WithDefaultPlugins(&goPluginV3.Plugin{}), + WithExtraCommands(extraCommand), + ) Expect(err).To(HaveOccurred()) }) }) @@ -733,7 +415,6 @@ var _ = Describe("CLI", func() { deprecationWarning = "DEPRECATED" ) var ( - projectVersion = config.Version{Number: 2} deprecatedPlugin = newMockDeprecatedPlugin("deprecated", "v1", deprecationWarning, projectVersion) ) @@ -746,11 +427,13 @@ var _ = Describe("CLI", func() { os.Stdout = w c, err = New( - WithDefaultProjectVersion(projectVersion), - WithDefaultPlugins(projectVersion, deprecatedPlugin), WithPlugins(deprecatedPlugin), + WithDefaultProjectVersion(projectVersion), + WithDefaultPlugins(deprecatedPlugin), ) + _ = w.Close() + Expect(err).NotTo(HaveOccurred()) printed, _ := ioutil.ReadAll(r) Expect(string(printed)).To(Equal( diff --git a/pkg/cli/init.go b/pkg/cli/init.go index f7f38ce8564..b6fe075d198 100644 --- a/pkg/cli/init.go +++ b/pkg/cli/init.go @@ -28,7 +28,6 @@ import ( "sigs.k8s.io/kubebuilder/v3/pkg/config" yamlstore "sigs.k8s.io/kubebuilder/v3/pkg/config/store/yaml" - cfgv2 "sigs.k8s.io/kubebuilder/v3/pkg/config/v2" "sigs.k8s.io/kubebuilder/v3/pkg/plugin" ) @@ -48,14 +47,7 @@ For further help about a specific project version, set --project-version. // Register --project-version on the dynamically created command // so that it shows up in help and does not cause a parse error. - cmd.Flags().String(projectVersionFlag, c.defaultProjectVersion.String(), - fmt.Sprintf("project version, possible values: (%s)", strings.Join(c.getAvailableProjectVersions(), ", "))) - // The --plugins flag can only be called to init projects v2+. - if c.projectVersion.Compare(cfgv2.Version) == 1 { - cmd.Flags().StringSlice(pluginsFlag, nil, - "Name and optionally version of the plugin to initialize the project with. "+ - fmt.Sprintf("Available plugins: (%s)", strings.Join(c.getAvailablePlugins(), ", "))) - } + cmd.Flags().String(projectVersionFlag, c.defaultProjectVersion.String(), "project version") // In case no plugin was resolved, instead of failing the construction of the CLI, fail the execution of // this subcommand. This allows the use of subcommands that do not require resolved plugins like help. diff --git a/pkg/cli/options.go b/pkg/cli/options.go index df89dd5c649..aea40ac8a45 100644 --- a/pkg/cli/options.go +++ b/pkg/cli/options.go @@ -44,53 +44,47 @@ func WithVersion(version string) Option { } } -// WithDefaultProjectVersion is an Option that sets the cli's default project version. -// Setting an unknown version will result in an error. -func WithDefaultProjectVersion(version config.Version) Option { +// WithPlugins is an Option that sets the cli's plugins. +func WithPlugins(plugins ...plugin.Plugin) Option { return func(c *cli) error { - if err := version.Validate(); err != nil { - return fmt.Errorf("broken pre-set default project version %q: %v", version, err) + for _, p := range plugins { + key := plugin.KeyFor(p) + if _, isConflicting := c.plugins[key]; isConflicting { + return fmt.Errorf("two plugins have the same key: %q", key) + } + if err := plugin.Validate(p); err != nil { + return fmt.Errorf("broken pre-set plugin %q: %v", key, err) + } + c.plugins[key] = p } - c.defaultProjectVersion = version return nil } } // 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 as default plugins") } 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 } } -// WithPlugins is an Option that sets the cli's plugins. -func WithPlugins(plugins ...plugin.Plugin) Option { +// WithDefaultProjectVersion is an Option that sets the cli's default project version. +// Setting an unknown version will result in an error. +func WithDefaultProjectVersion(version config.Version) Option { return func(c *cli) error { - for _, p := range plugins { - key := plugin.KeyFor(p) - if _, isConflicting := c.plugins[key]; isConflicting { - return fmt.Errorf("two plugins have the same key: %q", key) - } - if err := plugin.Validate(p); err != nil { - return fmt.Errorf("broken pre-set plugin %q: %v", key, err) - } - c.plugins[key] = p + if err := version.Validate(); err != nil { + return fmt.Errorf("broken pre-set default project version %q: %v", version, err) } + c.defaultProjectVersion = version return nil } } diff --git a/pkg/cli/options_test.go b/pkg/cli/options_test.go index 88ebe1b4b2c..9f846384e5b 100644 --- a/pkg/cli/options_test.go +++ b/pkg/cli/options_test.go @@ -17,9 +17,8 @@ limitations under the License. package cli import ( - "fmt" - . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" "github.com/spf13/cobra" @@ -68,144 +67,124 @@ var _ = Describe("CLI options", func() { }) }) - Context("WithDefaultProjectVersion", func() { - It("should return a valid CLI", func() { - defaultProjectVersions := []config.Version{ - {Number: 1}, - {Number: 2}, - {Number: 3, Stage: stage.Alpha}, - } - for _, defaultProjectVersion := range defaultProjectVersions { - By(fmt.Sprintf("using %q", defaultProjectVersion)) - c, err = newCLI(WithDefaultProjectVersion(defaultProjectVersion)) - Expect(err).NotTo(HaveOccurred()) - Expect(c).NotTo(BeNil()) - Expect(c.defaultProjectVersion).To(Equal(defaultProjectVersion)) - } - }) - - It("should return an error", func() { - defaultProjectVersions := []config.Version{ - {}, // Empty default project version - {Number: 1, Stage: stage.Stage(27)}, // Invalid stage in default project version - } - for _, defaultProjectVersion := range defaultProjectVersions { - By(fmt.Sprintf("using %q", defaultProjectVersion)) - _, err = newCLI(WithDefaultProjectVersion(defaultProjectVersion)) - Expect(err).To(HaveOccurred()) - } - }) - }) - - Context("WithDefaultPlugins", func() { + Context("WithPlugins", func() { It("should return a valid CLI", func() { - c, err = newCLI(WithDefaultPlugins(projectVersion, p)) + c, err = newCLI(WithPlugins(p)) Expect(err).NotTo(HaveOccurred()) Expect(c).NotTo(BeNil()) - Expect(c.defaultPlugins).To(Equal(map[config.Version][]string{projectVersion: {plugin.KeyFor(p)}})) + Expect(c.plugins).To(Equal(map[string]plugin.Plugin{plugin.KeyFor(p): p})) }) - When("providing an invalid project version", func() { + When("providing plugins with same keys", func() { It("should return an error", func() { - _, err = newCLI(WithDefaultPlugins(config.Version{}, p)) + _, err = newCLI(WithPlugins(p, p)) Expect(err).To(HaveOccurred()) }) }) - When("providing an empty set of plugins", func() { + When("providing plugins with same keys in two steps", func() { It("should return an error", func() { - _, err = newCLI(WithDefaultPlugins(projectVersion)) + _, err = newCLI(WithPlugins(p), WithPlugins(p)) 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(WithPlugins(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(WithPlugins(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(WithPlugins(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(WithPlugins(np4)) Expect(err).To(HaveOccurred()) }) }) }) - Context("WithPlugins", func() { + Context("WithDefaultPlugins", func() { It("should return a valid CLI", func() { - c, err = newCLI(WithPlugins(p)) + c, err = newCLI(WithDefaultPlugins(p)) Expect(err).NotTo(HaveOccurred()) Expect(c).NotTo(BeNil()) - Expect(c.plugins).To(Equal(map[string]plugin.Plugin{plugin.KeyFor(p): p})) - }) - - When("providing plugins with same keys", func() { - It("should return an error", func() { - _, err = newCLI(WithPlugins(p, p)) - Expect(err).To(HaveOccurred()) - }) + Expect(c.defaultPlugins).To(Equal([]string{plugin.KeyFor(p)})) }) - When("providing plugins with same keys in two steps", func() { + When("providing an empty set of plugins", func() { It("should return an error", func() { - _, err = newCLI(WithPlugins(p), WithPlugins(p)) + _, err = newCLI(WithDefaultPlugins()) Expect(err).To(HaveOccurred()) }) }) When("providing a plugin with an invalid name", func() { It("should return an error", func() { - _, err = newCLI(WithPlugins(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(WithPlugins(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(WithPlugins(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(WithPlugins(np4)) + _, err = newCLI(WithDefaultPlugins(np4)) Expect(err).To(HaveOccurred()) }) }) }) + Context("WithDefaultProjectVersion", func() { + DescribeTable("should return a valid CLI", + func(projectVersion config.Version) { + c, err = newCLI(WithDefaultProjectVersion(projectVersion)) + Expect(err).NotTo(HaveOccurred()) + Expect(c).NotTo(BeNil()) + Expect(c.defaultProjectVersion).To(Equal(projectVersion)) + }, + Entry("for version `2`", config.Version{Number: 2}), + Entry("for version `3-alpha`", config.Version{Number: 3, Stage: stage.Alpha}), + Entry("for version `3`", config.Version{Number: 3}), + ) + + DescribeTable("should fail", + func(projectVersion config.Version) { + _, err = newCLI(WithDefaultProjectVersion(projectVersion)) + Expect(err).To(HaveOccurred()) + }, + Entry("for empty version", config.Version{}), + Entry("for invalid stage", config.Version{Number: 1, Stage: stage.Stage(27)}), + ) + }) + Context("WithExtraCommands", func() { It("should return a valid CLI with extra commands", func() { commandTest := &cobra.Command{ diff --git a/pkg/cli/root.go b/pkg/cli/root.go index e0baa398180..7c960d19e4a 100644 --- a/pkg/cli/root.go +++ b/pkg/cli/root.go @@ -40,14 +40,11 @@ func (c cli) newRootCmd() *cobra.Command { }, } - // Global flags for all subcommands - // NOTE: the current plugin resolution doesn't allow to provide values to this flag different to those configured - // for the project, so default values need to be empty and considered when these two sources are compared. - // Another approach would be to allow users to overwrite the project configuration values. In this case, flags - // would take precedence over project configuration, which would take precedence over cli defaults. - fs := cmd.PersistentFlags() - fs.String(projectVersionFlag, "", "project version") - fs.StringSlice(pluginsFlag, nil, "plugin keys of the plugin to initialize the project with") + // Global flags for all subcommands. + cmd.PersistentFlags().StringSlice(pluginsFlag, nil, "plugin keys to be used for this subcommand execution") + + // Register --project-version on the root command so that it shows up in help. + cmd.Flags().String(projectVersionFlag, c.defaultProjectVersion.String(), "project version") // As the root command will be used to shot the help message under some error conditions, // like during plugin resolving, we need to allow unknown flags to prevent parsing errors. @@ -56,10 +53,10 @@ func (c cli) newRootCmd() *cobra.Command { return cmd } -// rootExamples builds the examples string for the root command +// rootExamples builds the examples string for the root command before resolving plugins func (c cli) rootExamples() string { str := fmt.Sprintf(`The first step is to initialize your project: - %[1]s init --project-version= --plugins= + %[1]s init [--plugins= [--project-version=]] is a comma-separated list of plugin keys from the following table and a supported project version for these plugins. @@ -68,14 +65,16 @@ and a supported project version for these plugins. For more specific help for the init command of a certain plugins and project version configuration please run: - %[1]s init --help --project-version= --plugins= + %[1]s init --help --plugins= [--project-version=] `, c.commandName, c.getPluginTable()) - str += fmt.Sprintf("\nDefault project version: %s\n", c.defaultProjectVersion) + if len(c.defaultPlugins) != 0 { + str += fmt.Sprintf("\nDefault plugin keys: %q\n", strings.Join(c.defaultPlugins, ",")) + } - if defaultPlugins, hasDefaultPlugins := c.defaultPlugins[c.defaultProjectVersion]; hasDefaultPlugins { - str += fmt.Sprintf("Default plugin keys: %q\n", strings.Join(defaultPlugins, ",")) + if c.defaultProjectVersion.Validate() == nil { + str += fmt.Sprintf("\nDefault project version: %q\n", c.defaultProjectVersion) } str += fmt.Sprintf(` diff --git a/pkg/plugin/filter.go b/pkg/plugin/filter.go new file mode 100644 index 00000000000..9a690263342 --- /dev/null +++ b/pkg/plugin/filter.go @@ -0,0 +1,61 @@ +/* +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 plugin + +import ( + "strings" + + "sigs.k8s.io/kubebuilder/v3/pkg/config" +) + +// FilterPluginsByKey returns the set of plugins that match the provided key (may be not-fully qualified) +func FilterPluginsByKey(plugins []Plugin, key string) ([]Plugin, error) { + name, ver := SplitKey(key) + hasVersion := ver != "" + var version Version + if hasVersion { + if err := version.Parse(ver); err != nil { + return nil, err + } + } + + filtered := make([]Plugin, 0, len(plugins)) + for _, plugin := range plugins { + if !strings.HasPrefix(plugin.Name(), name) { + continue + } + if hasVersion && plugin.Version().Compare(version) != 0 { + continue + } + filtered = append(filtered, plugin) + } + return filtered, nil +} + +// FilterPluginsByProjectVersion returns the set of plugins that support the provided project version +func FilterPluginsByProjectVersion(plugins []Plugin, projectVersion config.Version) []Plugin { + filtered := make([]Plugin, 0, len(plugins)) + for _, plugin := range plugins { + for _, supportedVersion := range plugin.SupportedProjectVersions() { + if supportedVersion.Compare(projectVersion) == 0 { + filtered = append(filtered, plugin) + break + } + } + } + return filtered +} diff --git a/pkg/plugin/filter_test.go b/pkg/plugin/filter_test.go new file mode 100644 index 00000000000..2b45dcb6fed --- /dev/null +++ b/pkg/plugin/filter_test.go @@ -0,0 +1,84 @@ +/* +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 plugin + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" + + "sigs.k8s.io/kubebuilder/v3/pkg/config" +) + +var ( + p1 = mockPlugin{ + name: "go.kubebuilder.io", + version: Version{Number: 2}, + supportedProjectVersions: []config.Version{{Number: 2}, {Number: 3}}, + } + p2 = mockPlugin{ + name: "go.kubebuilder.io", + version: Version{Number: 3}, + supportedProjectVersions: []config.Version{{Number: 3}}, + } + p3 = mockPlugin{ + name: "example.kubebuilder.io", + version: Version{Number: 1}, + supportedProjectVersions: []config.Version{{Number: 2}}, + } + p4 = mockPlugin{ + name: "test.kubebuilder.io", + version: Version{Number: 1}, + supportedProjectVersions: []config.Version{{Number: 3}}, + } + p5 = mockPlugin{ + name: "go.test.domain", + version: Version{Number: 2}, + supportedProjectVersions: []config.Version{{Number: 2}}, + } + + allPlugins = []Plugin{p1, p2, p3, p4, p5} +) + +var _ = Describe("FilterPluginsByKey", func() { + DescribeTable("should filter", + func(key string, plugins []Plugin) { + filtered, err := FilterPluginsByKey(allPlugins, key) + Expect(err).NotTo(HaveOccurred()) + Expect(filtered).To(Equal(plugins)) + }, + Entry("go plugins", "go", []Plugin{p1, p2, p5}), + Entry("go plugins (kubebuilder domain)", "go.kubebuilder", []Plugin{p1, p2}), + Entry("go v2 plugins", "go/v2", []Plugin{p1, p5}), + Entry("go v2 plugins (kubebuilder domain)", "go.kubebuilder/v2", []Plugin{p1}), + ) + + It("should fail for invalid versions", func() { + _, err := FilterPluginsByKey(allPlugins, "go/a") + Expect(err).To(HaveOccurred()) + }) +}) + +var _ = Describe("FilterPluginsByKey", func() { + DescribeTable("should filter", + func(projectVersion config.Version, plugins []Plugin) { + Expect(FilterPluginsByProjectVersion(allPlugins, projectVersion)).To(Equal(plugins)) + }, + Entry("project v2 plugins", config.Version{Number: 2}, []Plugin{p1, p3, p5}), + Entry("project v3 plugins", config.Version{Number: 3}, []Plugin{p1, p2, p4}), + ) +}) diff --git a/pkg/plugin/helpers.go b/pkg/plugin/helpers.go index e9e7aa68691..ceadae61019 100644 --- a/pkg/plugin/helpers.go +++ b/pkg/plugin/helpers.go @@ -21,7 +21,6 @@ import ( "path" "strings" - "sigs.k8s.io/kubebuilder/v3/pkg/config" "sigs.k8s.io/kubebuilder/v3/pkg/internal/validation" ) @@ -39,12 +38,6 @@ func SplitKey(key string) (string, string) { return keyParts[0], keyParts[1] } -// GetShortName returns plugin's short name (name before domain) if name -// is fully qualified (has a domain suffix), otherwise GetShortName returns name. -func GetShortName(name string) string { - return strings.SplitN(name, ".", 2)[0] -} - // Validate ensures a Plugin is valid. func Validate(p Plugin) error { if err := validateName(p.Name()); err != nil { @@ -87,13 +80,3 @@ func validateName(name string) error { } return nil } - -// SupportsVersion checks if a plugins supports a project version. -func SupportsVersion(p Plugin, projectVersion config.Version) bool { - for _, version := range p.SupportedProjectVersions() { - if projectVersion.Compare(version) == 0 { - return true - } - } - return false -} diff --git a/pkg/plugin/helpers_test.go b/pkg/plugin/helpers_test.go index 0a2856b1b5a..09f1b04692e 100644 --- a/pkg/plugin/helpers_test.go +++ b/pkg/plugin/helpers_test.go @@ -22,23 +22,11 @@ import ( . "github.com/onsi/gomega" "sigs.k8s.io/kubebuilder/v3/pkg/config" - "sigs.k8s.io/kubebuilder/v3/pkg/model/stage" ) -type mockPlugin struct { - name string - version Version - supportedProjectVersions []config.Version -} - -func (p mockPlugin) Name() string { return p.name } -func (p mockPlugin) Version() Version { return p.version } -func (p mockPlugin) SupportedProjectVersions() []config.Version { return p.supportedProjectVersions } - const ( - short = "go" - name = "go.kubebuilder.io" - key = "go.kubebuilder.io/v1" + name = "go.kubebuilder.io" + key = "go.kubebuilder.io/v1" ) var ( @@ -73,12 +61,6 @@ var _ = Describe("SplitKey", func() { }) }) -var _ = Describe("GetShortName", func() { - It("should extract base names from domains", func() { - Expect(GetShortName(name)).To(Equal(short)) - }) -}) - var _ = Describe("Validate", func() { It("should succeed for valid plugins", func() { plugin := mockPlugin{ @@ -129,19 +111,3 @@ var _ = Describe("ValidateKey", func() { Entry("for invalid versions", "go.kubebuilder.io/a"), ) }) - -var _ = Describe("SupportsVersion", func() { - plugin := mockPlugin{ - supportedProjectVersions: supportedProjectVersions, - } - - It("should return true for supported versions", func() { - Expect(SupportsVersion(plugin, config.Version{Number: 2})).To(BeTrue()) - Expect(SupportsVersion(plugin, config.Version{Number: 3})).To(BeTrue()) - }) - - It("should return false for non-supported versions", func() { - Expect(SupportsVersion(plugin, config.Version{Number: 1})).To(BeFalse()) - Expect(SupportsVersion(plugin, config.Version{Number: 3, Stage: stage.Alpha})).To(BeFalse()) - }) -}) diff --git a/pkg/plugin/suite_test.go b/pkg/plugin/suite_test.go index 35484a744c7..059ac751444 100644 --- a/pkg/plugin/suite_test.go +++ b/pkg/plugin/suite_test.go @@ -21,9 +21,21 @@ import ( . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + + "sigs.k8s.io/kubebuilder/v3/pkg/config" ) func TestPlugin(t *testing.T) { RegisterFailHandler(Fail) RunSpecs(t, "Plugin Suite") } + +type mockPlugin struct { + name string + version Version + supportedProjectVersions []config.Version +} + +func (p mockPlugin) Name() string { return p.name } +func (p mockPlugin) Version() Version { return p.version } +func (p mockPlugin) SupportedProjectVersions() []config.Version { return p.supportedProjectVersions } diff --git a/test/e2e/v2/plugin_cluster_test.go b/test/e2e/v2/plugin_cluster_test.go index 6832259788a..e38527b9e39 100644 --- a/test/e2e/v2/plugin_cluster_test.go +++ b/test/e2e/v2/plugin_cluster_test.go @@ -66,6 +66,7 @@ var _ = Describe("kubebuilder", func() { var controllerPodName string By("init v2 project") err := kbc.Init( + "--plugins", "go/v2", "--project-version", "2", "--domain", kbc.Domain, "--fetch-deps=false") diff --git a/test/e2e/v3/generate_test.go b/test/e2e/v3/generate_test.go index e754973743f..ed4f6117097 100644 --- a/test/e2e/v3/generate_test.go +++ b/test/e2e/v3/generate_test.go @@ -34,8 +34,8 @@ func GenerateV2(kbc *utils.TestContext) { By("initializing a project") err = kbc.Init( - "--project-version", "3", "--plugins", "go/v2", + "--project-version", "3", "--domain", kbc.Domain, "--fetch-deps=false", ) @@ -129,8 +129,8 @@ func GenerateV3(kbc *utils.TestContext, crdAndWebhookVersion string) { By("initializing a project") err = kbc.Init( - "--project-version", "3", "--plugins", "go/v3", + "--project-version", "3", "--domain", kbc.Domain, "--fetch-deps=false", ) diff --git a/test/testdata/generate.sh b/test/testdata/generate.sh index e9a15ae0217..607c2c0a996 100755 --- a/test/testdata/generate.sh +++ b/test/testdata/generate.sh @@ -111,9 +111,9 @@ 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=3 --plugins="go/v2,declarative" +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,declarative" # Project version 3 (default) uses plugin go/v3 (default). scaffold_test_project project-v3 scaffold_test_project project-v3-multigroup