From 624308134f81c45db47c6035bc358d42c0f5fbd3 Mon Sep 17 00:00:00 2001 From: Guillaume Tardif Date: Tue, 10 Nov 2020 16:58:56 +0100 Subject: [PATCH] Specific error message when specifying path as a volume source. Removed volume reference validation already done at https://github.com/compose-spec/compose-go/blob/5afecaa4cb/loader/validate.go#L38 Signed-off-by: Guillaume Tardif --- aci/convert/convert.go | 8 ++++---- aci/convert/volume.go | 20 +++++++++----------- aci/convert/volume_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/aci/convert/convert.go b/aci/convert/convert.go index f4479bd2c..accd50ad6 100644 --- a/aci/convert/convert.go +++ b/aci/convert/convert.go @@ -50,7 +50,7 @@ const ( func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types.Project, storageHelper login.StorageLogin) (containerinstance.ContainerGroup, error) { project := projectAciHelper(p) containerGroupName := strings.ToLower(project.Name) - volumesCache, volumesSlice, err := project.getAciFileVolumes(ctx, storageHelper) + volumesSlice, err := project.getAciFileVolumes(ctx, storageHelper) if err != nil { return containerinstance.ContainerGroup{}, err } @@ -90,7 +90,7 @@ func ToContainerGroup(ctx context.Context, aciContext store.AciContext, p types. var dnsLabelName *string for _, s := range project.Services { service := serviceConfigAciHelper(s) - containerDefinition, err := service.getAciContainer(volumesCache) + containerDefinition, err := service.getAciContainer() if err != nil { return containerinstance.ContainerGroup{}, err } @@ -162,8 +162,8 @@ type projectAciHelper types.Project type serviceConfigAciHelper types.ServiceConfig -func (s serviceConfigAciHelper) getAciContainer(volumesCache map[string]bool) (containerinstance.Container, error) { - aciServiceVolumes, err := s.getAciFileVolumeMounts(volumesCache) +func (s serviceConfigAciHelper) getAciContainer() (containerinstance.Container, error) { + aciServiceVolumes, err := s.getAciFileVolumeMounts() if err != nil { return containerinstance.Container{}, err } diff --git a/aci/convert/volume.go b/aci/convert/volume.go index c1f2585cf..51b6fd86a 100644 --- a/aci/convert/volume.go +++ b/aci/convert/volume.go @@ -38,18 +38,17 @@ const ( volumeReadOnly = "read_only" ) -func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.StorageLogin) (map[string]bool, []containerinstance.Volume, error) { - azureFileVolumesMap := make(map[string]bool, len(p.Volumes)) +func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.StorageLogin) ([]containerinstance.Volume, error) { var azureFileVolumesSlice []containerinstance.Volume for name, v := range p.Volumes { if v.Driver == azureFileDriverName { shareName, ok := v.DriverOpts[volumeDriveroptsShareNameKey] if !ok { - return nil, nil, fmt.Errorf("cannot retrieve fileshare name for Azurefile") + return nil, fmt.Errorf("cannot retrieve fileshare name for Azurefile") } accountName, ok := v.DriverOpts[volumeDriveroptsAccountNameKey] if !ok { - return nil, nil, fmt.Errorf("cannot retrieve account name for Azurefile") + return nil, fmt.Errorf("cannot retrieve account name for Azurefile") } readOnly, ok := v.DriverOpts[volumeReadOnly] if !ok { @@ -57,11 +56,11 @@ func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.St } ro, err := strconv.ParseBool(readOnly) if err != nil { - return nil, nil, fmt.Errorf("invalid mode %q for volume", readOnly) + return nil, fmt.Errorf("invalid mode %q for volume", readOnly) } accountKey, err := helper.GetAzureStorageAccountKey(ctx, accountName) if err != nil { - return nil, nil, err + return nil, err } aciVolume := containerinstance.Volume{ Name: to.StringPtr(name), @@ -72,18 +71,17 @@ func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.St ReadOnly: &ro, }, } - azureFileVolumesMap[name] = true azureFileVolumesSlice = append(azureFileVolumesSlice, aciVolume) } } - return azureFileVolumesMap, azureFileVolumesSlice, nil + return azureFileVolumesSlice, nil } -func (s serviceConfigAciHelper) getAciFileVolumeMounts(volumesCache map[string]bool) ([]containerinstance.VolumeMount, error) { +func (s serviceConfigAciHelper) getAciFileVolumeMounts() ([]containerinstance.VolumeMount, error) { var aciServiceVolumes []containerinstance.VolumeMount for _, sv := range s.Volumes { - if !volumesCache[sv.Source] { - return []containerinstance.VolumeMount{}, fmt.Errorf("could not find volume source %q", sv.Source) + if sv.Type == string(types.VolumeTypeBind) { + return []containerinstance.VolumeMount{}, fmt.Errorf("host path (%q) not allowed as volume source, you need to reference an Azure File Share defined in the 'volumes' section", sv.Source) } aciServiceVolumes = append(aciServiceVolumes, containerinstance.VolumeMount{ Name: to.StringPtr(sv.Source), diff --git a/aci/convert/volume_test.go b/aci/convert/volume_test.go index 941ed7841..36062bf7b 100644 --- a/aci/convert/volume_test.go +++ b/aci/convert/volume_test.go @@ -119,6 +119,30 @@ func TestComposeVolumes(t *testing.T) { assert.DeepEqual(t, (*group.Volumes)[0], expectedGroupVolume) } +func TestPathVolumeErrorMessage(t *testing.T) { + ctx := context.TODO() + accountName := "myAccount" + mockStorageHelper.On("GetAzureStorageAccountKey", ctx, accountName).Return("123456", nil) + project := types.Project{ + Services: []types.ServiceConfig{ + { + Name: "service1", + Image: "image1", + Volumes: []types.ServiceVolumeConfig{ + { + Source: "/path", + Target: "/target", + Type: string(types.VolumeTypeBind), + }, + }, + }, + }, + } + + _, err := ToContainerGroup(ctx, convertCtx, project, mockStorageHelper) + assert.ErrorContains(t, err, `host path ("/path") not allowed as volume source, you need to reference an Azure File Share defined in the 'volumes' section`) +} + func TestComposeVolumesRO(t *testing.T) { ctx := context.TODO() accountName := "myAccount"