Enable Central Repo feature by default (#105)

We want the central-repo feature to be enabled by default, but we want
a *hidden* way to disable it, just in case.  Therefore, we don't want
the feature flag to be present in the config file unless we actually
turn off the feature. This is important so that users don't think they
can turn off the feature.

However, if a feature flag is not set, it defaults to false.  This means
the central-repo feature would be disabled by default.  To work around
that, we flip the feature flag to be a way to *disable* the central
repo feature instead of enabling it like before.

So the feature flag becomes "no-central-repo-test-only":
- false -> central-repo feature enabled
- true  -> central-repo feature disabled

The suffix "test-only" is meant to make it even clearer that this
feature should not be turned off.

Unit tests affected by this change (now running with the central repo
feature enabled) have been adapted.

Signed-off-by: Marc Khouzam <kmarc@vmware.com>

hack/central-repo/README.md

@@ -18,7 +18,6 @@ Also, only versions `v0.0.1` and `v9.9.9` for the other plugins can be installed
 The steps to follow to use the test central repo are:
 
 1. Start the test repo with `make start-test-central-repo`.
-1. Enable the central repository using the temporary feature flag: `tz config set features.global.central-repository true`
 1. Configure the plugin source for the test central repo: `tz plugin source add -n default -t oci -u localhost:9876/tanzu-cli/plugins/central:small`
 1. Allow the use of a local registry: `export ALLOWED_REGISTRY=localhost:9876`
 
@@ -28,7 +27,6 @@ Here are the exact commands:
 cd tanzu-cli
 make build
 make start-test-central-repo
-tz config set features.global.central-repository true
 tz plugin source add -n default -t oci -u localhost:9876/tanzu-cli/plugins/central:small
 export ALLOWED_REGISTRY=localhost:9876
 

pkg/command/plugin.go

@@ -60,7 +60,7 @@ func newPluginCmd() *cobra.Command {
 
 	listPluginCmd.Flags().StringVarP(&outputFormat, "output", "o", "", "Output format (yaml|json|table)")
 	listPluginCmd.Flags().StringVarP(&local, "local", "l", "", "path to local plugin source")
-	if config.IsFeatureActivated(constants.FeatureCentralRepository) {
+	if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) {
 		// The --local flag no longer applies to the "list" command.
 		// Instead of removing it completely, we mark it hidden and print out an error
 		// in the RunE() function if it is used.  This provides better guidance to the user.
@@ -93,7 +93,7 @@ func newPluginCmd() *cobra.Command {
 		discoverySourceCmd,
 	)
 
-	if config.IsFeatureActivated(constants.FeatureCentralRepository) {
+	if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) {
 		installPluginCmd.MarkFlagsMutuallyExclusive("group", "local")
 		installPluginCmd.MarkFlagsMutuallyExclusive("group", "version")
 		installPluginCmd.MarkFlagsMutuallyExclusive("group", "target")
@@ -111,7 +111,7 @@ func newListPluginCmd() *cobra.Command {
 		Use:   "list",
 		Short: "List available plugins",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if config.IsFeatureActivated(constants.FeatureCentralRepository) {
+			if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) {
 				if local != "" {
 					return fmt.Errorf("the '--local' flag does not apply to this command. Please use 'tanzu plugin search --local'")
 				}
@@ -218,7 +218,7 @@ func newInstallPluginCmd() *cobra.Command {
 				return errors.New("invalid target specified. Please specify correct value of `--target` or `-t` flag from 'kubernetes/k8s/mission-control/tmc'")
 			}
 
-			if !config.IsFeatureActivated(constants.FeatureCentralRepository) {
+			if config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) {
 				return legacyPluginInstall(cmd, args)
 			}
 
@@ -353,7 +353,7 @@ func newUpgradePluginCmd() *cobra.Command {
 			}
 
 			var pluginVersion string
-			if config.IsFeatureActivated(constants.FeatureCentralRepository) {
+			if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) {
 				// With the Central Repository feature we can simply request to install
 				// the recommendedVersion.
 				pluginVersion = cli.VersionLatest

pkg/command/plugin_test.go

@@ -35,14 +35,14 @@ func TestPluginList(t *testing.T) {
 		expectedFailure    bool
 	}{
 		{
-			test:            "With empty config file(no discovery sources added) and no plugins installed",
+			test:            "With empty config file(no discovery sources added) and no plugins installed with no central repo",
 			plugins:         []string{},
 			args:            []string{"plugin", "list"},
 			expectedFailure: false,
 			expected:        "NAME DESCRIPTION TARGET DISCOVERY VERSION STATUS",
 		},
 		{
-			test:            "With empty config file(no discovery sources added) and when one additional plugin installed",
+			test:            "With empty config file(no discovery sources added) and when one additional plugin installed with no central repo",
 			plugins:         []string{"foo"},
 			versions:        []string{"v0.1.0"},
 			targets:         []configtypes.Target{configtypes.TargetK8s},
@@ -51,7 +51,7 @@ func TestPluginList(t *testing.T) {
 			expected:        "NAME DESCRIPTION TARGET DISCOVERY 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",
+			test:            "With empty config file(no discovery sources added) and when more than one plugin is installed with no central repo",
 			plugins:         []string{"foo", "bar"},
 			versions:        []string{"v0.1.0", "v0.2.0"},
 			targets:         []configtypes.Target{configtypes.TargetTMC, configtypes.TargetK8s},
@@ -60,7 +60,7 @@ func TestPluginList(t *testing.T) {
 			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",
 		},
 		{
-			test:            "when json output is requested",
+			test:            "when json output is requested with no central repo",
 			plugins:         []string{"foo"},
 			versions:        []string{"v0.1.0"},
 			targets:         []configtypes.Target{configtypes.TargetK8s},
@@ -69,7 +69,7 @@ func TestPluginList(t *testing.T) {
 			expected:        `[ { "description": "some foo description", "discovery": "", "name": "foo", "scope": "Standalone", "status": "installed", "version": "v0.1.0" } ]`,
 		},
 		{
-			test:            "when yaml output is requested",
+			test:            "when yaml output is requested with no central repo",
 			plugins:         []string{"foo"},
 			versions:        []string{"v0.1.0"},
 			targets:         []configtypes.Target{configtypes.TargetK8s},
@@ -78,22 +78,22 @@ func TestPluginList(t *testing.T) {
 			expected:        `- description: some foo description discovery: "" name: foo scope: Standalone status: installed version: v0.1.0`,
 		},
 		{
-			test:               "no '--local' option with central repo",
+			test:               "no '--local' option",
 			centralRepoFeature: true,
 			args:               []string{"plugin", "list", "--local", "someDirectory"},
 			expectedFailure:    true,
 			expected:           "the '--local' flag does not apply to this command. Please use 'tanzu plugin search --local'",
 		},
 		{
-			test:               "With empty config file(no discovery sources added) and no plugins installed with central repo",
+			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 with central repo",
+			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"},
@@ -103,7 +103,7 @@ func TestPluginList(t *testing.T) {
 			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 central repo",
+			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"},
@@ -113,7 +113,7 @@ func TestPluginList(t *testing.T) {
 			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 central repo",
+			test:               "when json output is requested",
 			centralRepoFeature: true,
 			plugins:            []string{"foo"},
 			versions:           []string{"v0.1.0"},
@@ -123,7 +123,7 @@ func TestPluginList(t *testing.T) {
 			expected:           `[ { "context": "", "description": "some foo description", "name": "foo", "status": "installed", "target": "kubernetes", "version": "v0.1.0" } ]`,
 		},
 		{
-			test:               "when yaml output is requested with central repo",
+			test:               "when yaml output is requested",
 			centralRepoFeature: true,
 			plugins:            []string{"foo"},
 			versions:           []string{"v0.1.0"},
@@ -153,9 +153,9 @@ func TestPluginList(t *testing.T) {
 		err = config.SetFeature(featureArray[1], featureArray[2], "true")
 		assert.Nil(t, err)
 
-		// Set the Central Repository feature
-		if spec.centralRepoFeature {
-			featureArray := strings.Split(constants.FeatureCentralRepository, ".")
+		// 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)
 		}
@@ -288,60 +288,60 @@ func TestDeletePlugin(t *testing.T) {
 
 func TestInstallPlugin(t *testing.T) {
 	tests := []struct {
-		test               string
-		centralRepoFeature string
-		args               []string
-		expectedErrorMsg   string
-		expectedFailure    bool
+		test                string
+		centralRepoDisabled string
+		args                []string
+		expectedErrorMsg    string
+		expectedFailure     bool
 	}{
 		{
-			test:               "need plugin name or 'all' without central repo",
-			centralRepoFeature: "false",
-			args:               []string{"plugin", "install"},
-			expectedFailure:    true,
-			expectedErrorMsg:   "missing plugin name or 'all' as an argument",
+			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 or 'all' with central repo if no group",
-			centralRepoFeature: "true",
-			args:               []string{"plugin", "install"},
-			expectedFailure:    true,
-			expectedErrorMsg:   "missing plugin name or 'all' as an argument",
+			test:                "need plugin name or 'all' if no group",
+			centralRepoDisabled: "false",
+			args:                []string{"plugin", "install"},
+			expectedFailure:     true,
+			expectedErrorMsg:    "missing plugin name or 'all' as an argument",
 		},
 		{
-			test:               "no 'all' option with central repo",
-			centralRepoFeature: "true",
-			args:               []string{"plugin", "install", "all"},
-			expectedFailure:    true,
-			expectedErrorMsg:   "the 'all' argument can only be used with the --group or --local flags",
+			test:                "no 'all' option",
+			centralRepoDisabled: "false",
+			args:                []string{"plugin", "install", "all"},
+			expectedFailure:     true,
+			expectedErrorMsg:    "the 'all' argument can only be used with the --group or --local flags",
 		},
 		{
-			test:               "invalid target",
-			centralRepoFeature: "true",
-			args:               []string{"plugin", "install", "--target", "invalid", "myplugin"},
-			expectedFailure:    true,
-			expectedErrorMsg:   "invalid target specified. Please specify correct value of `--target` or `-t` flag from 'kubernetes/k8s/mission-control/tmc'",
+			test:                "invalid target",
+			centralRepoDisabled: "false",
+			args:                []string{"plugin", "install", "--target", "invalid", "myplugin"},
+			expectedFailure:     true,
+			expectedErrorMsg:    "invalid target specified. Please specify correct value of `--target` or `-t` flag from 'kubernetes/k8s/mission-control/tmc'",
 		},
 		{
-			test:               "no --group and --local together",
-			centralRepoFeature: "true",
-			args:               []string{"plugin", "install", "--group", "testgroup", "--local", "./", "myplugin"},
-			expectedFailure:    true,
-			expectedErrorMsg:   "if any flags in the group [group local] are set none of the others can be",
+			test:                "no --group and --local together",
+			centralRepoDisabled: "false",
+			args:                []string{"plugin", "install", "--group", "testgroup", "--local", "./", "myplugin"},
+			expectedFailure:     true,
+			expectedErrorMsg:    "if any flags in the group [group local] are set none of the others can be",
 		},
 		{
-			test:               "no --group and --target together",
-			centralRepoFeature: "true",
-			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",
+			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 --version together",
-			centralRepoFeature: "true",
-			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",
+			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",
 		},
 	}
 
@@ -372,9 +372,9 @@ func TestInstallPlugin(t *testing.T) {
 
 	for _, spec := range tests {
 		t.Run(spec.test, func(t *testing.T) {
-			// Set the Central Repository feature
-			featureArray := strings.Split(constants.FeatureCentralRepository, ".")
-			err := config.SetFeature(featureArray[1], featureArray[2], spec.centralRepoFeature)
+			// 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()
@@ -392,18 +392,16 @@ func TestInstallPlugin(t *testing.T) {
 
 func TestUpgradePlugin(t *testing.T) {
 	tests := []struct {
-		test               string
-		centralRepoFeature string
-		args               []string
-		expectedErrorMsg   string
-		expectedFailure    bool
+		test             string
+		args             []string
+		expectedErrorMsg string
+		expectedFailure  bool
 	}{
 		{
-			test:               "invalid target",
-			centralRepoFeature: "true",
-			args:               []string{"plugin", "upgrade", "--target", "invalid", "myplugin"},
-			expectedFailure:    true,
-			expectedErrorMsg:   "invalid target specified. Please specify correct value of `--target` or `-t` flag from 'kubernetes/k8s/mission-control/tmc'",
+			test:             "invalid target",
+			args:             []string{"plugin", "upgrade", "--target", "invalid", "myplugin"},
+			expectedFailure:  true,
+			expectedErrorMsg: "invalid target specified. Please specify correct value of `--target` or `-t` flag from 'kubernetes/k8s/mission-control/tmc'",
 		},
 	}
 
@@ -430,11 +428,6 @@ func TestUpgradePlugin(t *testing.T) {
 
 	for _, spec := range tests {
 		t.Run(spec.test, func(t *testing.T) {
-			// Set the Central Repository feature
-			featureArray := strings.Split(constants.FeatureCentralRepository, ".")
-			err := config.SetFeature(featureArray[1], featureArray[2], spec.centralRepoFeature)
-			assert.Nil(err)
-
 			rootCmd, err := NewRootCmd()
 			assert.Nil(err)
 			rootCmd.SetArgs(spec.args)

pkg/command/root_test.go

@@ -13,11 +13,14 @@ import (
 
 	"github.com/stretchr/testify/assert"
 
+	"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/catalog"
 	"github.com/vmware-tanzu/tanzu-cli/pkg/cli"
+	configcli "github.com/vmware-tanzu/tanzu-cli/pkg/config"
+	"github.com/vmware-tanzu/tanzu-cli/pkg/constants"
 )
 
 const (
@@ -317,6 +320,15 @@ func TestEnvVarsSet(t *testing.T) {
 	os.Setenv("TANZU_CONFIG_NEXT_GEN", configFileNG.Name())
 	defer os.RemoveAll(configFileNG.Name())
 
+	// Setup default feature flags since we have created new config files
+	// TODO(khouzam): This is because AddDefaultFeatureFlagsIfMissing() has already
+	// been called in an init() function.  We should fix that in a more generic way.
+	c, err := config.GetClientConfigNoLock()
+	assert.Nil(err)
+	if configcli.AddDefaultFeatureFlagsIfMissing(c, constants.DefaultCliFeatureFlags) {
+		_ = config.StoreClientConfig(c)
+	}
+
 	rootCmd, err := NewRootCmd()
 	assert.Nil(err)
 

pkg/constants/featureflags.go

@@ -7,8 +7,8 @@ package constants
 const (
 	// FeatureContextCommand determines whether to surface the context command. This is disabled by default.
 	FeatureContextCommand = "features.global.context-target-v2"
-	// FeatureCentralRepository determines if the CLI uses the Central Repository of plugins.
-	FeatureCentralRepository = "features.global.central-repository"
+	// FeatureDisableCentralRepositoryForTesting determines if the CLI uses the Central Repository of plugins.
+	FeatureDisableCentralRepositoryForTesting = "features.global.no-central-repo-test-only"
 )
 
 // DefaultCliFeatureFlags is used to populate an initially empty config file with default values for feature flags.
@@ -25,7 +25,8 @@ const (
 var (
 	DefaultCliFeatureFlags = map[string]bool{
 		FeatureContextCommand: true,
-		// TODO(khouzam) turn this on before the 1.0 release, or remove the flag completely
-		FeatureCentralRepository: false,
+		// Do NOT include the test feature flag to disable the central repo.
+		// We don't want to publicize this feature flag.
+		// It defaults to false when not specified, which is what is needed.
 	}
 )

pkg/discovery/oci.go

@@ -33,7 +33,7 @@ type OCIDiscovery struct {
 
 // NewOCIDiscovery returns a new Discovery using the specified OCI image.
 func NewOCIDiscovery(name, image string, criteria *PluginDiscoveryCriteria) Discovery {
-	if config.IsFeatureActivated(constants.FeatureCentralRepository) {
+	if !config.IsFeatureActivated(constants.FeatureDisableCentralRepositoryForTesting) {
 		// The plugin inventory uses relative image URIs to be future-proof.
 		// Determine the image prefix from the main image.
 		// E.g., if the main image is at project.registry.vmware.com/tanzu-cli/plugins/plugin-inventory:latest