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

Relocation the right way

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
+}

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())
 }

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 {

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
 	}

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
+	}
+}

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 {

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

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)
 	}
 

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)
 		}

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.

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)

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
+}

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 {