Bug Fix: Add empty checks for config APIs

config/certs.go

@@ -178,7 +178,6 @@ func setCert(node *yaml.Node, cert *configtypes.Cert) (persist bool, err error)
 	return persist, err
 }
 
-//nolint:dupl
 func removeCert(node *yaml.Node, host string) error {
 	// Find the certs node in the yaml node
 	keys := []nodeutils.Key{

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.CoreCliOptions != nil && cfg.CoreCliOptions.DiscoverySources != nil {
 		for _, discoverySource := range cfg.CoreCliOptions.DiscoverySources {
-			_, discoverySourceName := getDiscoverySourceTypeAndName(discoverySource)
+			_, discoverySourceName, err := getDiscoverySourceTypeAndName(discoverySource)
+			if err != nil {
+				return nil, err
+			}
 			if discoverySourceName == name {
 				return &discoverySource, nil
 			}
@@ -172,7 +180,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

config/cli_discovery_sources_test.go

@@ -66,22 +66,46 @@ func TestGetCLIDiscoverySource(t *testing.T) {
 	}
 
 	tests := []struct {
-		name    string
-		in, out *configtypes.PluginDiscovery
+		name                string
+		discoverySourceName string
+		in                  *configtypes.PluginDiscovery
+		out                 *configtypes.PluginDiscovery
+		errStr              string
 	}{
 		{
-			name: "success get",
-			in:   discovery,
-			out:  discovery,
+			name:                "success get",
+			in:                  discovery,
+			out:                 discovery,
+			discoverySourceName: "test",
+		},
+		{
+			name: "failed discovery source with empty name",
+			in: &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 := SetCLIDiscoverySource(*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)
+			}
 		})
 	}
 }
@@ -95,9 +119,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",
@@ -213,11 +238,29 @@ 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))

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

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,19 @@ 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)
+			}
 		})
 	}
 }

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 {

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