Cleanup the use of feature flags (#482)

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 <kmarc@vmware.com>

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) {

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)

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)
@@ -312,60 +254,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",
 		},
 	}
 
@@ -396,11 +324,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)

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()

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)
 }