Merge pull request #392 from mikeparker/bundle-external-files

silvin-lubecki · web-flow · commit 0ec4226f0131 · 2018-09-26T15:35:34.000+02:00
Add attachments (extra files) into the package
diff --git a/e2e/commands_test.go b/e2e/commands_test.go
@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/docker/app/internal"
@@ -260,8 +261,8 @@ func TestSplitMerge(t *testing.T) {
 	result = icmd.RunCommand(dockerApp, "inspect", "remerged").Assert(t, icmd.Success)
 	assert.Assert(t, golden.String(result.Combined(), "envvariables-inspect.golden"))
 	// test inplace
-	icmd.RunCommand(dockerApp, "merge", "split")
-	icmd.RunCommand(dockerApp, "split", "split")
+	icmd.RunCommand(dockerApp, "merge", "split").Assert(t, icmd.Success)
+	icmd.RunCommand(dockerApp, "split", "split").Assert(t, icmd.Success)
 }
 
 func TestURL(t *testing.T) {
@@ -316,3 +317,38 @@ func testFork(registry string) func(*testing.T) {
 		assert.Assert(t, golden.Bytes(metadata2, "expected-fork-metadata-no-rename.golden"))
 	}
 }
+
+func TestAttachmentsWithRegistry(t *testing.T) {
+	r := startRegistry(t)
+	defer r.Stop(t)
+	registry := r.GetAddress(t)
+
+	dir := fs.NewDir(t, "testattachments",
+		fs.WithDir("attachments.dockerapp", fs.FromDir("testdata/attachments.dockerapp")),
+	)
+	defer dir.Remove()
+
+	icmd.RunCommand(dockerApp, "push", "--namespace", registry+"/acmecorp", dir.Join("attachments.dockerapp")).Assert(t, icmd.Success)
+
+	// inspect will run the core pull code too
+	result := icmd.RunCommand(dockerApp, "inspect", registry+"/acmecorp/attachments.dockerapp:0.1.0")
+
+	result.Assert(t, icmd.Success)
+	resultOutput := result.Combined()
+
+	assert.Assert(t, strings.Contains(resultOutput, "config.cfg"))
+	assert.Assert(t, strings.Contains(resultOutput, "nesteddir/config2.cfg"))
+	assert.Assert(t, strings.Contains(resultOutput, "nesteddir/nested2/nested3/config3.cfg"))
+
+	// Test forking with external files
+	tempDir := fs.NewDir(t, "dockerapptest")
+	defer tempDir.Remove()
+
+	icmd.RunCommand(dockerApp, "fork", registry+"/acmecorp/attachments.dockerapp:0.1.0",
+		"-p", tempDir.Path()).Assert(t, icmd.Success)
+	externalFile := golden.Get(t, tempDir.Join("attachments.dockerapp", "config.cfg"))
+	assert.Assert(t, golden.Bytes(externalFile, filepath.Join("attachments.dockerapp", "config.cfg")))
+
+	nestedAttachment := golden.Get(t, tempDir.Join("attachments.dockerapp", "nesteddir", "config2.cfg"))
+	assert.Assert(t, golden.Bytes(nestedAttachment, filepath.Join("attachments.dockerapp", "nesteddir", "config2.cfg")))
+}
diff --git a/e2e/testdata/attachments.dockerapp/config.cfg b/e2e/testdata/attachments.dockerapp/config.cfg
@@ -0,0 +1 @@
+Contents of config.cfg
diff --git a/e2e/testdata/attachments.dockerapp/docker-compose.yml b/e2e/testdata/attachments.dockerapp/docker-compose.yml
@@ -0,0 +1,29 @@
+version: "3.7"
+services:
+  front:
+    image: nginx:${myapp.nginx_version}
+    deploy:
+      replicas: ${myapp.nginx_replicas}
+  debug:
+    image: busybox:latest
+    ports:
+    - $aport
+    - $sport:$dport
+    privileged: ${privileged}
+    read_only: $read_only
+    tty: $tty
+    stdin_open: $stdin_open
+    deploy:
+      resources:
+        limits:
+          memory: $memory
+    healthcheck:
+      test: ["/ping", "debug"]
+      interval: 2m
+      timeout: $timeout
+  monitor:
+    image: busybox:latest
+    command: monitor --source ${app.name}-${app.version} $$dollar
+    x-enabled: "! ${myapp.debug}"
+  app-watcher:
+    image: $watcher.image
diff --git a/e2e/testdata/attachments.dockerapp/metadata.yml b/e2e/testdata/attachments.dockerapp/metadata.yml
@@ -0,0 +1,8 @@
+version: 0.1.0
+name: myapp
+description: ""
+maintainers:
+  - name: bearclaw
+    email: bearclaw@bearclaw.com
+  - name: bob
+    email: bob@bob.com
diff --git a/e2e/testdata/attachments.dockerapp/nesteddir/config2.cfg b/e2e/testdata/attachments.dockerapp/nesteddir/config2.cfg
@@ -0,0 +1 @@
+Contents of nesteddir/config2.cfg
diff --git a/e2e/testdata/attachments.dockerapp/nesteddir/nested2/nested3/config3.cfg b/e2e/testdata/attachments.dockerapp/nesteddir/nested2/nested3/config3.cfg
@@ -0,0 +1 @@
+Contents of nesteddir/nested2/nested3/config3.cfg
diff --git a/e2e/testdata/attachments.dockerapp/settings.yml b/e2e/testdata/attachments.dockerapp/settings.yml
@@ -0,0 +1,15 @@
+privileged: false
+myapp:
+  debug: false
+  nginx_version: latest
+  nginx_replicas: 2
+watcher:
+  image: watcher:latest
+read_only: true
+stdin_open: true
+tty: true
+aport: 10000
+sport: 11000
+dport: 12000
+memory: '100000000'
+timeout: 10s
diff --git a/internal/inspect/inspect.go b/internal/inspect/inspect.go
@@ -11,6 +11,7 @@ import (
 	"github.com/docker/app/types"
 	"github.com/docker/app/types/settings"
 	composetypes "github.com/docker/cli/cli/compose/types"
+	units "github.com/docker/go-units"
 )
 
 // Inspect dumps the metadata of an app
@@ -65,6 +66,15 @@ func Inspect(out io.Writer, app *types.App, argSettings map[string]string) error
 		}
 	}, "Setting", "Value")
 
+	// Add Attachments section
+	attachments := app.Attachments()
+	printSection(out, len(attachments), func(w io.Writer) {
+		for _, file := range attachments {
+			sizeString := units.HumanSize(float64(file.Size()))
+			fmt.Fprintf(w, "%s\t%s\n", file.Path(), sizeString)
+		}
+	}, "Attachment", "Size")
+
 	return nil
 }
 
diff --git a/internal/inspect/inspect_test.go b/internal/inspect/inspect_test.go
@@ -91,6 +91,7 @@ description: "this is sparta !"`),
 			fs.WithFile(internal.SettingsFileName, `
 port: 8080
 text: hello`),
+			fs.WithFile("config.cfg", "something"),
 		),
 	)
 	defer dir.Remove()
diff --git a/internal/inspect/testdata/inspect-full.golden b/internal/inspect/testdata/inspect-full.golden
@@ -24,3 +24,7 @@ Settings (2) Value
 ------------ -----
 port         8080
 text         hello
+
+Attachment (1) Size
+-------------- ----
+config.cfg     9B
diff --git a/internal/packager/fork.go b/internal/packager/fork.go
@@ -32,38 +32,35 @@ func Fork(originName, forkName, outputDir string, maintainers []string) error {
 
 	// create app dir in output-dir
 	namespace, name := splitPackageName(forkName)
-	appPath := filepath.Join(outputDir, internal.DirNameFromAppName(name))
-	if err := os.MkdirAll(appPath, 0755); err != nil {
+	appDir := filepath.Join(outputDir, internal.DirNameFromAppName(name))
+	if err := os.MkdirAll(appDir, 0755); err != nil {
+		return errors.Wrap(err, "failed to create output application directory")
+	}
+
+	// Create all the files on disk
+	if err := ExtractImagePayloadToDiskFiles(appDir, payload); err != nil {
 		return err
 	}
 
-	// iterate on contents
-	for k, vs := range payload {
-		v := []byte(vs)
-		if strings.Contains(k, "/") || strings.Contains(k, "\\") {
-			log.Infof("dropping payload element with unexpected path separator: %s", k)
-			continue
-		}
-		if k == internal.MetadataFileName {
-			log.Debug("Loading app metadata")
-			v, err = updateMetadata(v, namespace, name, maintainers)
-			if err != nil {
-				return err
-			}
-		}
-		dest := filepath.Join(appPath, k)
-		log.Debugf("Writing file at %s", dest)
-		if err := ioutil.WriteFile(dest, v, 0644); err != nil {
-			return errors.Wrap(err, "error writing output file")
-		}
+	// Update the metadata file
+	fullFilepath := filepath.Join(appDir, internal.MetadataFileName)
+	bytes, err := ioutil.ReadFile(fullFilepath)
+	if err != nil {
+		return errors.Wrapf(err, "failed to read metadata file from: %s", fullFilepath)
+	}
+	bytes, err = updateMetadata(bytes, namespace, name, maintainers)
+	if err != nil {
+		return err
+	}
+	if err := ioutil.WriteFile(fullFilepath, bytes, 0644); err != nil {
+		return errors.Wrapf(err, "failed to write metadata file: %s", fullFilepath)
 	}
 
 	return nil
 }
 
 func updateMetadata(raw []byte, namespace, name string, maintainers []string) ([]byte, error) {
 	// retrieve original metadata (maintainer/app name/app tag)
-	var yamlMeta []byte
 	meta, err := loadMetadata(raw)
 	if err != nil {
 		return nil, err
@@ -78,7 +75,7 @@ func updateMetadata(raw []byte, namespace, name string, maintainers []string) ([
 	)
 
 	// update metadata file
-	yamlMeta, err = yaml.Marshal(newMeta)
+	yamlMeta, err := yaml.Marshal(newMeta)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to render metadata structure")
 	}
diff --git a/internal/packager/registry.go b/internal/packager/registry.go
@@ -38,39 +38,63 @@ func splitImageName(repotag string) (*imageComponents, error) {
 
 // Pull loads an app from a registry and returns the extracted dir name
 func Pull(repotag string, outputDir string) (string, error) {
-	payload, err := resto.PullConfigMulti(context.Background(), repotag, resto.RegistryOptions{})
+	imgRef, err := splitImageName(repotag)
 	if err != nil {
-		return "", err
+		return "", errors.Wrapf(err, "origin %q is not a valid image name", repotag)
 	}
-	repoComps, err := splitImageName(repotag)
+	payload, err := resto.PullConfigMulti(context.Background(), repotag, resto.RegistryOptions{})
 	if err != nil {
 		return "", err
 	}
-	appDir := filepath.Join(outputDir, internal.DirNameFromAppName(repoComps.Name))
-	err = os.Mkdir(appDir, 0755)
-	if err != nil {
+	appDir := filepath.Join(outputDir, internal.DirNameFromAppName(imgRef.Name))
+	if err := os.Mkdir(appDir, 0755); err != nil {
 		return "", errors.Wrap(err, "failed to create output application directory")
 	}
-	for k, v := range payload {
-		// do not write files in any other directory
-		if strings.Contains(k, "/") || strings.Contains(k, "\\") {
-			log.Warnf("dropping image entry '%s' with unexpected path separator", k)
+	if err := ExtractImagePayloadToDiskFiles(appDir, payload); err != nil {
+		return "", err
+	}
+	return appDir, nil
+}
+
+// ExtractImagePayloadToDiskFiles extracts all the files out of the image payload and onto disk
+// creating all necessary folders in between.
+func ExtractImagePayloadToDiskFiles(appDir string, payload map[string]string) error {
+	for localFilepath, filedata := range payload {
+		fileBytes := []byte(filedata)
+		// Deal with windows/linux slashes
+		convertedFilepath := filepath.FromSlash(localFilepath)
+
+		// Check we aren't doing ./../../../ etc in the path
+		fullFilepath := filepath.Join(appDir, convertedFilepath)
+		if _, err := filepath.Rel(appDir, fullFilepath); err != nil {
+			log.Warnf("dropping image entry %q with unexpected path outside of app dir", localFilepath)
 			continue
 		}
-		target := filepath.Join(appDir, k)
-		if err := ioutil.WriteFile(target, []byte(v), 0644); err != nil {
-			return "", errors.Wrap(err, "failed to write output file")
+
+		// Create the directories for any nested files
+		basepath := filepath.Dir(fullFilepath)
+		if err := os.MkdirAll(basepath, os.ModePerm); err != nil {
+			return errors.Wrapf(err, "failed to create directories for file: %s", fullFilepath)
+		}
+		if err := ioutil.WriteFile(fullFilepath, fileBytes, 0644); err != nil {
+			return errors.Wrapf(err, "failed to write output file: %s", fullFilepath)
 		}
 	}
-	return appDir, nil
+
+	return nil
 }
 
 // Push pushes an app to a registry. Returns the image digest.
 func Push(app *types.App, namespace, tag, repo string) (string, error) {
-	payload := make(map[string]string)
-	payload[internal.MetadataFileName] = string(app.MetadataRaw())
-	payload[internal.ComposeFileName] = string(app.Composes()[0])
-	payload[internal.SettingsFileName] = string(app.SettingsRaw()[0])
+	payload, err := createPayload(app)
+	if err != nil {
+		return "", errors.Wrap(err, "failed to read external file while creating payload for push")
+	}
+	imageName := createImageName(app, namespace, tag, repo)
+	return resto.PushConfigMulti(context.Background(), payload, imageName, resto.RegistryOptions{}, nil)
+}
+
+func createImageName(app *types.App, namespace, tag, repo string) string {
 	if namespace == "" || tag == "" {
 		metadata := app.Metadata()
 		if namespace == "" {
@@ -86,6 +110,39 @@ func Push(app *types.App, namespace, tag, repo string) (string, error) {
 	if namespace != "" && namespace[len(namespace)-1] != '/' {
 		namespace += "/"
 	}
-	imageName := namespace + repo + ":" + tag
-	return resto.PushConfigMulti(context.Background(), payload, imageName, resto.RegistryOptions{}, nil)
+	return namespace + repo + ":" + tag
+}
+
+func createPayload(app *types.App) (map[string]string, error) {
+	payload := map[string]string{
+		internal.MetadataFileName: string(app.MetadataRaw()),
+		internal.ComposeFileName:  string(app.Composes()[0]),
+		internal.SettingsFileName: string(app.SettingsRaw()[0]),
+	}
+	if err := readAttachments(payload, app.Path, app.Attachments()); err != nil {
+		return nil, err
+	}
+	return payload, nil
+}
+
+func readAttachments(payload map[string]string, parentDirPath string, files []types.Attachment) error {
+	var errs []string
+	for _, file := range files {
+		// Convert to local OS filepath slash syntax
+		fullFilePath := filepath.Join(parentDirPath, filepath.FromSlash(file.Path()))
+		filedata, err := ioutil.ReadFile(fullFilePath)
+		if err != nil {
+			errs = append(errs, err.Error())
+			continue
+		}
+		payload[file.Path()] = string(filedata)
+	}
+	return newErrGroup(errs)
+}
+
+func newErrGroup(errs []string) error {
+	if len(errs) == 0 {
+		return nil
+	}
+	return errors.New(strings.Join(errs, "\n"))
 }
diff --git a/internal/packager/registry_test.go b/internal/packager/registry_test.go
diff --git a/loader/loader.go b/loader/loader.go
diff --git a/types/types.go b/types/types.go
diff --git a/types/types_test.go b/types/types_test.go

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Contents of nesteddir/nested2/nested3/config3.cfg`
Original file line number	Diff line number	Diff line change
@@ -91,6 +91,7 @@ description: "this is sparta !"`),
`91`	`91`	fs.WithFile(internal.SettingsFileName, `
`92`	`92`	`port: 8080`
`93`	`93`	text: hello`),
	`94`	`+ fs.WithFile("config.cfg", "something"),`
`94`	`95`	`),`
`95`	`96`	`)`
`96`	`97`	`defer dir.Remove()`