diff --git a/pkg/blobfuse/blobfuse.go b/pkg/blobfuse/blobfuse.go index 792667ebb..31900d3bf 100644 --- a/pkg/blobfuse/blobfuse.go +++ b/pkg/blobfuse/blobfuse.go @@ -167,42 +167,6 @@ func appendDefaultMountOptions(mountOptions []string) []string { return allMountOptions } -// get storage account from secrets map -// returns -func getStorageAccountFromSecretsMap(secrets map[string]string) (string, string, string, error) { - if secrets == nil { - return "", "", "", fmt.Errorf("unexpected: getStorageAccountFromSecretsMap secrets is nil") - } - - var accountName, accountKey, accountSasToken string - for k, v := range secrets { - switch strings.ToLower(k) { - case "accountname": - accountName = v - case "azurestorageaccountname": // for compatibility with built-in blobfuse plugin - accountName = v - case "accountkey": - accountKey = v - case "azurestorageaccountkey": // for compatibility with built-in blobfuse plugin - accountKey = v - case "azurestorageaccountsastoken": - accountSasToken = v - } - } - - if accountName == "" { - return "", "", "", fmt.Errorf("could not find accountname or azurestorageaccountname field secrets(%v)", secrets) - } - if accountKey == "" && accountSasToken == "" { - return "", "", "", fmt.Errorf("could not find accountkey, azurestorageaccountkey or azurestorageaccountsastoken field in secrets(%v)", secrets) - } - if accountKey != "" && accountSasToken != "" { - return "", "", "", fmt.Errorf("could not specify Access Key and SAS Token together") - } - - return accountName, accountKey, accountSasToken, nil -} - // A container name must be a valid DNS name, conforming to the following naming rules: // 1. Container names must start with a letter or number, and can contain only letters, numbers, and the dash (-) character. // 2. Every dash (-) character must be immediately preceded and followed by a letter or number; consecutive dashes are not permitted in container names. @@ -243,8 +207,118 @@ func isSASToken(key string) bool { return strings.Contains(key, "?sv=") } +// GetAuthEnv return +func (d *Driver) GetAuthEnv(ctx context.Context, volumeID string, attrib, secrets map[string]string) (string, string, []string, error) { + var ( + accountName string + accountKey string + accountSasToken string + containerName string + keyVaultURL string + keyVaultSecretName string + keyVaultSecretVersion string + authEnv []string + err error + ) + + for k, v := range attrib { + switch strings.ToLower(k) { + case "containername": + containerName = v + case "keyvaulturl": + keyVaultURL = v + case "keyvaultsecretname": + keyVaultSecretName = v + case "keyvaultsecretversion": + keyVaultSecretVersion = v + case "storageaccountname": + accountName = v + case "azurestorageidentityclientid": + authEnv = append(authEnv, "AZURE_STORAGE_IDENTITY_CLIENT_ID="+v) + case "azurestorageidentityobjectid": + authEnv = append(authEnv, "AZURE_STORAGE_IDENTITY_OBJECT_ID="+v) + case "azurestorageidentityresourceid": + authEnv = append(authEnv, "AZURE_STORAGE_IDENTITY_RESOURCE_ID="+v) + case "msiendpoint": + authEnv = append(authEnv, "MSI_ENDPOINT="+v) + case "azurestoragespnclientid": + authEnv = append(authEnv, "AZURE_STORAGE_SPN_CLIENT_ID="+v) + case "azurestoragespntenantid": + authEnv = append(authEnv, "AZURE_STORAGE_SPN_TENANT_ID="+v) + case "azurestorageaadendpoint": + authEnv = append(authEnv, "AZURE_STORAGE_AAD_ENDPOINT="+v) + } + } + + // 1. If keyVaultURL is not nil, preferentially use the key stored in key vault. + // 2. Then if secrets map is not nil, use the key stored in the secrets map. + // 3. Finally if both keyVaultURL and secrets map are nil, get the key from Azure. + if keyVaultURL != "" { + key, err := d.getKeyVaultSecretContent(ctx, keyVaultURL, keyVaultSecretName, keyVaultSecretVersion) + if err != nil { + return accountName, containerName, authEnv, err + } + if isSASToken(key) { + accountSasToken = key + } else { + accountKey = key + } + } else { + if len(secrets) == 0 { + var resourceGroupName string + resourceGroupName, accountName, containerName, err = GetContainerInfo(volumeID) + if err != nil { + return accountName, containerName, authEnv, err + } + + if resourceGroupName == "" { + resourceGroupName = d.cloud.ResourceGroup + } + + accountKey, err = d.cloud.GetStorageAccesskey(accountName, resourceGroupName) + if err != nil { + return accountName, containerName, authEnv, fmt.Errorf("no key for storage account(%s) under resource group(%s), err %v", accountName, resourceGroupName, err) + } + } else { + for k, v := range secrets { + switch strings.ToLower(k) { + case "accountname": + accountName = v + case "azurestorageaccountname": // for compatibility with built-in blobfuse plugin + accountName = v + case "accountkey": + accountKey = v + case "azurestorageaccountkey": // for compatibility with built-in blobfuse plugin + accountKey = v + case "azurestorageaccountsastoken": + accountSasToken = v + case "msisecret": + authEnv = append(authEnv, "MSI_SECRET="+v) + case "azurestoragespnclientsecret": + authEnv = append(authEnv, "AZURE_STORAGE_SPN_CLIENT_SECRET="+v) + } + } + } + } + + if containerName == "" { + err = fmt.Errorf("could not find containerName from attributes(%v) or volumeID(%v)", attrib, volumeID) + } + + if accountSasToken != "" { + authEnv = append(authEnv, "AZURE_STORAGE_SAS_TOKEN="+accountSasToken) + } + + if accountKey != "" { + authEnv = append(authEnv, "AZURE_STORAGE_ACCESS_KEY="+accountKey) + } + + return accountName, containerName, authEnv, err +} + // GetStorageAccountAndContainer: get storage account and container info // returns +// only for e2e testing func (d *Driver) GetStorageAccountAndContainer(ctx context.Context, volumeID string, attrib, secrets map[string]string) (string, string, string, string, error) { var ( accountName string @@ -304,11 +378,6 @@ func (d *Driver) GetStorageAccountAndContainer(ctx context.Context, volumeID str if err != nil { return "", "", "", "", fmt.Errorf("no key for storage account(%s) under resource group(%s), err %v", accountName, resourceGroupName, err) } - } else { - accountName, accountKey, accountSasToken, err = getStorageAccountFromSecretsMap(secrets) - if err != nil { - return "", "", "", "", err - } } } diff --git a/pkg/blobfuse/blobfuse_test.go b/pkg/blobfuse/blobfuse_test.go index 834ff5026..c423332f5 100644 --- a/pkg/blobfuse/blobfuse_test.go +++ b/pkg/blobfuse/blobfuse_test.go @@ -118,139 +118,6 @@ func TestGetContainerInfo(t *testing.T) { } } -func TestGetStorageAccountFromSecretsMap(t *testing.T) { - emptyAccountKeyMap := map[string]string{ - "accountname": "testaccount", - "accountkey": "", - } - - emptyAccountNameMap := map[string]string{ - "accountname": "", - "accountkey": "testkey", - } - - emptyAzureAccountKeyMap := map[string]string{ - "azurestorageaccountname": "testaccount", - "azurestorageaccountkey": "", - } - - emptyAzureAccountNameMap := map[string]string{ - "azurestorageaccountname": "", - "azurestorageaccountkey": "testkey", - } - - emptyAccountSasTokenMap := map[string]string{ - "azurestorageaccountname": "testaccount", - "azurestorageaccountsastoken": "", - } - - accesskeyAndaccountSasTokenMap := map[string]string{ - "azurestorageaccountname": "testaccount", - "azurestorageaccountkey": "testkey", - "azurestorageaccountsastoken": "testkey", - } - - tests := []struct { - options map[string]string - expected1 string - expected2 string - expected3 string - expected4 error - }{ - { - options: map[string]string{ - "accountname": "testaccount", - "accountkey": "testkey", - }, - expected1: "testaccount", - expected2: "testkey", - expected3: "", - expected4: nil, - }, - { - options: map[string]string{ - "azurestorageaccountname": "testaccount", - "azurestorageaccountkey": "testkey", - }, - expected1: "testaccount", - expected2: "testkey", - expected3: "", - expected4: nil, - }, - { - options: map[string]string{ - "accountname": "", - "accountkey": "", - }, - expected1: "", - expected2: "", - expected3: "", - expected4: fmt.Errorf("could not find accountname or azurestorageaccountname field secrets(map[accountname: accountkey:])"), - }, - { - options: emptyAccountKeyMap, - expected1: "", - expected2: "", - expected3: "", - expected4: fmt.Errorf("could not find accountkey, azurestorageaccountkey or azurestorageaccountsastoken field in secrets(%v)", emptyAccountKeyMap), - }, - { - options: emptyAccountNameMap, - expected1: "", - expected2: "", - expected3: "", - expected4: fmt.Errorf("could not find accountname or azurestorageaccountname field secrets(%v)", emptyAccountNameMap), - }, - { - options: emptyAzureAccountKeyMap, - expected1: "", - expected2: "", - expected3: "", - expected4: fmt.Errorf("could not find accountkey, azurestorageaccountkey or azurestorageaccountsastoken field in secrets(%v)", emptyAzureAccountKeyMap), - }, - { - options: emptyAzureAccountNameMap, - expected1: "", - expected2: "", - expected3: "", - expected4: fmt.Errorf("could not find accountname or azurestorageaccountname field secrets(%v)", emptyAzureAccountNameMap), - }, - { - options: emptyAccountSasTokenMap, - expected1: "", - expected2: "", - expected3: "", - expected4: fmt.Errorf("could not find accountkey, azurestorageaccountkey or azurestorageaccountsastoken field in secrets(%v)", emptyAzureAccountKeyMap), - }, - { - options: accesskeyAndaccountSasTokenMap, - expected1: "", - expected2: "", - expected3: "", - expected4: fmt.Errorf("could not specify Access Key and SAS Token together"), - }, - { - options: nil, - expected1: "", - expected2: "", - expected3: "", - expected4: fmt.Errorf("unexpected: getStorageAccountFromSecretsMap secrets is nil"), - }, - } - - for _, test := range tests { - result1, result2, result3, result4 := getStorageAccountFromSecretsMap(test.options) - if !reflect.DeepEqual(result1, test.expected1) || !reflect.DeepEqual(result2, test.expected2) || !reflect.DeepEqual(result3, test.expected3) { - t.Errorf("input: %q, getStorageAccountFromSecretsMap result1: %q, expected1: %q, result2: %q, expected2: %q, result3: %q, expected3: %q, result4: %q, expected4: %q", test.options, result1, test.expected1, result2, test.expected2, - result3, test.expected3, result4, test.expected4) - } else { - if result1 == "" || (result2 == "" && result3 == "") { - assert.Error(t, result4) - } - } - } -} - func TestGetValidContainerName(t *testing.T) { tests := []struct { volumeName string diff --git a/pkg/blobfuse/controllerserver.go b/pkg/blobfuse/controllerserver.go index db845101e..4bcbe0c9e 100644 --- a/pkg/blobfuse/controllerserver.go +++ b/pkg/blobfuse/controllerserver.go @@ -72,8 +72,6 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) resourceGroup = v case "containername": containerName = v - default: - return nil, fmt.Errorf("invalid option %q", k) } } diff --git a/pkg/blobfuse/nodeserver.go b/pkg/blobfuse/nodeserver.go index 836338f08..4c07af7a3 100644 --- a/pkg/blobfuse/nodeserver.go +++ b/pkg/blobfuse/nodeserver.go @@ -126,7 +126,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe attrib := req.GetVolumeContext() secrets := req.GetSecrets() - accountName, accountKey, accountSasToken, containerName, err := d.GetStorageAccountAndContainer(ctx, volumeID, attrib, secrets) + accountName, containerName, authEnv, err := d.GetAuthEnv(ctx, volumeID, attrib, secrets) if err != nil { return nil, err } @@ -144,14 +144,10 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe klog.V(2).Infof("target %v\nfstype %v\n\nvolumeId %v\ncontext %v\nmountflags %v\nmountOptions %v\nargs %v\nblobStorageEndPoint %v", targetPath, fsType, volumeID, attrib, mountFlags, mountOptions, args, blobStorageEndPoint) cmd := exec.Command("blobfuse", strings.Split(args, " ")...) - cmd.Env = append(os.Environ(), "AZURE_STORAGE_ACCOUNT="+accountName) - if accountSasToken != "" { - cmd.Env = append(cmd.Env, "AZURE_STORAGE_SAS_TOKEN="+accountSasToken) - } else { - cmd.Env = append(cmd.Env, "AZURE_STORAGE_ACCESS_KEY="+accountKey) - } + cmd.Env = append(os.Environ(), "AZURE_STORAGE_ACCOUNT="+accountName) cmd.Env = append(cmd.Env, "AZURE_STORAGE_BLOB_ENDPOINT="+blobStorageEndPoint) + cmd.Env = append(cmd.Env, authEnv...) output, err := cmd.CombinedOutput() if err != nil {