Modify parsing behavior of login credentials to handle Go escape char…

…acters (vmware-tanzu#564) * Signed-off-by: Varun Srinivasan <varuns6@vmware.com> Create common config and new function to parse config data * Always populate "port" key of parameters map after setting default VC port if config port value is empty Signed-off-by: Varun Srinivasan <varuns6@vmware.com> * Clean up test logging and unit tests Signed-off-by: Varun Srinivasan <varuns6@vmware.com> * Keep only necessary config values Signed-off-by: Varun Srinivasan <varuns6@vmware.com> * Refactor ParseConfig to return error for sanitized error handling Signed-off-by: Varun Srinivasan <varuns6@vmware.com> --------- Signed-off-by: Varun Srinivasan <varuns6@vmware.com>
lipingxue · Mar 11, 2024 · 5ea96ad · 5ea96ad
1 parent 3de8dbe
commit 5ea96ad
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 51 deletions.
diff --git a/go.mod b/go.mod
@@ -17,6 +17,7 @@ require (
 	github.com/vmware-tanzu/velero v1.10.2
 	github.com/vmware/govmomi v0.22.2-0.20200329013745-f2eef8fc745f
 	github.com/vmware/virtual-disks v0.0.4
+	gopkg.in/gcfg.v1 v1.2.3
 	k8s.io/api v0.24.2
 	k8s.io/apiextensions-apiserver v0.24.2
 	k8s.io/apimachinery v0.24.2
@@ -152,6 +153,7 @@ require (
 	google.golang.org/protobuf v1.30.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/ini.v1 v1.66.2 // indirect
+	gopkg.in/warnings.v0 v0.1.2 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	k8s.io/cli-runtime v0.24.0 // indirect

diff --git a/go.sum b/go.sum
@@ -1390,6 +1390,8 @@ gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntN
 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
 gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
 gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys=
+gopkg.in/gcfg.v1 v1.2.3 h1:m8OOJ4ccYHnx2f4gQwpno8nAX5OGOh7RLaaz0pj3Ogs=
+gopkg.in/gcfg.v1 v1.2.3/go.mod h1:yesOnuUOFQAhST5vPY4nbZsb/huCgGGXlipJsBn0b3o=
 gopkg.in/inf.v0 v0.9.1 h1:73M5CoZyi3ZLMOyDlQh031Cx6N9NDJ2Vvfl76EDAgDc=
 gopkg.in/inf.v0 v0.9.1/go.mod h1:cWUDdTG/fYaXco+Dcufb5Vnc6Gp2YChqWtbxRZE0mXw=
 gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k=
@@ -1401,6 +1403,8 @@ gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo=
 gopkg.in/square/go-jose.v2 v2.2.2/go.mod h1:M9dMgbHiYLoDGQrXy7OpJDJWiKiU//h+vD76mk0e1AI=
 gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 h1:uRGJdciOHaEIrze2W8Q3AKkepLTh2hOroT7a+7czfdQ=
 gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw=
+gopkg.in/warnings.v0 v0.1.2 h1:wFXVbFY8DY5/xOe1ECiWdKCzZlxgshcYVNkBHstARME=
+gopkg.in/warnings.v0 v0.1.2/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRNI=
 gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74=
 gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=

diff --git a/pkg/common/config/types.go b/pkg/common/config/types.go
@@ -0,0 +1,20 @@
+package config
+
+type Config struct {
+	Global struct {
+		VCenterIP           string `gcfg:"ip"`
+		ClusterID           string `gcfg:"cluster-id"`
+		ClusterDistribution string `gcfg:"cluster-distribution"`
+		CAFile              string `gcfg:"ca-file"`
+	}
+	// Virtual Center configurations
+	VirtualCenter map[string]*VirtualCenterConfig
+}
+
+type VirtualCenterConfig struct {
+	User         string `gcfg:"user"`
+	Password     string `gcfg:"password"`
+	VCenterPort  string `gcfg:"port"`
+	InsecureFlag bool   `gcfg:"insecure-flag"`
+	Datacenters  string `gcfg:"datacenters"`
+}
diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -28,9 +28,11 @@ import (
 	"time"
 
 	"github.com/hashicorp/go-version"
+	vcConfig "github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/common/config"
 	"github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/constants"
 	"github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/generated/clientset/versioned/typed/backupdriver/v1alpha1"
 	"github.com/vmware-tanzu/velero-plugin-for-vsphere/pkg/ivd"
+	"gopkg.in/gcfg.v1"
 	"k8s.io/client-go/tools/clientcmd"
 
 	"k8s.io/apimachinery/pkg/fields"
@@ -137,43 +139,44 @@ func RetrieveVcConfigSecret(params map[string]interface{}, config *rest.Config,
 		return errors.New(errMsg)
 	}
 
-	var sEnc string
-	var lines []string
-	for _, value := range secret.Data {
-		sEnc = string(value)
-		lines = strings.Split(sEnc, "\n")
-		logger.Debugf("Successfully retrieved vCenter configuration from secret %s", secret.Name)
-		break
+	err = ParseConfig(secret, params, logger)
+	if err != nil {
+		logger.WithError(err).Error("Failed to parse vSphere secret data")
+		return err
 	}
 
-	ParseLines(lines, params, logger)
-
-	// If port is missing, add an entry in the params to use the standard https port
-	if _, ok := params["port"]; !ok {
-		params["port"] = constants.DefaultVCenterPort
-	}
 	return nil
 }
 
-func ParseLines(lines []string, params map[string]interface{}, logger logrus.FieldLogger) {
-	for _, line := range lines {
-		if strings.Contains(line, "VirtualCenter") {
-			parts := strings.Split(line, "\"")
-			params["VirtualCenter"] = parts[1]
-		} else if strings.Contains(line, "=") {
-			parts := strings.SplitN(line, "=", 2)
-			key := strings.TrimSpace(parts[0])
-			value := strings.TrimSpace(parts[1])
-			// Skip the quotes in the value if present
-			unquotedValue, err := strconv.Unquote(string(value))
-			if err != nil {
-				logger.WithError(err).Debugf("Failed to unquote value %v for key %v. Just store the original value string", value, key)
-				params[key] = string(value)
-				continue
-			}
-			params[key] = unquotedValue
+func ParseConfig(secret *k8sv1.Secret, params map[string]interface{}, logger logrus.FieldLogger) error {
+	// Setup config
+	var conf vcConfig.Config
+	// Read secret data
+	for _, value := range secret.Data {
+		confStr := string(value) // Convert the secret data to a string
+		err := gcfg.FatalOnly(gcfg.ReadStringInto(&conf, confStr))
+		if err != nil {
+			logger.WithError(err).Error("Failed to parse vSphere secret data")
+			return err
 		}
+		logger.Debugf("Successfully parsed vCenter configuration from secret %s", secret.Name)
+		break
 	}
+
+	// Use config data from struct to populate params map passed by RetrieveVcConfigSecret callers
+	params["cluster-id"] = conf.Global.ClusterID
+
+	for ip, vcConfig := range conf.VirtualCenter {
+		params["VirtualCenter"] = ip
+		params["user"] = vcConfig.User
+		params["password"] = vcConfig.Password
+		if vcConfig.VCenterPort == "" {
+			vcConfig.VCenterPort = constants.DefaultVCenterPort
+		}
+		params["port"] = vcConfig.VCenterPort
+	}
+
+	return nil
 }
 
 func RetrieveParamsFromBSL(repositoryParams map[string]string, bslName string, config *rest.Config,
@@ -1022,7 +1025,7 @@ func waitForPvSecret(ctx context.Context, clientSet *kubernetes.Clientset, names
 }
 
 /*
- Adds the Velero label to exclude this K8S resource from the backup
+Adds the Velero label to exclude this K8S resource from the backup
 */
 func AddVeleroExcludeLabelToObjectMeta(objectMeta *metav1.ObjectMeta) {
 	objectMeta.Labels = AppendVeleroExcludeLabels(objectMeta.Labels)

diff --git a/pkg/utils/utils_unit_test.go b/pkg/utils/utils_unit_test.go
@@ -274,7 +274,7 @@ cluster-distribution = "CSI-Vanilla"
 
 [VirtualCenter "10.182.1.133"]
 user = "Administrator@vsphere.local"
-password = "Pass"
+password = "6^54#,RDvwgJ\\nEdg$2"
 datacenters = "VSAN-DC"
 port = "443"
 	`
@@ -361,7 +361,7 @@ func TestGetBool(t *testing.T) {
 	}
 }
 
-func TestParseLines(t *testing.T) {
+func TestParseConfig(t *testing.T) {
 	// Setup Logger
 	logger := logrus.New()
 	formatter := new(logrus.TextFormatter)
@@ -372,34 +372,32 @@ func TestParseLines(t *testing.T) {
 
 	tests := []struct {
 		name     string
-		sEnc     string
 		vc       string
+		user     string
 		password string
 	}{
 		{
-			name:     "Password with special character \\ in it",
-			sEnc:     "[VirtualCenter \"sc-rdops-vm06-dhcp-184-231.eng.vmware.com\"]\npassword = \"GpI4G`OK'?in40Fo/0\\\\;\"",
+			name:     `Password with special character \ in it`,
 			vc:       "sc-rdops-vm06-dhcp-184-231.eng.vmware.com",
-			password: "GpI4G`OK'?in40Fo/0\\;",
-		},
-		{
-			name:     "Password with multiple = in it",
-			sEnc:     "[VirtualCenter \"sc-rdops-vm06-dhcp-184-231.eng.vmware.com\"]\npassword = \"GpI4G`OK'?in40Fo/0\\\\;=h=\"",
-			vc:       "sc-rdops-vm06-dhcp-184-231.eng.vmware.com",
-			password: "GpI4G`OK'?in40Fo/0\\;=h=",
-		},
-		{
-			name:     "Password with special character \\t in it",
-			sEnc:     "[VirtualCenter \"sc-rdops-vm06-dhcp-184-231.eng.vmware.com\"]\npassword = \"G4\\t4t\"",
-			vc:       "sc-rdops-vm06-dhcp-184-231.eng.vmware.com",
-			password: "G4\t4t",
+			user:     "Administrator@vsphere.local",
+			password: `6^54#,RDvwgJ\Edg$2`,
 		},
 	}
+	confData := `[Global]
+	cluster-id = "cluster1"
+	
+	[VirtualCenter "sc-rdops-vm06-dhcp-184-231.eng.vmware.com"]
+	user = "Administrator@vsphere.local"
+	password = "6^54#,RDvwgJ\\Edg$2"
+	port = "443"`
+	mockSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{Name: "velero-vsphere-config-secret"},
+		Data:       map[string][]byte{"csi-vsphere.conf": []byte(confData)},
+	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			lines := strings.Split(test.sEnc, "\n")
 			params := make(map[string]interface{})
-			ParseLines(lines, params, logger)
+			ParseConfig(mockSecret, params, logger)
 			assert.Equal(t, test.vc, params["VirtualCenter"])
 			assert.Equal(t, test.password, params["password"])
 		})
@@ -980,12 +978,34 @@ func TestRetrieveVcConfigSecret(t *testing.T) {
 	formatter.FullTimestamp = true
 	logger.SetFormatter(formatter)
 	logger.SetLevel(logrus.DebugLevel)
+	confData := `[Global]
+	cluster-id = "cluster1"
+	
+	[VirtualCenter "10.182.1.133"]
+	user = "user@vsphere.local"
+	password = "password"
+	port = "443"`
+	mockSecret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{Name: "velero-vsphere-config-secret"},
+		Data:       map[string][]byte{"csi-vsphere.conf": []byte(confData)},
+	}
 	tests := []struct {
 		name        string
 		runtimeObjs []runtime.Object
 		config      *rest.Config
 		expectError bool
 	}{
+		{
+			name: "Test secret retrieval",
+			runtimeObjs: []runtime.Object{
+				decoupleVSphereCSIDriverFeatureDisabled,
+				csi201VanillaDeployment,
+				csi201VSphereCredentialSecret,
+				mockSecret,
+			},
+			config:      &rest.Config{},
+			expectError: false,
+		},
 		{
 			name: "Decouple CSI driver feature enabled, plugin config absent, supervisor csi 2.3, csi secret present",
 			runtimeObjs: []runtime.Object{