TRID-9101 - Enforcing one authentication method while creating backend (

NetApp#949) Enforcing one authentication method while creating backend
YeYoot · Jun 23, 2022 · 655e707 · 655e707
1 parent 6e5bc69
commit 655e707
Show file tree

Hide file tree

Showing 3 changed files with 145 additions and 9 deletions.
diff --git a/storage_drivers/ontap/ontap_common.go b/storage_drivers/ontap/ontap_common.go
@@ -193,6 +193,11 @@ func InitializeOntapConfig(
 			return nil, fmt.Errorf("could not inject backend secret; err: %v", err)
 		}
 	}
+	// Ensure only one authentication type is specified in the backend config
+	if config.ClientPrivateKey != "" && config.Username != "" {
+		return nil, fmt.Errorf("more than one authentication method (username/password and clientPrivateKey)" +
+			" present in backend config; please ensure only one authentication method is provided")
+	}
 
 	// Load default config parameters
 	err = PopulateConfigurationDefaults(ctx, config)

diff --git a/storage_drivers/ontap/ontap_nas_test.go b/storage_drivers/ontap/ontap_nas_test.go
@@ -36,6 +36,15 @@ import (
 // via abstraction layer (ONTAPI interface)
 // //////////////////////////////////////////////////////////////////////////////////////////
 
+var (
+	ctx             = context.TODO()
+	debugTraceFlags = map[string]bool{"method": true, "api": true, "discovery": true}
+)
+
+const (
+	BackendUUID = "deadbeef-03af-4394-ace4-e177cdbcaf28"
+)
+
 func TestOntapNasStorageDriverConfigString(t *testing.T) {
 	vserverAdminHost := ONTAPTEST_LOCALHOST
 	vserverAdminPort := "0"
@@ -208,3 +217,111 @@ func TestInitializeStoragePoolsLabels(t *testing.T) {
 		assert.Equal(t, c.virtualExpected, label, c.virtualErrorMessage)
 	}
 }
+
+func TestOntapNasStorageDriverInitialize_WithTwoAuthMethods(t *testing.T) {
+	vserverAdminHost := ONTAPTEST_LOCALHOST
+	vserverAdminPort := "0"
+	vserverAggrName := ONTAPTEST_VSERVER_AGGR_NAME
+	ctx := context.TODO()
+	commonConfig := &drivers.CommonStorageDriverConfig{
+		Version:           1,
+		StorageDriverName: "ontap-nas",
+		BackendName:       "myOntapNasBackend",
+		DriverContext:     tridentconfig.ContextCSI,
+		DebugTraceFlags:   debugTraceFlags,
+	}
+
+	configJSON := `
+	{
+		"version":           1,
+		"storageDriverName": "ontap-nas",
+		"managementLIF":     "1.1.1.1:10",
+		"svm":               "SVM1",
+		"aggregate":         "data",
+		"username":          "dummyuser",
+		"password":          "dummypassword",
+		"clientcertificate": "dummy-certificate",
+		"clientprivatekey":  "dummy-client-private-key"
+	}`
+	ontapNasDriver := newTestOntapNASDriver(vserverAdminHost, vserverAdminPort, vserverAggrName,
+		tridentconfig.DriverContext("CSI"), false)
+
+	result := ontapNasDriver.Initialize(ctx, tridentconfig.DriverContext("CSI"), configJSON, commonConfig, map[string]string{}, BackendUUID)
+
+	assert.Error(t, result, "driver initialization succeeded despite more than one authentication methods in config")
+	assert.Contains(t, result.Error(), "more than one authentication method", "expected error string not found")
+}
+
+func TestOntapNasStorageDriverInitialize_WithTwoAuthMethodsWithSecrets(t *testing.T) {
+	vserverAdminHost := ONTAPTEST_LOCALHOST
+	vserverAdminPort := "0"
+	vserverAggrName := ONTAPTEST_VSERVER_AGGR_NAME
+	ctx := context.TODO()
+	commonConfig := &drivers.CommonStorageDriverConfig{
+		Version:           1,
+		StorageDriverName: "ontap-nas",
+		BackendName:       "myOntapNasBackend",
+		DriverContext:     tridentconfig.ContextCSI,
+		DebugTraceFlags:   debugTraceFlags,
+	}
+
+	configJSON := `
+	{
+		"version":           1,
+		"storageDriverName": "ontap-nas",
+		"managementLIF":     "1.1.1.1:10",
+		"svm":               "SVM1",
+		"aggregate":         "data"
+	}`
+	secrets := map[string]string{
+		"username":          "dummyuser",
+		"password":          "dummypassword",
+		"clientprivatekey":  "dummy-client-private-key",
+		"clientcertificate": "dummy-certificate",
+	}
+	ontapNasDriver := newTestOntapNASDriver(vserverAdminHost, vserverAdminPort, vserverAggrName,
+		tridentconfig.DriverContext("CSI"), false)
+
+	result := ontapNasDriver.Initialize(ctx, tridentconfig.DriverContext("CSI"), configJSON, commonConfig, secrets,
+		BackendUUID)
+
+	assert.Error(t, result, "driver initialization succeeded despite more than one authentication methods in config")
+	assert.Contains(t, result.Error(), "more than one authentication method", "expected error string not found")
+}
+
+func TestOntapNasStorageDriverInitialize_WithTwoAuthMethodsWithConfigAndSecrets(t *testing.T) {
+	vserverAdminHost := ONTAPTEST_LOCALHOST
+	vserverAdminPort := "0"
+	vserverAggrName := ONTAPTEST_VSERVER_AGGR_NAME
+	ctx := context.TODO()
+	commonConfig := &drivers.CommonStorageDriverConfig{
+		Version:           1,
+		StorageDriverName: "ontap-nas",
+		BackendName:       "myOntapNasBackend",
+		DriverContext:     tridentconfig.ContextCSI,
+		DebugTraceFlags:   debugTraceFlags,
+	}
+
+	configJSON := `
+	{
+		"version":           1,
+		"storageDriverName": "ontap-nas",
+		"managementLIF":     "1.1.1.1:10",
+		"svm":               "SVM1",
+		"aggregate":         "data",
+		"username":          "dummyuser",
+		"password":          "dummypassword"
+	}`
+	secrets := map[string]string{
+		"clientprivatekey":  "dummy-client-private-key",
+		"clientcertificate": "dummy-certificate",
+	}
+	ontapNasDriver := newTestOntapNASDriver(vserverAdminHost, vserverAdminPort, vserverAggrName,
+		tridentconfig.DriverContext("CSI"), false)
+
+	result := ontapNasDriver.Initialize(ctx, tridentconfig.DriverContext("CSI"), configJSON, commonConfig, secrets,
+		BackendUUID)
+
+	assert.Error(t, result, "driver initialization succeeded despite more than one authentication methods in config")
+	assert.Contains(t, result.Error(), "more than one authentication method", "expected error string not found")
+}
diff --git a/storage_drivers/types.go b/storage_drivers/types.go
@@ -153,14 +153,24 @@ func (d *OntapStorageDriverConfig) InjectSecrets(secretMap map[string]string) er
 	// NOTE: When the backend secrets are read in the CRD persistance layer they are converted to lower-case.
 
 	var ok bool
-	if d.ClientPrivateKey, ok = secretMap[strings.ToLower("ClientPrivateKey")]; !ok || d.
-		ClientPrivateKey == "" {
-		if d.Username, ok = secretMap[strings.ToLower("Username")]; !ok {
-			return injectionError("Username or ClientPrivateKey")
+	// Inject the credentials from the secretMap into the driver's config
+	if _, ok = secretMap[strings.ToLower("ClientPrivateKey")]; ok {
+		if d.ClientPrivateKey != "" {
+			log.Warn("clientPrivateKey is specified in both config and secret; overriding from secret.")
 		}
-		if d.Password, ok = secretMap[strings.ToLower("Password")]; !ok {
-			return injectionError("Password")
+		d.ClientPrivateKey = secretMap[strings.ToLower("ClientPrivateKey")]
+	}
+	if _, ok = secretMap[strings.ToLower("Username")]; ok {
+		if d.Username != "" {
+			log.Warn("Username is specified in both config and secret; overriding from secret.")
+		}
+		d.Username = secretMap[strings.ToLower("Username")]
+	}
+	if _, ok = secretMap[strings.ToLower("Password")]; ok {
+		if d.Password != "" {
+			log.Warn("Password is specified in both config and secret; overriding from secret.")
 		}
+		d.Password = secretMap[strings.ToLower("Password")]
 	}
 	// CHAP settings
 	if d.UseCHAP {
@@ -209,9 +219,13 @@ func (d *OntapStorageDriverConfig) ExtractSecrets() map[string]string {
 // HideSensitiveWithSecretName function replaces sensitive fields it contains (credentials, etc.),
 // with secretName.
 func (d *OntapStorageDriverConfig) HideSensitiveWithSecretName(secretName string) {
-	d.ClientPrivateKey = secretName
-	d.Username = secretName
-	d.Password = secretName
+	if d.ClientPrivateKey != "" {
+		d.ClientPrivateKey = secretName
+	}
+	if d.Username != "" {
+		d.Username = secretName
+		d.Password = secretName
+	}
 
 	// CHAP settings
 	if d.UseCHAP {