Skip to content

Commit

Permalink
Cleanup the use of feature flags
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@marckhouzam
marckhouzam committed Sep 15, 2023
1 parent 2895b0c commit c5a0218
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 658 deletions.
1 change: 1 addition & 0 deletions pkg/command/discovery_source_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
329 changes: 69 additions & 260 deletions pkg/command/plugin.go
View file

Large diffs are not rendered by default.

24 changes: 4 additions & 20 deletions pkg/command/plugin_search_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down Expand Up @@ -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)
Expand Down
193 changes: 58 additions & 135 deletions pkg/command/plugin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" } ]`,
},
}

Expand All @@ -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)
Expand Down Expand Up @@ -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",
},
}

Expand Down Expand Up @@ -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)
Expand Down
27 changes: 11 additions & 16 deletions pkg/command/root.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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()
Expand Down
7 changes: 1 addition & 6 deletions pkg/config/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Loading

0 comments on commit c5a0218

Please sign in to comment.