Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Non-context aware plugin listing is broken #1765

Closed
3 of 10 tasks
stmcginnis opened this issue Mar 3, 2022 · 2 comments · Fixed by #3192
Closed
3 of 10 tasks

Non-context aware plugin listing is broken #1765

stmcginnis opened this issue Mar 3, 2022 · 2 comments · Fixed by #3192
Assignees

Comments

@stmcginnis
Copy link
Contributor

Bug description

There are a few spots in the code that have not been updated for the context aware plugin handling. One of those is the tanzu generate-all-docs command. Running this command removes all references to subcommands from the generated CLI docs because it doesn't find any plugins.

The places in the code where this had previously been called that are still working have been updated to have something like:

if config.IsFeatureActivated(config.FeatureContextAwareCLIForPlugins) {
		currentServerName := ""

		server, err := config.GetCurrentServer()
		if err == nil && server != nil {
			currentServerName = server.Name
		}

		serverPlugin, standalonePlugins, err := pluginmanager.InstalledPlugins(currentServerName)
		if err != nil {
			return nil, fmt.Errorf("find installed plugins: %w", err)
		}

		//nolint:gocritic
		p := append(serverPlugin, standalonePlugins...)
		for i := range p {
			plugins = append(plugins, &p[i])
		}
	} else {
		plugins, err = cli.ListPlugins()
		if err != nil {
			return nil, fmt.Errorf("find available plugins: %w", err)
		}
	}

But other places, like the generate-all-docs code, just call ListPlugins to get the set of installed plugins. That fails to find any because it looks like the plugin cache has been changed from being ~/.local/share/tanzu-cli/[set of binaries] to be ~/.local/share/tanzu-cli/[plugin_name]/[version_named_binary].

The ListPlugins code explicitly checks if the first level under ~/.local/share/tanzu-cli/ is a directory, and if so ignores it and continues:

if info.IsDir() {

Since it appears this can now contain multiple versions per plugin directory, special handling would need to be added to look for the most recent binary version.

Some of the places where there is no conditional handling for context aware plugins:

descs, err := cli.ListPlugins("test")

plugins, err := cli.ListPlugins()

installedPlugins, err := ListPlugins()

descriptors, err := cli.ListPlugins()

descriptors, err := cli.ListPlugins()

plugins, err := cli.ListPlugins()

Affected product area (please put an X in all that apply)

  • APIs
  • Addons
  • CLI
  • Docs
  • IAM
  • Installation
  • Plugin
  • Security
  • Test and Release
  • User Experience

Expected behavior

Non-context aware plugins get returned for any instance of ListPlugins and that tanzu generate-all-docs generates the docs for all CLI commands.

@stmcginnis stmcginnis added kind/bug PR/Issue related to a bug needs-triage Indicates an issue or PR needs to be triaged labels Mar 3, 2022
@stmcginnis
Copy link
Contributor Author

Hey @anujc25 - I think you worked on a lot of the context aware bits. Could you take a look at this?

@chandrareddyp
Copy link
Contributor

chandrareddyp commented Sep 7, 2022

This bug has two parts/issues:

  1. The tanzu generate-all-docs command is not working as expected, this is fixed in Update generate-all-docs command logic to fetch installed plugins #3192
  2. The API cli.ListPlugins() is deprecated, it should be replaced with a context-aware plugin list, and also needs to remove feature flag context-aware-cli-for-plugins, to address this created a new issue Remove feature flag context-aware-cli-for-plugins and code which is not relevant #3224

We have addressed all issues in this bug, hence closing this one.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants