Separate secret convert code into its own file

Signed-off-by: Guillaume Tardif <guillaume.tardif@docker.com>

aci/convert/convert.go

@@ -18,12 +18,9 @@ package convert
 
 import (
 	"context"
-	"encoding/base64"
 	"fmt"
-	"io/ioutil"
 	"math"
 	"os"
-	"path"
 	"strconv"
 	"strings"
 
@@ -50,9 +47,6 @@ const (
 	volumeDriveroptsShareNameKey   = "share_name"
 	volumeDriveroptsAccountNameKey = "storage_account_name"
 	volumeReadOnly                 = "read_only"
-
-	defaultSecretsPath         = "/run/secrets"
-	serviceSecretAbsPathPrefix = "aci-service-secret-path-"
 )
 
 // ToContainerGroup converts a compose project into a ACI container group
@@ -190,58 +184,6 @@ func getDNSSidecar(containers []containerinstance.Container) containerinstance.C
 
 type projectAciHelper types.Project
 
-func getServiceSecretKey(serviceName, targetDir string) string {
-	return fmt.Sprintf("%s-%s--%s",
-		serviceSecretAbsPathPrefix, serviceName, strings.ReplaceAll(targetDir, "/", "-"))
-}
-
-func (p projectAciHelper) getAciSecretVolumes() ([]containerinstance.Volume, error) {
-	var secretVolumes []containerinstance.Volume
-	for _, svc := range p.Services {
-		squashedTargetVolumes := make(map[string]containerinstance.Volume)
-		for _, scr := range svc.Secrets {
-			data, err := ioutil.ReadFile(p.Secrets[scr.Source].File)
-			if err != nil {
-				return secretVolumes, err
-			}
-			if len(data) == 0 {
-				continue
-			}
-			dataStr := base64.StdEncoding.EncodeToString(data)
-			if scr.Target == "" {
-				scr.Target = scr.Source
-			}
-
-			if !path.IsAbs(scr.Target) && strings.ContainsAny(scr.Target, "\\/") {
-				return []containerinstance.Volume{},
-					errors.Errorf("in service %q, secret with source %q cannot have a relative path as target. "+
-						"Only absolute paths are allowed. Found %q",
-						svc.Name, scr.Source, scr.Target)
-			}
-
-			if !path.IsAbs(scr.Target) {
-				scr.Target = path.Join(defaultSecretsPath, scr.Target)
-			}
-
-			targetDir := path.Dir(scr.Target)
-			targetDirKey := getServiceSecretKey(svc.Name, targetDir)
-			if _, ok := squashedTargetVolumes[targetDir]; !ok {
-				squashedTargetVolumes[targetDir] = containerinstance.Volume{
-					Name:   to.StringPtr(targetDirKey),
-					Secret: make(map[string]*string),
-				}
-			}
-
-			squashedTargetVolumes[targetDir].Secret[path.Base(scr.Target)] = &dataStr
-		}
-		for _, v := range squashedTargetVolumes {
-			secretVolumes = append(secretVolumes, v)
-		}
-	}
-
-	return secretVolumes, nil
-}
-
 func (p projectAciHelper) getAciFileVolumes(ctx context.Context, helper login.StorageLogin) (map[string]bool, []containerinstance.Volume, error) {
 	azureFileVolumesMap := make(map[string]bool, len(p.Volumes))
 	var azureFileVolumesSlice []containerinstance.Volume
@@ -347,64 +289,6 @@ func (s serviceConfigAciHelper) getAciFileVolumeMounts(volumesCache map[string]b
 	return aciServiceVolumes, nil
 }
 
-func (s serviceConfigAciHelper) getAciSecretsVolumeMounts() ([]containerinstance.VolumeMount, error) {
-	vms := []containerinstance.VolumeMount{}
-	presenceSet := make(map[string]bool)
-	for _, scr := range s.Secrets {
-		if scr.Target == "" {
-			scr.Target = scr.Source
-		}
-		if !path.IsAbs(scr.Target) {
-			scr.Target = path.Join(defaultSecretsPath, scr.Target)
-		}
-
-		presenceKey := path.Dir(scr.Target)
-		if !presenceSet[presenceKey] {
-			vms = append(vms, containerinstance.VolumeMount{
-				Name:      to.StringPtr(getServiceSecretKey(s.Name, path.Dir(scr.Target))),
-				MountPath: to.StringPtr(path.Dir(scr.Target)),
-				ReadOnly:  to.BoolPtr(true),
-			})
-			presenceSet[presenceKey] = true
-		}
-	}
-	err := validateMountPathCollisions(vms)
-	if err != nil {
-		return []containerinstance.VolumeMount{}, err
-	}
-	return vms, nil
-}
-
-func validateMountPathCollisions(vms []containerinstance.VolumeMount) error {
-	for i, vm1 := range vms {
-		for j, vm2 := range vms {
-			if i == j {
-				continue
-			}
-			var (
-				biggerVMPath  = strings.Split(*vm1.MountPath, "/")
-				smallerVMPath = strings.Split(*vm2.MountPath, "/")
-			)
-			if len(smallerVMPath) > len(biggerVMPath) {
-				tmp := biggerVMPath
-				biggerVMPath = smallerVMPath
-				smallerVMPath = tmp
-			}
-			isPrefixed := true
-			for i := 0; i < len(smallerVMPath); i++ {
-				if smallerVMPath[i] != biggerVMPath[i] {
-					isPrefixed = false
-					break
-				}
-			}
-			if isPrefixed {
-				return errors.Errorf("mount paths %q and %q collide. A volume mount cannot include another one.", *vm1.MountPath, *vm2.MountPath)
-			}
-		}
-	}
-	return nil
-}
-
 func (s serviceConfigAciHelper) getAciContainer(volumesCache map[string]bool) (containerinstance.Container, error) {
 	aciServiceVolumes, err := s.getAciFileVolumeMounts(volumesCache)
 	if err != nil {

aci/convert/convert_test.go

@@ -18,10 +18,7 @@ package convert
 
 import (
 	"context"
-	"fmt"
-	"io/ioutil"
 	"os"
-	"path"
 	"testing"
 
 	"github.com/stretchr/testify/mock"
@@ -715,157 +712,6 @@ func TestConvertContainerGroupStatus(t *testing.T) {
 	assert.Equal(t, "Unknown", GetStatus(container(nil), group(nil)))
 }
 
-func TestConvertSecrets(t *testing.T) {
-	serviceName := "testservice"
-	secretName := "testsecret"
-	absBasePath := "/home/user"
-	tmpFile, err := ioutil.TempFile(os.TempDir(), "TestConvertProjectSecrets-")
-	assert.NilError(t, err)
-	_, err = tmpFile.Write([]byte("test content"))
-	assert.NilError(t, err)
-	t.Cleanup(func() {
-		_ = os.Remove(tmpFile.Name())
-	})
-
-	t.Run("mix default and absolute", func(t *testing.T) {
-		pSquashedDefaultAndAbs := projectAciHelper{
-			Services: []types.ServiceConfig{
-				{
-					Name: serviceName,
-					Secrets: []types.ServiceSecretConfig{
-						{
-							Source: secretName,
-							Target: "some_target1",
-						},
-						{
-							Source: secretName,
-						},
-						{
-							Source: secretName,
-							Target: path.Join(defaultSecretsPath, "some_target2"),
-						},
-						{
-							Source: secretName,
-							Target: path.Join(absBasePath, "some_target3"),
-						},
-						{
-							Source: secretName,
-							Target: path.Join(absBasePath, "some_target4"),
-						},
-					},
-				},
-			},
-			Secrets: map[string]types.SecretConfig{
-				secretName: {
-					File: tmpFile.Name(),
-				},
-			},
-		}
-		volumes, err := pSquashedDefaultAndAbs.getAciSecretVolumes()
-		assert.NilError(t, err)
-		assert.Equal(t, len(volumes), 2)
-
-		defaultVolumeName := getServiceSecretKey(serviceName, defaultSecretsPath)
-		homeVolumeName := getServiceSecretKey(serviceName, absBasePath)
-		// random order since this was created from a map...
-		for _, vol := range volumes {
-			switch *vol.Name {
-			case defaultVolumeName:
-				assert.Equal(t, len(vol.Secret), 3)
-			case homeVolumeName:
-				assert.Equal(t, len(vol.Secret), 2)
-			default:
-				assert.Assert(t, false, "unexpected volume name: "+*vol.Name)
-			}
-		}
-
-		s := serviceConfigAciHelper(pSquashedDefaultAndAbs.Services[0])
-		vms, err := s.getAciSecretsVolumeMounts()
-		assert.NilError(t, err)
-		assert.Equal(t, len(vms), 2)
-
-		assert.Equal(t, *vms[0].Name, defaultVolumeName)
-		assert.Equal(t, *vms[0].MountPath, defaultSecretsPath)
-
-		assert.Equal(t, *vms[1].Name, homeVolumeName)
-		assert.Equal(t, *vms[1].MountPath, absBasePath)
-	})
-
-	t.Run("convert invalid target", func(t *testing.T) {
-		targetName := "some/invalid/relative/path/target"
-		pInvalidRelativePathTarget := projectAciHelper{
-			Services: []types.ServiceConfig{
-				{
-					Name: serviceName,
-					Secrets: []types.ServiceSecretConfig{
-						{
-							Source: secretName,
-							Target: targetName,
-						},
-					},
-				},
-			},
-			Secrets: map[string]types.SecretConfig{
-				secretName: {
-					File: tmpFile.Name(),
-				},
-			},
-		}
-		_, err := pInvalidRelativePathTarget.getAciSecretVolumes()
-		assert.Equal(t, err.Error(),
-			fmt.Sprintf(`in service %q, secret with source %q cannot have a relative path as target. Only absolute paths are allowed. Found %q`,
-				serviceName, secretName, targetName))
-	})
-
-	t.Run("convert colliding default targets", func(t *testing.T) {
-		targetName1 := path.Join(defaultSecretsPath, "target1")
-		targetName2 := path.Join(defaultSecretsPath, "sub/folder/target2")
-
-		service := serviceConfigAciHelper{
-			Name: serviceName,
-			Secrets: []types.ServiceSecretConfig{
-				{
-					Source: secretName,
-					Target: targetName1,
-				},
-				{
-					Source: secretName,
-					Target: targetName2,
-				},
-			},
-		}
-
-		_, err := service.getAciSecretsVolumeMounts()
-		assert.Equal(t, err.Error(),
-			fmt.Sprintf(`mount paths %q and %q collide. A volume mount cannot include another one.`,
-				path.Dir(targetName1), path.Dir(targetName2)))
-	})
-
-	t.Run("convert colliding absolute targets", func(t *testing.T) {
-		targetName1 := path.Join(absBasePath, "target1")
-		targetName2 := path.Join(absBasePath, "sub/folder/target2")
-
-		service := serviceConfigAciHelper{
-			Name: serviceName,
-			Secrets: []types.ServiceSecretConfig{
-				{
-					Source: secretName,
-					Target: targetName1,
-				},
-				{
-					Source: secretName,
-					Target: targetName2,
-				},
-			},
-		}
-
-		_, err := service.getAciSecretsVolumeMounts()
-		assert.Equal(t, err.Error(),
-			fmt.Sprintf(`mount paths %q and %q collide. A volume mount cannot include another one.`,
-				path.Dir(targetName1), path.Dir(targetName2)))
-	})
-}
-
 func container(status *string) containerinstance.Container {
 	var state *containerinstance.ContainerState = nil
 	if status != nil {

aci/convert/secrets.go

@@ -0,0 +1,144 @@
+/*
+   Copyright 2020 Docker Compose CLI authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package convert
+
+import (
+	"encoding/base64"
+	"fmt"
+	"io/ioutil"
+	"path"
+	"strings"
+
+	"github.com/Azure/azure-sdk-for-go/services/containerinstance/mgmt/2018-10-01/containerinstance"
+	"github.com/Azure/go-autorest/autorest/to"
+	"github.com/pkg/errors"
+)
+
+const (
+	defaultSecretsPath         = "/run/secrets"
+	serviceSecretAbsPathPrefix = "aci-service-secret-path-"
+)
+
+func getServiceSecretKey(serviceName, targetDir string) string {
+	return fmt.Sprintf("%s-%s--%s",
+		serviceSecretAbsPathPrefix, serviceName, strings.ReplaceAll(targetDir, "/", "-"))
+}
+
+func (p projectAciHelper) getAciSecretVolumes() ([]containerinstance.Volume, error) {
+	var secretVolumes []containerinstance.Volume
+	for _, svc := range p.Services {
+		squashedTargetVolumes := make(map[string]containerinstance.Volume)
+		for _, scr := range svc.Secrets {
+			data, err := ioutil.ReadFile(p.Secrets[scr.Source].File)
+			if err != nil {
+				return secretVolumes, err
+			}
+			if len(data) == 0 {
+				continue
+			}
+			dataStr := base64.StdEncoding.EncodeToString(data)
+			if scr.Target == "" {
+				scr.Target = scr.Source
+			}
+
+			if !path.IsAbs(scr.Target) && strings.ContainsAny(scr.Target, "\\/") {
+				return []containerinstance.Volume{},
+					errors.Errorf("in service %q, secret with source %q cannot have a relative path as target. "+
+						"Only absolute paths are allowed. Found %q",
+						svc.Name, scr.Source, scr.Target)
+			}
+
+			if !path.IsAbs(scr.Target) {
+				scr.Target = path.Join(defaultSecretsPath, scr.Target)
+			}
+
+			targetDir := path.Dir(scr.Target)
+			targetDirKey := getServiceSecretKey(svc.Name, targetDir)
+			if _, ok := squashedTargetVolumes[targetDir]; !ok {
+				squashedTargetVolumes[targetDir] = containerinstance.Volume{
+					Name:   to.StringPtr(targetDirKey),
+					Secret: make(map[string]*string),
+				}
+			}
+
+			squashedTargetVolumes[targetDir].Secret[path.Base(scr.Target)] = &dataStr
+		}
+		for _, v := range squashedTargetVolumes {
+			secretVolumes = append(secretVolumes, v)
+		}
+	}
+
+	return secretVolumes, nil
+}
+
+func (s serviceConfigAciHelper) getAciSecretsVolumeMounts() ([]containerinstance.VolumeMount, error) {
+	vms := []containerinstance.VolumeMount{}
+	presenceSet := make(map[string]bool)
+	for _, scr := range s.Secrets {
+		if scr.Target == "" {
+			scr.Target = scr.Source
+		}
+		if !path.IsAbs(scr.Target) {
+			scr.Target = path.Join(defaultSecretsPath, scr.Target)
+		}
+
+		presenceKey := path.Dir(scr.Target)
+		if !presenceSet[presenceKey] {
+			vms = append(vms, containerinstance.VolumeMount{
+				Name:      to.StringPtr(getServiceSecretKey(s.Name, path.Dir(scr.Target))),
+				MountPath: to.StringPtr(path.Dir(scr.Target)),
+				ReadOnly:  to.BoolPtr(true),
+			})
+			presenceSet[presenceKey] = true
+		}
+	}
+	err := validateMountPathCollisions(vms)
+	if err != nil {
+		return []containerinstance.VolumeMount{}, err
+	}
+	return vms, nil
+}
+
+func validateMountPathCollisions(vms []containerinstance.VolumeMount) error {
+	for i, vm1 := range vms {
+		for j, vm2 := range vms {
+			if i == j {
+				continue
+			}
+			var (
+				biggerVMPath  = strings.Split(*vm1.MountPath, "/")
+				smallerVMPath = strings.Split(*vm2.MountPath, "/")
+			)
+			if len(smallerVMPath) > len(biggerVMPath) {
+				tmp := biggerVMPath
+				biggerVMPath = smallerVMPath
+				smallerVMPath = tmp
+			}
+			isPrefixed := true
+			for i := 0; i < len(smallerVMPath); i++ {
+				if smallerVMPath[i] != biggerVMPath[i] {
+					isPrefixed = false
+					break
+				}
+			}
+			if isPrefixed {
+				return errors.Errorf("mount paths %q and %q collide. A volume mount cannot include another one.", *vm1.MountPath, *vm2.MountPath)
+			}
+		}
+	}
+	return nil
+}

aci/convert/secrets_test.go

@@ -0,0 +1,180 @@
+/*
+   Copyright 2020 Docker Compose CLI authors
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package convert
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path"
+	"testing"
+
+	"github.com/compose-spec/compose-go/types"
+	"gotest.tools/v3/assert"
+)
+
+
+func TestConvertSecrets(t *testing.T) {
+	serviceName := "testservice"
+	secretName := "testsecret"
+	absBasePath := "/home/user"
+	tmpFile, err := ioutil.TempFile(os.TempDir(), "TestConvertProjectSecrets-")
+	assert.NilError(t, err)
+	_, err = tmpFile.Write([]byte("test content"))
+	assert.NilError(t, err)
+	t.Cleanup(func() {
+		_ = os.Remove(tmpFile.Name())
+	})
+
+	t.Run("mix default and absolute", func(t *testing.T) {
+		pSquashedDefaultAndAbs := projectAciHelper{
+			Services: []types.ServiceConfig{
+				{
+					Name: serviceName,
+					Secrets: []types.ServiceSecretConfig{
+						{
+							Source: secretName,
+							Target: "some_target1",
+						},
+						{
+							Source: secretName,
+						},
+						{
+							Source: secretName,
+							Target: path.Join(defaultSecretsPath, "some_target2"),
+						},
+						{
+							Source: secretName,
+							Target: path.Join(absBasePath, "some_target3"),
+						},
+						{
+							Source: secretName,
+							Target: path.Join(absBasePath, "some_target4"),
+						},
+					},
+				},
+			},
+			Secrets: map[string]types.SecretConfig{
+				secretName: {
+					File: tmpFile.Name(),
+				},
+			},
+		}
+		volumes, err := pSquashedDefaultAndAbs.getAciSecretVolumes()
+		assert.NilError(t, err)
+		assert.Equal(t, len(volumes), 2)
+
+		defaultVolumeName := getServiceSecretKey(serviceName, defaultSecretsPath)
+		homeVolumeName := getServiceSecretKey(serviceName, absBasePath)
+		// random order since this was created from a map...
+		for _, vol := range volumes {
+			switch *vol.Name {
+			case defaultVolumeName:
+				assert.Equal(t, len(vol.Secret), 3)
+			case homeVolumeName:
+				assert.Equal(t, len(vol.Secret), 2)
+			default:
+				assert.Assert(t, false, "unexpected volume name: "+*vol.Name)
+			}
+		}
+
+		s := serviceConfigAciHelper(pSquashedDefaultAndAbs.Services[0])
+		vms, err := s.getAciSecretsVolumeMounts()
+		assert.NilError(t, err)
+		assert.Equal(t, len(vms), 2)
+
+		assert.Equal(t, *vms[0].Name, defaultVolumeName)
+		assert.Equal(t, *vms[0].MountPath, defaultSecretsPath)
+
+		assert.Equal(t, *vms[1].Name, homeVolumeName)
+		assert.Equal(t, *vms[1].MountPath, absBasePath)
+	})
+
+	t.Run("convert invalid target", func(t *testing.T) {
+		targetName := "some/invalid/relative/path/target"
+		pInvalidRelativePathTarget := projectAciHelper{
+			Services: []types.ServiceConfig{
+				{
+					Name: serviceName,
+					Secrets: []types.ServiceSecretConfig{
+						{
+							Source: secretName,
+							Target: targetName,
+						},
+					},
+				},
+			},
+			Secrets: map[string]types.SecretConfig{
+				secretName: {
+					File: tmpFile.Name(),
+				},
+			},
+		}
+		_, err := pInvalidRelativePathTarget.getAciSecretVolumes()
+		assert.Equal(t, err.Error(),
+			fmt.Sprintf(`in service %q, secret with source %q cannot have a relative path as target. Only absolute paths are allowed. Found %q`,
+				serviceName, secretName, targetName))
+	})
+
+	t.Run("convert colliding default targets", func(t *testing.T) {
+		targetName1 := path.Join(defaultSecretsPath, "target1")
+		targetName2 := path.Join(defaultSecretsPath, "sub/folder/target2")
+
+		service := serviceConfigAciHelper{
+			Name: serviceName,
+			Secrets: []types.ServiceSecretConfig{
+				{
+					Source: secretName,
+					Target: targetName1,
+				},
+				{
+					Source: secretName,
+					Target: targetName2,
+				},
+			},
+		}
+
+		_, err := service.getAciSecretsVolumeMounts()
+		assert.Equal(t, err.Error(),
+			fmt.Sprintf(`mount paths %q and %q collide. A volume mount cannot include another one.`,
+				path.Dir(targetName1), path.Dir(targetName2)))
+	})
+
+	t.Run("convert colliding absolute targets", func(t *testing.T) {
+		targetName1 := path.Join(absBasePath, "target1")
+		targetName2 := path.Join(absBasePath, "sub/folder/target2")
+
+		service := serviceConfigAciHelper{
+			Name: serviceName,
+			Secrets: []types.ServiceSecretConfig{
+				{
+					Source: secretName,
+					Target: targetName1,
+				},
+				{
+					Source: secretName,
+					Target: targetName2,
+				},
+			},
+		}
+
+		_, err := service.getAciSecretsVolumeMounts()
+		assert.Equal(t, err.Error(),
+			fmt.Sprintf(`mount paths %q and %q collide. A volume mount cannot include another one.`,
+				path.Dir(targetName1), path.Dir(targetName2)))
+	})
+}