vmware-tanzu / tanzu-cli

The Tanzu Core CLI project provides the core functionality of the Tanzu CLI. The CLI is based on a plugin architecture where CLI command functionality can be delivered through independently developed plugin binaries
Apache License 2.0
35 stars 22 forks source link

Add plugin-level remapping support #687

Closed vuil closed 8 months ago

vuil commented 8 months ago

What this PR does / why we need it

    Adds experimental functionality to remap in the CLI command tree
    commands at an entire plugin level.

    The mapping information is currently solely derived from the plugin's
    `info` command output it can extend potentially to mapping metadata 
    specified outside of the plugin binaries.

    Mapping to a nonexistent location should only be possible if the parent
    Command exists AND parent is not one associated another plugin's command
    tree root. (iow where the Command's Annotations indicate it is a wrapper for a
    plugin)

    Any command group that exists in the destination location of the mapping
    is removed from the command tree.

    Mapping is contingent on whether an active CLI context is of a type
    that is one of the supportedContextType list, if the latter is explicitly
    expressed by the plugin.

    An upcoming change will provide more contextual information to the
    plugin on how it's commands are invoked by the Tanzu CLI. Until then
    this remapping functionality is best limited to scenarios of relocating
    without command group renaming, or ones that only involve minor and
    non-surprising renames of the command group for the plugin's commands.

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

Update unit tests. Additionally

Assuming k8s-targetted plugin apps is installed

Build and install additional plugin with different PluginDescriptor ... 1)

        p, err := plugin.NewPlugin(&plugin.PluginDescriptor{
                Name:                 "appsv2",
                Description:          "Applicationz2 on Kubernetes",  << for ease of disambuigate of top level help
                SupportedContextType: []string{string(types.ContextTypeTanzu)},
                InvokedAs:            []string{"apps"},
...
        })

verified that the apps cmd node (invoked via tanzu apps ... ) is replaced with that for the new plugin. Similarly for tanzu kubernetes app, only when an active CLI context is of type tanzu

> tanzu

Usage:
  tanzu [command]

Available command groups:

  Admin
    builder                 Build Tanzu components
  Build
    apps                    Applicationz2 on Kubernetes

----

> tanzu kubernetes

Usage:
  tanzu kubernetes [command]

Aliases:
  kubernetes, k8s

Available command groups:

  Build
    apps                    Applicationz2 on Kubernetes

2)

        p, err := plugin.NewPlugin(&plugin.PluginDescriptor{
                Name:                 "appsv2",
                Description:          "Applicationz2 on Kubernetes",
                InvokedAs:            []string{"apps"},
...
        })

verified that the apps cmd node (invoked via tanzu apps ... ) is unconditionally replaced with that for the new plugin. Similarly for tanzu kubernetes app

3)

        p, err := plugin.NewPlugin(&plugin.PluginDescriptor{
                Name:                 "appsv2",
                Description:          "Applicationz2 on Kubernetes",
                InvokedAs:            []string{"appsalt"},
...
        })

verified that the appsalt cmd node (invoked via tanzu appsalt ... ) is introduced. This is not a recommended scenario as the presence of the original apps command group could lead to UX artifacts like duplicated aliases warning.

Release note

    Adds experimental functionality to remap in the CLI commands at an entire plugin level.

Additional information

Special notes for your reviewer

marckhouzam commented 8 months ago

I assume that the explanation: "An upcoming change will provide more contextual information to the plugin on how it's commands are invoked by the Tanzu CLI. Until then this remapping functionality is best limited to scenarios of relocating without command group renaming, or ones that only involve minor and non-surprising renames of the command group for the plugin's commands."

means that we have to accept that the help text shows the plugin name and not the invokeAs name?

vuil commented 8 months ago

I assume that the explanation: "An upcoming change will provide more contextual information to the plugin on how it's commands are invoked by the Tanzu CLI. Until then this remapping functionality is best limited to scenarios of relocating without command group renaming, or ones that only involve minor and non-surprising renames of the command group for the plugin's commands."

means that we have to accept that the help text shows the plugin name and not the invokeAs name?

It is a rough edge, but I am inclined keep this and address improvement as a followup. Partly because changes will likely involve additiona runtime changes, and when made should hopefully be thorough enough for plugin code (not just the help text) can also account for the mapping.