Get storage account key from azure login, no need to specify it in compose file or run -v option

aci/convert/convert.go

@@ -18,7 +18,6 @@ package convert
 
 import (
 	"encoding/base64"
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"math"
@@ -26,6 +25,10 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/pkg/errors"
+
+	"github.com/docker/api/aci/login"
+
 	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
 	"github.com/Azure/go-autorest/autorest/to"
 	"github.com/compose-spec/compose-go/types"
@@ -42,7 +45,6 @@ const (
 	azureFileDriverName            = "azure_file"
 	volumeDriveroptsShareNameKey   = "share_name"
 	volumeDriveroptsAccountNameKey = "storage_account_name"
-	volumeDriveroptsAccountKeyKey  = "storage_account_key"
 	secretInlineMark               = "inline:"
 )
 
@@ -50,7 +52,15 @@ const (
 func ToContainerGroup(aciContext store.AciContext, p types.Project) (containerinstance.ContainerGroup, error) {
 	project := projectAciHelper(p)
 	containerGroupName := strings.ToLower(project.Name)
-	volumesCache, volumesSlice, err := project.getAciFileVolumes()
+	loginService, err := login.NewAzureLoginService()
+	if err != nil {
+		return containerinstance.ContainerGroup{}, err
+	}
+	storageHelper := login.StorageAccountHelper{
+		LoginService: *loginService,
+		AciContext:   aciContext,
+	}
+	volumesCache, volumesSlice, err := project.getAciFileVolumes(storageHelper)
 	if err != nil {
 		return containerinstance.ContainerGroup{}, err
 	}
@@ -191,7 +201,7 @@ func (p projectAciHelper) getAciSecretVolumes() ([]containerinstance.Volume, err
 	return secretVolumes, nil
 }
 
-func (p projectAciHelper) getAciFileVolumes() (map[string]bool, []containerinstance.Volume, error) {
+func (p projectAciHelper) getAciFileVolumes(helper login.StorageAccountHelper) (map[string]bool, []containerinstance.Volume, error) {
 	azureFileVolumesMap := make(map[string]bool, len(p.Volumes))
 	var azureFileVolumesSlice []containerinstance.Volume
 	for name, v := range p.Volumes {
@@ -204,9 +214,9 @@ func (p projectAciHelper) getAciFileVolumes() (map[string]bool, []containerinsta
 			if !ok {
 				return nil, nil, fmt.Errorf("cannot retrieve account name for Azurefile")
 			}
-			accountKey, ok := v.DriverOpts[volumeDriveroptsAccountKeyKey]
+			accountKey, err := helper.GetAzureStorageAccountKey(accountName)
-			if !ok {
+			if err != nil {
-				return nil, nil, fmt.Errorf("cannot retrieve account key for Azurefile")
+				return nil, nil, err
 			}
 			aciVolume := containerinstance.Volume{
 				Name: to.StringPtr(name),

aci/convert/volume.go

@@ -18,7 +18,6 @@ package convert
 
 import (
 	"fmt"
-	"net/url"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -44,7 +43,6 @@ func GetRunVolumes(volumes []string) (map[string]types.VolumeConfig, []types.Ser
 			Driver: azureFileDriverName,
 			DriverOpts: map[string]string{
 				volumeDriveroptsAccountNameKey: vi.username,
-				volumeDriveroptsAccountKeyKey:  vi.key,
 				volumeDriveroptsShareNameKey:   vi.share,
 			},
 		}
@@ -62,73 +60,26 @@ func GetRunVolumes(volumes []string) (map[string]types.VolumeConfig, []types.Ser
 type volumeInput struct {
 	name     string
 	username string
-	key      string
 	share    string
 	target   string
 }
 
-func escapeKeySlashes(rawURL string) (string, error) {
-	urlSplit := strings.Split(rawURL, "@")
-	if len(urlSplit) < 1 {
-		return "", fmt.Errorf("invalid URL format: %s", rawURL)
-	}
-	userPasswd := strings.ReplaceAll(urlSplit[0], "/", "_")
-
-	atIndex := strings.Index(rawURL, "@")
-	if atIndex < 0 {
-		return "", fmt.Errorf("no share specified in: %s", rawURL)
-	}
-
-	scaped := userPasswd + rawURL[atIndex:]
-
-	return scaped, nil
-}
-
-func unescapeKey(key string) string {
-	return strings.ReplaceAll(key, "_", "/")
-}
-
-// Removes the second ':' that separates the source from target
-func volumeURL(pathURL string) (*url.URL, error) {
-	scapedURL, err := escapeKeySlashes(pathURL)
-	if err != nil {
-		return nil, err
-	}
-	pathURL = "//" + scapedURL
-
-	count := strings.Count(pathURL, ":")
-	if count > 2 {
-		return nil, fmt.Errorf("invalid path URL: %s", pathURL)
-	}
-	if count == 2 {
-		tokens := strings.Split(pathURL, ":")
-		pathURL = fmt.Sprintf("%s:%s%s", tokens[0], tokens[1], tokens[2])
-	}
-	return url.Parse(pathURL)
-}
-
 func (v *volumeInput) parse(name string, s string) error {
-	volumeURL, err := volumeURL(s)
-	if err != nil {
-		return errors.Wrapf(errdefs.ErrParsingFailed, "unable to parse volume specification: %s", err.Error())
-	}
-	v.username = volumeURL.User.Username()
-	if v.username == "" {
-		return errors.Wrapf(errdefs.ErrParsingFailed, "volume specification %q does not include a storage username", v)
-	}
-	key, ok := volumeURL.User.Password()
-	if !ok || key == "" {
-		return errors.Wrapf(errdefs.ErrParsingFailed, "volume specification %q does not include a storage key", v)
-	}
-	v.key = unescapeKey(key)
-	v.share = volumeURL.Host
-	if v.share == "" {
-		return errors.Wrapf(errdefs.ErrParsingFailed, "volume specification %q does not include a storage file share", v)
-	}
 	v.name = name
-	v.target = volumeURL.Path
+	tokens := strings.Split(s, "@")
-	if v.target == "" {
+	if len(tokens) < 2 || tokens[0] == "" {
-		// Do not use filepath.Join, on Windows it will replace / by \
+		return errors.Wrapf(errdefs.ErrParsingFailed, "volume specification %q does not include a storage account before '@'", v)
+	}
+	v.username = tokens[0]
+	remaining := tokens[1]
+	tokens = strings.Split(remaining, ":")
+	if tokens[0] == "" {
+		return errors.Wrapf(errdefs.ErrParsingFailed, "volume specification %q does not include a storage file share after '@'", v)
+	}
+	v.share = tokens[0]
+	if len(tokens) > 1 {
+		v.target = tokens[1]
+	} else {
 		v.target = "/run/volumes/" + v.share
 	}
 	return nil

aci/convert/volume_test.go

@@ -21,21 +21,18 @@ import (
 
 	"github.com/compose-spec/compose-go/types"
 	"gotest.tools/v3/assert"
-
-	"github.com/docker/api/errdefs"
 )
 
 const (
 	storageAccountNameKey = "storage_account_name"
-	storageAccountKeyKey  = "storage_account_key"
 	shareNameKey          = "share_name"
 )
 
 func TestGetRunVolumes(t *testing.T) {
 	volumeStrings := []string{
-		"myuser1:mykey1@myshare1/my/path/to/target1",
+		"myuser1@myshare1:/my/path/to/target1",
-		"myuser2:mykey2@myshare2/my/path/to/target2",
+		"myuser2@myshare2:/my/path/to/target2",
-		"myuser3:mykey3@mydefaultsharename", // Use default placement at '/run/volumes/<share_name>'
+		"myuser3@mydefaultsharename", // Use default placement at '/run/volumes/<share_name>'
 	}
 	var goldenVolumeConfigs = map[string]types.VolumeConfig{
 		"volume-0": {
@@ -43,7 +40,6 @@ func TestGetRunVolumes(t *testing.T) {
 			Driver: "azure_file",
 			DriverOpts: map[string]string{
 				storageAccountNameKey: "myuser1",
-				storageAccountKeyKey:  "mykey1",
 				shareNameKey:          "myshare1",
 			},
 		},
@@ -52,7 +48,6 @@ func TestGetRunVolumes(t *testing.T) {
 			Driver: "azure_file",
 			DriverOpts: map[string]string{
 				storageAccountNameKey: "myuser2",
-				storageAccountKeyKey:  "mykey2",
 				shareNameKey:          "myshare2",
 			},
 		},
@@ -61,7 +56,6 @@ func TestGetRunVolumes(t *testing.T) {
 			Driver: "azure_file",
 			DriverOpts: map[string]string{
 				storageAccountNameKey: "myuser3",
-				storageAccountKeyKey:  "mykey3",
 				shareNameKey:          "mydefaultsharename",
 			},
 		},
@@ -95,29 +89,16 @@ func TestGetRunVolumes(t *testing.T) {
 }
 
 func TestGetRunVolumesMissingFileShare(t *testing.T) {
-	_, _, err := GetRunVolumes([]string{"myuser:mykey@"})
+	_, _, err := GetRunVolumes([]string{"myaccount@"})
-	assert.Assert(t, errdefs.IsErrParsingFailed(err))
+	assert.ErrorContains(t, err, "does not include a storage file share after '@'")
-	assert.ErrorContains(t, err, "does not include a storage file share")
 }
 
 func TestGetRunVolumesMissingUser(t *testing.T) {
-	_, _, err := GetRunVolumes([]string{":mykey@myshare"})
+	_, _, err := GetRunVolumes([]string{"@myshare"})
-	assert.Assert(t, errdefs.IsErrParsingFailed(err))
+	assert.ErrorContains(t, err, "does not include a storage account before '@'")
-	assert.ErrorContains(t, err, "does not include a storage username")
-}
-
-func TestGetRunVolumesMissingKey(t *testing.T) {
-	_, _, err := GetRunVolumes([]string{"userwithnokey:@myshare"})
-	assert.Assert(t, errdefs.IsErrParsingFailed(err))
-	assert.ErrorContains(t, err, "does not include a storage key")
-
-	_, _, err = GetRunVolumes([]string{"userwithnokeytoo@myshare"})
-	assert.Assert(t, errdefs.IsErrParsingFailed(err))
-	assert.ErrorContains(t, err, "does not include a storage key")
 }
 
 func TestGetRunVolumesNoShare(t *testing.T) {
 	_, _, err := GetRunVolumes([]string{"noshare"})
-	assert.Assert(t, errdefs.IsErrParsingFailed(err))
+	assert.ErrorContains(t, err, "does not include a storage account before '@'")
-	assert.ErrorContains(t, err, "no share specified")
 }

aci/login/StorageAccountHelper.go

@@ -0,0 +1,60 @@
+package login
+
+import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+
+	"github.com/docker/api/context/store"
+)
+
+const authenticationURL = "https://management.azure.com/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Storage/storageAccounts/%s/listKeys?api-version=2019-06-01"
+
+// StorageAccountHelper helper for Azure Storage Account
+type StorageAccountHelper struct {
+	LoginService AzureLoginService
+	AciContext   store.AciContext
+}
+
+type storageAcountKeys struct {
+	Keys []storageAcountKey `json:"keys"`
+}
+type storageAcountKey struct {
+	KeyName string `json:"keyName"`
+	Value   string `json:"value"`
+}
+
+// GetAzureStorageAccountKey retrieves the storage account ket from the current azure login
+func (helper StorageAccountHelper) GetAzureStorageAccountKey(accountName string) (string, error) {
+	token, err := helper.LoginService.GetValidToken()
+	if err != nil {
+		return "", err
+	}
+	authURL := fmt.Sprintf(authenticationURL, helper.AciContext.SubscriptionID, helper.AciContext.ResourceGroup, accountName)
+	req, err := http.NewRequest(http.MethodPost, authURL, nil)
+	if err != nil {
+		return "", err
+	}
+	req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", token.AccessToken))
+	res, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return "", err
+	}
+	bits, err := ioutil.ReadAll(res.Body)
+	if err != nil {
+		return "", err
+	}
+	if res.StatusCode >= 400 {
+		return "", fmt.Errorf("could not access storage account acountKeys for %s, using the azure login. Status %d : %s", accountName, res.StatusCode, string(bits))
+	}
+
+	acountKeys := storageAcountKeys{}
+	if err := json.Unmarshal(bits, &acountKeys); err != nil {
+		return "", err
+	}
+	if len(acountKeys.Keys) < 1 {
+		return "", fmt.Errorf("no key could be obtained for storage account %s from your azure login", accountName)
+	}
+	return acountKeys.Keys[0].Value, nil
+}

tests/aci-e2e/e2e-aci_test.go

@@ -165,7 +165,7 @@ func TestContainerRun(t *testing.T) {
 		mountTarget := "/usr/share/nginx/html"
 		res := c.RunDockerCmd(
 			"run", "-d",
-			"-v", fmt.Sprintf("%s:%s@%s:%s", saName, k, testShareName, mountTarget),
+			"-v", fmt.Sprintf("%s@%s:%s", saName, testShareName, mountTarget),
 			"-p", "80:80",
 			"nginx",
 		)

tests/composefiles/aci-demo/aci_demo_port_secrets_volumes.yaml

@@ -14,10 +14,6 @@ services:
     image: gtardif/sentences-web
     ports:
       - "80:80"
-    secrets:
-      - source: mysecret1
-        target: mytarget1
-      - mysecret2
     volumes:
       - mydata:/mount/testvolumes
 
@@ -25,12 +21,5 @@ volumes:
   mydata:
     driver: azure_file
     driver_opts:
-      share_name: gtashare1      
+      share_name: minecraft-volume
-      storage_account_name: gtastorageaccount1       
+      storage_account_name: minecraftdocker
-      storage_account_key: UZyyUyZJA0LYrPrXqvB+HP+gGWD0K54LNmtfV+xwGQ18JufaAQ7vtUhcJoEcFUUrm40mehLKtvi4n58w0ivDtQ==
-
-secrets:
-  mysecret1:
-    file: ./my_secret1.txt
-  mysecret2:
-    file: ./my_secret2.txt