From c5a0218d6598a41b87bf15f658f941f2723154fe Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Fri, 15 Sep 2023 10:03:01 -0400 Subject: [PATCH] Cleanup the use of feature flags This commit removes the code that was no longer run due to feature flags not being supported anymore. The unit tests are also updated to match this reality. Furthermore, now that we don't support turning off the Central repo of plugins feature, the only OCI discovery that is supported is the "DBBackedOCIDiscovery", so this commit removes the "OCIDiscovery" type and its supporting code. Signed-off-by: Marc Khouzam --- pkg/command/discovery_source_test.go | 1 + pkg/command/plugin.go | 329 ++++++--------------------- pkg/command/plugin_search_test.go | 24 +- pkg/command/plugin_test.go | 193 +++++----------- pkg/command/root.go | 27 +-- pkg/config/init.go | 7 +- pkg/discovery/oci.go | 89 +------- pkg/discovery/oci_test.go | 136 ----------- pkg/pluginmanager/manager_test.go | 4 + 9 files changed, 152 insertions(+), 658 deletions(-) diff --git a/pkg/command/discovery_source_test.go b/pkg/command/discovery_source_test.go index 6487048f0..7533e4aa7 100644 --- a/pkg/command/discovery_source_test.go +++ b/pkg/command/discovery_source_test.go @@ -107,6 +107,7 @@ func Test_createAndListDiscoverySources(t *testing.T) { os.Unsetenv(configlib.EnvConfigNextGenKey) os.Unsetenv(constants.CEIPOptInUserPromptAnswer) os.Unsetenv(constants.EULAPromptAnswer) + os.Unsetenv(constants.ConfigVariableAdditionalDiscoveryForTesting) } func Test_initDiscoverySources(t *testing.T) { diff --git a/pkg/command/plugin.go b/pkg/command/plugin.go index 460ec3bdf..39dd4c7e2 100644 --- a/pkg/command/plugin.go +++ b/pkg/command/plugin.go @@ -16,13 +16,11 @@ import ( kerrors "k8s.io/apimachinery/pkg/util/errors" "github.com/vmware-tanzu/tanzu-plugin-runtime/component" - "github.com/vmware-tanzu/tanzu-plugin-runtime/config" configtypes "github.com/vmware-tanzu/tanzu-plugin-runtime/config/types" "github.com/vmware-tanzu/tanzu-plugin-runtime/plugin" "github.com/vmware-tanzu/tanzu-cli/pkg/cli" "github.com/vmware-tanzu/tanzu-cli/pkg/common" - "github.com/vmware-tanzu/tanzu-cli/pkg/constants" "github.com/vmware-tanzu/tanzu-cli/pkg/discovery" "github.com/vmware-tanzu/tanzu-cli/pkg/pluginmanager" "github.com/vmware-tanzu/tanzu-cli/pkg/pluginsupplier" @@ -69,40 +67,38 @@ func newPluginCmd() *cobra.Command { listPluginCmd.Flags().StringVarP(&outputFormat, "output", "o", "", "Output format (yaml|json|table)") describePluginCmd.Flags().StringVarP(&outputFormat, "output", "o", "", "Output format (yaml|json|table)") - if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) { - installPluginCmd.Flags().StringVar(&group, "group", "", "install the plugins specified by a plugin-group version") + installPluginCmd.Flags().StringVar(&group, "group", "", "install the plugins specified by a plugin-group version") - // --local is renamed to --local-source - installPluginCmd.Flags().StringVarP(&local, "local", "", "", "path to local plugin source") - msg := "this was done in the v1.0.0 release, it will be removed following the deprecation policy (6 months). Use the --local-source flag instead.\n" - if err := installPluginCmd.Flags().MarkDeprecated("local", msg); err != nil { - // Will only fail if the flag does not exist, which would indicate a coding error, - // so let's panic so we notice immediately. - panic(err) - } + // --local is renamed to --local-source + installPluginCmd.Flags().StringVarP(&local, "local", "", "", "path to local plugin source") + msg := "this was done in the v1.0.0 release, it will be removed following the deprecation policy (6 months). Use the --local-source flag instead.\n" + if err := installPluginCmd.Flags().MarkDeprecated("local", msg); err != nil { + // Will only fail if the flag does not exist, which would indicate a coding error, + // so let's panic so we notice immediately. + panic(err) + } - // The --local-source flag for installing plugins is only used in development testing - // and should not be used in production. We mark it as hidden to help convey this reality. - installPluginCmd.Flags().StringVarP(&local, "local-source", "l", "", "path to local plugin source") - if err := installPluginCmd.Flags().MarkHidden("local-source"); err != nil { - // Will only fail if the flag does not exist, which would indicate a coding error, - // so let's panic so we notice immediately. - panic(err) - } - } else { - installPluginCmd.Flags().StringVarP(&local, "local", "l", "", "path to local discovery/distribution source") - listPluginCmd.Flags().StringVarP(&local, "local", "l", "", "path to local plugin source") + // The --local-source flag for installing plugins is only used in development testing + // and should not be used in production. We mark it as hidden to help convey this reality. + installPluginCmd.Flags().StringVarP(&local, "local-source", "l", "", "path to local plugin source") + if err := installPluginCmd.Flags().MarkHidden("local-source"); err != nil { + // Will only fail if the flag does not exist, which would indicate a coding error, + // so let's panic so we notice immediately. + panic(err) } installPluginCmd.Flags().StringVarP(&version, "version", "v", cli.VersionLatest, "version of the plugin") deletePluginCmd.Flags().BoolVarP(&forceDelete, "yes", "y", false, "delete the plugin without asking for confirmation") - if config.IsFeatureActivated(constants.FeatureContextCommand) { - targetFlagDesc := fmt.Sprintf("target of the plugin (%s)", common.TargetList) - installPluginCmd.Flags().StringVarP(&targetStr, "target", "t", "", targetFlagDesc) - upgradePluginCmd.Flags().StringVarP(&targetStr, "target", "t", "", targetFlagDesc) - deletePluginCmd.Flags().StringVarP(&targetStr, "target", "t", "", targetFlagDesc) - describePluginCmd.Flags().StringVarP(&targetStr, "target", "t", "", targetFlagDesc) - } + targetFlagDesc := fmt.Sprintf("target of the plugin (%s)", common.TargetList) + installPluginCmd.Flags().StringVarP(&targetStr, "target", "t", "", targetFlagDesc) + upgradePluginCmd.Flags().StringVarP(&targetStr, "target", "t", "", targetFlagDesc) + deletePluginCmd.Flags().StringVarP(&targetStr, "target", "t", "", targetFlagDesc) + describePluginCmd.Flags().StringVarP(&targetStr, "target", "t", "", targetFlagDesc) + + installPluginCmd.MarkFlagsMutuallyExclusive("group", "local") + installPluginCmd.MarkFlagsMutuallyExclusive("group", "local-source") + installPluginCmd.MarkFlagsMutuallyExclusive("group", "version") + installPluginCmd.MarkFlagsMutuallyExclusive("group", "target") pluginCmd.AddCommand( listPluginCmd, @@ -113,23 +109,12 @@ func newPluginCmd() *cobra.Command { cleanPluginCmd, syncPluginCmd, discoverySourceCmd, + newSearchPluginCmd(), + newPluginGroupCmd(), + newDownloadBundlePluginCmd(), + newUploadBundlePluginCmd(), ) - if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) { - installPluginCmd.MarkFlagsMutuallyExclusive("group", "local") - installPluginCmd.MarkFlagsMutuallyExclusive("group", "local-source") - installPluginCmd.MarkFlagsMutuallyExclusive("group", "version") - if config.IsFeatureActivated(constants.FeatureContextCommand) { - installPluginCmd.MarkFlagsMutuallyExclusive("group", "target") - } - pluginCmd.AddCommand( - newSearchPluginCmd(), - newPluginGroupCmd(), - newDownloadBundlePluginCmd(), - newUploadBundlePluginCmd(), - ) - } - return pluginCmd } @@ -140,61 +125,32 @@ func newListPluginCmd() *cobra.Command { Long: "List installed standalone plugins or plugins recommended by the contexts being used", RunE: func(cmd *cobra.Command, args []string) error { errorList := make([]error, 0) - if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) { - // List installed standalone plugins - standalonePlugins, err := pluginsupplier.GetInstalledStandalonePlugins() - if err != nil { - errorList = append(errorList, err) - log.Warningf("there was an error while getting installed standalone plugins, error information: '%v'", err.Error()) - } - sort.Sort(cli.PluginInfoSorter(standalonePlugins)) - - // List installed context plugins and also missing context plugins. - // Showing missing ones guides the user to know some plugins are recommended for the - // active contexts, but are not installed. - installedContextPlugins, missingContextPlugins, pluginSyncRequired, err := getInstalledAndMissingContextPlugins() - if err != nil { - errorList = append(errorList, err) - log.Warningf(errorWhileGettingContextPlugins, err.Error()) - } - sort.Sort(discovery.DiscoveredSorter(installedContextPlugins)) - sort.Sort(discovery.DiscoveredSorter(missingContextPlugins)) - - if config.IsFeatureActivated(constants.FeatureContextCommand) && (outputFormat == "" || outputFormat == string(component.TableOutputType)) { - displayInstalledAndMissingSplitView(standalonePlugins, installedContextPlugins, missingContextPlugins, pluginSyncRequired, cmd.OutOrStdout()) - } else { - displayInstalledAndMissingListView(standalonePlugins, installedContextPlugins, missingContextPlugins, cmd.OutOrStdout()) - } - - return kerrors.NewAggregate(errorList) - } - - // Plugin listing before the Central Repository feature - var err error - var availablePlugins []discovery.Discovered - if local != "" { - // get absolute local path - local, err = filepath.Abs(local) - if err != nil { - return err - } - availablePlugins, err = pluginmanager.AvailablePluginsFromLocalSource(local) - } else { - availablePlugins, err = pluginmanager.AvailablePlugins() + // List installed standalone plugins + standalonePlugins, err := pluginsupplier.GetInstalledStandalonePlugins() + if err != nil { + errorList = append(errorList, err) + log.Warningf("there was an error while getting installed standalone plugins, error information: '%v'", err.Error()) } + sort.Sort(cli.PluginInfoSorter(standalonePlugins)) + // List installed context plugins and also missing context plugins. + // Showing missing ones guides the user to know some plugins are recommended for the + // active contexts, but are not installed. + installedContextPlugins, missingContextPlugins, pluginSyncRequired, err := getInstalledAndMissingContextPlugins() if err != nil { - log.Warningf("there was an error while getting available plugins, error information: '%v'", err.Error()) + errorList = append(errorList, err) + log.Warningf(errorWhileGettingContextPlugins, err.Error()) } - sort.Sort(discovery.DiscoveredSorter(availablePlugins)) + sort.Sort(discovery.DiscoveredSorter(installedContextPlugins)) + sort.Sort(discovery.DiscoveredSorter(missingContextPlugins)) - if config.IsFeatureActivated(constants.FeatureContextCommand) && (outputFormat == "" || outputFormat == string(component.TableOutputType)) { - displayPluginListOutputSplitViewContext(availablePlugins, cmd.OutOrStdout()) + if outputFormat == "" || outputFormat == string(component.TableOutputType) { + displayInstalledAndMissingSplitView(standalonePlugins, installedContextPlugins, missingContextPlugins, pluginSyncRequired, cmd.OutOrStdout()) } else { - displayPluginListOutputListView(availablePlugins, cmd.OutOrStdout()) + displayInstalledAndMissingListView(standalonePlugins, installedContextPlugins, missingContextPlugins, cmd.OutOrStdout()) } - return err + return kerrors.NewAggregate(errorList) }, } @@ -230,12 +186,28 @@ func newDescribePluginCmd() *cobra.Command { return describeCmd } -// nolint: gocyclo func newInstallPluginCmd() *cobra.Command { var installCmd = &cobra.Command{ Use: "install [" + pluginNameCaps + "]", Short: "Install a plugin", - Args: cobra.MaximumNArgs(1), + Long: "Install a specific plugin by name or specify all to install all plugins of a group", + Example: ` + # Install all plugins of the vmware-tkg/default plugin group version v2.1.0 + tanzu plugin install --group vmware-tkg/default:v2.1.0 + + # Install all plugins of the latest version of the vmware-tkg/default plugin group + tanzu plugin install --group vmware-tkg/default + + # Install the latest version of plugin "myPlugin" + # If the plugin exists for more than one target, an error will be thrown + tanzu plugin install myPlugin + + # Install the latest version of plugin "myPlugin" for target kubernetes + tanzu plugin install myPlugin --target k8s + + # Install version v1.0.0 of plugin "myPlugin" + tanzu plugin install myPlugin --version v1.0.0`, + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { var err error var pluginName string @@ -243,9 +215,6 @@ func newInstallPluginCmd() *cobra.Command { if !configtypes.IsValidTarget(targetStr, true, true) { return errors.New(invalidTargetMsg) } - if config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) { - return legacyPluginInstall(cmd, args) - } if group != "" { // We are installing from a group @@ -312,80 +281,9 @@ func newInstallPluginCmd() *cobra.Command { return nil }, } - if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) { - installCmd.Example = ` - # Install all plugins of the vmware-tkg/default plugin group version v2.1.0 - tanzu plugin install --group vmware-tkg/default:v2.1.0 - - # Install all plugins of the latest version of the vmware-tkg/default plugin group - tanzu plugin install --group vmware-tkg/default - - # Install the latest version of plugin "myPlugin" - # If the plugin exists for more than one target, an error will be thrown - tanzu plugin install myPlugin - - # Install the latest version of plugin "myPlugin" for target kubernetes - tanzu plugin install myPlugin --target k8s - - # Install version v1.0.0 of plugin "myPlugin" - tanzu plugin install myPlugin --version v1.0.0` - installCmd.Long = "Install a specific plugin by name or specify all to install all plugins of a group" - } return installCmd } -func legacyPluginInstall(cmd *cobra.Command, args []string) error { - var err error - if len(args) == 0 { - return fmt.Errorf("missing plugin name or '%s' as an argument", cli.AllPlugins) - } - pluginName := args[0] - - // Invoke install plugin from local source if local files are provided - if local != "" { - // get absolute local path - local, err = filepath.Abs(local) - if err != nil { - return err - } - err = pluginmanager.InstallPluginsFromLocalSource(pluginName, version, getTarget(), local, false) - if err != nil { - return err - } - if pluginName == cli.AllPlugins { - log.Successf("successfully installed all plugins") - } else { - log.Successf("successfully installed '%s' plugin", pluginName) - } - return nil - } - - // Invoke plugin sync if install all plugins is mentioned - if pluginName == cli.AllPlugins { - err = pluginmanager.SyncPlugins() - if err != nil { - return err - } - log.Successf("successfully installed all plugins") - return nil - } - - pluginVersion := version - if pluginVersion == cli.VersionLatest { - pluginVersion, err = pluginmanager.GetRecommendedVersionOfPlugin(pluginName, getTarget()) - if err != nil { - return err - } - } - - err = pluginmanager.InstallStandalonePlugin(pluginName, pluginVersion, getTarget()) - if err != nil { - return err - } - log.Successf("successfully installed '%s' plugin", pluginName) - return nil -} - func newUpgradePluginCmd() *cobra.Command { var upgradeCmd = &cobra.Command{ Use: "upgrade " + pluginNameCaps, @@ -401,19 +299,9 @@ func newUpgradePluginCmd() *cobra.Command { return errors.New(invalidTargetMsg) } - var pluginVersion string - if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) { - // With the Central Repository feature we can simply request to install - // the recommendedVersion. - pluginVersion = cli.VersionLatest - } else { - pluginVersion, err = pluginmanager.GetRecommendedVersionOfPlugin(pluginName, getTarget()) - if err != nil { - return err - } - } - - err = pluginmanager.UpgradePlugin(pluginName, pluginVersion, getTarget()) + // With the Central Repository feature we can simply request to install + // the recommendedVersion. + err = pluginmanager.UpgradePlugin(pluginName, cli.VersionLatest, getTarget()) if err != nil { return err } @@ -493,16 +381,6 @@ Plugins installed with this command will only be available while the context rem return syncCmd } -// getInstalledElseAvailablePluginVersion return installed plugin version if plugin is installed -// if not installed it returns available recommended plugin version -func getInstalledElseAvailablePluginVersion(p *discovery.Discovered) string { - installedOrAvailableVersion := p.InstalledVersion - if installedOrAvailableVersion == "" { - installedOrAvailableVersion = p.RecommendedVersion - } - return installedOrAvailableVersion -} - // getInstalledAndMissingContextPlugins returns any context plugins that are not installed func getInstalledAndMissingContextPlugins() (installed, missing []discovery.Discovered, pluginSyncRequired bool, err error) { errorList := make([]error, 0) @@ -549,75 +427,6 @@ func getInstalledAndMissingContextPlugins() (installed, missing []discovery.Disc return installed, missing, pluginSyncRequired, kerrors.NewAggregate(errorList) } -func displayPluginListOutputListView(availablePlugins []discovery.Discovered, writer io.Writer) { - var data [][]string - var output component.OutputWriter - - for index := range availablePlugins { - data = append(data, []string{availablePlugins[index].Name, availablePlugins[index].Description, availablePlugins[index].Scope, - availablePlugins[index].Source, getInstalledElseAvailablePluginVersion(&availablePlugins[index]), availablePlugins[index].Status}) - } - output = component.NewOutputWriter(writer, outputFormat, "Name", "Description", "Scope", "Discovery", "Version", "Status") - - for _, row := range data { - vals := make([]interface{}, len(row)) - for i, val := range row { - vals[i] = val - } - output.AddRow(vals...) - } - output.Render() -} - -func displayPluginListOutputSplitViewContext(availablePlugins []discovery.Discovered, writer io.Writer) { - var dataStandalone [][]string - var outputStandalone component.OutputWriter - dataContext := make(map[string][][]string) - outputContext := make(map[string]component.OutputWriter) - - outputStandalone = component.NewOutputWriter(writer, outputFormat, "Name", "Description", "Target", "Discovery", "Version", "Status") - - for index := range availablePlugins { - if availablePlugins[index].Scope == common.PluginScopeStandalone { - newRow := []string{availablePlugins[index].Name, availablePlugins[index].Description, string(availablePlugins[index].Target), - availablePlugins[index].Source, getInstalledElseAvailablePluginVersion(&availablePlugins[index]), availablePlugins[index].Status} - dataStandalone = append(dataStandalone, newRow) - } else { - newRow := []string{availablePlugins[index].Name, availablePlugins[index].Description, string(availablePlugins[index].Target), - getInstalledElseAvailablePluginVersion(&availablePlugins[index]), availablePlugins[index].Status} - outputContext[availablePlugins[index].ContextName] = component.NewOutputWriter(writer, outputFormat, "Name", "Description", "Target", "Version", "Status") - data := dataContext[availablePlugins[index].ContextName] - data = append(data, newRow) - dataContext[availablePlugins[index].ContextName] = data - } - } - - addDataToOutputWriter := func(output component.OutputWriter, data [][]string) { - for _, row := range data { - vals := make([]interface{}, len(row)) - for i, val := range row { - vals[i] = val - } - output.AddRow(vals...) - } - } - - cyanBold := color.New(color.FgCyan).Add(color.Bold) - cyanBoldItalic := color.New(color.FgCyan).Add(color.Bold, color.Italic) - - _, _ = cyanBold.Println("Standalone Plugins") - addDataToOutputWriter(outputStandalone, dataStandalone) - outputStandalone.Render() - - for context, writer := range outputContext { - fmt.Println("") - _, _ = cyanBold.Println("Plugins from Context: ", cyanBoldItalic.Sprintf(context)) - data := dataContext[context] - addDataToOutputWriter(writer, data) - writer.Render() - } -} - func displayInstalledAndMissingSplitView(installedStandalonePlugins []cli.PluginInfo, installedContextPlugins, missingContextPlugins []discovery.Discovered, pluginSyncRequired bool, writer io.Writer) { // List installed standalone plugins cyanBold := color.New(color.FgCyan).Add(color.Bold) diff --git a/pkg/command/plugin_search_test.go b/pkg/command/plugin_search_test.go index ab2626933..339291b2f 100644 --- a/pkg/command/plugin_search_test.go +++ b/pkg/command/plugin_search_test.go @@ -18,18 +18,11 @@ import ( func TestPluginSearch(t *testing.T) { tests := []struct { - test string - centralRepoDisabled string - args []string - expected string - expectedFailure bool + test string + args []string + expected string + expectedFailure bool }{ - { - test: "no 'plugin search' without central repo", - centralRepoDisabled: "true", - args: []string{"plugin", "search"}, - expected: "Provides all lifecycle operations for plugins", - }, { test: "invalid target", args: []string{"plugin", "search", "--target", "invalid"}, @@ -83,15 +76,6 @@ func TestPluginSearch(t *testing.T) { for _, spec := range tests { t.Run(spec.test, func(t *testing.T) { - // Disable the Central Repository feature if needed - enabled := "true" - if !strings.EqualFold(spec.centralRepoDisabled, "true") { - enabled = "false" - } - featureArray := strings.Split(constants.FeatureDisableCentralRepositoryForTesting, ".") - err := config.SetFeature(featureArray[1], featureArray[2], enabled) - assert.Nil(err) - rootCmd, err := NewRootCmd() assert.Nil(err) rootCmd.SetArgs(spec.args) diff --git a/pkg/command/plugin_test.go b/pkg/command/plugin_test.go index 95d38b13e..b074b938b 100644 --- a/pkg/command/plugin_test.go +++ b/pkg/command/plugin_test.go @@ -25,122 +25,71 @@ import ( func TestPluginList(t *testing.T) { tests := []struct { - test string - centralRepoFeature bool - plugins []string - versions []string - targets []configtypes.Target - args []string - expected string - expectedFailure bool + test string + plugins []string + versions []string + targets []configtypes.Target + args []string + expected string + expectedFailure bool }{ { - test: "With empty config file(no discovery sources added) and no plugins installed with no central repo", + test: "no '--local' option", + args: []string{"plugin", "list", "--local", "someDirectory"}, + expectedFailure: true, + expected: "unknown flag: --local", + }, + { + test: "With empty config file(no discovery sources added) and no plugins installed", plugins: []string{}, args: []string{"plugin", "list"}, expectedFailure: false, - expected: "NAME DESCRIPTION TARGET DISCOVERY VERSION STATUS", + expected: "NAME DESCRIPTION TARGET VERSION STATUS", }, { - test: "With empty config file(no discovery sources added) and when one additional plugin installed with no central repo", + test: "With empty config file(no discovery sources added) and when one additional plugin installed", plugins: []string{"foo"}, versions: []string{"v0.1.0"}, targets: []configtypes.Target{configtypes.TargetK8s}, args: []string{"plugin", "list"}, expectedFailure: false, - expected: "NAME DESCRIPTION TARGET DISCOVERY VERSION STATUS foo some foo description kubernetes v0.1.0 installed", + expected: "NAME DESCRIPTION TARGET VERSION STATUS foo some foo description kubernetes v0.1.0 installed", }, { - test: "With empty config file(no discovery sources added) and when more than one plugin is installed with no central repo", + test: "With empty config file(no discovery sources added) and when more than one plugin is installed", plugins: []string{"foo", "bar"}, versions: []string{"v0.1.0", "v0.2.0"}, targets: []configtypes.Target{configtypes.TargetTMC, configtypes.TargetK8s}, args: []string{"plugin", "list"}, expectedFailure: false, - expected: "NAME DESCRIPTION TARGET DISCOVERY VERSION STATUS bar some bar description kubernetes v0.2.0 installed foo some foo description mission-control v0.1.0 installed", + expected: "NAME DESCRIPTION TARGET VERSION STATUS bar some bar description kubernetes v0.2.0 installed foo some foo description mission-control v0.1.0 installed", }, { - test: "when json output is requested with no central repo", + test: "when json output is requested", plugins: []string{"foo"}, versions: []string{"v0.1.0"}, targets: []configtypes.Target{configtypes.TargetK8s}, args: []string{"plugin", "list", "-o", "json"}, expectedFailure: false, - expected: `[ { "description": "some foo description", "discovery": "", "name": "foo", "scope": "Standalone", "status": "installed", "version": "v0.1.0" } ]`, + expected: `[ { "context": "", "description": "some foo description", "name": "foo", "status": "installed", "target": "kubernetes", "version": "v0.1.0" } ]`, }, { - test: "when yaml output is requested with no central repo", + test: "when yaml output is requested", plugins: []string{"foo"}, versions: []string{"v0.1.0"}, targets: []configtypes.Target{configtypes.TargetK8s}, args: []string{"plugin", "list", "-o", "yaml"}, expectedFailure: false, - expected: `- description: some foo description discovery: "" name: foo scope: Standalone status: installed version: v0.1.0`, - }, - { - test: "no '--local' option", - centralRepoFeature: true, - args: []string{"plugin", "list", "--local", "someDirectory"}, - expectedFailure: true, - expected: "unknown flag: --local", - }, - { - test: "With empty config file(no discovery sources added) and no plugins installed", - centralRepoFeature: true, - plugins: []string{}, - args: []string{"plugin", "list"}, - expectedFailure: false, - expected: "NAME DESCRIPTION TARGET VERSION STATUS", - }, - { - test: "With empty config file(no discovery sources added) and when one additional plugin installed", - centralRepoFeature: true, - plugins: []string{"foo"}, - versions: []string{"v0.1.0"}, - targets: []configtypes.Target{configtypes.TargetK8s}, - args: []string{"plugin", "list"}, - expectedFailure: false, - expected: "NAME DESCRIPTION TARGET VERSION STATUS foo some foo description kubernetes v0.1.0 installed", - }, - { - test: "With empty config file(no discovery sources added) and when more than one plugin is installed", - centralRepoFeature: true, - plugins: []string{"foo", "bar"}, - versions: []string{"v0.1.0", "v0.2.0"}, - targets: []configtypes.Target{configtypes.TargetTMC, configtypes.TargetK8s}, - args: []string{"plugin", "list"}, - expectedFailure: false, - expected: "NAME DESCRIPTION TARGET VERSION STATUS bar some bar description kubernetes v0.2.0 installed foo some foo description mission-control v0.1.0 installed", - }, - { - test: "when json output is requested", - centralRepoFeature: true, - plugins: []string{"foo"}, - versions: []string{"v0.1.0"}, - targets: []configtypes.Target{configtypes.TargetK8s}, - args: []string{"plugin", "list", "-o", "json"}, - expectedFailure: false, - expected: `[ { "context": "", "description": "some foo description", "name": "foo", "status": "installed", "target": "kubernetes", "version": "v0.1.0" } ]`, - }, - { - test: "when yaml output is requested", - centralRepoFeature: true, - plugins: []string{"foo"}, - versions: []string{"v0.1.0"}, - targets: []configtypes.Target{configtypes.TargetK8s}, - args: []string{"plugin", "list", "-o", "yaml"}, - expectedFailure: false, - expected: `- context: "" description: some foo description name: foo status: installed target: kubernetes version: v0.1.0`, + expected: `- context: "" description: some foo description name: foo status: installed target: kubernetes version: v0.1.0`, }, { - test: "plugin describe json output requested", - centralRepoFeature: true, - plugins: []string{"foo"}, - versions: []string{"v0.1.0"}, - targets: []configtypes.Target{configtypes.TargetK8s}, - args: []string{"plugin", "describe", "foo", "-o", "json"}, - expectedFailure: false, - expected: `[ { "description": "some foo description", "installationpath": "%v", "name": "foo", "status": "installed", "target": "kubernetes", "version": "v0.1.0" } ]`, + test: "plugin describe json output requested", + plugins: []string{"foo"}, + versions: []string{"v0.1.0"}, + targets: []configtypes.Target{configtypes.TargetK8s}, + args: []string{"plugin", "describe", "foo", "-o", "json"}, + expectedFailure: false, + expected: `[ { "description": "some foo description", "installationpath": "%v", "name": "foo", "status": "installed", "target": "kubernetes", "version": "v0.1.0" } ]`, }, } @@ -165,13 +114,6 @@ func TestPluginList(t *testing.T) { err = config.SetFeature(featureArray[1], featureArray[2], "true") assert.Nil(t, err) - // Disable the Central Repository feature if needed - if !spec.centralRepoFeature { - featureArray := strings.Split(constants.FeatureDisableCentralRepositoryForTesting, ".") - err := config.SetFeature(featureArray[1], featureArray[2], "true") - assert.Nil(t, err) - } - var completionType uint8 t.Run(spec.test, func(t *testing.T) { assert := assert.New(t) @@ -310,60 +252,46 @@ func TestDeletePlugin(t *testing.T) { func TestInstallPlugin(t *testing.T) { tests := []struct { - test string - centralRepoDisabled string - args []string - expectedErrorMsg string - expectedFailure bool + test string + args []string + expectedErrorMsg string + expectedFailure bool }{ { - test: "need plugin name or 'all' with no central repo", - centralRepoDisabled: "true", - args: []string{"plugin", "install"}, - expectedFailure: true, - expectedErrorMsg: "missing plugin name or 'all' as an argument", - }, - { - test: "need plugin name if no group", - centralRepoDisabled: "false", - args: []string{"plugin", "install"}, - expectedFailure: true, - expectedErrorMsg: "missing plugin name as an argument", + test: "need plugin name if no group", + args: []string{"plugin", "install"}, + expectedFailure: true, + expectedErrorMsg: "missing plugin name as an argument", }, { - test: "no 'all' option", - centralRepoDisabled: "false", - args: []string{"plugin", "install", "all"}, - expectedFailure: true, - expectedErrorMsg: "the 'all' argument can only be used with the '--group' flag", + test: "no 'all' option", + args: []string{"plugin", "install", "all"}, + expectedFailure: true, + expectedErrorMsg: "the 'all' argument can only be used with the '--group' flag", }, { - test: "invalid target", - centralRepoDisabled: "false", - args: []string{"plugin", "install", "--target", "invalid", "myplugin"}, - expectedFailure: true, - expectedErrorMsg: invalidTargetMsg, + test: "invalid target", + args: []string{"plugin", "install", "--target", "invalid", "myplugin"}, + expectedFailure: true, + expectedErrorMsg: invalidTargetMsg, }, { - test: "no --group and --local-source together", - centralRepoDisabled: "false", - args: []string{"plugin", "install", "--group", "testgroup", "--local-source", "./", "myplugin"}, - expectedFailure: true, - expectedErrorMsg: "if any flags in the group [group local-source] are set none of the others can be", + test: "no --group and --local-source together", + args: []string{"plugin", "install", "--group", "testgroup", "--local-source", "./", "myplugin"}, + expectedFailure: true, + expectedErrorMsg: "if any flags in the group [group local-source] are set none of the others can be", }, { - test: "no --group and --target together", - centralRepoDisabled: "false", - args: []string{"plugin", "install", "--group", "testgroup", "--target", "k8s", "myplugin"}, - expectedFailure: true, - expectedErrorMsg: "if any flags in the group [group target] are set none of the others can be", + test: "no --group and --target together", + args: []string{"plugin", "install", "--group", "testgroup", "--target", "k8s", "myplugin"}, + expectedFailure: true, + expectedErrorMsg: "if any flags in the group [group target] are set none of the others can be", }, { - test: "no --group and --version together", - centralRepoDisabled: "false", - args: []string{"plugin", "install", "--group", "testgroup", "--version", "v1.1.1", "myplugin"}, - expectedFailure: true, - expectedErrorMsg: "if any flags in the group [group version] are set none of the others can be", + test: "no --group and --version together", + args: []string{"plugin", "install", "--group", "testgroup", "--version", "v1.1.1", "myplugin"}, + expectedFailure: true, + expectedErrorMsg: "if any flags in the group [group version] are set none of the others can be", }, } @@ -394,11 +322,6 @@ func TestInstallPlugin(t *testing.T) { for _, spec := range tests { t.Run(spec.test, func(t *testing.T) { - // Disable the Central Repository feature if needed - featureArray := strings.Split(constants.FeatureDisableCentralRepositoryForTesting, ".") - err := config.SetFeature(featureArray[1], featureArray[2], spec.centralRepoDisabled) - assert.Nil(err) - rootCmd, err := NewRootCmd() assert.Nil(err) rootCmd.SetArgs(spec.args) diff --git a/pkg/command/root.go b/pkg/command/root.go index bab53e74f..88cc71ddc 100644 --- a/pkg/command/root.go +++ b/pkg/command/root.go @@ -19,7 +19,6 @@ import ( "github.com/vmware-tanzu/tanzu-cli/pkg/cli" "github.com/vmware-tanzu/tanzu-cli/pkg/common" cliconfig "github.com/vmware-tanzu/tanzu-cli/pkg/config" - "github.com/vmware-tanzu/tanzu-cli/pkg/constants" "github.com/vmware-tanzu/tanzu-cli/pkg/pluginmanager" "github.com/vmware-tanzu/tanzu-cli/pkg/pluginsupplier" "github.com/vmware-tanzu/tanzu-cli/pkg/telemetry" @@ -30,7 +29,7 @@ import ( ) // NewRootCmd creates a root command. -func NewRootCmd() (*cobra.Command, error) { //nolint:gocyclo +func NewRootCmd() (*cobra.Command, error) { var rootCmd = newRootCmd() uFunc := cli.NewMainUsage().UsageFunc() rootCmd.SetUsageFunc(uFunc) @@ -45,6 +44,9 @@ func NewRootCmd() (*cobra.Command, error) { //nolint:gocyclo initCmd, completionCmd, configCmd, + contextCmd, + k8sCmd, + tmcCmd, genAllDocsCmd, // Note(TODO:prkalle): The below ceip-participation command(experimental) added may be removed in the next release, // If we decide to fold this functionality into existing 'tanzu telemetry' plugin @@ -53,20 +55,13 @@ func NewRootCmd() (*cobra.Command, error) { //nolint:gocyclo if _, err := ensureCLIInstanceID(); err != nil { return nil, errors.Wrap(err, "failed to ensure CLI ID") } - // If the context and target feature is enabled, add the corresponding commands under root. - if config.IsFeatureActivated(constants.FeatureContextCommand) { - rootCmd.AddCommand( - contextCmd, - k8sCmd, - tmcCmd, - ) - mapTargetToCmd := map[configtypes.Target]*cobra.Command{ - configtypes.TargetK8s: k8sCmd, - configtypes.TargetTMC: tmcCmd, - } - if err := addPluginsToTarget(mapTargetToCmd); err != nil { - return nil, err - } + + mapTargetToCmd := map[configtypes.Target]*cobra.Command{ + configtypes.TargetK8s: k8sCmd, + configtypes.TargetTMC: tmcCmd, + } + if err := addPluginsToTarget(mapTargetToCmd); err != nil { + return nil, err } plugins, err := pluginsupplier.GetInstalledPlugins() diff --git a/pkg/config/init.go b/pkg/config/init.go index a712ef174..1bf426653 100644 --- a/pkg/config/init.go +++ b/pkg/config/init.go @@ -33,10 +33,5 @@ func init() { // PopulateDefaultCentralDiscovery() handles the locking of the config file itself by // using the config file higher-level APIs config.ReleaseTanzuConfigLock() - - // Can only check for config.IsFeatureActivated() once the feature flags are setup - // by the above calls. - if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) { - _ = PopulateDefaultCentralDiscovery(false) - } + _ = PopulateDefaultCentralDiscovery(false) } diff --git a/pkg/discovery/oci.go b/pkg/discovery/oci.go index 626179392..286f20b21 100644 --- a/pkg/discovery/oci.go +++ b/pkg/discovery/oci.go @@ -6,31 +6,11 @@ package discovery import ( "path" "path/filepath" - "strings" - "github.com/pkg/errors" - apimachineryjson "k8s.io/apimachinery/pkg/runtime/serializer/json" - - cliv1alpha1 "github.com/vmware-tanzu/tanzu-cli/apis/cli/v1alpha1" - "github.com/vmware-tanzu/tanzu-cli/pkg/carvelhelpers" "github.com/vmware-tanzu/tanzu-cli/pkg/common" - "github.com/vmware-tanzu/tanzu-cli/pkg/constants" "github.com/vmware-tanzu/tanzu-cli/pkg/plugininventory" - "github.com/vmware-tanzu/tanzu-plugin-runtime/config" ) -// OCIDiscovery is an artifact discovery endpoint utilizing OCI image -type OCIDiscovery struct { - // name is a name of the discovery - name string - // image is an OCI compliant image. Which include DNS-compatible registry name, - // a valid URI path(MAY contain zero or more ‘/’) and a valid tag - // E.g., harbor.my-domain.local/tanzu-cli/plugins-manifest:latest - // Contains a directory containing YAML files, each of which contains single - // CLIPlugin API resource. - image string -} - // NewOCIDiscovery returns a new Discovery using the specified OCI image. func NewOCIDiscovery(name, image string, options ...DiscoveryOptions) Discovery { // Initialize discovery options @@ -39,17 +19,10 @@ func NewOCIDiscovery(name, image string, options ...DiscoveryOptions) Discovery option(opts) } - if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) { - discovery := newDBBackedOCIDiscovery(name, image) - discovery.pluginCriteria = opts.PluginDiscoveryCriteria - discovery.useLocalCacheOnly = opts.UseLocalCacheOnly - return discovery - } - - return &OCIDiscovery{ - name: name, - image: image, - } + discovery := newDBBackedOCIDiscovery(name, image) + discovery.pluginCriteria = opts.PluginDiscoveryCriteria + discovery.useLocalCacheOnly = opts.UseLocalCacheOnly + return discovery } // NewOCIGroupDiscovery returns a new plugn group Discovery using the specified OCI image. @@ -84,57 +57,3 @@ func newDBBackedOCIDiscovery(name, image string) *DBBackedOCIDiscovery { inventory: inventory, } } - -// List available plugins. -func (od *OCIDiscovery) List() (plugins []Discovered, err error) { - return od.Manifest() -} - -// Name of the repository. -func (od *OCIDiscovery) Name() string { - return od.name -} - -// Type of the discovery. -func (od *OCIDiscovery) Type() string { - return common.DiscoveryTypeOCI -} - -// Manifest returns the manifest for a local repository. -func (od *OCIDiscovery) Manifest() ([]Discovered, error) { - outputData, err := carvelhelpers.ProcessCarvelPackage(od.image) - if err != nil { - return nil, errors.Wrap(err, "error while processing package") - } - - return processDiscoveryManifestData(outputData, od.name) -} - -func processDiscoveryManifestData(data []byte, discoveryName string) ([]Discovered, error) { - plugins := make([]Discovered, 0) - - for _, resourceYAML := range strings.Split(string(data), "---") { - scheme, err := cliv1alpha1.SchemeBuilder.Build() - if err != nil { - return nil, errors.Wrap(err, "failed to create scheme") - } - s := apimachineryjson.NewSerializerWithOptions(apimachineryjson.DefaultMetaFactory, scheme, scheme, - apimachineryjson.SerializerOptions{Yaml: true, Pretty: false, Strict: false}) - var p cliv1alpha1.CLIPlugin - _, _, err = s.Decode([]byte(resourceYAML), nil, &p) - if err != nil { - return nil, errors.Wrap(err, "could not decode discovery manifests") - } - - dp, err := DiscoveredFromK8sV1alpha1(&p) - if err != nil { - return nil, err - } - dp.Source = discoveryName - dp.DiscoveryType = common.DiscoveryTypeOCI - if dp.Name != "" { - plugins = append(plugins, dp) - } - } - return plugins, nil -} diff --git a/pkg/discovery/oci_test.go b/pkg/discovery/oci_test.go index 503bf9215..cb6e54df0 100644 --- a/pkg/discovery/oci_test.go +++ b/pkg/discovery/oci_test.go @@ -15,126 +15,6 @@ import ( "github.com/vmware-tanzu/tanzu-plugin-runtime/config" ) -var testData1 = `--- -apiVersion: cli.tanzu.vmware.com/v1alpha1 -kind: CLIPlugin -metadata: - name: foo -spec: - artifacts: - v0.0.1: - - arch: amd64 - image: tanzu-cli-plugins/foo-darwin-amd64:latest - os: darwin - type: oci - - arch: amd64 - image: tanzu-cli-plugins/foo-linux-amd64:latest - os: linux - type: oci - - arch: amd64 - image: tanzu-cli-plugins/foo-windows-amd64:latest - os: windows - type: foo - description: Foo description - optional: false - recommendedVersion: v0.0.1 -` - -var testData2 = `--- -apiVersion: cli.tanzu.vmware.com/v1alpha1 -kind: CLIPlugin -metadata: - name: foo -spec: - artifacts: - v0.0.1: - - arch: amd64 - image: tanzu-cli-plugins/foo-darwin-amd64:latest - os: darwin - type: oci - - arch: amd64 - image: tanzu-cli-plugins/foo-linux-amd64:latest - os: linux - type: oci - - arch: amd64 - image: tanzu-cli-plugins/foo-windows-amd64:latest - os: windows - type: oci - description: Foo description - optional: false - recommendedVersion: v0.0.1 ---- -apiVersion: cli.tanzu.vmware.com/v1alpha1 -kind: CLIPlugin -metadata: - name: bar -spec: - artifacts: - v0.0.1: - - arch: amd64 - image: tanzu-cli-plugins/foo-darwin-amd64:latest - os: darwin - type: oci - - arch: amd64 - image: tanzu-cli-plugins/foo-linux-amd64:latest - os: linux - type: oci - - arch: amd64 - image: tanzu-cli-plugins/foo-windows-amd64:latest - os: windows - type: oci - v0.0.2: - - arch: amd64 - image: tanzu-cli-plugins/foo-darwin-amd64:latest - os: darwin - type: oci - - arch: amd64 - image: tanzu-cli-plugins/foo-linux-amd64:latest - os: linux - type: oci - - arch: amd64 - image: tanzu-cli-plugins/foo-windows-amd64:latest - os: windows - type: oci - description: Bar description - optional: false - recommendedVersion: v0.0.2 -` - -func Test_ProcessOCIPluginManifest(t *testing.T) { - assert := assert.New(t) - - plugins, err := processDiscoveryManifestData([]byte(testData1), "test-discovery") - assert.Nil(err) - assert.NotNil(plugins) - assert.Equal(1, len(plugins)) - assert.Equal("foo", plugins[0].Name) - assert.Equal("v0.0.1", plugins[0].RecommendedVersion) - assert.Equal("Foo description", plugins[0].Description) - assert.Equal("test-discovery", plugins[0].Source) - assert.EqualValues([]string{"v0.0.1"}, plugins[0].SupportedVersions) - - plugins, err = processDiscoveryManifestData([]byte(testData2), "test-discovery") - assert.Nil(err) - assert.NotNil(plugins) - assert.Equal(2, len(plugins)) - - assert.Equal("foo", plugins[0].Name) - assert.Equal("v0.0.1", plugins[0].RecommendedVersion) - assert.Equal("Foo description", plugins[0].Description) - assert.Equal("test-discovery", plugins[0].Source) - assert.Equal(1, len(plugins[0].SupportedVersions)) - assert.EqualValues([]string{"v0.0.1"}, plugins[0].SupportedVersions) - - assert.Equal("bar", plugins[1].Name) - assert.Equal("v0.0.2", plugins[1].RecommendedVersion) - assert.Equal("Bar description", plugins[1].Description) - assert.Equal("test-discovery", plugins[1].Source) - assert.Equal(2, len(plugins[1].SupportedVersions)) - assert.Contains(plugins[1].SupportedVersions, "v0.0.1") - assert.Contains(plugins[1].SupportedVersions, "v0.0.2") -} - func Test_NewOCIDiscovery(t *testing.T) { assert := assert.New(t) @@ -174,22 +54,6 @@ func Test_NewOCIDiscovery(t *testing.T) { assert.Equal(discoveryImage, dbDiscovery.image) assert.Equal(discoveryCriteria, dbDiscovery.pluginCriteria) assert.Nil(dbDiscovery.groupCriteria) - - // Turn off central repo feature - featureArray = strings.Split(constants.FeatureDisableCentralRepositoryForTesting, ".") - err = config.SetFeature(featureArray[1], featureArray[2], "true") - assert.Nil(err) - - // Check that the correct discovery type is returned - pd = NewOCIDiscovery(discoveryName, discoveryImage, WithPluginDiscoveryCriteria(discoveryCriteria)) - assert.NotNil(pd) - assert.Equal(discoveryName, pd.Name()) - assert.Equal(common.DiscoveryTypeOCI, pd.Type()) - - ociDiscovery, ok := pd.(*OCIDiscovery) - assert.True(ok) - assert.Equal(discoveryName, ociDiscovery.name) - assert.Equal(discoveryImage, ociDiscovery.image) } func Test_NewOCIGroupDiscovery(t *testing.T) { diff --git a/pkg/pluginmanager/manager_test.go b/pkg/pluginmanager/manager_test.go index 9821cb2fc..980480d97 100644 --- a/pkg/pluginmanager/manager_test.go +++ b/pkg/pluginmanager/manager_test.go @@ -1353,6 +1353,8 @@ func TestGetAdditionalTestPluginDiscoveries(t *testing.T) { assertions.Equal(expectedDiscoveries[1], discoveries[1].OCI.Image) assertions.Equal(expectedDiscoveries[2], discoveries[2].OCI.Image) assertions.Equal(expectedDiscoveries[3], discoveries[3].OCI.Image) + + os.Unsetenv(constants.ConfigVariableAdditionalDiscoveryForTesting) } func TestGetPluginDiscoveries(t *testing.T) { @@ -1417,6 +1419,8 @@ func TestGetPluginDiscoveries(t *testing.T) { assertions.Equal(2, len(discoveries)) assertions.Equal("default-local", discoveries[0].Local.Name) assertions.Equal("fake", discoveries[1].Local.Name) + + os.Unsetenv(constants.ConfigVariableAdditionalDiscoveryForTesting) } func TestMergeDuplicatePlugins(t *testing.T) {