Skip to content

Commit d3d71a1

Browse files
SoManyHsSoManyHs
committed
Return error for unsupported volume keys
1 parent e319cac commit d3d71a1

File tree

3 files changed

+130
-31
lines changed

3 files changed

+130
-31
lines changed

‎ecs-cli/modules/cli/compose/project/project_test.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,9 +209,9 @@ func TestParseComposeForVersion2Files(t *testing.T) {
209209
mysqlImage := "mysql"
210210
ports := []string{"80:80"}
211211
memoryReservation := yaml.MemStringorInt(500000000)
212-
memory:= yaml.MemStringorInt(536870912) // 512 * 1024 * 1024
212+
memory := yaml.MemStringorInt(536870912) // 512 * 1024 * 1024
213213
shmSize := yaml.MemStringorInt(1073741824) // 1 gb = 1024 * 1024 * 1024 bytes
214-
tmpfs := yaml.Stringorslice{"/run:size=1gb", "/tmp:size=65536k"}
214+
tmpfs := yaml.Stringorslice{"/run:size=1gb", "/tmp:size=65536k"}
215215

216216
composeFileString := `version: '2'
217217
services:
@@ -225,7 +225,13 @@ services:
225225
- /run:size=1gb
226226
- /tmp:size=65536k
227227
mysql:
228-
image: mysql`
228+
image: mysql
229+
volumes:
230+
- banana:/tmp/cache
231+
- :/tmp/cache
232+
- ./cache:/tmp/cache:ro
233+
volumes:
234+
banana:`
229235

230236
// setup project and parse
231237
composeBytes := [][]byte{}

‎ecs-cli/modules/utils/compose/convert_task_definition.go

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ func ConvertToTaskDefinition(context *project.Context, volumeConfigs map[string]
9999
taskRoleArn = taskDefParams.taskRoleArn
100100
}
101101

102-
volumes := ConvertToVolumes(volumeConfigs)
102+
volumes, err := ConvertToVolumes(volumeConfigs)
103+
if err != nil {
104+
return nil, err
105+
}
103106

104107
// Create containerDefinitions
105108
containerDefinitions := []*ecs.ContainerDefinition{}
@@ -766,7 +769,9 @@ func ConvertToMemoryInMB(bytes int64) int64 {
766769
return memory
767770
}
768771

769-
func ConvertToVolumes(volumeConfigs map[string]*config.VolumeConfig) *volumes {
772+
// ConvertToVolumes converts the VolumeConfigs map on a libcompose project into
773+
// a Volumes struct and populates the volumeEmptyHost field with any named volumes
774+
func ConvertToVolumes(volumeConfigs map[string]*config.VolumeConfig) (*volumes, error) {
770775
volumes := &volumes{
771776
volumeWithHost: make(map[string]string), // map with key:=hostSourcePath value:=VolumeName
772777
}
@@ -775,28 +780,20 @@ func ConvertToVolumes(volumeConfigs map[string]*config.VolumeConfig) *volumes {
775780
if volumeConfigs != nil {
776781
for name, config := range volumeConfigs {
777782
if config != nil {
783+
// NOTE: If Driver field is not empty, this
784+
// will add a prefix to the named volume on the container
778785
if config.Driver != "" {
779-
log.WithFields(log.Fields{
780-
"volume name": name,
781-
"option name": "driver",
782-
}).Warn("Skipping unsupported YAML option...")
786+
return nil, errors.New("Volume driver is not supported")
783787
}
788+
// Driver Options must relate to a specific volume driver
784789
if len(config.DriverOpts) != 0 {
785-
log.WithFields(log.Fields{
786-
"volume name": name,
787-
"option name": "driver_opts",
788-
}).Warn("Skipping unsupported YAML option...")
789-
}
790-
if config.External.External {
791-
log.WithFields(log.Fields{
792-
"volume name": name,
793-
"option name": "external",
794-
}).Warn("Skipping unsupported YAML option...")
790+
return nil, errors.New("Volume driver options is not supported")
795791
}
792+
return nil, errors.New("External option is not supported")
796793
}
797794
volumes.volumeEmptyHost = append(volumes.volumeEmptyHost, name)
798795
}
799796
}
800797

801-
return volumes
798+
return volumes, nil
802799
}

‎ecs-cli/modules/utils/compose/convert_task_definition_test.go

Lines changed: 107 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -767,6 +767,7 @@ func TestConvertToTaskDefinitionWithVolumes(t *testing.T) {
767767
}
768768
volumeDef := *taskDefinition.Volumes[0]
769769
mountPoint := *containerDef.MountPoints[0]
770+
770771
if hostPath != aws.StringValue(volumeDef.Host.SourcePath) {
771772
t.Errorf("Expected HostSourcePath [%s] But was [%s]", hostPath, aws.StringValue(volumeDef.Host.SourcePath))
772773
}
@@ -788,7 +789,7 @@ func TestConvertToTaskDefinitionWithNamedVolume(t *testing.T) {
788789
Networks: &yaml.Networks{Networks: []*yaml.Network{defaultNetwork}},
789790
}
790791

791-
taskDefinition := convertToTaskDefinitionInTest(t, "name", &config.VolumeConfig{}, serviceConfig, "", "")
792+
taskDefinition := convertToTaskDefinitionInTest(t, "name", nil, serviceConfig, "", "")
792793
containerDef := *taskDefinition.ContainerDefinitions[0]
793794

794795
volumeDef := *taskDefinition.Volumes[0]
@@ -918,8 +919,17 @@ func TestConvertToMountPoints(t *testing.T) {
918919
}
919920

920921
// Valid inputs with host and container paths set
921-
mountPointsIn := yaml.Volumes{Volumes: []*yaml.Volume{&onlyContainerPath, &onlyContainerPath2, &hostAndContainerPath,
922-
&onlyContainerPathWithRO, &hostAndContainerPathWithRO, &hostAndContainerPathWithRW, &namedVolumeAndContainerPath}}
922+
mountPointsIn := yaml.Volumes{
923+
Volumes: []*yaml.Volume{
924+
&onlyContainerPath,
925+
&onlyContainerPath2,
926+
&hostAndContainerPath,
927+
&onlyContainerPathWithRO,
928+
&hostAndContainerPathWithRO,
929+
&hostAndContainerPathWithRW,
930+
&namedVolumeAndContainerPath,
931+
},
932+
}
923933

924934
mountPointsOut, err := ConvertToMountPoints(&mountPointsIn, volumes)
925935
if err != nil {
@@ -944,16 +954,37 @@ func TestConvertToMountPoints(t *testing.T) {
944954
if mountPointsOut[1].SourceVolume == mountPointsOut[3].SourceVolume {
945955
t.Errorf("Expected volume %v (onlyContainerPath2) and %v (onlyContainerPathWithRO) to be different", mountPointsOut[0].SourceVolume, mountPointsOut[1].SourceVolume)
946956
}
957+
}
958+
959+
func TestConvertToMountPointsWithInvalidAccessMode(t *testing.T) {
960+
volumes := &volumes{
961+
volumeWithHost: make(map[string]string),
962+
volumeEmptyHost: []string{namedVolume},
963+
}
964+
965+
hostAndContainerPathWithIncorrectAccess := yaml.Volume{
966+
Source: hostPath,
967+
Destination: containerPath,
968+
AccessMode: "readonly",
969+
}
970+
971+
mountPointsIn := yaml.Volumes{
972+
Volumes: []*yaml.Volume{&hostAndContainerPathWithIncorrectAccess},
973+
}
974+
975+
_, err := ConvertToMountPoints(&mountPointsIn, volumes)
947976

948-
// Invalid access mode input
949-
hostAndContainerPathWithIncorrectAccess := yaml.Volume{Source: hostPath, Destination: containerPath, AccessMode: "readonly"}
950-
mountPointsIn = yaml.Volumes{Volumes: []*yaml.Volume{&hostAndContainerPathWithIncorrectAccess}}
951-
mountPointsOut, err = ConvertToMountPoints(&mountPointsIn, volumes)
952977
if err == nil {
953978
t.Errorf("Expected to get error for mountPoint[%s] but didn't.", hostAndContainerPathWithIncorrectAccess)
954979
}
980+
}
955981

956-
mountPointsOut, err = ConvertToMountPoints(nil, volumes)
982+
func TestConvertToMountPointsNullContainerVolumes(t *testing.T) {
983+
volumes := &volumes{
984+
volumeWithHost: make(map[string]string),
985+
volumeEmptyHost: []string{namedVolume},
986+
}
987+
mountPointsOut, err := ConvertToMountPoints(nil, volumes)
957988
if err != nil {
958989
t.Fatalf("Expected to convert nil mountPoints without errors. But got [%v]", err)
959990
}
@@ -1064,11 +1095,76 @@ func verifyUlimit(t *testing.T, output *ecs.Ulimit, name string, softLimit, hard
10641095
}
10651096
}
10661097

1098+
func TestConvertToVolumes(t *testing.T) {
1099+
libcomposeVolumeConfigs := map[string]*config.VolumeConfig{
1100+
namedVolume: nil,
1101+
}
1102+
1103+
expected := &volumes{
1104+
volumeWithHost: make(map[string]string), // map with key:=hostSourcePath value:=VolumeName
1105+
volumeEmptyHost: []string{namedVolume}, // Declare one volume with an empty host
1106+
}
1107+
1108+
actual, err := ConvertToVolumes(libcomposeVolumeConfigs)
1109+
1110+
assert.NoError(t, err, "Unexpected error converting libcompose volume configs")
1111+
assert.Equal(t, expected, actual, "Named volumes should match")
1112+
}
1113+
1114+
func TestConvertToVolumes_ErrorsWithDriverSubfield(t *testing.T) {
1115+
libcomposeVolumeConfigs := map[string]*config.VolumeConfig{
1116+
namedVolume: &config.VolumeConfig{
1117+
Driver: "noodles",
1118+
},
1119+
}
1120+
1121+
_, err := ConvertToVolumes(libcomposeVolumeConfigs)
1122+
1123+
assert.Error(t, err, "Expected error converting libcompose volume configs when driver is specified")
1124+
}
1125+
1126+
func TestConvertToVolumes_ErrorsWithDriverOptsSubfield(t *testing.T) {
1127+
driverOpts := map[string]string{"foo": "bar"}
1128+
1129+
libcomposeVolumeConfigs := map[string]*config.VolumeConfig{
1130+
namedVolume: &config.VolumeConfig{
1131+
DriverOpts: driverOpts,
1132+
},
1133+
}
1134+
1135+
_, err := ConvertToVolumes(libcomposeVolumeConfigs)
1136+
1137+
assert.Error(t, err, "Expected error converting libcompose volume configs when driver options are specified")
1138+
}
1139+
1140+
func TestConvertToVolumes_ErrorsWithExternalSubfield(t *testing.T) {
1141+
external := yaml.External{External: false}
1142+
1143+
libcomposeVolumeConfigs := map[string]*config.VolumeConfig{
1144+
namedVolume: &config.VolumeConfig{
1145+
External: external,
1146+
},
1147+
}
1148+
1149+
_, err := ConvertToVolumes(libcomposeVolumeConfigs)
1150+
1151+
assert.Error(t, err, "Expected error converting libcompose volume configs when external is specified")
1152+
1153+
external = yaml.External{External: true}
1154+
libcomposeVolumeConfigs = map[string]*config.VolumeConfig{
1155+
namedVolume: &config.VolumeConfig{
1156+
External: external,
1157+
},
1158+
}
1159+
1160+
_, err = ConvertToVolumes(libcomposeVolumeConfigs)
1161+
1162+
assert.Error(t, err, "Expected error converting libcompose volume configs when external is specified")
1163+
}
1164+
10671165
func convertToTaskDefinitionInTest(t *testing.T, name string, volumeConfig *config.VolumeConfig, serviceConfig *config.ServiceConfig, taskRoleArn string, launchType string) *ecs.TaskDefinition {
10681166
volumeConfigs := make(map[string]*config.VolumeConfig)
1069-
if volumeConfig != nil {
1070-
volumeConfigs[namedVolume] = volumeConfig
1071-
}
1167+
volumeConfigs[namedVolume] = volumeConfig
10721168

10731169
serviceConfigs := config.NewServiceConfigs()
10741170
serviceConfigs.Add(name, serviceConfig)

0 commit comments

Comments
 (0)