Merge pull request #765 from eunomie/relocation-the-right-way

silvin-lubecki · web-flow · commit dc6d86d1163f · 2019-11-28T14:42:32.000+01:00
Relocation the right way
diff --git a/cmd/cnab-run/bundle.go b/cmd/cnab-run/bundle.go
@@ -0,0 +1,40 @@
+package main
+
+import (
+	"github.com/deislabs/cnab-go/bundle"
+	"github.com/docker/app/internal/relocated"
+	"github.com/docker/cnab-to-oci/relocation"
+)
+
+const (
+	// bundlePath is where the CNAB runtime will put the actual Bundle definition
+	bundlePath = "/cnab/bundle.json"
+	// relocationMapPath is where the CNAB runtime will put the relocation map
+	// See https://github.com/cnabio/cnab-spec/blob/master/103-bundle-runtime.md#image-relocation
+	relocationMapPath = "/cnab/app/relocation-mapping.json"
+)
+
+func getBundle() (*bundle.Bundle, error) {
+	return relocated.BundleJSON(bundlePath)
+}
+
+func getRelocationMap() (relocation.ImageRelocationMap, error) {
+	return relocated.RelocationMapJSON(relocationMapPath)
+}
+
+func getRelocatedBundle() (*relocated.Bundle, error) {
+	bndl, err := getBundle()
+	if err != nil {
+		return nil, err
+	}
+
+	relocationMap, err := getRelocationMap()
+	if err != nil {
+		return nil, err
+	}
+
+	return &relocated.Bundle{
+		Bundle:        bndl,
+		RelocationMap: relocationMap,
+	}, nil
+}
diff --git a/cmd/cnab-run/inspect.go b/cmd/cnab-run/inspect.go
@@ -15,11 +15,11 @@ func inspectAction(instanceName string) error {
 	}
 	defer app.Cleanup()
 
-	imageMap, err := getBundleImageMap()
+	bndl, err := getRelocatedBundle()
 	if err != nil {
 		return err
 	}
 
 	parameters := packager.ExtractCNABParametersValues(packager.ExtractCNABParameterMapping(app.Parameters()), os.Environ())
-	return appinspect.ImageInspect(os.Stdout, app, parameters, imageMap)
+	return appinspect.ImageInspect(os.Stdout, app, parameters, bndl.RelocatedImages())
 }
diff --git a/cmd/cnab-run/install.go b/cmd/cnab-run/install.go
@@ -6,7 +6,6 @@ import (
 	"os"
 	"strconv"
 
-	"github.com/deislabs/cnab-go/bundle"
 	"github.com/docker/app/internal"
 	"github.com/docker/app/internal/packager"
 	"github.com/docker/app/render"
@@ -19,12 +18,6 @@ import (
 	"github.com/spf13/pflag"
 )
 
-const (
-	// imageMapFilePath is the path where the CNAB runtime will put the actual
-	// service to image mapping to use
-	imageMapFilePath = "/cnab/app/image-map.json"
-)
-
 func installAction(instanceName string) error {
 	cli, err := setupDockerContext()
 	if err != nil {
@@ -42,12 +35,12 @@ func installAction(instanceName string) error {
 	if err != nil {
 		return err
 	}
-	imageMap, err := getBundleImageMap()
+	bndl, err := getRelocatedBundle()
 	if err != nil {
 		return err
 	}
 	parameters := packager.ExtractCNABParametersValues(packager.ExtractCNABParameterMapping(app.Parameters()), os.Environ())
-	rendered, err := render.Render(app, parameters, imageMap)
+	rendered, err := render.Render(app, parameters, bndl.RelocatedImages())
 	if err != nil {
 		return err
 	}
@@ -79,18 +72,6 @@ func getFlagset(orchestrator command.Orchestrator) *pflag.FlagSet {
 	return result
 }
 
-func getBundleImageMap() (map[string]bundle.Image, error) {
-	mapJSON, err := ioutil.ReadFile(imageMapFilePath)
-	if err != nil {
-		return nil, err
-	}
-	var result map[string]bundle.Image
-	if err := json.Unmarshal(mapJSON, &result); err != nil {
-		return nil, err
-	}
-	return result, nil
-}
-
 func addLabels(rendered *composetypes.Config) error {
 	args, err := ioutil.ReadFile(internal.DockerArgsPath)
 	if err != nil {
diff --git a/cmd/cnab-run/render.go b/cmd/cnab-run/render.go
@@ -19,7 +19,7 @@ func renderAction(instanceName string) error {
 	}
 	defer app.Cleanup()
 
-	imageMap, err := getBundleImageMap()
+	bndl, err := getRelocatedBundle()
 	if err != nil {
 		return err
 	}
@@ -31,7 +31,7 @@ func renderAction(instanceName string) error {
 
 	parameters := packager.ExtractCNABParametersValues(packager.ExtractCNABParameterMapping(app.Parameters()), os.Environ())
 
-	rendered, err := render.Render(app, parameters, imageMap)
+	rendered, err := render.Render(app, parameters, bndl.RelocatedImages())
 	if err != nil {
 		return err
 	}
diff --git a/internal/cnab/driver.go b/internal/cnab/driver.go
@@ -2,20 +2,23 @@ package cnab
 
 import (
 	"bytes"
+	"encoding/json"
 	"io"
 	"os"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"github.com/docker/app/internal/cliopts"
-	store2 "github.com/docker/app/internal/store"
+	"github.com/docker/app/internal/store"
 
 	"github.com/deislabs/cnab-go/claim"
 	"github.com/deislabs/cnab-go/driver"
 	dockerDriver "github.com/deislabs/cnab-go/driver/docker"
 	"github.com/docker/app/internal"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/context/docker"
-	"github.com/docker/cli/cli/context/store"
+	cliContext "github.com/docker/cli/cli/context/store"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/mount"
 )
@@ -39,7 +42,7 @@ func RequiredClaimBindMount(c claim.Claim, dockerCli command.Cli) (BindMount, er
 
 // RequiredBindMount Returns the path required to bind mount when running
 // the invocation image.
-func RequiredBindMount(targetContextName string, targetOrchestrator string, s store.Store) (BindMount, error) {
+func RequiredBindMount(targetContextName string, targetOrchestrator string, s cliContext.Store) (BindMount, error) {
 	if targetOrchestrator == "kubernetes" {
 		return BindMount{}, nil
 	}
@@ -119,7 +122,7 @@ func prepareDriver(dockerCli command.Cli, bindMount BindMount, stdout io.Writer)
 	return d, errBuf
 }
 
-func SetupDriver(installation *store2.Installation, dockerCli command.Cli, opts *cliopts.InstallerContextOptions, stdout io.Writer) (driver.Driver, *bytes.Buffer, error) {
+func SetupDriver(installation *store.Installation, dockerCli command.Cli, opts *cliopts.InstallerContextOptions, stdout io.Writer) (driver.Driver, *bytes.Buffer, error) {
 	dockerCli, err := opts.SetInstallerContext(dockerCli)
 	if err != nil {
 		return nil, nil, err
@@ -131,3 +134,31 @@ func SetupDriver(installation *store2.Installation, dockerCli command.Cli, opts
 	driverImpl, errBuf := prepareDriver(dockerCli, bind, stdout)
 	return driverImpl, errBuf, nil
 }
+
+func WithRelocationMap(installation *store.Installation) func(op *driver.Operation) error {
+	return func(op *driver.Operation) error {
+		if err := addRelocationMapToFiles(op, installation); err != nil {
+			return err
+		}
+		relocateInvocationImage(op, installation)
+		return nil
+	}
+}
+
+func addRelocationMapToFiles(op *driver.Operation, installation *store.Installation) error {
+	data, err := json.Marshal(installation.RelocationMap)
+	if err != nil {
+		return errors.Wrap(err, "could not marshal relocation map")
+	}
+	op.Files["/cnab/app/relocation-mapping.json"] = string(data)
+
+	return nil
+}
+
+func relocateInvocationImage(op *driver.Operation, installation *store.Installation) {
+	invocImage := op.Image
+	if relocatedImage, ok := installation.RelocationMap[invocImage.Image]; ok {
+		invocImage.Image = relocatedImage
+		op.Image = invocImage
+	}
+}
diff --git a/internal/commands/image/inspect.go b/internal/commands/image/inspect.go
@@ -88,7 +88,7 @@ func runInspect(dockerCli command.Cli, appname string, opts inspectOptions, inst
 		}
 
 		installation.SetParameter(internal.ParameterInspectFormatName, format)
-		if err = a.Run(&installation.Claim, nil); err != nil {
+		if err = a.Run(&installation.Claim, nil, cnab.WithRelocationMap(installation)); err != nil {
 			return fmt.Errorf("inspect failed: %s\n%s", err, errBuf)
 		}
 	} else {
diff --git a/internal/commands/image/render.go b/internal/commands/image/render.go
@@ -70,7 +70,7 @@ func runRender(dockerCli command.Cli, appname string, opts renderOptions, instal
 	}
 	installation.Parameters[internal.ParameterRenderFormatName] = opts.formatDriver
 
-	if err := action.Run(&installation.Claim, nil, cfgFunc); err != nil {
+	if err := action.Run(&installation.Claim, nil, cfgFunc, cnab.WithRelocationMap(installation)); err != nil {
 		return fmt.Errorf("render failed: %s\n%s", err, errBuf)
 	}
 	return nil
diff --git a/internal/commands/inspect.go b/internal/commands/inspect.go
@@ -73,7 +73,7 @@ func runInspect(dockerCli command.Cli, appName string, inspectOptions inspectOpt
 	} else {
 		return fmt.Errorf("inspect failed: status action is not supported by the App")
 	}
-	if err := a.Run(&installation.Claim, creds); err != nil {
+	if err := a.Run(&installation.Claim, creds, cnab.WithRelocationMap(installation)); err != nil {
 		return fmt.Errorf("inspect failed: %s\n%s", err, errBuf)
 	}
 
diff --git a/internal/commands/remove.go b/internal/commands/remove.go
@@ -82,7 +82,7 @@ func runRemove(dockerCli command.Cli, installationName string, opts removeOption
 		op.Out = dockerCli.Out()
 		return nil
 	}
-	if err := uninst.Run(&installation.Claim, creds, cfgFunc); err != nil {
+	if err := uninst.Run(&installation.Claim, creds, cfgFunc, cnab.WithRelocationMap(installation)); err != nil {
 		if err2 := installationStore.Store(installation); err2 != nil {
 			return fmt.Errorf("%s while %s", err2, errBuf)
 		}
diff --git a/internal/commands/run.go b/internal/commands/run.go
@@ -158,7 +158,7 @@ func runBundle(dockerCli command.Cli, bndl *relocated.Bundle, opts runOptions, i
 			op.Out = dockerCli.Out()
 			return nil
 		}
-		err = inst.Run(&installation.Claim, creds, cfgFunc)
+		err = inst.Run(&installation.Claim, creds, cfgFunc, cnab.WithRelocationMap(installation))
 	}
 	// Even if the installation failed, the installation is persisted with its failure status,
 	// so any installation needs a clean uninstallation.
diff --git a/internal/commands/update.go b/internal/commands/update.go
@@ -89,7 +89,7 @@ func runUpdate(dockerCli command.Cli, installationName string, opts updateOption
 		op.Out = dockerCli.Out()
 		return nil
 	}
-	err = u.Run(&installation.Claim, creds, cfgFunc)
+	err = u.Run(&installation.Claim, creds, cfgFunc, cnab.WithRelocationMap(installation))
 	err2 := installationStore.Store(installation)
 	if err != nil {
 		return fmt.Errorf("Update failed: %s\n%s", err, errBuf)
diff --git a/internal/relocated/bundle.go b/internal/relocated/bundle.go
@@ -32,13 +32,13 @@ func FromBundle(bndl *bundle.Bundle) *Bundle {
 
 // BundleFromFile creates a relocated bundle based on the bundle file and relocation map.
 func BundleFromFile(filename string) (*Bundle, error) {
-	bndl, err := bundleJSON(filename)
+	bndl, err := BundleJSON(filename)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to read bundle")
 	}
 
 	relocationMapFileName := filepath.Join(filepath.Dir(filename), RelocationMapFilename)
-	relocationMap, err := relocationMapJSON(relocationMapFileName)
+	relocationMap, err := RelocationMapJSON(relocationMapFileName)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to read relocation map")
 	}
@@ -75,7 +75,7 @@ func (b *Bundle) Store(dir string) error {
 	return nil
 }
 
-func bundleJSON(bundlePath string) (*bundle.Bundle, error) {
+func BundleJSON(bundlePath string) (*bundle.Bundle, error) {
 	data, err := ioutil.ReadFile(bundlePath)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to read file %s", bundlePath)
@@ -87,10 +87,11 @@ func bundleJSON(bundlePath string) (*bundle.Bundle, error) {
 	return bndl, nil
 }
 
-func relocationMapJSON(relocationMapPath string) (relocation.ImageRelocationMap, error) {
+func RelocationMapJSON(relocationMapPath string) (relocation.ImageRelocationMap, error) {
 	relocationMap := relocation.ImageRelocationMap{}
 	_, err := os.Stat(relocationMapPath)
 	if os.IsNotExist(err) {
+		// it's ok to not have a relocation map, just act as if the file were empty
 		return relocationMap, nil
 	}
 	data, err := ioutil.ReadFile(relocationMapPath)
@@ -102,3 +103,15 @@ func relocationMapJSON(relocationMapPath string) (relocation.ImageRelocationMap,
 	}
 	return relocationMap, nil
 }
+
+func (b *Bundle) RelocatedImages() map[string]bundle.Image {
+	images := b.Images
+	for name, def := range images {
+		if img, ok := b.RelocationMap[def.Image]; ok {
+			def.Image = img
+			images[name] = def
+		}
+	}
+
+	return images
+}
diff --git a/internal/store/installation.go b/internal/store/installation.go
@@ -39,7 +39,6 @@ func NewInstallation(name string, reference string, bndl *relocated.Bundle) (*In
 		Reference:     reference,
 		RelocationMap: bndl.RelocationMap,
 	}
-	i.applyRelocationMap()
 
 	return i, nil
 }
@@ -52,21 +51,6 @@ func (i Installation) SetParameter(name string, value string) {
 	}
 }
 
-func (i *Installation) applyRelocationMap() {
-	for idx, def := range i.Bundle.InvocationImages {
-		if img, ok := i.RelocationMap[def.Image]; ok {
-			def.Image = img
-			i.Bundle.InvocationImages[idx] = def
-		}
-	}
-	for name, def := range i.Bundle.Images {
-		if img, ok := i.RelocationMap[def.Image]; ok {
-			def.Image = img
-			i.Bundle.Images[name] = def
-		}
-	}
-}
-
 var _ InstallationStore = &installationStore{}
 
 type installationStore struct {
diff --git a/internal/store/installation_test.go b/internal/store/installation_test.go

Original file line number	Diff line number	Diff line change
`@@ -15,11 +15,11 @@ func inspectAction(instanceName string) error {`
`15`	`15`	`}`
`16`	`16`	`defer app.Cleanup()`
`17`	`17`
`18`		`- imageMap, err := getBundleImageMap()`
	`18`	`+ bndl, err := getRelocatedBundle()`
`19`	`19`	`if err != nil {`
`20`	`20`	`return err`
`21`	`21`	`}`
`22`	`22`
`23`	`23`	`parameters := packager.ExtractCNABParametersValues(packager.ExtractCNABParameterMapping(app.Parameters()), os.Environ())`
`24`		`- return appinspect.ImageInspect(os.Stdout, app, parameters, imageMap)`
	`24`	`+ return appinspect.ImageInspect(os.Stdout, app, parameters, bndl.RelocatedImages())`
`25`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ func renderAction(instanceName string) error {`
`19`	`19`	`}`
`20`	`20`	`defer app.Cleanup()`
`21`	`21`
`22`		`- imageMap, err := getBundleImageMap()`
	`22`	`+ bndl, err := getRelocatedBundle()`
`23`	`23`	`if err != nil {`
`24`	`24`	`return err`
`25`	`25`	`}`
`@@ -31,7 +31,7 @@ func renderAction(instanceName string) error {`
`31`	`31`
`32`	`32`	`parameters := packager.ExtractCNABParametersValues(packager.ExtractCNABParameterMapping(app.Parameters()), os.Environ())`
`33`	`33`
`34`		`- rendered, err := render.Render(app, parameters, imageMap)`
	`34`	`+ rendered, err := render.Render(app, parameters, bndl.RelocatedImages())`
`35`	`35`	`if err != nil {`
`36`	`36`	`return err`
`37`	`37`	`}`
Original file line number	Diff line number	Diff line change
`@@ -88,7 +88,7 @@ func runInspect(dockerCli command.Cli, appname string, opts inspectOptions, inst`
`88`	`88`	`}`
`89`	`89`
`90`	`90`	`installation.SetParameter(internal.ParameterInspectFormatName, format)`
`91`		`- if err = a.Run(&installation.Claim, nil); err != nil {`
	`91`	`+ if err = a.Run(&installation.Claim, nil, cnab.WithRelocationMap(installation)); err != nil {`
`92`	`92`	`return fmt.Errorf("inspect failed: %s\n%s", err, errBuf)`
`93`	`93`	`}`
`94`	`94`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ func runRender(dockerCli command.Cli, appname string, opts renderOptions, instal`
`70`	`70`	`}`
`71`	`71`	`installation.Parameters[internal.ParameterRenderFormatName] = opts.formatDriver`
`72`	`72`
`73`		`- if err := action.Run(&installation.Claim, nil, cfgFunc); err != nil {`
	`73`	`+ if err := action.Run(&installation.Claim, nil, cfgFunc, cnab.WithRelocationMap(installation)); err != nil {`
`74`	`74`	`return fmt.Errorf("render failed: %s\n%s", err, errBuf)`
`75`	`75`	`}`
`76`	`76`	`return nil`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ func runInspect(dockerCli command.Cli, appName string, inspectOptions inspectOpt`
`73`	`73`	`} else {`
`74`	`74`	`return fmt.Errorf("inspect failed: status action is not supported by the App")`
`75`	`75`	`}`
`76`		`- if err := a.Run(&installation.Claim, creds); err != nil {`
	`76`	`+ if err := a.Run(&installation.Claim, creds, cnab.WithRelocationMap(installation)); err != nil {`
`77`	`77`	`return fmt.Errorf("inspect failed: %s\n%s", err, errBuf)`
`78`	`78`	`}`
`79`	`79`
Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ func runRemove(dockerCli command.Cli, installationName string, opts removeOption`
`82`	`82`	`op.Out = dockerCli.Out()`
`83`	`83`	`return nil`
`84`	`84`	`}`
`85`		`- if err := uninst.Run(&installation.Claim, creds, cfgFunc); err != nil {`
	`85`	`+ if err := uninst.Run(&installation.Claim, creds, cfgFunc, cnab.WithRelocationMap(installation)); err != nil {`
`86`	`86`	`if err2 := installationStore.Store(installation); err2 != nil {`
`87`	`87`	`return fmt.Errorf("%s while %s", err2, errBuf)`
`88`	`88`	`}`
Original file line number	Diff line number	Diff line change
`@@ -158,7 +158,7 @@ func runBundle(dockerCli command.Cli, bndl *relocated.Bundle, opts runOptions, i`
`158`	`158`	`op.Out = dockerCli.Out()`
`159`	`159`	`return nil`
`160`	`160`	`}`
`161`		`- err = inst.Run(&installation.Claim, creds, cfgFunc)`
	`161`	`+ err = inst.Run(&installation.Claim, creds, cfgFunc, cnab.WithRelocationMap(installation))`
`162`	`162`	`}`
`163`	`163`	`// Even if the installation failed, the installation is persisted with its failure status,`
`164`	`164`	`// so any installation needs a clean uninstallation.`