diff --git a/aci/convert/convert.go b/aci/convert/convert.go index 1119ae8b2..9afabb10a 100644 --- a/aci/convert/convert.go +++ b/aci/convert/convert.go @@ -23,6 +23,7 @@ import ( "io/ioutil" "math" "os" + "path/filepath" "strconv" "strings" @@ -50,7 +51,8 @@ const ( volumeDriveroptsAccountNameKey = "storage_account_name" volumeReadOnly = "read_only" - serviceSecretPrefix = "aci-service-secret-" + defaultSecretsPath = "/run/secrets" + serviceSecretAbsPathPrefix = "aci-service-secret-path-" ) // ToContainerGroup converts a compose project into a ACI container group @@ -188,13 +190,15 @@ 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 { - secretServiceVolume := containerinstance.Volume{ - Name: to.StringPtr(serviceSecretPrefix + svc.Name), - Secret: make(map[string]*string), - } + squashedTargetVolumes := make(map[string]containerinstance.Volume) for _, scr := range svc.Secrets { data, err := ioutil.ReadFile(p.Secrets[scr.Source].File) if err != nil { @@ -207,14 +211,31 @@ func (p projectAciHelper) getAciSecretVolumes() ([]containerinstance.Volume, err if scr.Target == "" { scr.Target = scr.Source } - if strings.ContainsAny(scr.Target, "\\/") { + + if !filepath.IsAbs(scr.Target) && strings.ContainsAny(scr.Target, "\\/") { return []containerinstance.Volume{}, - errors.Errorf("in service %q, secret with source %q cannot have a path as target. Found %q", svc.Name, scr.Source, scr.Target) + 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) } - secretServiceVolume.Secret[scr.Target] = &dataStr + + if !filepath.IsAbs(scr.Target) { + scr.Target = filepath.Join(defaultSecretsPath, scr.Target) + } + + targetDir := filepath.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[filepath.Base(scr.Target)] = &dataStr } - if len(secretServiceVolume.Secret) > 0 { - secretVolumes = append(secretVolumes, secretServiceVolume) + for _, v := range squashedTargetVolumes { + secretVolumes = append(secretVolumes, v) } } @@ -326,15 +347,62 @@ func (s serviceConfigAciHelper) getAciFileVolumeMounts(volumesCache map[string]b return aciServiceVolumes, nil } -func (s serviceConfigAciHelper) getAciSecretsVolumeMount() *containerinstance.VolumeMount { - if len(s.Secrets) == 0 { - return 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 !filepath.IsAbs(scr.Target) { + scr.Target = filepath.Join(defaultSecretsPath, scr.Target) + } + + presenceKey := filepath.Dir(scr.Target) + if !presenceSet[presenceKey] { + vms = append(vms, containerinstance.VolumeMount{ + Name: to.StringPtr(getServiceSecretKey(s.Name, filepath.Dir(scr.Target))), + MountPath: to.StringPtr(filepath.Dir(scr.Target)), + ReadOnly: to.BoolPtr(true), + }) + presenceSet[presenceKey] = true + } } - return &containerinstance.VolumeMount{ - Name: to.StringPtr(serviceSecretPrefix + s.Name), - MountPath: to.StringPtr("/run/secrets"), - ReadOnly: to.BoolPtr(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, string(filepath.Separator)) + smallerVMPath = strings.Split(*vm2.MountPath, string(filepath.Separator)) + ) + 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) { @@ -342,11 +410,11 @@ func (s serviceConfigAciHelper) getAciContainer(volumesCache map[string]bool) (c if err != nil { return containerinstance.Container{}, err } - allVolumes := aciServiceVolumes - secretVolumeMount := s.getAciSecretsVolumeMount() - if secretVolumeMount != nil { - allVolumes = append(allVolumes, *secretVolumeMount) + serviceSecretVolumes, err := s.getAciSecretsVolumeMounts() + if err != nil { + return containerinstance.Container{}, err } + allVolumes := append(aciServiceVolumes, serviceSecretVolumes...) var volumes *[]containerinstance.VolumeMount if len(allVolumes) > 0 { volumes = &allVolumes diff --git a/aci/convert/convert_test.go b/aci/convert/convert_test.go index c2beecb5b..77f8cde9e 100644 --- a/aci/convert/convert_test.go +++ b/aci/convert/convert_test.go @@ -18,7 +18,10 @@ package convert import ( "context" + "fmt" + "io/ioutil" "os" + "path/filepath" "testing" "github.com/stretchr/testify/mock" @@ -712,6 +715,151 @@ 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() { + assert.NilError(t, 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: filepath.Join(defaultSecretsPath, "some_target2"), + }, + { + Source: secretName, + Target: filepath.Join(absBasePath, "some_target3"), + }, + { + Source: secretName, + Target: filepath.Join(absBasePath, "some_target4"), + }, + }, + }, + }, + Secrets: map[string]types.SecretConfig{ + secretName: { + File: tmpFile.Name(), + }, + }, + } + vs, err := pSquashedDefaultAndAbs.getAciSecretVolumes() + assert.NilError(t, err) + assert.Equal(t, len(vs), 2) + + defaultVolumeName := getServiceSecretKey(serviceName, defaultSecretsPath) + assert.Equal(t, *vs[0].Name, defaultVolumeName) + assert.Equal(t, len(vs[0].Secret), 3) + + homeVolumeName := getServiceSecretKey(serviceName, absBasePath) + assert.Equal(t, *vs[1].Name, homeVolumeName) + assert.Equal(t, len(vs[1].Secret), 2) + + 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 := filepath.Join(defaultSecretsPath, "target1") + targetName2 := filepath.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.`, + filepath.Dir(targetName1), filepath.Dir(targetName2))) + }) + + t.Run("convert colliding absolute targets", func(t *testing.T) { + targetName1 := filepath.Join(absBasePath, "target1") + targetName2 := filepath.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.`, + filepath.Dir(targetName1), filepath.Dir(targetName2))) + }) +} + func container(status *string) containerinstance.Container { var state *containerinstance.ContainerState = nil if status != nil { diff --git a/tests/aci-e2e/e2e-aci_test.go b/tests/aci-e2e/e2e-aci_test.go index ddb491057..9e77e2575 100644 --- a/tests/aci-e2e/e2e-aci_test.go +++ b/tests/aci-e2e/e2e-aci_test.go @@ -564,19 +564,12 @@ func TestUpSecrets(t *testing.T) { secret2Value = "another_password\n" ) var ( - basefilePath = filepath.Join("..", "composefiles", composeProjectName) - composefilePath = filepath.Join(basefilePath, "compose.yml") - composefileInvalidTargetPath = filepath.Join(basefilePath, "compose-invalid-target.yml") + basefilePath = filepath.Join("..", "composefiles", composeProjectName) + composefilePath = filepath.Join(basefilePath, "compose.yml") ) c := NewParallelE2eCLI(t, binDir) _, _, _ = setupTestResourceGroup(t, c) - t.Run("compose up invalid target", func(t *testing.T) { - res := c.RunDockerOrExitError("compose", "up", "-f", composefileInvalidTargetPath, "--project-name", composeProjectName) - assert.Equal(t, res.ExitCode, 1) - assert.Equal(t, res.Combined(), "in service \"web\", secret with source \"mysecret1\" cannot have a path as target. Found \"my/invalid/target1\"\n") - }) - t.Run("compose up", func(t *testing.T) { c.RunDockerCmd("compose", "up", "-f", composefilePath, "--project-name", composeProjectName) res := c.RunDockerCmd("ps") diff --git a/tests/composefiles/aci_secrets/compose-invalid-target.yml b/tests/composefiles/aci_secrets/compose-invalid-target.yml deleted file mode 100644 index cd67b1269..000000000 --- a/tests/composefiles/aci_secrets/compose-invalid-target.yml +++ /dev/null @@ -1,16 +0,0 @@ -services: - web: - build: . - image: ulyssessouza/secrets_server - ports: - - "80:80" - secrets: - - source: mysecret1 - target: my/invalid/target1 - - mysecret2 - -secrets: - mysecret1: - file: ./my_secret1.txt - mysecret2: - file: ./my_secret2.txt