Merge pull request #1748 from Adirio/plugin-subcommand-concept

⚠️ [Rename] Disambiguate plugin by calling subcommand what the plugin getters returns instead of plugin

Author: k8s-ci-robot

pkg/cli/api.go

@@ -56,38 +56,38 @@ func (c cli) newAPIContext() plugin.Context {
 
 // nolint:dupl
 func (c cli) bindCreateAPI(ctx plugin.Context, cmd *cobra.Command) {
-	var getter plugin.CreateAPIPluginGetter
+	var createAPIPlugin plugin.CreateAPI
 	for _, p := range c.resolvedPlugins {
-		tmpGetter, isGetter := p.(plugin.CreateAPIPluginGetter)
-		if isGetter {
-			if getter != nil {
-				err := fmt.Errorf("duplicate API creation plugins for project version %q (%s, %s), "+
-					"use a more specific plugin key", c.projectVersion, plugin.KeyFor(getter), plugin.KeyFor(p))
+		tmpPlugin, isValid := p.(plugin.CreateAPI)
+		if isValid {
+			if createAPIPlugin != nil {
+				err := fmt.Errorf("duplicate API creation plugins (%s, %s), use a more specific plugin key",
+					plugin.KeyFor(createAPIPlugin), plugin.KeyFor(p))
 				cmdErr(cmd, err)
 				return
 			}
-			getter = tmpGetter
+			createAPIPlugin = tmpPlugin
 		}
 	}
 
-	cfg, err := config.LoadInitialized()
-	if err != nil {
+	if createAPIPlugin == nil {
+		err := fmt.Errorf("relevant plugins do not provide an API creation plugin")
 		cmdErr(cmd, err)
 		return
 	}
 
-	if getter == nil {
-		err := fmt.Errorf("layout plugin %q does not support an API creation plugin", cfg.Layout)
+	cfg, err := config.LoadInitialized()
+	if err != nil {
 		cmdErr(cmd, err)
 		return
 	}
 
-	createAPI := getter.GetCreateAPIPlugin()
-	createAPI.InjectConfig(&cfg.Config)
-	createAPI.BindFlags(cmd.Flags())
-	createAPI.UpdateContext(&ctx)
+	subcommand := createAPIPlugin.GetCreateAPISubcommand()
+	subcommand.InjectConfig(&cfg.Config)
+	subcommand.BindFlags(cmd.Flags())
+	subcommand.UpdateContext(&ctx)
 	cmd.Long = ctx.Description
 	cmd.Example = ctx.Examples
-	cmd.RunE = runECmdFunc(cfg, createAPI,
-		fmt.Sprintf("failed to create API with version %q", c.projectVersion))
+	cmd.RunE = runECmdFunc(cfg, subcommand,
+		fmt.Sprintf("failed to create API with %q", plugin.KeyFor(createAPIPlugin)))
 }

pkg/cli/cli.go

@@ -55,7 +55,7 @@ type Option func(*cli) error
 // cli defines the command line structure and interfaces that are used to
 // scaffold kubebuilder project files.
 type cli struct {
-	// Base command name. Can be injected downstream.
+	// Root command name. Can be injected downstream.
 	commandName string
 	// Default project version. Used in CLI flag setup.
 	defaultProjectVersion string
@@ -70,16 +70,16 @@ type cli struct {
 	completionCommand bool
 
 	// Plugins injected by options.
-	pluginsFromOptions map[string][]plugin.Base
+	pluginsFromOptions map[string][]plugin.Plugin
 	// Default plugins injected by options. Only one plugin per project version
 	// is allowed.
-	defaultPluginsFromOptions map[string]plugin.Base
+	defaultPluginsFromOptions map[string]plugin.Plugin
 	// A plugin key passed to --plugins on invoking 'init'.
 	cliPluginKey string
 	// A filtered set of plugins that should be used by command constructors.
-	resolvedPlugins []plugin.Base
+	resolvedPlugins []plugin.Plugin
 
-	// Base command.
+	// Root command.
 	cmd *cobra.Command
 	// Commands injected by options.
 	extraCommands []*cobra.Command
@@ -90,8 +90,8 @@ func New(opts ...Option) (CLI, error) {
 	c := &cli{
 		commandName:               "kubebuilder",
 		defaultProjectVersion:     internalconfig.DefaultVersion,
-		pluginsFromOptions:        make(map[string][]plugin.Base),
-		defaultPluginsFromOptions: make(map[string]plugin.Base),
+		pluginsFromOptions:        make(map[string][]plugin.Plugin),
+		defaultPluginsFromOptions: make(map[string]plugin.Plugin),
 	}
 	for _, opt := range opts {
 		if err := opt(c); err != nil {
@@ -131,7 +131,7 @@ func WithDefaultProjectVersion(version string) Option {
 }
 
 // WithPlugins is an Option that sets the cli's plugins.
-func WithPlugins(plugins ...plugin.Base) Option {
+func WithPlugins(plugins ...plugin.Plugin) Option {
 	return func(c *cli) error {
 		for _, p := range plugins {
 			for _, version := range p.SupportedProjectVersions() {
@@ -149,7 +149,7 @@ func WithPlugins(plugins ...plugin.Base) Option {
 
 // WithDefaultPlugins is an Option that sets the cli's default plugins. Only
 // one plugin per project version is allowed.
-func WithDefaultPlugins(plugins ...plugin.Base) Option {
+func WithDefaultPlugins(plugins ...plugin.Plugin) Option {
 	return func(c *cli) error {
 		for _, p := range plugins {
 			for _, version := range p.SupportedProjectVersions() {
@@ -230,7 +230,7 @@ func (c *cli) initialize() error {
 	// in situations like 'init --plugins "go"' when multiple go-type plugins
 	// are available but only one default is for a particular project version.
 	allPlugins := c.pluginsFromOptions[c.projectVersion]
-	defaultPlugin := []plugin.Base{c.defaultPluginsFromOptions[c.projectVersion]}
+	defaultPlugin := []plugin.Plugin{c.defaultPluginsFromOptions[c.projectVersion]}
 	switch {
 	case c.cliPluginKey != "":
 		// Filter plugin by keys passed in CLI.

pkg/cli/cli_test.go

@@ -40,59 +40,71 @@ func (p mockPlugin) Name() string                       { return p.name }
 func (p mockPlugin) Version() plugin.Version            { return p.version }
 func (p mockPlugin) SupportedProjectVersions() []string { return p.projectVersions }
 
-func (mockPlugin) UpdateContext(*plugin.Context) {}
-func (mockPlugin) BindFlags(*pflag.FlagSet)      {}
-func (mockPlugin) InjectConfig(*config.Config)   {}
-func (mockPlugin) Run() error                    { return nil }
-
-func makeBasePlugin(name, version string, projVers ...string) plugin.Base {
+func makeBasePlugin(name, version string, projVers ...string) plugin.Plugin {
 	v, err := plugin.ParseVersion(version)
 	if err != nil {
 		panic(err)
 	}
 	return mockPlugin{name, v, projVers}
 }
 
-func makePluginsForKeys(keys ...string) (plugins []plugin.Base) {
+func makePluginsForKeys(keys ...string) (plugins []plugin.Plugin) {
 	for _, key := range keys {
 		n, v := plugin.SplitKey(key)
 		plugins = append(plugins, makeBasePlugin(n, v, internalconfig.DefaultVersion))
 	}
 	return
 }
 
+type mockSubcommand struct{}
+
+func (mockSubcommand) UpdateContext(*plugin.Context) {}
+func (mockSubcommand) BindFlags(*pflag.FlagSet)      {}
+func (mockSubcommand) InjectConfig(*config.Config)   {}
+func (mockSubcommand) Run() error                    { return nil }
+
+// nolint:maligned
 type mockAllPlugin struct {
 	mockPlugin
 	mockInitPlugin
 	mockCreateAPIPlugin
 	mockCreateWebhookPlugin
+	mockEditPlugin
 }
 
-type mockInitPlugin struct{ mockPlugin }
-type mockCreateAPIPlugin struct{ mockPlugin }
-type mockCreateWebhookPlugin struct{ mockPlugin }
+type mockInitPlugin struct{ mockSubcommand }
+type mockCreateAPIPlugin struct{ mockSubcommand }
+type mockCreateWebhookPlugin struct{ mockSubcommand }
+type mockEditPlugin struct{ mockSubcommand }
 
-// GetInitPlugin will return the plugin which is responsible for initialized the project
-func (p mockInitPlugin) GetInitPlugin() plugin.Init { return p }
+// GetInitSubcommand implements plugin.Init
+func (p mockInitPlugin) GetInitSubcommand() plugin.InitSubcommand { return p }
 
-// GetCreateAPIPlugin will return the plugin which is responsible for scaffolding APIs for the project
-func (p mockCreateAPIPlugin) GetCreateAPIPlugin() plugin.CreateAPI { return p }
+// GetCreateAPISubcommand implements plugin.CreateAPI
+func (p mockCreateAPIPlugin) GetCreateAPISubcommand() plugin.CreateAPISubcommand { return p }
+
+// GetCreateWebhookSubcommand implements plugin.CreateWebhook
+func (p mockCreateWebhookPlugin) GetCreateWebhookSubcommand() plugin.CreateWebhookSubcommand {
+	return p
+}
 
-// GetCreateWebhookPlugin will return the plugin which is responsible for scaffolding webhooks for the project
-func (p mockCreateWebhookPlugin) GetCreateWebhookPlugin() plugin.CreateWebhook { return p }
+// GetEditSubcommand implements plugin.Edit
+func (p mockEditPlugin) GetEditSubcommand() plugin.EditSubcommand { return p }
 
-func makeAllPlugin(name, version string, projectVersions ...string) plugin.Base {
+func makeAllPlugin(name, version string, projectVersions ...string) plugin.Plugin {
 	p := makeBasePlugin(name, version, projectVersions...).(mockPlugin)
+	subcommand := mockSubcommand{}
 	return mockAllPlugin{
 		p,
-		mockInitPlugin{p},
-		mockCreateAPIPlugin{p},
-		mockCreateWebhookPlugin{p},
+		mockInitPlugin{subcommand},
+		mockCreateAPIPlugin{subcommand},
+		mockCreateWebhookPlugin{subcommand},
+		mockEditPlugin{subcommand},
 	}
 }
 
-func makeSetByProjVer(ps ...plugin.Base) map[string][]plugin.Base {
-	set := make(map[string][]plugin.Base)
+func makeSetByProjVer(ps ...plugin.Plugin) map[string][]plugin.Plugin {
+	set := make(map[string][]plugin.Plugin)
 	for _, p := range ps {
 		for _, version := range p.SupportedProjectVersions() {
 			set[version] = append(set[version], p)
@@ -117,7 +129,7 @@ var _ = Describe("CLI", func() {
 		pluginAV2       = makeAllPlugin(pluginNameA, "v2", projectVersions...)
 		pluginBV1       = makeAllPlugin(pluginNameB, "v1", projectVersions...)
 		pluginBV2       = makeAllPlugin(pluginNameB, "v2", projectVersions...)
-		allPlugins      = []plugin.Base{pluginAV1, pluginAV2, pluginBV1, pluginBV2}
+		allPlugins      = []plugin.Plugin{pluginAV1, pluginAV2, pluginBV1, pluginBV2}
 	)
 
 	Describe("New", func() {
@@ -129,28 +141,28 @@ var _ = Describe("CLI", func() {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c).NotTo(BeNil())
 				Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1)))
-				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
+				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
 
 				By("setting two plugins with different names and versions")
 				c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginBV2))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c).NotTo(BeNil())
 				Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginBV2)))
-				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
+				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
 
 				By("setting two plugins with the same names and different versions")
 				c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginAV2))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c).NotTo(BeNil())
 				Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginAV2)))
-				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
+				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
 
 				By("setting two plugins with different names and the same version")
 				c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginBV1))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c).NotTo(BeNil())
 				Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginBV1)))
-				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
+				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
 			})
 
 			It("should return an error", func() {
@@ -193,31 +205,31 @@ var _ = Describe("CLI", func() {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c).NotTo(BeNil())
 				Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginAV2)))
-				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
+				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
 
 				By(`setting cliPluginKey to "go/v1"`)
 				setPluginsFlag("go/v1")
 				c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginBV2))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c).NotTo(BeNil())
 				Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginBV2)))
-				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
+				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
 
 				By(`setting cliPluginKey to "go/v2"`)
 				setPluginsFlag("go/v2")
 				c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginBV2))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c).NotTo(BeNil())
 				Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginBV2)))
-				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginBV2}))
+				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginBV2}))
 
 				By(`setting cliPluginKey to "go.test.com/v2"`)
 				setPluginsFlag("go.test.com/v2")
 				c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(allPlugins...))
 				Expect(err).NotTo(HaveOccurred())
 				Expect(c).NotTo(BeNil())
 				Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(allPlugins...)))
-				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginBV2}))
+				Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginBV2}))
 			})
 
 			It("should return an error", func() {

pkg/cli/cmd_helpers.go

@@ -45,14 +45,15 @@ func errCmdFunc(err error) func(*cobra.Command, []string) error {
 	}
 }
 
-// runECmdFunc returns a cobra RunE function that runs gsub and saves the
-// config, which may have been modified by gsub.
+// runECmdFunc returns a cobra RunE function that runs subcommand and saves the
+// config, which may have been modified by subcommand.
 func runECmdFunc(
 	c *config.Config,
-	gsub plugin.GenericSubcommand, // nolint:interfacer
-	msg string) func(*cobra.Command, []string) error {
+	subcommand plugin.Subcommand, // nolint:interfacer
+	msg string,
+) func(*cobra.Command, []string) error {
 	return func(*cobra.Command, []string) error {
-		if err := gsub.Run(); err != nil {
+		if err := subcommand.Run(); err != nil {
 			return fmt.Errorf("%s: %v", msg, err)
 		}
 		return c.Save()

pkg/cli/edit.go

@@ -46,47 +46,48 @@ func (c *cli) newEditCmd() *cobra.Command {
 func (c *cli) newEditContext() plugin.Context {
 	ctx := plugin.Context{
 		CommandName: c.commandName,
-		Description: `This command will edit the project configuration. You can have single or multi group project.`,
+		Description: `Edit the project configuration.
+`,
 	}
 
 	return ctx
 }
 
 // nolint:dupl
 func (c *cli) bindEdit(ctx plugin.Context, cmd *cobra.Command) {
-	var getter plugin.EditPluginGetter
+	var editPlugin plugin.Edit
 	for _, p := range c.resolvedPlugins {
-		tmpGetter, isGetter := p.(plugin.EditPluginGetter)
-		if isGetter {
-			if getter != nil {
-				err := fmt.Errorf("duplicate edit project plugins for project version %q (%s, %s), "+
-					"use a more specific plugin key", c.projectVersion, plugin.KeyFor(getter), plugin.KeyFor(p))
+		tmpPlugin, isValid := p.(plugin.Edit)
+		if isValid {
+			if editPlugin != nil {
+				err := fmt.Errorf(
+					"duplicate edit project plugins (%s, %s), use a more specific plugin key",
+					plugin.KeyFor(editPlugin), plugin.KeyFor(p))
 				cmdErr(cmd, err)
 				return
 			}
-			getter = tmpGetter
+			editPlugin = tmpPlugin
 		}
 	}
 
-	cfg, err := config.LoadInitialized()
-	if err != nil {
+	if editPlugin == nil {
+		err := fmt.Errorf("relevant plugins do not provide a project edit plugin")
 		cmdErr(cmd, err)
 		return
 	}
 
-	if getter == nil {
-		err := fmt.Errorf("layout plugin %q does not support a edit project plugin", cfg.Layout)
+	cfg, err := config.LoadInitialized()
+	if err != nil {
 		cmdErr(cmd, err)
 		return
 	}
 
-	editProject := getter.GetEditPlugin()
-	editProject.InjectConfig(&cfg.Config)
-	editProject.BindFlags(cmd.Flags())
-	editProject.UpdateContext(&ctx)
+	subcommand := editPlugin.GetEditSubcommand()
+	subcommand.InjectConfig(&cfg.Config)
+	subcommand.BindFlags(cmd.Flags())
+	subcommand.UpdateContext(&ctx)
 	cmd.Long = ctx.Description
 	cmd.Example = ctx.Examples
-	cmd.RunE = runECmdFunc(cfg, editProject,
-		fmt.Sprintf("failed to edit project with version %q", c.projectVersion))
-
+	cmd.RunE = runECmdFunc(cfg, subcommand,
+		fmt.Sprintf("failed to edit project with %q", plugin.KeyFor(editPlugin)))
 }