diff --git a/aci/convert/convert.go b/aci/convert/convert.go index c8134b7c4..b151433ae 100644 --- a/aci/convert/convert.go +++ b/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] - if !ok { - return nil, nil, fmt.Errorf("cannot retrieve account key for Azurefile") + accountKey, err := helper.GetAzureStorageAccountKey(accountName) + if err != nil { + return nil, nil, err } aciVolume := containerinstance.Volume{ Name: to.StringPtr(name), diff --git a/aci/convert/volume.go b/aci/convert/volume.go index e3b37a83d..bdda13618 100644 --- a/aci/convert/volume.go +++ b/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 - if v.target == "" { - // Do not use filepath.Join, on Windows it will replace / by \ + tokens := strings.Split(s, "@") + if len(tokens) < 2 || tokens[0] == "" { + 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 diff --git a/aci/convert/volume_test.go b/aci/convert/volume_test.go index 6a9da4470..b3bdebcdd 100644 --- a/aci/convert/volume_test.go +++ b/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", - "myuser2:mykey2@myshare2/my/path/to/target2", - "myuser3:mykey3@mydefaultsharename", // Use default placement at '/run/volumes/' + "myuser1@myshare1:/my/path/to/target1", + "myuser2@myshare2:/my/path/to/target2", + "myuser3@mydefaultsharename", // Use default placement at '/run/volumes/' } 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@"}) - assert.Assert(t, errdefs.IsErrParsingFailed(err)) - assert.ErrorContains(t, err, "does not include a storage file share") + _, _, err := GetRunVolumes([]string{"myaccount@"}) + assert.ErrorContains(t, err, "does not include a storage file share after '@'") } func TestGetRunVolumesMissingUser(t *testing.T) { - _, _, err := GetRunVolumes([]string{":mykey@myshare"}) - assert.Assert(t, errdefs.IsErrParsingFailed(err)) - 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") + _, _, err := GetRunVolumes([]string{"@myshare"}) + assert.ErrorContains(t, err, "does not include a storage account before '@'") } func TestGetRunVolumesNoShare(t *testing.T) { _, _, err := GetRunVolumes([]string{"noshare"}) - assert.Assert(t, errdefs.IsErrParsingFailed(err)) - assert.ErrorContains(t, err, "no share specified") + assert.ErrorContains(t, err, "does not include a storage account before '@'") } diff --git a/aci/login/StorageAccountHelper.go b/aci/login/StorageAccountHelper.go new file mode 100644 index 000000000..622c7a683 --- /dev/null +++ b/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 +} diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index 67fff876e..436abd06f 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/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", ) diff --git a/tests/composefiles/aci-demo/aci_demo_port_secrets_volumes.yaml b/tests/composefiles/aci-demo/aci_demo_port_secrets_volumes.yaml index 1469e65d8..31f982ed2 100644 --- a/tests/composefiles/aci-demo/aci_demo_port_secrets_volumes.yaml +++ b/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 - storage_account_name: gtastorageaccount1 - storage_account_key: UZyyUyZJA0LYrPrXqvB+HP+gGWD0K54LNmtfV+xwGQ18JufaAQ7vtUhcJoEcFUUrm40mehLKtvi4n58w0ivDtQ== - -secrets: - mysecret1: - file: ./my_secret1.txt - mysecret2: - file: ./my_secret2.txt + share_name: minecraft-volume + storage_account_name: minecraftdocker \ No newline at end of file