Skip to content

Commit

Permalink
Merge pull request #146 from andyzhangx/msi-endpoint
Browse files Browse the repository at this point in the history 
feat: support Managed Identity and Service Principal Name auth
  • Loading branch information
@andyzhangx
andyzhangx committed May 20, 2020
2 parents 024456b + 668fdbf commit 3355d49
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 183 deletions.
151 changes: 110 additions & 41 deletions pkg/blobfuse/blobfuse.go
View file
Expand Up @@ -167,42 +167,6 @@ func appendDefaultMountOptions(mountOptions []string) []string {
return allMountOptions
}

// get storage account from secrets map
// returns <accountName, accountKey, accountSasToken>
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.
Expand Down Expand Up @@ -243,8 +207,118 @@ func isSASToken(key string) bool {
return strings.Contains(key, "?sv=")
}

// GetAuthEnv return <accountName, containerName, authEnv, error>
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 <accountName, accountKey, accountSasToken, containerName>
// 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
Expand Down Expand Up @@ -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
}
}
}

Expand Down
133 changes: 0 additions & 133 deletions pkg/blobfuse/blobfuse_test.go
View file
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions pkg/blobfuse/controllerserver.go
View file
Expand Up @@ -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)
}
}

Expand Down
10 changes: 3 additions & 7 deletions pkg/blobfuse/nodeserver.go
View file
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down

0 comments on commit 3355d49

Please sign in to comment.