Add support for arbitrary writable volume mounts during pack build

acceptance/acceptance_test.go

@@ -884,55 +884,90 @@ func testAcceptance(
 					})
 
 					when("--volume", func() {
-						var buildpackTgz, tempVolume string
+						var (
+							readBuildpackTgz,
+							readWriteBuildpackTgz,
+							tmpVolumeSrc string
+						)
 
 						it.Before(func() {
 							h.SkipUnless(t,
-								pack.SupportsFeature(invoke.CustomVolumeMounts),
-								"pack 0.11.0 shipped with a volume mounting bug",
+								pack.SupportsFeature(invoke.ReadWriteVolumeMounts),
+								"pack version does not support read/write volume mounts",
 							)
 
-							buildpackTgz = h.CreateTGZ(t, filepath.Join(bpDir, "volume-buildpack"), "./", 0755)
+							readBuildpackTgz = h.CreateTGZ(t, filepath.Join(bpDir, "read-volume-buildpack"), "./", 0755)
+							readWriteBuildpackTgz = h.CreateTGZ(t, filepath.Join(bpDir, "read-write-volume-buildpack"), "./", 0755)
 
 							var err error
-							tempVolume, err = ioutil.TempDir("", "my-volume-mount-source")
+							tmpVolumeSrc, err = ioutil.TempDir("", "volume-mount-source")
 							assert.Nil(err)
-							assert.Nil(os.Chmod(tempVolume, 0755)) // Override umask
+							assert.Nil(os.Chmod(tmpVolumeSrc, 0777)) // Override umask
 
 							// Some OSes (like macOS) use symlinks for the standard temp dir.
 							// Resolve it so it can be properly mounted by the Docker daemon.
-							tempVolume, err = filepath.EvalSymlinks(tempVolume)
+							tmpVolumeSrc, err = filepath.EvalSymlinks(tmpVolumeSrc)
 							assert.Nil(err)
 
-							err = ioutil.WriteFile(filepath.Join(tempVolume, "some-file"), []byte("some-string\n"), 0755)
+							err = ioutil.WriteFile(filepath.Join(tmpVolumeSrc, "some-file"), []byte("some-content\n"), 0777)
 							assert.Nil(err)
 						})
 
 						it.After(func() {
-							_ = os.Remove(buildpackTgz)
+							_ = os.Remove(readBuildpackTgz)
+							_ = os.Remove(readWriteBuildpackTgz)
 							_ = h.DockerRmi(dockerCli, repoName)
-
-							_ = os.RemoveAll(tempVolume)
+							_ = os.RemoveAll(tmpVolumeSrc)
 						})
 
-						it("mounts the provided volume in the detect and build phases", func() {
-							output := pack.RunSuccessfully(
-								"build", repoName,
-								"-p", filepath.Join("testdata", "mock_app"),
-								"--buildpack", buildpackTgz,
-								"--volume", fmt.Sprintf("%s:%s", tempVolume, "/my-volume-mount-target"),
-							)
+						when("volume is read-only", func() {
+							it("mounts the provided volume in the detect and build phases", func() {
+								output := pack.RunSuccessfully(
+									"build", repoName,
+									"-p", filepath.Join("testdata", "mock_app"),
+									"--volume", fmt.Sprintf("%s:/platform/volume-mount-target", tmpVolumeSrc),
+									"--buildpack", readBuildpackTgz,
+									"--env", "TEST_FILE_PATH=/platform/volume-mount-target/some-file",
+								)
 
-							expectedPhase := "Build"
-							if pack.SupportsFeature(invoke.ReadFromVolumeInDetect) {
-								expectedPhase = "Detect"
-							}
+								bpOutputAsserts := assertions.NewTestBuildpackOutputAssertionManager(t, output)
+								bpOutputAsserts.ReportsReadingFileContents("Detect", "/platform/volume-mount-target/some-file", "some-content")
+								bpOutputAsserts.ReportsReadingFileContents("Build", "/platform/volume-mount-target/some-file", "some-content")
+							})
 
-							assertions.NewTestBuildpackOutputAssertionManager(t, output).ReportsReadingFileContents(
-								"/my-volume-mount-target/some-file",
-								"some-string",
-								expectedPhase,
-							)
+							it("should fail to write", func() {
+								output := pack.RunSuccessfully(
+									"build", repoName,
+									"-p", filepath.Join("testdata", "mock_app"),
+									"--volume", fmt.Sprintf("%s:/platform/volume-mount-target", tmpVolumeSrc),
+									"--buildpack", readWriteBuildpackTgz,
+									"--env", "DETECT_TEST_FILE_PATH=/platform/volume-mount-target/detect-file",
+									"--env", "BUILD_TEST_FILE_PATH=/platform/volume-mount-target/build-file",
+								)
+
+								bpOutputAsserts := assertions.NewTestBuildpackOutputAssertionManager(t, output)
+								bpOutputAsserts.ReportsFailingToWriteFileContents("Detect", "/platform/volume-mount-target/detect-file")
+								bpOutputAsserts.ReportsFailingToWriteFileContents("Build", "/platform/volume-mount-target/build-file")
+							})
+						})
+
+						when("volume is read-write", func() {
+							it("can be written to", func() {
+								output := pack.RunSuccessfully(
+									"build", repoName,
+									"-p", filepath.Join("testdata", "mock_app"),
+									"--volume", fmt.Sprintf("%s:/volume-mount-target:rw", tmpVolumeSrc),
+									"--buildpack", readWriteBuildpackTgz,
+									"--env", "DETECT_TEST_FILE_PATH=/volume-mount-target/detect-file",
+									"--env", "BUILD_TEST_FILE_PATH=/volume-mount-target/build-file",
+								)
+
+								bpOutputAsserts := assertions.NewTestBuildpackOutputAssertionManager(t, output)
+								bpOutputAsserts.ReportsWritingFileContents("Detect", "/volume-mount-target/detect-file")
+								bpOutputAsserts.ReportsReadingFileContents("Detect", "/volume-mount-target/detect-file", "some-content")
+								bpOutputAsserts.ReportsWritingFileContents("Build", "/volume-mount-target/build-file")
+								bpOutputAsserts.ReportsReadingFileContents("Build", "/volume-mount-target/build-file", "some-content")
+							})
 						})
 					})
 
@@ -1189,9 +1224,7 @@ func testAcceptance(
 
 					when("--env", func() {
 						it.Before(func() {
-							h.AssertNil(t,
-								os.Setenv("ENV2_CONTENTS", "Env2 Layer Contents From Environment"),
-							)
+							assert.Nil(os.Setenv("ENV2_CONTENTS", "Env2 Layer Contents From Environment"))
 						})
 
 						it.After(func() {

acceptance/assertions/test_buildpack_output.go

@@ -22,10 +22,22 @@ func NewTestBuildpackOutputAssertionManager(t *testing.T, output string) TestBui
 	}
 }
 
-func (t TestBuildpackOutputAssertionManager) ReportsReadingFileContents(path, expectedContent, phase string) {
+func (t TestBuildpackOutputAssertionManager) ReportsReadingFileContents(phase, path, content string) {
 	t.testObject.Helper()
 
-	t.assert.ContainsF(t.output, "%s: Reading file '/platform%s': %s", phase, path, expectedContent)
+	t.assert.ContainsF(t.output, "%s: Reading file '%s': %s", phase, path, content)
+}
+
+func (t TestBuildpackOutputAssertionManager) ReportsWritingFileContents(phase, path string) {
+	t.testObject.Helper()
+
+	t.assert.ContainsF(t.output, "%s: Writing file '%s': written", phase, path)
+}
+
+func (t TestBuildpackOutputAssertionManager) ReportsFailingToWriteFileContents(phase, path string) {
+	t.testObject.Helper()
+
+	t.assert.ContainsF(t.output, "%s: Writing file '%s': failed", phase, path)
 }
 
 func (t TestBuildpackOutputAssertionManager) ReportsConnectedToInternet() {

acceptance/invoke/pack.go

@@ -212,29 +212,25 @@ const (
 	BuilderTomlValidation Feature = iota
 	ExcludeAndIncludeDescriptor
 	CreatorInPack
-	CustomVolumeMounts
+	ReadWriteVolumeMounts
 	NoColorInBuildpacks
-	ReadFromVolumeInDetect
 )
 
-var featureTests = map[Feature]func(e *PackInvoker) bool{
-	BuilderTomlValidation: func(e *PackInvoker) bool {
-		return e.laterThan090()
+var featureTests = map[Feature]func(i *PackInvoker) bool{
+	BuilderTomlValidation: func(i *PackInvoker) bool {
+		return i.laterThan("0.9.0")
 	},
-	ExcludeAndIncludeDescriptor: func(e *PackInvoker) bool {
-		return e.laterThan090()
+	ExcludeAndIncludeDescriptor: func(i *PackInvoker) bool {
+		return i.laterThan("0.9.0")
 	},
-	CreatorInPack: func(e *PackInvoker) bool {
-		return e.laterThan0_10_0()
+	CreatorInPack: func(i *PackInvoker) bool {
+		return i.atLeast("0.10.0")
 	},
-	CustomVolumeMounts: func(e *PackInvoker) bool {
-		return e.not0_11_0()
+	ReadWriteVolumeMounts: func(i *PackInvoker) bool {
+		return i.laterThan("0.12.0")
 	},
-	NoColorInBuildpacks: func(e *PackInvoker) bool {
-		return e.atLeast0_12_0()
-	},
-	ReadFromVolumeInDetect: func(e *PackInvoker) bool {
-		return e.laterThan090()
+	NoColorInBuildpacks: func(i *PackInvoker) bool {
+		return i.atLeast("0.12.0")
 	},
 }
 
@@ -250,25 +246,18 @@ func (i *PackInvoker) semanticVersion() *semver.Version {
 	return semanticVersion
 }
 
-func (i *PackInvoker) laterThan090() bool {
-	ver := i.semanticVersion()
-	return ver.Compare(semver.MustParse("0.9.0")) > 0 || ver.Equal(semver.MustParse("0.0.0"))
-}
-
-func (i *PackInvoker) laterThan0_10_0() bool {
-	ver := i.semanticVersion()
-	return ver.GreaterThan(semver.MustParse("0.10.0")) || ver.Equal(semver.MustParse("0.0.0"))
-}
-
-func (i *PackInvoker) not0_11_0() bool {
+// laterThan returns true if pack version is older than the provided version
+func (i *PackInvoker) laterThan(version string) bool {
+	providedVersion := semver.MustParse(version)
 	ver := i.semanticVersion()
-	return !ver.Equal(semver.MustParse("0.11.0"))
+	return ver.Compare(providedVersion) > 0 || ver.Equal(semver.MustParse("0.0.0"))
 }
 
-func (i *PackInvoker) atLeast0_12_0() bool {
+// atLeast returns true if pack version is the same or older than the provided version
+func (i *PackInvoker) atLeast(version string) bool {
+	minimalVersion := semver.MustParse(version)
 	ver := i.semanticVersion()
-	minimumVersion := semver.MustParse("0.12.0")
-	return ver.Equal(minimumVersion) || ver.GreaterThan(minimumVersion) || ver.Equal(semver.MustParse("0.0.0"))
+	return ver.Equal(minimalVersion) || ver.GreaterThan(minimalVersion) || ver.Equal(semver.MustParse("0.0.0"))
 }
 
 func (i *PackInvoker) ConfigFileContents() string {

acceptance/testdata/mock_buildpacks/0.2/read-volume-buildpack/bin/build

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+
+TEST_FILE_PATH=${TEST_FILE_PATH:?"env var must be set"}
+
+echo "---> Build: Volume Buildpack"
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+echo "Build: Reading file '${TEST_FILE_PATH}': $(< "${TEST_FILE_PATH}")"
+
+echo "---> Done"

acceptance/testdata/mock_buildpacks/0.2/read-volume-buildpack/bin/detect

@@ -0,0 +1,13 @@
+#!/usr/bin/env bash
+
+TEST_FILE_PATH=${TEST_FILE_PATH:?"env var must be set"}
+
+echo "---> Detect: Volume Buildpack"
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+echo "Detect: Reading file '${TEST_FILE_PATH}': $(< "${TEST_FILE_PATH}")"
+
+echo "---> Done"

acceptance/testdata/mock_buildpacks/0.2/read-write-volume-buildpack/bin/build

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+
+TEST_FILE_PATH=${BUILD_TEST_FILE_PATH:?"env var must be set"}
+
+echo "---> Build: Read/Write Volume Buildpack"
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+echo "Build: Writing file '${TEST_FILE_PATH}': $(echo "some-content" > "${TEST_FILE_PATH}" && echo "written" || echo "failed")"
+echo "Build: Reading file '${TEST_FILE_PATH}': $(< "${TEST_FILE_PATH}")"
+
+echo "---> Done"

acceptance/testdata/mock_buildpacks/0.2/read-write-volume-buildpack/bin/detect

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+
+TEST_FILE_PATH=${DETECT_TEST_FILE_PATH:?"env var must be set"}
+
+echo "---> Detect: Read/Write Volume Buildpack"
+
+set -o errexit
+set -o nounset
+set -o pipefail
+
+echo "Detect: Writing file '${TEST_FILE_PATH}': $(echo "some-content" > "${TEST_FILE_PATH}" && echo "written" || echo "failed")"
+echo "Detect: Reading file '${TEST_FILE_PATH}': $(< "${TEST_FILE_PATH}")"
+
+echo "---> Done"

acceptance/testdata/mock_buildpacks/0.2/read-write-volume-buildpack/buildpack.toml

@@ -0,0 +1,9 @@
+api = "0.2"
+
+[buildpack]
+  id = "rw-volume/bp"
+  version = "rw-volume-bp-version"
+  name = "Read/Write Volume Buildpack"
+
+[[stacks]]
+  id = "pack.test.stack"

build.go

@@ -6,7 +6,6 @@ import (
 	"math/rand"
 	"net/url"
 	"os"
-	"path"
 	"path/filepath"
 	"runtime"
 	"sort"
@@ -142,11 +141,15 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
 		return errors.Errorf("Builder %s is incompatible with this version of pack", style.Symbol(opts.Builder))
 	}
 
-	platformVolumes, err := buildPlatformVolumes(opts.ContainerConfig.Volumes)
+	processedVolumes, warnings, err := processVolumes(opts.ContainerConfig.Volumes)
 	if err != nil {
 		return err
 	}
 
+	for _, warning := range warnings {
+		c.logger.Warn(warning)
+	}
+
 	lifecycleOpts := build.LifecycleOptions{
 		AppPath:            appPath,
 		Image:              imageRef,
@@ -161,7 +164,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
 		HTTPSProxy:         proxyConfig.HTTPSProxy,
 		NoProxy:            proxyConfig.NoProxy,
 		Network:            opts.ContainerConfig.Network,
-		Volumes:            platformVolumes,
+		Volumes:            processedVolumes,
 		DefaultProcessType: opts.DefaultProcessType,
 		FileFilter:         opts.FileFilter,
 	}
@@ -625,19 +628,30 @@ func randString(n int) string {
 	return string(b)
 }
 
-func buildPlatformVolumes(volumes []string) ([]string, error) {
-	platformVolumes := make([]string, len(volumes))
+func processVolumes(volumes []string) (processed []string, warnings []string, err error) {
 	// Assume a linux container
 	parser := mounts.NewParser(mounts.OSLinux)
-	for i, v := range volumes {
+	for _, v := range volumes {
 		volume, err := parser.ParseMountRaw(v, "")
 		if err != nil {
-			return nil, errors.Wrapf(err, "Platform volume %q has invalid format", v)
+			return nil, nil, errors.Wrapf(err, "platform volume %q has invalid format", v)
+		}
+
+		for _, p := range [...]string{"/cnb", "/layers"} {
+			if strings.HasPrefix(volume.Spec.Target, p) {
+				warnings = append(warnings, fmt.Sprintf("Mounting to a sensitive directory %s", style.Symbol(volume.Spec.Target)))
+			}
 		}
 
-		// Use path.Join instead of filepath.Join because we assume the container OS is linux but the host may be windows
-		dest := path.Join("/platform", volume.Destination)
-		platformVolumes[i] = fmt.Sprintf("%v:%v:ro", volume.Spec.Source, dest)
+		processed = append(processed, fmt.Sprintf("%s:%s:%s", volume.Spec.Source, volume.Spec.Target, processMode(volume.Mode)))
 	}
-	return platformVolumes, nil
+	return processed, warnings, nil
+}
+
+func processMode(mode string) string {
+	if mode == "" {
+		return "ro"
+	}
+
+	return mode
 }