Phase 2 Plugins should be able to parse flags passed in by a user

[everettraven]

What do you want to happen?

Problem

The current implementation of Phase 2 Plugins does not provide logic to parse flags that a user passes to Kubebuilder to use with an external plugin.

For example, running kubebuilder init --plugins myexternalplugin/v1 --domain example.com you will get an output similar to:

INFO[0000] Adding external plugin: myexternalplugin     
Error: unknown flag: --domain
Usage:
  kubebuilder init [flags]

Examples:
  # Help for initializing a project with version "2"
  kubebuilder init --project-version="2" -h

  # Help for initializing a project with version "3"
  kubebuilder init --project-version="3" -h

Flags:
  -h, --help                     help for init
      --project-version string   project version (default "3")

Global Flags:
      --plugins strings   plugin keys to be used for this subcommand execution

2022/05/16 09:38:53 unknown flag: --domain

This output occurs even if the external plugin defines this flag because the external plugin does not do any logic to bind flags. We can see in this file that a BindFlags function is missing: https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/plugins/external/init.go

Proposed Solution

I propose that a solution is implemented similar to this:

flowchart TD
    A[BindFlags] --> B[Get Flags from External Plugin];
    B ----> C{Error?};
    C -- Yes --> D[Parse all flags and provide general message to consult documentation];
    C -- No --> E[Parse only flags specified by External Plugin response];
    D ----> F[Call External Plugin with flags];
    E ----> F;

Loading

This would allow for external plugin authors to define a command that could be used to get the flags that are allowed, making it so that Kubebuilder can provide an experience to its users that is similar to using the built in plugins. If an external plugin author does not implement this command Kubebuilder can default to allowing all flags to be parsed when using an external plugin and provide a general message to consult the external plugin documentation for more information regarding the flags.

Extra Labels

No response

[everettraven]

/cc @rashmigottipati

[everettraven]

I would like to work on this if this a feature that is agreed to.

[camilamacedo86]

@everettraven,

The same scenario is faced for the plugins shipped on the CLI, i.e: #2563. The flag does not exist for the specific plugin. I think if we could in the Kubebuilder CLI do something like it could help out users to understand that the plugin N/version does not support the flag (for external or internal plugins).

2022/05/16 09:38:53 unknown flag. for the plugin (plugin-key): --domain

[everettraven]

Hi @camilamacedo86,

The same scenario is faced for the plugins shipped on the CLI, i.e: #2563. The flag does not exist for the specific plugin. I think if we could in the Kubebuilder CLI do something like it could help out users to understand that the plugin N/version does not support the flag (for external or internal plugins).

2022/05/16 09:38:53 unknown flag. for the plugin (plugin-key): --domain

The problem that is faced here with Phase 2 Plugins is that even if the flag is defined by the external plugin, this error will occur.

Just to show that the --domain flag is present in the external plugin, here is a snippet from the external plugin source:

def parse_init_args(args_list):
  p = argparse.ArgumentParser(description='Parse "init" plugin flags', conflict_handler="resolve")
  p.add_argument('--domain', type=str, required=False, default='my.domain', help='Project domain')
  p.add_argument('--license', type=str, required=False, default='apache2', help='Project license')
  p.add_argument('--boolean', type=bool, required=False, default=False, help='Boolean flag', action=argparse.BooleanOptionalAction)
  p.add_argument('--help', type=bool, required=False, default=False, help='Display help message', action=argparse.BooleanOptionalAction)
  return p.parse_args(args_list), p

We can see that the --domain flag is defined by the external plugin but we still get the unknown flag error from Kubebuilder because the external plugin implementation does not implement the BindFlags method.

This can be seen here: https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/plugins/external/init.go

Without implementing that BindFlags method Kubebuilder will always throw an unknown flag error when trying to run an external plugin because the flags are not being parsed by the external plugin implementation.

[camilamacedo86]

@rashmigottipati I was speaking with @everettraven to understand it better and it shows a bug.

By using the python samples for testing and the external plugin sample created does define the --domain flag but it is still failing when running kubebuilder init --plugins myexternalplugin/v1 --domain example.com

+1 for me here. @everettraven if you wish to push a PR with a suggestion to fix it your help is more than welcome.
And I am sure that @rashmigottipati will be happy in helping out with the review. If we have the samples here we could easily test them out.

We need to add the samples + e2e tests asap as well.
Great catch 🥇