Merge pull request #757 from docker/secrets-refactor

Squash all secrets in a single one

aci/convert/convert.go

@@ -49,6 +49,8 @@ const (
 	volumeDriveroptsShareNameKey   = "share_name"
 	volumeDriveroptsAccountNameKey = "storage_account_name"
 	volumeReadOnly                 = "read_only"
+
+	serviceSecretPrefix = "aci-service-secret-"
 )
 
 // ToContainerGroup converts a compose project into a ACI container group
@@ -188,22 +190,34 @@ type projectAciHelper types.Project
 
 func (p projectAciHelper) getAciSecretVolumes() ([]containerinstance.Volume, error) {
 	var secretVolumes []containerinstance.Volume
-	for secretName, filepathToRead := range p.Secrets {
-		data, err := ioutil.ReadFile(filepathToRead.File)
-		if err != nil {
-			return secretVolumes, err
+	for _, svc := range p.Services {
+		secretServiceVolume := containerinstance.Volume{
+			Name:   to.StringPtr(serviceSecretPrefix + svc.Name),
+			Secret: make(map[string]*string),
 		}
-		if len(data) == 0 {
-			continue
+		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 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)
+			}
+			secretServiceVolume.Secret[scr.Target] = &dataStr
+		}
+		if len(secretServiceVolume.Secret) > 0 {
+			secretVolumes = append(secretVolumes, secretServiceVolume)
 		}
-		dataStr := base64.StdEncoding.EncodeToString(data)
-		secretVolumes = append(secretVolumes, containerinstance.Volume{
-			Name: to.StringPtr(secretName),
-			Secret: map[string]*string{
-				secretName: &dataStr,
-			},
-		})
 	}
+
 	return secretVolumes, nil
 }
 
@@ -312,37 +326,29 @@ func (s serviceConfigAciHelper) getAciFileVolumeMounts(volumesCache map[string]b
 	return aciServiceVolumes, nil
 }
 
-func (s serviceConfigAciHelper) getAciSecretsVolumeMounts() []containerinstance.VolumeMount {
-	var secretVolumeMounts []containerinstance.VolumeMount
-	for _, secret := range s.Secrets {
-		secretsMountPath := "/run/secrets"
-		if secret.Target == "" {
-			secret.Target = secret.Source
-		}
-		// Specifically use "/" here and not filepath.Join() to avoid windows path being sent and used inside containers
-		secretsMountPath = secretsMountPath + "/" + secret.Target
-		vmName := strings.Split(secret.Source, "=")[0]
-		vm := containerinstance.VolumeMount{
-			Name:      to.StringPtr(vmName),
-			MountPath: to.StringPtr(secretsMountPath),
-			ReadOnly:  to.BoolPtr(true), // TODO Confirm if the secrets are read only
-		}
-		secretVolumeMounts = append(secretVolumeMounts, vm)
+func (s serviceConfigAciHelper) getAciSecretsVolumeMount() *containerinstance.VolumeMount {
+	if len(s.Secrets) == 0 {
+		return nil
+	}
+	return &containerinstance.VolumeMount{
+		Name:      to.StringPtr(serviceSecretPrefix + s.Name),
+		MountPath: to.StringPtr("/run/secrets"),
+		ReadOnly:  to.BoolPtr(true),
 	}
-	return secretVolumeMounts
 }
 
 func (s serviceConfigAciHelper) getAciContainer(volumesCache map[string]bool) (containerinstance.Container, error) {
-	secretVolumeMounts := s.getAciSecretsVolumeMounts()
 	aciServiceVolumes, err := s.getAciFileVolumeMounts(volumesCache)
 	if err != nil {
 		return containerinstance.Container{}, err
 	}
-	allVolumes := append(aciServiceVolumes, secretVolumeMounts...)
+	allVolumes := aciServiceVolumes
+	secretVolumeMount := s.getAciSecretsVolumeMount()
+	if secretVolumeMount != nil {
+		allVolumes = append(allVolumes, *secretVolumeMount)
+	}
 	var volumes *[]containerinstance.VolumeMount
-	if len(allVolumes) == 0 {
-		volumes = nil
-	} else {
+	if len(allVolumes) > 0 {
 		volumes = &allVolumes
 	}
 

docs/aci-compose-features.md

@@ -121,9 +121,8 @@ Credentials for storage accounts will be automatically fetched at deployment tim
 ## Secrets
 
 Secrets can be defined in compose files, and will need secret files available at deploy time next to the compose file. 
-The content of the secret file will be made available inside selected containers, under `/run/secrets/<SECRET_NAME>/<SECRET_NAME>
+The content of the secret file will be made available inside selected containers, under `/run/secrets/<SECRET_NAME>`.
 External secrets are not supported with the ACI integration.
-Due to ACI secret volume mounting, each secret file is mounted in its own folder named after the secret.
 
 ```yaml
 services:
@@ -145,6 +144,8 @@ secrets:
 
 The nginx container will have secret1 mounted as `/run/secrets/mysecret1/mysecret1`, the db container will have secret2 mounted as `/run/secrets/mysecret1/mysecret2`
 
+**Note that file paths are not allowed in the target**
+
 ## Container Resources
 
 CPU and memory reservations and limits can be set in compose.

tests/aci-e2e/e2e-aci_test.go

@@ -515,6 +515,68 @@ func TestUpResources(t *testing.T) {
 	})
 }
 
+func TestUpSecrets(t *testing.T) {
+	const (
+		composeProjectName = "aci_secrets"
+		serverContainer    = composeProjectName + "_web"
+
+		secret1Name  = "mytarget1"
+		secret1Value = "myPassword1\n"
+
+		secret2Name  = "mysecret2"
+		secret2Value = "another_password\n"
+	)
+	var (
+		basefilePath                 = filepath.Join("..", "composefiles", composeProjectName)
+		composefilePath              = filepath.Join(basefilePath, "compose.yml")
+		composefileInvalidTargetPath = filepath.Join(basefilePath, "compose-invalid-target.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")
+		out := lines(res.Stdout())
+		// Check one container running
+		assert.Assert(t, is.Len(out, 2))
+		webRunning := false
+		for _, l := range out {
+			if strings.Contains(l, serverContainer) {
+				webRunning = true
+				strings.Contains(l, ":80->80/tcp")
+			}
+		}
+		assert.Assert(t, webRunning, "web container not running ; ps:\n"+res.Stdout())
+
+		res = c.RunDockerCmd("inspect", serverContainer)
+
+		containerInspect, err := ParseContainerInspect(res.Stdout())
+		assert.NilError(t, err)
+		assert.Assert(t, is.Len(containerInspect.Ports, 1))
+		endpoint := fmt.Sprintf("http://%s:%d", containerInspect.Ports[0].HostIP, containerInspect.Ports[0].HostPort)
+
+		output := HTTPGetWithRetry(t, endpoint+"/"+secret1Name, http.StatusOK, 2*time.Second, 20*time.Second)
+		assert.Equal(t, output, secret1Value)
+
+		output = HTTPGetWithRetry(t, endpoint+"/"+secret2Name, http.StatusOK, 2*time.Second, 20*time.Second)
+		assert.Equal(t, output, secret2Value)
+
+		t.Cleanup(func() {
+			c.RunDockerCmd("compose", "down", "--project-name", composeProjectName)
+			res := c.RunDockerCmd("ps")
+			out := lines(res.Stdout())
+			assert.Equal(t, len(out), 1)
+		})
+	})
+}
+
 func TestUpUpdate(t *testing.T) {
 	const (
 		composeProjectName = "acidemo"

tests/composefiles/aci_secrets/Dockerfile

@@ -0,0 +1,20 @@
+#   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.
+
+FROM python:3.8
+WORKDIR /run/secrets
+
+EXPOSE 80
+ENTRYPOINT ["python"]
+CMD ["-m", "http.server", "80"]

tests/composefiles/aci_secrets/compose-invalid-target.yml

@@ -0,0 +1,16 @@
+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

tests/composefiles/aci_secrets/compose.yml

@@ -0,0 +1,16 @@
+services:
+  web:
+    build: .
+    image: ulyssessouza/secrets_server
+    ports:
+      - "80:80"
+    secrets:
+      - source: mysecret1
+        target: mytarget1
+      - mysecret2
+
+secrets:
+  mysecret1:
+    file: ./my_secret1.txt
+  mysecret2:
+    file: ./my_secret2.txt

tests/composefiles/aci_secrets/my_secret1.txt

@@ -0,0 +1 @@
+myPassword1

tests/composefiles/aci_secrets/my_secret2.txt

@@ -0,0 +1 @@
+another_password