designs: extensible CLI and scaffolding plugins #1250

joelanford · 2019-12-10T02:26:35Z

Description of the change
This PR adds a design proposal for Kubebuilder CLI and scaffolding extensibility

Motivation for the change
To achieve agreement on a way forward for Kubebuilder CLI and scaffolding extensibility so that the Kubernetes controller/operator communities can align on Go project layouts and enable extension points for other customizations.

joelanford · 2019-12-10T03:19:23Z

To answer @DirectXMan12's questions from #1249

How do you specify init plugins (e.g. --project-version v2:osdk,add-ascii-art)? I imagine that some of them want to persist there plugin-ness to PROJECT, so being able to add config sections to PROJECT and to remember that they're enabled would be nice.

In the prototype, the --project-version flag defines the single plugin to use for project initialization. If that plugin also supports either of the create subcommands, the pkg/cli code expects to find that same project version in the PROJECT file.

Basically, the plugin is loaded during init based on --project-version and during create based on the PROJECT file.

I'm thinking along the lines of addon operator or OSDK, where you might want to automatically invoke the OSDK api plugin when a project was init-ed with the OSDK init plugin.

The prototype doesn't really get into this, but I agree it is something we should discuss and cover. Open to ideas on this. I can think of three options:

Leave it to a particular "root" plugin to implement any further customization. For example, if the current Kubebuilder Go project code was implemented as a root init plugin as defined by this proposal, the --pattern option would be an implementation detail of the Go plugin's create api support (IIRC).
Have explicit definitions and configuration for separate root-level plugins and transform/pattern plugins. The former have basically no input other than what's hardcoded or passed via config/CLI flags. The latter would be based on the existing addon plugin model such that it could modify the world piped to it by the "root" plugin or by other transform plugins.
Define all plugins using the transform model. The first plugin in the list would be the "root" plugin and would create the world from scratch. The rest of the plugins would transform the world. Plugins could theoretically work in either mode, just like many Unix commands do.

Don't let this derail the discussion, but: can we have some way to just turn config blocks into CLI args, and then we can have a single point of config for args vs PROJECT, and not depend on pflag externally? This might be too much of a "rewrite the world" though. e.g.
type OSDKConfig struct {
ImageVersion string cli:"img-version"
}
// becomes --osdk-img-version or something
// maybe have a validate method too

I like this idea. We would need a way to know which config struct to parse to build the flag set for each command. For example, this seems like it implies that the Go V2 scaffolding would need 3 config structs, one for each subcommand it implements (e.g. type GoV2InitConfig struct {}), and possibly a parent struct that implements the Plugin interface so that they can all be linked to the same project version.

Do plugins still get the world state somehow?

I'll answer your question in two ways:

In the prototype, there's an InjectCommandName() interface that tells the plugin what command name it should use in help output (as an aside, I experimented with returning Go templates from plugins so they didn't have to each implement that interface, but that seemed more brittle). The injection interface is meant to be a simple example of injecting some state. We could expand that.
If by "world" you mean, the existing state of files to be scaffolded, then no, that doesn't exist in the prototype/proposal. But perhaps it should based on your previous question about defining multiple plugins.

camilamacedo86 · 2019-12-10T10:59:16Z

designs/extensible-cli-and-scaffolding-plugins.md

+type Plugin interface {
+    // Version is the project version that this plugin implements.
+    // For example, Kubebuilder's Go v2 plugin implementation would return "2"
+    Version() string


WDYT about we have here the []types as well? (ansible, helm, go)?

I think we probably want like name and version here, unless we want to automatically deduce name from elsewhere, but that seems 🤢

That way we get kubebuilder:v2 or ansible:v3 or osdk:v1 and such.

I was thinking that kubebuilder or sdk are equals when is the go type, so I mean it doesn't really matter since the scaffold will be the same and what may change are the top customizations. Because of this, in my mind types would represent the language used. But may I am unable to get some point.

WDYT about we have type=[go|helm|ansible] and builder=[sdk|kube]?

Things we seem to need from the base Plugin interface:

Method to get a version string (Version())

Always an integer as string.

Method to get an identifier for a plugin (Name())

Should this identifier format be up the implementer, or be name or name:vX?

If we go with a self-specified identifier, do we care about prevent naming conflicts between plugins?

+1 on name and version

We should not force folks to determine if they are are type or a builder, a name for the given plugin that is being used and a version of that plugin will give us the flexibility we need IMO.

if Version is always an int, then can we make it an unsigned int type to get compile-time guarantees?

DirectXMan12 · 2019-12-10T21:08:59Z

Define all plugins using the transform model. The first plugin in the list would be the "root" plugin and would create the world from scratch.

I think I'd kinda lean towards this or option 2. It's a lot more flexible, makes it easier to do stuff like "I need project or environment specific tweaks to some base scaffolding" (e.g. I want Bazel build rules set up automatically).

Re project file (cc @camilamacedo86's comment above), I'd imagine something like:

domain: test.kubebuilder.io
repo: sigs.k8s.io/kubebuilder/testdata/project-v3
version: "3x" # version strings are mostly meaningless at this point, but the x is a nice marker that we are using plugins
resources:
- group: crew
  kind: Captain
  version: v1
model:
  # these are persistent plugins -- they'll be automatically invoked
  # for additional commands that they support, unless otherwise disabled
  - { name: "kubebuilder", version: "2" }  # "root" plugin -- gets an empty world tree
  - name: "add-bazel"
    version: "1"
    config: { pruneMakefile: true } # custom config

DirectXMan12 · 2019-12-10T21:19:27Z

I'd loosely imagine that corresponds to

$ kubebuilder init --domain=test.kubebuilder.io --scaffold=kubebuilder:v2,add-bazel:v1 --add-bazel-prune-makefile=true
$ kubebuilder create api --group crew --kind Captain --version v1 # automatically gets bazel files

kubebuilder init --project-version 1 corresponds to --scaffold kubebuilder:v1.

For ease of use, we'd probably want to do the following:

--scaffold kubebuilder,add-bazel uses the default versions for each plugin
For compatibility reasons existing flags that would be specific to the KB root plugin wouldn't get a prefix, but most other things would probably get autoprefixed with plugin name.
--scaffold add-bazel would notice that add-bazel can't deal with being a root plugin, and default to KB as the root, so --scaffold add-bazel is eventually equivalent to --scaffold kubebuilder:v2,add-bazel:v1

DirectXMan12 · 2019-12-10T21:29:05Z

So, for example, an osdk operator might look use kubebuilder init --scaffold osdk, which is equivalent to kubebuilder init --scaffold kubebuilder:2,osdk:2 or something (assuming for Go operators osdk is additional config for operator framework stuff)

or

internally in the osdk command (pulling from your example):

func main() {
	c, err := cli.New(
		cli.WithCommandName("operator-sdk"),
		cli.WithDefaultModel(&golangv2.Plugin{}, &osdkmods.Plugin{}),
		cli.WithExtraCommands(newCustomCobraCmd()),
		cli.WithPlugins(
			&golangv1.Plugin{},
			&golangv2.Plugin{},
			&helmv1.Plugin{},
			&ansiblev1.Plugin{},
		),
	)
	if err != nil {
		log.Fatal(err)
	}
	if err := c.Run(); err != nil {
		log.Fatal(err)
	}
}

DirectXMan12 · 2019-12-10T21:34:17Z

designs/extensible-cli-and-scaffolding-plugins.md

+type Plugin interface {
+    // Version is the project version that this plugin implements.
+    // For example, Kubebuilder's Go v2 plugin implementation would return "2"
+    Version() string


I think we probably want like name and version here, unless we want to automatically deduce name from elsewhere, but that seems 🤢

That way we get kubebuilder:v2 or ansible:v3 or osdk:v1 and such.

DirectXMan12 · 2019-12-10T21:34:48Z

designs/extensible-cli-and-scaffolding-plugins.md

+// Deprecated is an interface that, if implemented, informs the CLI
+// that the plugin is deprecated.  The CLI uses this to print deprecation
+// warnings when the plugin is in use.
+type Deprecated interface {


DirectXMan12 · 2019-12-10T21:37:46Z

designs/extensible-cli-and-scaffolding-plugins.md

+    // BindInitFlags binds the plugin's init flags to the CLI. This allows each
+    // plugin to define its own command line flags for the `kubebuilder init`
+    // subcommand.
+    BindInitFlags(fs *pflag.FlagSet)


So, I kinda don't want to suggest this, but I figured I throw it out: all the CLI arg logic from CT is in its own package, and can be used with arbitrary CLI markers. It's used in a couple of the helper commands. but it completely ignores existing UX for CLI programs, so we probably don't want to use it

What would be the benefit of doing this vs. using pflag.FlagSet and the plugin are responsible for adding its own flags and the types that it uses internally for the flags.

I am having a tough time seeing the benefit of that approach vs this approach which feels pretty common to me?

I guess not exposing pflag?

We could wrap *pflag.FlagSet in a PluginFlagSet struct so we can change the underlying flagset implementation later while keeping the interface forward-compatible:

type PluginFlagSet struct { *pflag.FlagSet } type InitPlugin interface { ... BindInitFlags(PluginFlagSet) }

That way we can deprecate pflag by using a conversion function to transform *pflag.FlagSet to a new flagset type until we remove support entirely. Thoughts?

I think the result of all this back and forth might just be that avoiding pflag (and having cobra in the interface) is more work than it's worth ATM. Sorry for starting all that. Let's just go forward with pflag for now, and we can always re-adjust a bit later if we really need to.

DirectXMan12 · 2019-12-10T21:42:56Z

designs/extensible-cli-and-scaffolding-plugins.md

+
+With all of that said, even this exposure of `cobra.Command` could be problematic. If Kubebuilder decides in the future to transition to a different CLI framework (or to roll its own) it has to either continue maintaining support for these extra cobra commands passed into it, or it was to break the CLI API.
+
+Are there other ideas for how to handle the following requirements?


As much as I don't want to expose cobra, I kinda feel like if we try we might end up re-implementing it poorly. As a thought exercise:

What do we need/want?

Cohesive --help with additional subcommands listed based on the --scaffold flag and the persistent plugins

Additional subcommands that basically have all args and unknown flags passed to them

(nice to have from a UX perspective) common flags may be specified anywhere on the command line, not just before the subcommand

anything I'm missing?

It's going to be hard to avoid exposing cobra.

droot

Took a look, have a few comments.

droot · 2019-12-10T21:58:29Z

designs/extensible-cli-and-scaffolding-plugins.md

+
+## Overview 
+I would like for Kubebuilder to become more extensible, such that it could be imported and used as a library in other projects. Specifically, I'm looking for a way to use Kubebuilder's existing CLI and scaffolding for Go projects, but to also be able to augment Kubebuilder's project versions with other custom project versions so that I can support the Kubebuilder workflow with non-Go operators (e.g. operator-sdk's Ansible and Helm-based operators).
+


We use version for project versioning e.g. 1, 2. For this use-case, we can use the term types to refer different types of Operator projects.

+1, I has to re-read from the start once I understood that version was not refering to a version but to a type

droot · 2019-12-10T22:01:27Z

designs/extensible-cli-and-scaffolding-plugins.md

+    // For example, Kubebuilder's Go v2 plugin implementation would return "2"
+    Version() string
+}
+```


Better would be call this method Name instead of Version. Version indicates the version of the plugin itself that could v1, v2,....

see my comment at #1250 (comment)

I would also lean to both Name() string and Version() string

droot · 2019-12-10T22:05:01Z

designs/extensible-cli-and-scaffolding-plugins.md

+    // Init initializes a project.
+    Init() error
+}
+```


So if I have a plugin that implements InitPlugin interface, does it override the default init behavior or does it augment the default behavior ?

According to #1250 (comment) it seems you queue Init commands using kubebuilder Init as the root.

droot · 2019-12-10T22:13:46Z

designs/extensible-cli-and-scaffolding-plugins.md

+# Extensible CLI and Scaffolding Plugins
+
+## Overview 
+I would like for Kubebuilder to become more extensible, such that it could be imported and used as a library in other projects. Specifically, I'm looking for a way to use Kubebuilder's existing CLI and scaffolding for Go projects, but to also be able to augment Kubebuilder's project versions with other custom project versions so that I can support the Kubebuilder workflow with non-Go operators (e.g. operator-sdk's Ansible and Helm-based operators).


One observation: I think the proposal is mixing extensibility with using KB CLI as library. I am wondering if decoupling them will simplify the proposal (if possible).

To me these things go hand in hand, since using KB CLI as a library by itself is easy to do (CLI option structs like cmd.apiOptions could be exported) while making KB extensible in the manner suggested gives you a CLI library "for free". Does that make sense @droot?

droot · 2019-12-10T22:45:48Z

designs/extensible-cli-and-scaffolding-plugins.md

+
+    // Init initializes a project.
+    Init() error
+}


One high level pattern is -- There will be metadata associated with the project (ex. domain, groups...) and type (helm, golang...), we need a way to persist that data in probably PROJECT file and each subcommand invocation can be provided with that metadata and can be written back at the end during the invocation or at the end.
If we can come up with a generic metadata interface (Generic Key-Value may be ?) on PROJECT file, then we can decouple commandline args from the plugins.

Yes this sounds like a necessity. I like @DirectXMan12's suggestion from above. Is this along the lines of what you're looking for?

droot · 2019-12-10T22:49:59Z

designs/extensible-cli-and-scaffolding-plugins.md

+
+With all of that said, even this exposure of `cobra.Command` could be problematic. If Kubebuilder decides in the future to transition to a different CLI framework (or to roll its own) it has to either continue maintaining support for these extra cobra commands passed into it, or it was to break the CLI API.
+
+Are there other ideas for how to handle the following requirements?


It's going to be hard to avoid exposing cobra.

mengqiy · 2019-12-11T01:09:45Z

Eliminate use of cobra in CLI interface
Allow other projects to have custom subcommands
Support cohesive help output

I have an idea that seems to address these 3 requirements.

We have a "root" plugin that will dump some metadata in PROJECT file during init.
The file roughly looks like the following:

domain: test.kubebuilder.io
repo: sigs.k8s.io/kubebuilder/testdata/project-v3
version: "3x" # version strings are mostly meaningless at this point, but the x is a nice marker that we are using plugins
resources:
- group: crew
  kind: Captain
  version: v1
clis:
  - name: helm
    type: subcommand
    path: / # "/" means this subcommand is added at the root of the kubebuilder command
    cmd: ["osdk", "helm", "other-stuff"]
    config: { pruneMakefile: true } # custom config
  - name: "add-bazel"
    type: plugin
    path: /create/api # this plugin will be executed with "kubebuilder create api" command
    cmd: ["add-bazel", "other-stuff"]
    config: { pruneMakefile: true } # custom config

We distinguish subcommand and plugin in the metadata file above.
Besides that, subcommand must implement a Help interface which returns help text related information in json; plugin (transformer) must implement Pipe.

Each time kubebuilder executes, it loads PROJECT file. If the type is subcommand, it delegates to it. IOW, subcommand is responsible to write files etc. If the type is plugin, it pipes the Universe to it.

In terms of help text integration, kubebuilder exec the subcommand e.g. helm --help=json to get the help text information back and construct a "fake" command to use Cobra to write the help text output.

DirectXMan12 · 2019-12-11T01:25:50Z

@mengqiy that doesn't necessarily give us cohesive help though, or command line parsing. We'd have to treat options like git does (flags for parent commands come before the subcommand, anything after goes to the child command), and we wouldn't get combined help.

Plus, I'm not certain we want to persist optional subcommands to the project file -- that feels weird to me.

At any rate, for the subcommand issue, let's say for now subcommands only exist when KB is being used as a library, ignoring the project file, and figure out how to deal with it there.

DirectXMan12 · 2019-12-11T02:09:49Z

Point of note: we don't currently use any persistent flags in kubebuilder. Meaning maybe persistent flag parsing is probably not much of an issue. We just need cohesive help.

Adirio · 2019-12-11T10:03:57Z

designs/extensible-cli-and-scaffolding-plugins.md

+```
+
+### Optional
+Next, a plugin could optionally implement further interfaces to declare its support for specific Kubebuilder subcommands. For example:


Would this be achievable without specific subcommand interfaces? Having specific interfaces for Init, CreateAPI, ... would limit the subcommands to these list and would require modifying the source when any other subcommand wants to be added. What about the following:

type Plugin interface { // Name is the name of this plugin. // For example, Kubebuilder's Go v2 plugin implementation would return "kubebuilder" Name() string // Version is the project version that this plugin implements. // For example, Kubebuilder's Go v2 plugin implementation would return "2" Version() string // Subcommands returns the different subcommands that this plugin implements. Subcommands() []Subcommand } type Subcommand struct { // Command returns the subcommand that this type is implementing. // For example, `kubebuilder create api` subcommand implementation would return `[]string{"create", "api"}` Command() []string // Description returns a description of what this subcommand does. It is used to display help. Description() string // Example returns one or more examples of the command-line usage // of this plugin's project subcommand support. It is used to display help. Example() string // BindFlags binds the plugin's flags to the CLI. This allows each plugin to define its own // command line flags for the kubebuilder subcommand. BindFlags(fs *pflag.FlagSet) // Run runs the subcommand. Run() error }

Given how plugin interfaces are lain out in the current POC, I'd say this is a good idea.

If we went with the above approach, I would think that Subcommand may need optional Subcommands. I don't like the idea of having to understand "create", "API" -> create API. Reading this into a CLI parser would be difficult right?

I think my vote would be for having specific interfaces for specific commands.

If we run into a case where it makes sense to pass more info around, we would need to make this generic to fit the above model. This could lead to lots of confusion in the future IMO.

Being explicit about when I want my plugin to run if I am choosing an override means that if and when kubebuilder adds a new command, I am not accidentally overriding/layering it in my plugin, with a generic subcommand. I think that this is a good thing.

Marking deprecation and changes to a plugin interface will be easier if we have concrete types rather than implementations of a generic interface IMO.

If we went with the above approach, I would think that Subcommand may need optional Subcommands. I don't like the idea of having to understand "create", "API" -> create API. Reading this into a CLI parser would be difficult right?

Subcommand.Command is a []string. kubebuilder create api would return []string{"create", "api"}. The recursive approach could also be done by adding a Subcommands() []Subcommand and changing Command string but I usually tend to avoid recursion.

If we run into a case where it makes sense to pass more info around, we would need to make this generic to fit the above model. This could lead to lots of confusion in the future IMO.

The info is passed through the flags, the commands layer has no info to pass around. Could you think of any example?

Being explicit about when I want my plugin to run if I am choosing an override means that if and when kubebuilder adds a new command, I am not accidentally overriding/layering it in my plugin, with a generic subcommand. I think that this is a good thing.

The counter part is that no extension could develop any command that kubebuilder doesn't offer as a Plugin interface, while the geenric approach would allow it.

Marking deprecation and changes to a plugin interface will be easier if we have concrete types rather than implementations of a generic interface IMO.

The Deprecated interface could still be implemented and checked with generic interface, which difficulty are you seeing?

The info is passed through the flags, the commands layer has no info to pass around. Could you think of any example?

There might be other metadata or state to inject. For example, configuration from the PROJECT file, plugin configuration, environment info, etc. I suppose that could also be defined similar to the Deprecated interface, where the CLI checks if the subcommand implements some injection function(s).

The counter part is that no extension could develop any command that kubebuilder doesn't offer as a Plugin interface, while the geenric approach would allow it.

The WithExtraCommands() functional option to cli.New would allow other binaries to implement their own subcommands, with the caveat that they wouldn't be allowed to override subcommands defined and used by Kubebuilder. So init and create would have well-defined subcommands and interfaces for customizing their logic, but they would not allow plugins to define other custom subcommands.

Using Operator SDK as an example, it would be possible to create a new subcommand that provides integrations with OLM and pass it into the CLI with cli.New(cli.WithExtraCommands(olmCmd))

IMO, preventing open-ended subcommand building in Kubebuilder's built-in subcommand trees would be a feature that ensure's that the Kubebuilder-defined operator development workflow is consistent across different extensions/binaries.

Another potential pain point with this approach is that it sort of locks Kubebuilder into this interface for all subcommands. I can't think of a great example at the moment, but I could envision a requirement where maybe the init subcommand interface needs to have an extra required method on its plugin interface.

Perhaps a middle ground would be something like this:

type Plugin interface { ... } type InitPluginGetter interface { Plugin GetInitPlugin() InitPlugin } type InitPlugin interface { GenericSubcommand } type GenericSubcommand interface { // Description returns a description of what this subcommand does. It is used to display help. Description() string // Example returns one or more examples of the command-line usage // of this plugin's project subcommand support. It is used to display help. Example() string // BindFlags binds the plugin's flags to the CLI. This allows each plugin to define its own // command line flags for the kubebuilder subcommand. BindFlags(fs *pflag.FlagSet) // Run runs the subcommand. Run() error }

In this case, we would still have explicit subcommands and type checking, most subcommand plugins could use the GenericSubcommand interface, but it would give us some flexibility in the future to allow changing individual plugin definitions, while still retaining compiler enforced type checking/safety on Kubebuilder's subcommands.

mengqiy · 2019-12-11T18:09:28Z

Plus, I'm not certain we want to persist optional subcommands to the project file -- that feels weird to me.

It doesn't have to be persisted in the PROJECT file. Maybe in a separate .plugin file is also fine. Or it doesn't persist it anywhere, kubebuilder always gets it and parse it from the root plugin.

mengqiy · 2019-12-11T19:38:00Z

kubebuilder init --scaffold kubebuilder:v2,osdk:v1

One concern I have with this approach is that letting the user specify the plugin version may confuse the users and the users may do the wrong things.
If each plugin has multiple versions, 2+ plugins can have multiple combinations, but only a few of them are supposed to work. E.g. we may have combinations of kubebuilder:v2,osdk:v1, kubebuilder:v2,osdk:v2 kubebuilder:v3,osdk:v1 and kubebuilder:v3,osdk:v2, but only kubebuilder:v2,osdk:v1 and kubebuilder:v3,osdk:v2 are supposed to work.

shawn-hurley

I want to explore the custom config in the project file and where that maps in the interface for a plugin. Unless I am misunderstanding how that would worK?

If each plugin has multiple versions, 2+ plugins can have multiple combinations, but only a few of them are supposed to work.

You are correct, is there a way to determine this by the plugin saying the name/version that it is layered on top of? The questions becomes kubebuilder:v2,osdk:v2,add-bazel:v1. add-bazel would not know about osdk:v2`. I think we could do something clever like "does it layer the root, if it does then we assume it is fine. If it does not, does it layer the next layer if it does great, if not....." WDYT?

This would allow the CLI time check to with some level of knowledge and allow for composition that we may not have expected?

shawn-hurley · 2019-12-18T01:18:37Z

designs/extensible-cli-and-scaffolding-plugins.md

+type Plugin interface {
+    // Version is the project version that this plugin implements.
+    // For example, Kubebuilder's Go v2 plugin implementation would return "2"
+    Version() string


+1 on name and version

We should not force folks to determine if they are are type or a builder, a name for the given plugin that is being used and a version of that plugin will give us the flexibility we need IMO.

if Version is always an int, then can we make it an unsigned int type to get compile-time guarantees?

shawn-hurley · 2019-12-18T01:31:42Z

designs/extensible-cli-and-scaffolding-plugins.md

+```
+
+### Optional
+Next, a plugin could optionally implement further interfaces to declare its support for specific Kubebuilder subcommands. For example:


If we went with the above approach, I would think that Subcommand may need optional Subcommands. I don't like the idea of having to understand "create", "API" -> create API. Reading this into a CLI parser would be difficult right?

I think my vote would be for having specific interfaces for specific commands.

If we run into a case where it makes sense to pass more info around, we would need to make this generic to fit the above model. This could lead to lots of confusion in the future IMO.

Being explicit about when I want my plugin to run if I am choosing an override means that if and when kubebuilder adds a new command, I am not accidentally overriding/layering it in my plugin, with a generic subcommand. I think that this is a good thing.

Marking deprecation and changes to a plugin interface will be easier if we have concrete types rather than implementations of a generic interface IMO.

shawn-hurley · 2019-12-18T01:38:39Z

designs/extensible-cli-and-scaffolding-plugins.md

+    // BindInitFlags binds the plugin's init flags to the CLI. This allows each
+    // plugin to define its own command line flags for the `kubebuilder init`
+    // subcommand.
+    BindInitFlags(fs *pflag.FlagSet)


What would be the benefit of doing this vs. using pflag.FlagSet and the plugin are responsible for adding its own flags and the types that it uses internally for the flags.

I am having a tough time seeing the benefit of that approach vs this approach which feels pretty common to me?

estroz · 2019-12-19T20:04:43Z

@DirectXMan12

I think I'd kinda lean towards this or option 2. It's a lot more flexible, makes it easier to do stuff like "I need project or environment specific tweaks to some base scaffolding" (e.g. I want Bazel build rules set up automatically).

I think something like this is a requirement for a fully-implemented and configurable plugin system. However we may be able to separate that discussion from how we want to write the Plugin interface system, since the internal details can mostly be separated from CLI composition and that discussion itself is complex by itself. Can we break plugin piping into a separate proposal, and just implement pkg/cli for now? That may have been what @droot was saying above.

I hacked together a branch implementing the proposal as-is (no CLI changes though). Take a look and post what you do/don't like: https://github.com/estroz/kubebuilder/tree/feature/plugins

@shawn-hurley

I think we could do something clever like "does it layer the root, if it does then we assume it is fine. If it does not, does it layer the next layer if it does great, if not....."

This layering logic seems feasible. Perhaps we can add a Layer() LayerType method on a Plugin, where LayerType is one of root, modifier, tool:

Only one root plugin is allowed.
Any number of modifier and tool plugins are allowed.
To layer modifiers, each modifier plugin in a PROJECT will have 0..N dependencies on other plugins.
- If you wanted to apply a modification to osdk:v1 you'd specify {name: my-mod:v1, dependency: [osdk:v1]}.
We could also add a Path() string method to Plugin, with a "only modifies files in this path" semantic.
- No two dependency-less modifiers can share parent path elements, to prevent conflicts.

However like I mentioned above, perhaps we should push this layering off into a follow-up proposal since it needs more discussion and it seems like we can mostly implement the underlying interfaces beforehand.

Adirio · 2020-01-02T08:17:28Z

designs/extensible-cli-and-scaffolding-plugins.md

-    Version() string
+    // Version returns the project version that this plugin implements.
+    // For example, Kubebuilder's Go v2 plugin implementation would return 2.
+    Version() uint


I think you should keep this as Version() string as that will allow more flexible versioning formats as 2.1.0 or 2.3-alpha1 similar to api version names-

DirectXMan12 · 2020-01-08T18:39:29Z

Created https://github.com/kubernetes-sigs/kubebuilder/tree/feature/plugins-part-2-electric-boogaloo to collaborate

DirectXMan12 · 2020-01-31T23:03:04Z

doing tentative merge on this design doc. Will continue updating as we develop the feature branch.

/lgtm
/approve

DirectXMan12 · 2020-01-31T23:04:09Z

/hold

@joelanford @estroz can one of you update this with the latest details and we'll get it merged

estroz · 2020-02-05T19:07:14Z

designs/extensible-cli-and-scaffolding-plugins-phase-1.md

+    Name() string
+    // SupportedProjectVersions lists all project configuration versions this
+    // plugin supports, ex. []string{"2", "3"}. The returned slice cannot be empty.
+    SupportedProjectVersions() []string


After looking over our discussion notes, I realized that Plugin.Version() and a PROJECT file's version field can/should drift since the former describes a plugin's development cycle while the latter describes the overall project's. Furthermore, indexing the CLI's plugins, which are indexed by semantic version, with a single digit version string (version: "2") will not work, especially due to the development cycle disconnect described above.

The SupportedProjectVersions() allow a plugin to be added to the CLI's plugins such that they can be indexed by project version from PROJECT/--project-version:

func WithPlugins(plugins ...plugin.Base) Option { return func(c *cli) error { for _, p := range plugins { for _, ver := range p.SupportedProjectVersions() { if _, ok := c.plugins[ver]; !ok { c.plugins[ver] = []plugin.Base{} } c.plugins[ver] = append(c.plugins[ver], p) } } return nil } }

@DirectXMan12 @mengqiy @joelanford @Adirio do my concerns hold water, and if so does this solution make sense?

+1 on that.

+1 seems fine to me

So basically each plugin will have a method that returns which versions it should be added to. That sounds ok to me.

camilamacedo86

It is ok for me.
Waiting for other oks in order to flag it and we see it merged.

DirectXMan12 · 2020-02-05T21:29:13Z

/lgtm
/approve

/hold cancel

🎉

k8s-ci-robot · 2020-02-05T21:29:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12, joelanford

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [DirectXMan12]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

feature: implement plugin proposal in #1250

designs: extensible CLI and scaffolding plugins

53afd47

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 10, 2019

k8s-ci-robot requested review from DirectXMan12 and pwittrock December 10, 2019 02:26

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 10, 2019

joelanford mentioned this pull request Dec 10, 2019

Feature Request: Extensible CLI and scaffolding plugins #1249

Closed

camilamacedo86 reviewed Dec 10, 2019

View reviewed changes

DirectXMan12 reviewed Dec 10, 2019

View reviewed changes

droot reviewed Dec 10, 2019

View reviewed changes

Adirio reviewed Dec 11, 2019

View reviewed changes

shawn-hurley reviewed Dec 18, 2019

View reviewed changes

clarify sentence on project type; add Name() to Plugin interface

c284873

Adirio reviewed Jan 2, 2020

View reviewed changes

estroz mentioned this pull request Jan 8, 2020

feature: implement plugin proposal in #1250 #1290

Merged

add new interface system and link to feature branch

b647850

hasbro17 mentioned this pull request Jan 13, 2020

Mock kubebuilder integration with 'operator-sdk alpha kubebuilder' cmd operator-framework/operator-sdk#2396

Merged

k8s-ci-robot assigned DirectXMan12 Jan 31, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 31, 2020

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 31, 2020

hasbro17 mentioned this pull request Feb 2, 2020

Testing with operator-sdk ?? #1336

Closed

explain PROJECT file updates and plugin version/name conventions

5187a15

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 5, 2020

estroz reviewed Feb 5, 2020

View reviewed changes

camilamacedo86 approved these changes Feb 5, 2020

View reviewed changes

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Feb 5, 2020

Adirio approved these changes Feb 5, 2020

View reviewed changes

k8s-ci-robot merged commit 48ea4cb into kubernetes-sigs:master Feb 5, 2020

estroz mentioned this pull request Feb 20, 2020

Feature Request: Plugins Phase 2 #1378

Closed

k8s-ci-robot added a commit that referenced this pull request Feb 20, 2020

Merge pull request #1290 from estroz/feature/plugins

e322071

feature: implement plugin proposal in #1250

joelanford deleted the extensible-cli-design branch June 2, 2020 13:48


		With all of that said, even this exposure of `cobra.Command` could be problematic. If Kubebuilder decides in the future to transition to a different CLI framework (or to roll its own) it has to either continue maintaining support for these extra cobra commands passed into it, or it was to break the CLI API.

		Are there other ideas for how to handle the following requirements?


		## Overview
		I would like for Kubebuilder to become more extensible, such that it could be imported and used as a library in other projects. Specifically, I'm looking for a way to use Kubebuilder's existing CLI and scaffolding for Go projects, but to also be able to augment Kubebuilder's project versions with other custom project versions so that I can support the Kubebuilder workflow with non-Go operators (e.g. operator-sdk's Ansible and Helm-based operators).

designs: extensible CLI and scaffolding plugins #1250

designs: extensible CLI and scaffolding plugins #1250

Conversation

joelanford commented Dec 10, 2019

joelanford commented Dec 10, 2019

camilamacedo86 Dec 10, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 Dec 11, 2019 • edited

Choose a reason for hiding this comment

estroz Dec 17, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DirectXMan12 commented Dec 10, 2019 • edited

DirectXMan12 commented Dec 10, 2019 • edited

DirectXMan12 commented Dec 10, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estroz Dec 18, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

What do we need/want?

Choose a reason for hiding this comment

droot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mengqiy commented Dec 11, 2019

DirectXMan12 commented Dec 11, 2019

DirectXMan12 commented Dec 11, 2019

Choose a reason for hiding this comment

estroz Dec 17, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelanford Dec 19, 2019 • edited

Choose a reason for hiding this comment

mengqiy commented Dec 11, 2019

mengqiy commented Dec 11, 2019

shawn-hurley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estroz commented Dec 19, 2019 • edited

Choose a reason for hiding this comment

DirectXMan12 commented Jan 8, 2020

DirectXMan12 commented Jan 31, 2020

DirectXMan12 commented Jan 31, 2020

estroz Feb 5, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 left a comment

Choose a reason for hiding this comment

DirectXMan12 commented Feb 5, 2020

k8s-ci-robot commented Feb 5, 2020

camilamacedo86 Dec 10, 2019 •

edited

camilamacedo86 Dec 11, 2019 •

edited

estroz Dec 17, 2019 •

edited

DirectXMan12 commented Dec 10, 2019 •

edited

DirectXMan12 commented Dec 10, 2019 •

edited

DirectXMan12 commented Dec 10, 2019 •

edited

estroz Dec 18, 2019 •

edited

estroz Dec 17, 2019 •

edited

joelanford Dec 19, 2019 •

edited

estroz commented Dec 19, 2019 •

edited

estroz Feb 5, 2020 •

edited