diff --git a/config/cli_discovery_sources.go b/config/cli_discovery_sources.go index 1a8c81c5..45bd7496 100644 --- a/config/cli_discovery_sources.go +++ b/config/cli_discovery_sources.go @@ -118,13 +118,21 @@ func getCLIDiscoverySources(node *yaml.Node) ([]configtypes.PluginDiscovery, err } func getCLIDiscoverySource(node *yaml.Node, name string) (*configtypes.PluginDiscovery, error) { + // check if context name is empty + if name == "" { + return nil, errors.New("discovery source name cannot be empty") + } + cfg, err := convertNodeToClientConfig(node) if err != nil { return nil, err } if cfg.ClientOptions != nil && cfg.ClientOptions.CLI != nil && cfg.ClientOptions.CLI.DiscoverySources != nil { for _, discoverySource := range cfg.ClientOptions.CLI.DiscoverySources { - _, discoverySourceName := getDiscoverySourceTypeAndName(discoverySource) + _, discoverySourceName, err := getDiscoverySourceTypeAndName(discoverySource) + if err != nil { + return nil, err + } if discoverySourceName == name { return &discoverySource, nil } @@ -185,7 +193,12 @@ func deleteCLIDiscoverySource(node *yaml.Node, name string) error { if err != nil { return err } - discoverySourceType, discoverySourceName := getDiscoverySourceTypeAndName(*discoverySource) + + discoverySourceType, discoverySourceName, err := getDiscoverySourceTypeAndName(*discoverySource) + if err != nil { + return err + } + var result []*yaml.Node for _, discoverySourceNode := range cliDiscoverySourcesNode.Content { // Find discovery source matched by discoverySourceType diff --git a/config/cli_discovery_sources_test.go b/config/cli_discovery_sources_test.go index 2fbd9c3b..9b841707 100644 --- a/config/cli_discovery_sources_test.go +++ b/config/cli_discovery_sources_test.go @@ -73,9 +73,11 @@ func TestGetCLIDiscoverySource(t *testing.T) { }() tests := []struct { - name string - in *configtypes.ClientConfig - out *configtypes.PluginDiscovery + name string + discoverySourceName string + in *configtypes.ClientConfig + out *configtypes.PluginDiscovery + errStr string }{ { name: "success get", @@ -101,15 +103,44 @@ func TestGetCLIDiscoverySource(t *testing.T) { ManifestPath: "test-manifest-path", }, }, + discoverySourceName: "test", + }, + { + name: "failed discovery source with empty name", + in: &configtypes.ClientConfig{ + ClientOptions: &configtypes.ClientOptions{ + CLI: &configtypes.CLIOptions{ + DiscoverySources: []configtypes.PluginDiscovery{ + { + GCP: &configtypes.GCPDiscovery{ + Name: "", + Bucket: "updated-test-bucket", + ManifestPath: "test-manifest-path", + }, + }, + }, + }, + }, + }, + discoverySourceName: "", + errStr: "discovery source name cannot be empty", }, } for _, spec := range tests { t.Run(spec.name, func(t *testing.T) { err := StoreClientConfig(spec.in) - assert.NoError(t, err) - c, err := GetCLIDiscoverySource("test") - assert.Equal(t, spec.out, c) - assert.NoError(t, err) + if spec.errStr != "" { + assert.Equal(t, spec.errStr, err.Error()) + } else { + assert.NoError(t, err) + } + c, err := GetCLIDiscoverySource(spec.discoverySourceName) + if spec.errStr != "" { + assert.Equal(t, spec.errStr, err.Error()) + } else { + assert.Equal(t, spec.out, c) + assert.NoError(t, err) + } }) } } @@ -123,9 +154,10 @@ func TestSetCLIDiscoverySources(t *testing.T) { }() tests := []struct { - name string - input []configtypes.PluginDiscovery - total int + name string + input []configtypes.PluginDiscovery + total int + errStr string }{ { name: "success add test", @@ -246,14 +278,33 @@ func TestSetCLIDiscoverySources(t *testing.T) { }, total: 4, }, + { + name: "failed discovery source with empty name", + input: []configtypes.PluginDiscovery{ + { + Local: &configtypes.LocalDiscovery{ + Name: "", + Path: "test-path", + }, + }, + }, + errStr: "discovery source name cannot be empty", + total: 4, + }, } for _, spec := range tests { t.Run(spec.name, func(t *testing.T) { err := SetCLIDiscoverySources(spec.input) - assert.NoError(t, err) + if spec.errStr != "" { + assert.Equal(t, spec.errStr, err.Error()) + } else { + assert.NoError(t, err) + } + sources, err := GetCLIDiscoverySources() assert.NoError(t, err) assert.Equal(t, spec.total, len(sources)) + }) } } diff --git a/config/cli_options.go b/config/cli_options.go index d12a15a9..79a64073 100644 --- a/config/cli_options.go +++ b/config/cli_options.go @@ -22,6 +22,10 @@ func GetEdition() (string, error) { // SetEdition adds or updates edition value func SetEdition(val string) (err error) { + // Check if val is empty + if val == "" { + return errors.New("value cannot be empty") + } // Retrieve client config node AcquireTanzuConfigLock() defer ReleaseTanzuConfigLock() diff --git a/config/cli_options_test.go b/config/cli_options_test.go index 98cad1fb..b44fd29d 100644 --- a/config/cli_options_test.go +++ b/config/cli_options_test.go @@ -7,11 +7,9 @@ import ( "testing" "github.com/stretchr/testify/assert" - - configtypes "github.com/vmware-tanzu/tanzu-plugin-runtime/config/types" ) -func TestGetEdition(t *testing.T) { +func TestSetEdition(t *testing.T) { // Setup config test data _, cleanUp := setupTestConfig(t, &CfgTestData{}) @@ -20,47 +18,17 @@ func TestGetEdition(t *testing.T) { }() tests := []struct { - name string - in *configtypes.ClientConfig - out string - errStr string + name string + value string + errStr string + errStrForGet string }{ { - name: "success k8s", - in: &configtypes.ClientConfig{ - ClientOptions: &configtypes.ClientOptions{ - Env: map[string]string{ - "test": "test", - }, - }, - }, - out: "test", + name: "should return error for empty val", + value: "", + errStr: "value cannot be empty", + errStrForGet: "edition not found", }, - } - for _, spec := range tests { - t.Run(spec.name, func(t *testing.T) { - err := StoreClientConfig(spec.in) - assert.NoError(t, err) - c, err := GetEnv("test") - assert.NoError(t, err) - assert.Equal(t, spec.out, c) - assert.NoError(t, err) - }) - } -} - -func TestSetEdition(t *testing.T) { - // Setup config test data - _, cleanUp := setupTestConfig(t, &CfgTestData{}) - - defer func() { - cleanUp() - }() - - tests := []struct { - name string - value string - }{ { name: "should persist tanzu when empty client config", value: "tanzu", @@ -78,10 +46,20 @@ func TestSetEdition(t *testing.T) { for _, spec := range tests { t.Run(spec.name, func(t *testing.T) { err := SetEdition(spec.value) - assert.NoError(t, err) + if spec.errStr != "" { + assert.Equal(t, spec.errStr, err.Error()) + } else { + assert.NoError(t, err) + } + c, err := GetEdition() - assert.Equal(t, spec.value, c) - assert.NoError(t, err) + if spec.errStrForGet != "" { + assert.Equal(t, spec.errStrForGet, err.Error()) + } else { + assert.Equal(t, spec.value, c) + assert.NoError(t, err) + } + }) } } diff --git a/config/cli_repositories.go b/config/cli_repositories.go index 51c43fc2..3da34403 100644 --- a/config/cli_repositories.go +++ b/config/cli_repositories.go @@ -159,7 +159,9 @@ func deleteCLIRepository(node *yaml.Node, name string) error { } repositoryType, repositoryName := getRepositoryTypeAndName(*repository) - + if repositoryType == "" || repositoryName == "" { + return errors.New("not found") + } var result []*yaml.Node for _, repositoryNode := range cliRepositoriesNode.Content { if repositoryIndex := nodeutils.GetNodeIndex(repositoryNode.Content, repositoryType); repositoryIndex != -1 { diff --git a/config/contexts.go b/config/contexts.go index fa6c245e..e386662e 100644 --- a/config/contexts.go +++ b/config/contexts.go @@ -6,6 +6,7 @@ package config import ( "fmt" + "github.com/pkg/errors" "gopkg.in/yaml.v3" "github.com/vmware-tanzu/tanzu-plugin-runtime/config/nodeutils" @@ -246,6 +247,11 @@ func EndpointFromContext(s *configtypes.Context) (endpoint string, err error) { } func getContext(node *yaml.Node, name string) (*configtypes.Context, error) { + // check if context name is empty + if name == "" { + return nil, errors.New("context name cannot be empty") + } + cfg, err := convertNodeToClientConfig(node) if err != nil { return nil, err @@ -285,6 +291,11 @@ func setContexts(node *yaml.Node, contexts []*configtypes.Context) (err error) { } func setContext(node *yaml.Node, ctx *configtypes.Context) (persist bool, err error) { + // Check if ctx.Name is empty + if ctx.Name == "" { + return false, errors.New("context name cannot be empty") + } + // Get Patch Strategies from config metadata patchStrategies, err := GetConfigMetadataPatchStrategy() if err != nil { @@ -394,6 +405,11 @@ func removeCurrentContext(node *yaml.Node, ctx *configtypes.Context) error { //nolint:dupl func removeContext(node *yaml.Node, name string) error { + // check if context name is empty + if name == "" { + return errors.New("context name cannot be empty") + } + // Find the contexts node in the yaml node keys := []nodeutils.Key{ {Name: KeyContexts}, diff --git a/config/contexts_it_test.go b/config/contexts_it_test.go index f1a5de1a..18829d1e 100644 --- a/config/contexts_it_test.go +++ b/config/contexts_it_test.go @@ -41,6 +41,34 @@ servers: manifestPath: updated-test-manifest-path annotation: one required: true + - type: managementcluster + managementClusterOpts: + endpoint: updated-test-endpoint + path: updated-test-path + context: updated-test-context + annotation: one + required: true + discoverySources: + - gcp: + name: test + bucket: updated-test-bucket + manifestPath: updated-test-manifest-path + annotation: one + required: true + - type: managementcluster + managementClusterOpts: + endpoint: updated-test-endpoint + path: updated-test-path + context: updated-test-context + annotation: one + required: true + discoverySources: + - gcp: + name: test + bucket: updated-test-bucket + manifestPath: updated-test-manifest-path + annotation: one + required: true current: test-mc ` expectedCfg := `clientOptions: @@ -71,6 +99,34 @@ servers: manifestPath: updated-test-manifest-path annotation: one required: true + - type: managementcluster + managementClusterOpts: + endpoint: updated-test-endpoint + path: updated-test-path + context: updated-test-context + annotation: one + required: true + discoverySources: + - gcp: + name: test + bucket: updated-test-bucket + manifestPath: updated-test-manifest-path + annotation: one + required: true + - type: managementcluster + managementClusterOpts: + endpoint: updated-test-endpoint + path: updated-test-path + context: updated-test-context + annotation: one + required: true + discoverySources: + - gcp: + name: test + bucket: updated-test-bucket + manifestPath: updated-test-manifest-path + annotation: one + required: true - name: test-mc2 type: managementcluster managementClusterOpts: @@ -103,6 +159,42 @@ current: test-mc2 manifestPath: test-manifest-path annotation: one required: true + - target: kubernetes + group: one + clusterOpts: + isManagementCluster: true + annotation: one + required: true + annotationStruct: + one: one + endpoint: test-endpoint + path: test-path + context: test-context + discoverySources: + - gcp: + name: test + bucket: test-bucket + manifestPath: test-manifest-path + annotation: one + required: true + - target: kubernetes + group: one + clusterOpts: + isManagementCluster: true + annotation: one + required: true + annotationStruct: + one: one + endpoint: test-endpoint + path: test-path + context: test-context + discoverySources: + - gcp: + name: test + bucket: test-bucket + manifestPath: test-manifest-path + annotation: one + required: true currentContext: kubernetes: test-mc ` @@ -126,6 +218,42 @@ currentContext: manifestPath: test-manifest-path annotation: one required: true + - target: kubernetes + group: one + clusterOpts: + isManagementCluster: true + annotation: one + required: true + annotationStruct: + one: one + endpoint: test-endpoint + path: test-path + context: test-context + discoverySources: + - gcp: + name: test + bucket: test-bucket + manifestPath: test-manifest-path + annotation: one + required: true + - target: kubernetes + group: one + clusterOpts: + isManagementCluster: true + annotation: one + required: true + annotationStruct: + one: one + endpoint: test-endpoint + path: test-path + context: test-context + discoverySources: + - gcp: + name: test + bucket: test-bucket + manifestPath: test-manifest-path + annotation: one + required: true - name: test-mc2 target: kubernetes clusterOpts: @@ -175,6 +303,7 @@ func TestContextsIntegration(t *testing.T) { } assert.Nil(t, err) assert.Equal(t, expected, context) + // Add new Context newCtx := &configtypes.Context{ Name: "test-mc2", @@ -199,6 +328,32 @@ func TestContextsIntegration(t *testing.T) { ctx, err := GetContext("test-mc2") assert.Nil(t, err) assert.Equal(t, newCtx, ctx) + + // Try to add context with empty name + contextWithEmptyName := &configtypes.Context{ + Name: "", + Target: configtypes.TargetK8s, + ClusterOpts: &configtypes.ClusterServer{ + Path: "test-path", + Context: "test-context", + IsManagementCluster: true, + }, + DiscoverySources: []configtypes.PluginDiscovery{ + { + GCP: &configtypes.GCPDiscovery{ + Name: "test", + Bucket: "test-bucket", + ManifestPath: "test-manifest-path", + }, + }, + }, + } + err = SetContext(contextWithEmptyName, true) + assert.Equal(t, "context name cannot be empty", err.Error()) + ctx, err = GetContext("") + assert.Equal(t, "context name cannot be empty", err.Error()) + assert.Nil(t, ctx) + // Update existing Context updatedCtx := &configtypes.Context{ Name: "test-mc2", @@ -224,6 +379,7 @@ func TestContextsIntegration(t *testing.T) { assert.Nil(t, err) assert.Equal(t, updatedCtx, ctx) + //Read config files file, err := os.ReadFile(cfgTestFiles[0].Name()) assert.NoError(t, err) assert.Equal(t, expectedCfg, string(file)) diff --git a/config/contexts_test.go b/config/contexts_test.go index 3c023397..b146bdb3 100644 --- a/config/contexts_test.go +++ b/config/contexts_test.go @@ -1136,3 +1136,59 @@ func TestSetContextWithUniquePermissions(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 7, len(s.GlobalOpts.Auth.Permissions)) } + +func TestSetContextWithEmptyName(t *testing.T) { + // setup + func() { + LocalDirName = TestLocalDirName + }() + defer func() { + cleanupDir(LocalDirName) + }() + tcs := []struct { + name string + ctx *configtypes.Context + current bool + errStr string + }{ + { + name: "success current", + ctx: &configtypes.Context{ + Name: "", + Target: configtypes.TargetK8s, + ClusterOpts: &configtypes.ClusterServer{ + Endpoint: "test-endpoint", + Path: "test-path", + Context: "test-context", + IsManagementCluster: true, + }, + }, + errStr: "context name cannot be empty", + }, + { + name: "success re empty current", + ctx: &configtypes.Context{ + Name: "", + Target: configtypes.TargetK8s, + ClusterOpts: &configtypes.ClusterServer{ + Endpoint: "test-endpoint", + Path: "test-path", + Context: "test-context", + IsManagementCluster: true, + }, + }, + errStr: "context name cannot be empty", + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + err := SetContext(tc.ctx, tc.current) + if tc.errStr == "" { + assert.NoError(t, err) + } else { + assert.EqualError(t, err, tc.errStr) + } + }) + } +} diff --git a/config/discovery_sources_node.go b/config/discovery_sources_node.go index 057fe48d..a2a849bb 100644 --- a/config/discovery_sources_node.go +++ b/config/discovery_sources_node.go @@ -64,9 +64,9 @@ func setDiscoverySource(discoverySourcesNode *yaml.Node, discoverySource configt var result []*yaml.Node // Get discovery source type and name - newOrUpdatedDiscoverySourceType, newOrUpdatedDiscoverySourceName := getDiscoverySourceTypeAndName(discoverySource) - if newOrUpdatedDiscoverySourceType == "" || newOrUpdatedDiscoverySourceName == "" { - return persist, errors.New("not found") + newOrUpdatedDiscoverySourceType, newOrUpdatedDiscoverySourceName, err := getDiscoverySourceTypeAndName(discoverySource) + if err != nil { + return persist, err } // Loop through each discovery source node @@ -134,19 +134,34 @@ func setDiscoverySource(discoverySourcesNode *yaml.Node, discoverySource configt return persist, err } -func getDiscoverySourceTypeAndName(discoverySource configtypes.PluginDiscovery) (string, string) { - if discoverySource.GCP != nil && discoverySource.GCP.Name != "" { //nolint:staticcheck - return DiscoveryTypeGCP, discoverySource.GCP.Name //nolint:staticcheck - } else if discoverySource.OCI != nil && discoverySource.OCI.Name != "" { - return DiscoveryTypeOCI, discoverySource.OCI.Name - } else if discoverySource.Local != nil && discoverySource.Local.Name != "" { - return DiscoveryTypeLocal, discoverySource.Local.Name - } else if discoverySource.Kubernetes != nil && discoverySource.Kubernetes.Name != "" { - return DiscoveryTypeKubernetes, discoverySource.Kubernetes.Name - } else if discoverySource.REST != nil && discoverySource.REST.Name != "" { - return DiscoveryTypeREST, discoverySource.REST.Name +func getDiscoverySourceTypeAndName(discoverySource configtypes.PluginDiscovery) (string, string, error) { + var discoverySourceType, discoverySourceName string + + switch { + case discoverySource.GCP != nil: + discoverySourceType = DiscoveryTypeGCP + discoverySourceName = discoverySource.GCP.Name + case discoverySource.OCI != nil: + discoverySourceType = DiscoveryTypeOCI + discoverySourceName = discoverySource.OCI.Name + case discoverySource.Local != nil: + discoverySourceType = DiscoveryTypeLocal + discoverySourceName = discoverySource.Local.Name + case discoverySource.Kubernetes != nil: + discoverySourceType = DiscoveryTypeKubernetes + discoverySourceName = discoverySource.Kubernetes.Name + case discoverySource.REST != nil: + discoverySourceType = DiscoveryTypeREST + discoverySourceName = discoverySource.REST.Name + default: + return "", "", errors.New("discovery source type cannot be empty") } - return "", "" + + if discoverySourceName == "" { + return "", "", errors.New("discovery source name cannot be empty") + } + + return discoverySourceType, discoverySourceName, nil } // Find the matching discovery source type and index from accepted discovery sources diff --git a/config/envs.go b/config/envs.go index 48ddad38..512a64e9 100644 --- a/config/envs.go +++ b/config/envs.go @@ -42,11 +42,16 @@ func GetEnv(key string) (string, error) { } func getEnv(node *yaml.Node, key string) (string, error) { + // check if key is empty + if key == "" { + return "", errors.New("key cannot be empty") + } + cfg, err := convertNodeToClientConfig(node) if err != nil { return "", err } - if cfg.ClientOptions == nil && cfg.ClientOptions.Env == nil { + if cfg.ClientOptions == nil || cfg.ClientOptions.Env == nil { return "", errors.New("not found") } if val, ok := cfg.ClientOptions.Env[key]; ok { @@ -72,6 +77,11 @@ func DeleteEnv(key string) error { } func deleteEnv(node *yaml.Node, key string) (err error) { + // check if key is empty + if key == "" { + return errors.New("key cannot be empty") + } + // find env node keys := []nodeutils.Key{ {Name: KeyClientOptions}, @@ -123,6 +133,16 @@ func SetEnv(key, value string) (err error) { //nolint:dupl func setEnv(node *yaml.Node, key, value string) (persist bool, err error) { + // check if key is empty + if key == "" { + return false, errors.New("key cannot be empty") + } + + // check if value is empty + if value == "" { + return false, errors.New("value cannot be empty") + } + // find env node keys := []nodeutils.Key{ {Name: KeyClientOptions, Type: yaml.MappingNode}, diff --git a/config/envs_test.go b/config/envs_test.go index 2fbf5659..06857f25 100644 --- a/config/envs_test.go +++ b/config/envs_test.go @@ -59,11 +59,13 @@ func TestGetEnv(t *testing.T) { defer func() { cleanupDir(LocalDirName) }() + tests := []struct { - name string - in *configtypes.ClientConfig - out string - errStr string + name string + in *configtypes.ClientConfig + out string + errStr string + errStrForInput string }{ { name: "success k8s", @@ -76,15 +78,61 @@ func TestGetEnv(t *testing.T) { }, out: "test", }, + { + name: "get options with empty key", + in: &configtypes.ClientConfig{ + ClientOptions: &configtypes.ClientOptions{ + Env: map[string]string{ + "test": "test", + }, + }, + }, + out: "", + errStr: "key cannot be empty", + }, + { + name: "store options with empty key", + in: &configtypes.ClientConfig{ + ClientOptions: &configtypes.ClientOptions{ + Env: map[string]string{ + "": "test", + }, + }, + }, + out: "", + errStr: "key cannot be empty", + errStrForInput: "key cannot be empty", + }, + { + name: "store options with empty val", + in: &configtypes.ClientConfig{ + ClientOptions: &configtypes.ClientOptions{ + Env: map[string]string{ + "test-empty-val": "", + }, + }, + }, + out: "test-empty-val", + errStr: "not found", + errStrForInput: "value cannot be empty", + }, } for _, spec := range tests { t.Run(spec.name, func(t *testing.T) { err := StoreClientConfig(spec.in) - assert.NoError(t, err) - c, err := GetEnv("test") - assert.NoError(t, err) - assert.Equal(t, spec.out, c) - assert.NoError(t, err) + if spec.errStrForInput != "" { + assert.Equal(t, spec.errStrForInput, err.Error()) + } else { + assert.NoError(t, err) + } + + c, err := GetEnv(spec.out) + if spec.errStr != "" { + assert.Equal(t, spec.errStr, err.Error()) + } else { + assert.NoError(t, err) + assert.Equal(t, spec.out, c) + } }) } } diff --git a/config/features.go b/config/features.go index ad077594..4bf60498 100644 --- a/config/features.go +++ b/config/features.go @@ -31,6 +31,16 @@ func IsFeatureEnabled(plugin, key string) (bool, error) { } func getFeature(node *yaml.Node, plugin, key string) (string, error) { + // check if plugin is empty + if plugin == "" { + return "", errors.New("plugin cannot be empty") + } + + // check if key is empty + if key == "" { + return "", errors.New("key cannot be empty") + } + cfg, err := convertNodeToClientConfig(node) if err != nil { return "", err @@ -61,6 +71,15 @@ func DeleteFeature(plugin, key string) error { } func deleteFeature(node *yaml.Node, plugin, key string) error { + // check if plugin is empty + if plugin == "" { + return errors.New("plugin cannot be empty") + } + + // check if key is empty + if key == "" { + return errors.New("key cannot be empty") + } // Find plugin node keys := []nodeutils.Key{ {Name: KeyClientOptions}, @@ -105,6 +124,21 @@ func SetFeature(plugin, key, value string) (err error) { } func setFeature(node *yaml.Node, plugin, key, value string) (persist bool, err error) { + // check if plugin is empty + if plugin == "" { + return false, errors.New("plugin cannot be empty") + } + + // check if key is empty + if key == "" { + return false, errors.New("key cannot be empty") + } + + // check if value is empty + if value == "" { + return false, errors.New("value cannot be empty") + } + // find plugin node keys := []nodeutils.Key{ {Name: KeyClientOptions, Type: yaml.MappingNode}, diff --git a/config/features_test.go b/config/features_test.go index fe0cc13b..88709ff8 100644 --- a/config/features_test.go +++ b/config/features_test.go @@ -21,11 +21,49 @@ func TestIsFeatureEnabled(t *testing.T) { cleanupDir(LocalDirName) }() tests := []struct { - name string - feature map[string]configtypes.FeatureMap - plugin string - key string + name string + feature map[string]configtypes.FeatureMap + plugin string + key string + errStr string + errStrForStore string }{ + { + name: "empty plugin", + feature: map[string]configtypes.FeatureMap{ + "": { + "context-aware-cli-for-plugins": "true", + }, + }, + plugin: "", + key: "context-aware-cli-for-plugins", + errStr: "plugin cannot be empty", + errStrForStore: "plugin cannot be empty", + }, + { + name: "empty key", + feature: map[string]configtypes.FeatureMap{ + "global": { + "": "true", + }, + }, + plugin: "global", + key: "", + errStr: "key cannot be empty", + errStrForStore: "key cannot be empty", + }, + { + name: "empty value", + feature: map[string]configtypes.FeatureMap{ + "global": { + "context-aware-cli-for-plugins": "", + }, + }, + plugin: "global", + key: "context-aware-cli-for-plugins", + errStr: "not found", + errStrForStore: "value cannot be empty", + }, { name: "success context-aware-cli-for-plugins", feature: map[string]configtypes.FeatureMap{ @@ -45,10 +83,19 @@ func TestIsFeatureEnabled(t *testing.T) { }, } err := StoreClientConfig(cfg) - assert.NoError(t, err) + if tc.errStrForStore != "" { + assert.Equal(t, tc.errStrForStore, err.Error()) + } else { + assert.NoError(t, err) + } + ok, err := IsFeatureEnabled(tc.plugin, tc.key) - assert.NoError(t, err) - assert.Equal(t, ok, true) + if tc.errStr != "" { + assert.Equal(t, tc.errStr, err.Error()) + } else { + assert.NoError(t, err) + assert.Equal(t, ok, true) + } }) } } @@ -118,12 +165,32 @@ func TestSetFeature(t *testing.T) { cleanupDir(LocalDirName) }() tests := []struct { - name string - cfg *configtypes.ClientConfig - plugin string - key string - value bool + name string + cfg *configtypes.ClientConfig + plugin string + key string + value bool + errStrForSet string + errStrForGet string }{ + { + name: "empty plugin", + cfg: &configtypes.ClientConfig{}, + plugin: "", + key: "context-aware-cli-for-plugins", + value: false, + errStrForSet: "plugin cannot be empty", + errStrForGet: "plugin cannot be empty", + }, + { + name: "empty key", + cfg: &configtypes.ClientConfig{}, + plugin: "global", + key: "", + value: false, + errStrForSet: "key cannot be empty", + errStrForGet: "key cannot be empty", + }, { name: "success context-aware-cli-for-plugins", cfg: &configtypes.ClientConfig{ @@ -175,11 +242,21 @@ func TestSetFeature(t *testing.T) { t.Run(tc.name, func(t *testing.T) { err := StoreClientConfig(tc.cfg) assert.NoError(t, err) + err = SetFeature(tc.plugin, tc.key, strconv.FormatBool(tc.value)) - assert.NoError(t, err) + if tc.errStrForSet != "" { + assert.Equal(t, tc.errStrForSet, err.Error()) + } else { + assert.NoError(t, err) + } + ok, err := IsFeatureEnabled(tc.plugin, tc.key) - assert.NoError(t, err) - assert.Equal(t, ok, tc.value) + if tc.errStrForGet != "" { + assert.Equal(t, tc.errStrForGet, err.Error()) + } else { + assert.NoError(t, err) + assert.Equal(t, ok, tc.value) + } }) } } diff --git a/config/metadata_api.go b/config/metadata_api.go index cd688116..345e2402 100644 --- a/config/metadata_api.go +++ b/config/metadata_api.go @@ -125,6 +125,11 @@ func setConfigMetadataPatchStrategies(node *yaml.Node, patchStrategies map[strin } func setConfigMetadataPatchStrategy(node *yaml.Node, key, value string) error { + // check if key is empty + if key == "" { + return errors.New("key cannot be empty") + } + if !strings.EqualFold(value, "replace") && !strings.EqualFold(value, "merge") { return errors.New("allowed values are replace or merge") } diff --git a/config/metadata_settings_api.go b/config/metadata_settings_api.go index 003b9a2e..9c77bc6c 100644 --- a/config/metadata_settings_api.go +++ b/config/metadata_settings_api.go @@ -105,6 +105,11 @@ func getSettings(node *yaml.Node) (map[string]string, error) { } func getSetting(node *yaml.Node, key string) (string, error) { + // check if key is empty + if key == "" { + return "", errors.New("key cannot be empty") + } + cfgMetadata, err := convertNodeToMetadata(node) if err != nil { return "", err @@ -122,6 +127,11 @@ func getSetting(node *yaml.Node, key string) (string, error) { } func deleteSetting(node *yaml.Node, key string) (err error) { + // check if key is empty + if key == "" { + return errors.New("key cannot be empty") + } + // find settings node keys := []nodeutils.Key{ {Name: KeyConfigMetadata, Type: yaml.MappingNode}, @@ -152,6 +162,15 @@ func deleteSetting(node *yaml.Node, key string) (err error) { //nolint:dupl func setSetting(node *yaml.Node, key, value string) (persist bool, err error) { + // check if key is empty + if key == "" { + return false, errors.New("key cannot be empty") + } + // check if value is empty + if value == "" { + return false, errors.New("value cannot be empty") + } + // find settings node keys := []nodeutils.Key{ {Name: KeyConfigMetadata, Type: yaml.MappingNode}, diff --git a/config/servers.go b/config/servers.go index 227602b0..1e836f0c 100644 --- a/config/servers.go +++ b/config/servers.go @@ -272,6 +272,11 @@ func setCurrentServer(node *yaml.Node, name string) (persist bool, err error) { } func getServer(node *yaml.Node, name string) (*configtypes.Server, error) { + // check if name is empty + if name == "" { + return nil, errors.New("name cannot be empty") + } + cfg, err := convertNodeToClientConfig(node) if err != nil { return nil, err @@ -298,6 +303,11 @@ func getCurrentServer(node *yaml.Node) (s *configtypes.Server, err error) { } func removeCurrentServer(node *yaml.Node, name string) error { + // check if name is empty + if name == "" { + return errors.New("name cannot be empty") + } + // find current server node keys := []nodeutils.Key{ {Name: KeyCurrentServer}, @@ -314,6 +324,11 @@ func removeCurrentServer(node *yaml.Node, name string) error { //nolint:dupl func removeServer(node *yaml.Node, name string) error { + // check if name is empty + if name == "" { + return errors.New("name cannot be empty") + } + // find servers node keys := []nodeutils.Key{ {Name: KeyServers}, @@ -344,6 +359,11 @@ func setServers(node *yaml.Node, servers []*configtypes.Server) error { } func setServer(node *yaml.Node, s *configtypes.Server) (persist bool, err error) { + // check if name is empty + if s.Name == "" { + return false, errors.New("server name cannot be empty") + } + // Get Patch Strategies patchStrategies, err := GetConfigMetadataPatchStrategy() if err != nil {