Skip to content

Commit b68226d

Browse files
AdirioAdirio
committed
Separate the plugin and subcommand concepts
Previously, both the getters and the execution objects were called plugins, leading to missunderstandings Signed-off-by: Adrian Orive <[email protected]>
1 parent 9c02d55 commit b68226d

22 files changed

+314
-308
lines changed

‎pkg/cli/api.go

+17-17
Original file line numberDiff line numberDiff line change
@@ -56,38 +56,38 @@ func (c cli) newAPIContext() plugin.Context {
5656

5757
// nolint:dupl
5858
func (c cli) bindCreateAPI(ctx plugin.Context, cmd *cobra.Command) {
59-
var getter plugin.CreateAPIPluginGetter
59+
var createAPIPlugin plugin.CreateAPIPlugin
6060
for _, p := range c.resolvedPlugins {
61-
tmpGetter, isGetter := p.(plugin.CreateAPIPluginGetter)
62-
if isGetter {
63-
if getter != nil {
64-
err := fmt.Errorf("duplicate API creation plugins for project version %q (%s, %s), "+
65-
"use a more specific plugin key", c.projectVersion, plugin.KeyFor(getter), plugin.KeyFor(p))
61+
tmpPlugin, isValid := p.(plugin.CreateAPIPlugin)
62+
if isValid {
63+
if createAPIPlugin != nil {
64+
err := fmt.Errorf("duplicate API creation plugins (%s, %s), use a more specific plugin key",
65+
plugin.KeyFor(createAPIPlugin), plugin.KeyFor(p))
6666
cmdErr(cmd, err)
6767
return
6868
}
69-
getter = tmpGetter
69+
createAPIPlugin = tmpPlugin
7070
}
7171
}
7272

73-
cfg, err := config.LoadInitialized()
74-
if err != nil {
73+
if createAPIPlugin == nil {
74+
err := fmt.Errorf("relevant plugins do not provide an API creation plugin")
7575
cmdErr(cmd, err)
7676
return
7777
}
7878

79-
if getter == nil {
80-
err := fmt.Errorf("layout plugin %q does not support an API creation plugin", cfg.Layout)
79+
cfg, err := config.LoadInitialized()
80+
if err != nil {
8181
cmdErr(cmd, err)
8282
return
8383
}
8484

85-
createAPI := getter.GetCreateAPIPlugin()
86-
createAPI.InjectConfig(&cfg.Config)
87-
createAPI.BindFlags(cmd.Flags())
88-
createAPI.UpdateContext(&ctx)
85+
subcommand := createAPIPlugin.GetCreateAPISubcommand()
86+
subcommand.InjectConfig(&cfg.Config)
87+
subcommand.BindFlags(cmd.Flags())
88+
subcommand.UpdateContext(&ctx)
8989
cmd.Long = ctx.Description
9090
cmd.Example = ctx.Examples
91-
cmd.RunE = runECmdFunc(cfg, createAPI,
92-
fmt.Sprintf("failed to create API with version %q", c.projectVersion))
91+
cmd.RunE = runECmdFunc(cfg, subcommand,
92+
fmt.Sprintf("failed to create API with %q", plugin.KeyFor(createAPIPlugin)))
9393
}

‎pkg/cli/cli.go

+10-10
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ type Option func(*cli) error
5555
// cli defines the command line structure and interfaces that are used to
5656
// scaffold kubebuilder project files.
5757
type cli struct {
58-
// Base command name. Can be injected downstream.
58+
// Root command name. Can be injected downstream.
5959
commandName string
6060
// Default project version. Used in CLI flag setup.
6161
defaultProjectVersion string
@@ -67,16 +67,16 @@ type cli struct {
6767
doGenericHelp bool
6868

6969
// Plugins injected by options.
70-
pluginsFromOptions map[string][]plugin.Base
70+
pluginsFromOptions map[string][]plugin.Plugin
7171
// Default plugins injected by options. Only one plugin per project version
7272
// is allowed.
73-
defaultPluginsFromOptions map[string]plugin.Base
73+
defaultPluginsFromOptions map[string]plugin.Plugin
7474
// A plugin key passed to --plugins on invoking 'init'.
7575
cliPluginKey string
7676
// A filtered set of plugins that should be used by command constructors.
77-
resolvedPlugins []plugin.Base
77+
resolvedPlugins []plugin.Plugin
7878

79-
// Base command.
79+
// Root command.
8080
cmd *cobra.Command
8181
// Commands injected by options.
8282
extraCommands []*cobra.Command
@@ -87,8 +87,8 @@ func New(opts ...Option) (CLI, error) {
8787
c := &cli{
8888
commandName: "kubebuilder",
8989
defaultProjectVersion: internalconfig.DefaultVersion,
90-
pluginsFromOptions: make(map[string][]plugin.Base),
91-
defaultPluginsFromOptions: make(map[string]plugin.Base),
90+
pluginsFromOptions: make(map[string][]plugin.Plugin),
91+
defaultPluginsFromOptions: make(map[string]plugin.Plugin),
9292
}
9393
for _, opt := range opts {
9494
if err := opt(c); err != nil {
@@ -128,7 +128,7 @@ func WithDefaultProjectVersion(version string) Option {
128128
}
129129

130130
// WithPlugins is an Option that sets the cli's plugins.
131-
func WithPlugins(plugins ...plugin.Base) Option {
131+
func WithPlugins(plugins ...plugin.Plugin) Option {
132132
return func(c *cli) error {
133133
for _, p := range plugins {
134134
for _, version := range p.SupportedProjectVersions() {
@@ -146,7 +146,7 @@ func WithPlugins(plugins ...plugin.Base) Option {
146146

147147
// WithDefaultPlugins is an Option that sets the cli's default plugins. Only
148148
// one plugin per project version is allowed.
149-
func WithDefaultPlugins(plugins ...plugin.Base) Option {
149+
func WithDefaultPlugins(plugins ...plugin.Plugin) Option {
150150
return func(c *cli) error {
151151
for _, p := range plugins {
152152
for _, version := range p.SupportedProjectVersions() {
@@ -221,7 +221,7 @@ func (c *cli) initialize() error {
221221
// in situations like 'init --plugins "go"' when multiple go-type plugins
222222
// are available but only one default is for a particular project version.
223223
allPlugins := c.pluginsFromOptions[c.projectVersion]
224-
defaultPlugin := []plugin.Base{c.defaultPluginsFromOptions[c.projectVersion]}
224+
defaultPlugin := []plugin.Plugin{c.defaultPluginsFromOptions[c.projectVersion]}
225225
switch {
226226
case c.cliPluginKey != "":
227227
// Filter plugin by keys passed in CLI.

‎pkg/cli/cli_suite_test.go

+34-22
Original file line numberDiff line numberDiff line change
@@ -45,59 +45,71 @@ func (p mockPlugin) Name() string { return p.name }
4545
func (p mockPlugin) Version() plugin.Version { return p.version }
4646
func (p mockPlugin) SupportedProjectVersions() []string { return p.projectVersions }
4747

48-
func (mockPlugin) UpdateContext(*plugin.Context) {}
49-
func (mockPlugin) BindFlags(*pflag.FlagSet) {}
50-
func (mockPlugin) InjectConfig(*config.Config) {}
51-
func (mockPlugin) Run() error { return nil }
52-
53-
func makeBasePlugin(name, version string, projVers ...string) plugin.Base {
48+
func makeBasePlugin(name, version string, projVers ...string) plugin.Plugin {
5449
v, err := plugin.ParseVersion(version)
5550
if err != nil {
5651
panic(err)
5752
}
5853
return mockPlugin{name, v, projVers}
5954
}
6055

61-
func makePluginsForKeys(keys ...string) (plugins []plugin.Base) {
56+
func makePluginsForKeys(keys ...string) (plugins []plugin.Plugin) {
6257
for _, key := range keys {
6358
n, v := plugin.SplitKey(key)
6459
plugins = append(plugins, makeBasePlugin(n, v, internalconfig.DefaultVersion))
6560
}
6661
return
6762
}
6863

64+
type mockSubcommand struct{}
65+
66+
func (mockSubcommand) UpdateContext(*plugin.Context) {}
67+
func (mockSubcommand) BindFlags(*pflag.FlagSet) {}
68+
func (mockSubcommand) InjectConfig(*config.Config) {}
69+
func (mockSubcommand) Run() error { return nil }
70+
71+
// nolint:maligned
6972
type mockAllPlugin struct {
7073
mockPlugin
7174
mockInitPlugin
7275
mockCreateAPIPlugin
7376
mockCreateWebhookPlugin
77+
mockEditPlugin
7478
}
7579

76-
type mockInitPlugin struct{ mockPlugin }
77-
type mockCreateAPIPlugin struct{ mockPlugin }
78-
type mockCreateWebhookPlugin struct{ mockPlugin }
80+
type mockInitPlugin struct{ mockSubcommand }
81+
type mockCreateAPIPlugin struct{ mockSubcommand }
82+
type mockCreateWebhookPlugin struct{ mockSubcommand }
83+
type mockEditPlugin struct{ mockSubcommand }
7984

80-
// GetInitPlugin will return the plugin which is responsible for initialized the project
81-
func (p mockInitPlugin) GetInitPlugin() plugin.Init { return p }
85+
// GetInitSubcommand implements plugin.InitPlugin
86+
func (p mockInitPlugin) GetInitSubcommand() plugin.InitSubcommand { return p }
8287

83-
// GetCreateAPIPlugin will return the plugin which is responsible for scaffolding APIs for the project
84-
func (p mockCreateAPIPlugin) GetCreateAPIPlugin() plugin.CreateAPI { return p }
88+
// GetCreateAPISubcommand implements plugin.CreateAPIPlugin
89+
func (p mockCreateAPIPlugin) GetCreateAPISubcommand() plugin.CreateAPISubcommand { return p }
90+
91+
// GetCreateWebhookSubcommand implements plugin.CreateWebhookPlugin
92+
func (p mockCreateWebhookPlugin) GetCreateWebhookSubcommand() plugin.CreateWebhookSubcommand {
93+
return p
94+
}
8595

86-
// GetCreateWebhookPlugin will return the plugin which is responsible for scaffolding webhooks for the project
87-
func (p mockCreateWebhookPlugin) GetCreateWebhookPlugin() plugin.CreateWebhook { return p }
96+
// GetEditSubcommand implements plugin.EditPlugin
97+
func (p mockEditPlugin) GetEditSubcommand() plugin.EditSubcommand { return p }
8898

89-
func makeAllPlugin(name, version string, projectVersions ...string) plugin.Base {
99+
func makeAllPlugin(name, version string, projectVersions ...string) plugin.Plugin {
90100
p := makeBasePlugin(name, version, projectVersions...).(mockPlugin)
101+
subcommand := mockSubcommand{}
91102
return mockAllPlugin{
92103
p,
93-
mockInitPlugin{p},
94-
mockCreateAPIPlugin{p},
95-
mockCreateWebhookPlugin{p},
104+
mockInitPlugin{subcommand},
105+
mockCreateAPIPlugin{subcommand},
106+
mockCreateWebhookPlugin{subcommand},
107+
mockEditPlugin{subcommand},
96108
}
97109
}
98110

99-
func makeSetByProjVer(ps ...plugin.Base) map[string][]plugin.Base {
100-
set := make(map[string][]plugin.Base)
111+
func makeSetByProjVer(ps ...plugin.Plugin) map[string][]plugin.Plugin {
112+
set := make(map[string][]plugin.Plugin)
101113
for _, p := range ps {
102114
for _, version := range p.SupportedProjectVersions() {
103115
set[version] = append(set[version], p)

‎pkg/cli/cli_test.go

+9-9
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ var _ = Describe("CLI", func() {
3838
pluginAV2 = makeAllPlugin(pluginNameA, "v2", projectVersions...)
3939
pluginBV1 = makeAllPlugin(pluginNameB, "v1", projectVersions...)
4040
pluginBV2 = makeAllPlugin(pluginNameB, "v2", projectVersions...)
41-
allPlugins = []plugin.Base{pluginAV1, pluginAV2, pluginBV1, pluginBV2}
41+
allPlugins = []plugin.Plugin{pluginAV1, pluginAV2, pluginBV1, pluginBV2}
4242
)
4343

4444
Describe("New", func() {
@@ -50,28 +50,28 @@ var _ = Describe("CLI", func() {
5050
Expect(err).NotTo(HaveOccurred())
5151
Expect(c).NotTo(BeNil())
5252
Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1)))
53-
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
53+
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
5454

5555
By("setting two plugins with different names and versions")
5656
c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginBV2))
5757
Expect(err).NotTo(HaveOccurred())
5858
Expect(c).NotTo(BeNil())
5959
Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginBV2)))
60-
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
60+
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
6161

6262
By("setting two plugins with the same names and different versions")
6363
c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginAV2))
6464
Expect(err).NotTo(HaveOccurred())
6565
Expect(c).NotTo(BeNil())
6666
Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginAV2)))
67-
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
67+
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
6868

6969
By("setting two plugins with different names and the same version")
7070
c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginBV1))
7171
Expect(err).NotTo(HaveOccurred())
7272
Expect(c).NotTo(BeNil())
7373
Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginBV1)))
74-
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
74+
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
7575
})
7676

7777
It("should return an error", func() {
@@ -114,31 +114,31 @@ var _ = Describe("CLI", func() {
114114
Expect(err).NotTo(HaveOccurred())
115115
Expect(c).NotTo(BeNil())
116116
Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginAV2)))
117-
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
117+
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
118118

119119
By(`setting cliPluginKey to "go/v1"`)
120120
setPluginsFlag("go/v1")
121121
c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginBV2))
122122
Expect(err).NotTo(HaveOccurred())
123123
Expect(c).NotTo(BeNil())
124124
Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginBV2)))
125-
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginAV1}))
125+
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginAV1}))
126126

127127
By(`setting cliPluginKey to "go/v2"`)
128128
setPluginsFlag("go/v2")
129129
c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(pluginAV1, pluginBV2))
130130
Expect(err).NotTo(HaveOccurred())
131131
Expect(c).NotTo(BeNil())
132132
Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(pluginAV1, pluginBV2)))
133-
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginBV2}))
133+
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginBV2}))
134134

135135
By(`setting cliPluginKey to "go.test.com/v2"`)
136136
setPluginsFlag("go.test.com/v2")
137137
c, err = New(WithDefaultPlugins(pluginAV1), WithPlugins(allPlugins...))
138138
Expect(err).NotTo(HaveOccurred())
139139
Expect(c).NotTo(BeNil())
140140
Expect(c.(*cli).pluginsFromOptions).To(Equal(makeSetByProjVer(allPlugins...)))
141-
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Base{pluginBV2}))
141+
Expect(c.(*cli).resolvedPlugins).To(Equal([]plugin.Plugin{pluginBV2}))
142142
})
143143

144144
It("should return an error", func() {

‎pkg/cli/cmd_helpers.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,15 @@ func errCmdFunc(err error) func(*cobra.Command, []string) error {
4545
}
4646
}
4747

48-
// runECmdFunc returns a cobra RunE function that runs gsub and saves the
49-
// config, which may have been modified by gsub.
48+
// runECmdFunc returns a cobra RunE function that runs subcommand and saves the
49+
// config, which may have been modified by subcommand.
5050
func runECmdFunc(
5151
c *config.Config,
52-
gsub plugin.GenericSubcommand, // nolint:interfacer
53-
msg string) func(*cobra.Command, []string) error {
52+
subcommand plugin.Subcommand, // nolint:interfacer
53+
msg string,
54+
) func(*cobra.Command, []string) error {
5455
return func(*cobra.Command, []string) error {
55-
if err := gsub.Run(); err != nil {
56+
if err := subcommand.Run(); err != nil {
5657
return fmt.Errorf("%s: %v", msg, err)
5758
}
5859
return c.Save()

‎pkg/cli/edit.go

+20-19
Original file line numberDiff line numberDiff line change
@@ -46,47 +46,48 @@ func (c *cli) newEditCmd() *cobra.Command {
4646
func (c *cli) newEditContext() plugin.Context {
4747
ctx := plugin.Context{
4848
CommandName: c.commandName,
49-
Description: `This command will edit the project configuration. You can have single or multi group project.`,
49+
Description: `Edit the project configuration.
50+
`,
5051
}
5152

5253
return ctx
5354
}
5455

5556
// nolint:dupl
5657
func (c *cli) bindEdit(ctx plugin.Context, cmd *cobra.Command) {
57-
var getter plugin.EditPluginGetter
58+
var editPlugin plugin.EditPlugin
5859
for _, p := range c.resolvedPlugins {
59-
tmpGetter, isGetter := p.(plugin.EditPluginGetter)
60-
if isGetter {
61-
if getter != nil {
62-
err := fmt.Errorf("duplicate edit project plugins for project version %q (%s, %s), "+
63-
"use a more specific plugin key", c.projectVersion, plugin.KeyFor(getter), plugin.KeyFor(p))
60+
tmpPlugin, isValid := p.(plugin.EditPlugin)
61+
if isValid {
62+
if editPlugin != nil {
63+
err := fmt.Errorf(
64+
"duplicate edit project plugins (%s, %s), use a more specific plugin key",
65+
plugin.KeyFor(editPlugin), plugin.KeyFor(p))
6466
cmdErr(cmd, err)
6567
return
6668
}
67-
getter = tmpGetter
69+
editPlugin = tmpPlugin
6870
}
6971
}
7072

71-
cfg, err := config.LoadInitialized()
72-
if err != nil {
73+
if editPlugin == nil {
74+
err := fmt.Errorf("relevant plugins do not provide a project edit plugin")
7375
cmdErr(cmd, err)
7476
return
7577
}
7678

77-
if getter == nil {
78-
err := fmt.Errorf("layout plugin %q does not support a edit project plugin", cfg.Layout)
79+
cfg, err := config.LoadInitialized()
80+
if err != nil {
7981
cmdErr(cmd, err)
8082
return
8183
}
8284

83-
editProject := getter.GetEditPlugin()
84-
editProject.InjectConfig(&cfg.Config)
85-
editProject.BindFlags(cmd.Flags())
86-
editProject.UpdateContext(&ctx)
85+
subcommand := editPlugin.GetEditSubcommand()
86+
subcommand.InjectConfig(&cfg.Config)
87+
subcommand.BindFlags(cmd.Flags())
88+
subcommand.UpdateContext(&ctx)
8789
cmd.Long = ctx.Description
8890
cmd.Example = ctx.Examples
89-
cmd.RunE = runECmdFunc(cfg, editProject,
90-
fmt.Sprintf("failed to edit project with version %q", c.projectVersion))
91-
91+
cmd.RunE = runECmdFunc(cfg, subcommand,
92+
fmt.Sprintf("failed to edit project with %q", plugin.KeyFor(editPlugin)))
9293
}

0 commit comments

Comments
 (0)