Update relocation map during porter publish

pkg/porter/publish.go

@@ -54,7 +54,7 @@ func (o *PublishOptions) Validate(cxt *portercontext.Context) error {
 		}
 
 		if o.File == "" {
-			return errors.New("could not find porter.yaml in the current directory, make sure you are in the right directory or specify the porter manifest with --file")
+			return fmt.Errorf("could not find porter.yaml in the current directory %s, make sure you are in the right directory or specify the porter manifest with --file", o.Dir)
 		}
 	}
 
@@ -197,14 +197,9 @@ func (p *Porter) publishFromFile(ctx context.Context, opts PublishOptions) error
 // OCI Layout, rename each based on the registry/org values derived from the provided tag
 // and then push each updated image with the original digests
 //
-// Finally, we generate a new bundle from the old, with all image names and digests updated, based
-// on the newly copied images, and then push this new bundle using the provided tag.
+// Finally, we update the relocation map in the original bundle, based
+// on the newly copied images, and then push the bundle using the provided tag.
 // (Currently we use the docker/cnab-to-oci library for this logic.)
-//
-// In the generation of a new bundle, we therefore don't preserve content digests and can't maintain
-// signature verification throughout the process.  Once we wish to preserve content digest and such verification,
-// this approach will need to be refactored, via preserving the original bundle and employing
-// a relocation mapping approach to associate the bundle's (old) images with the newly copied images.
 func (p *Porter) publishFromArchive(ctx context.Context, opts PublishOptions) error {
 	ctx, log := tracing.StartSpan(ctx)
 	defer log.EndSpan()
@@ -235,45 +230,21 @@ func (p *Porter) publishFromArchive(ctx context.Context, opts PublishOptions) er
 
 	// Push updated images (renamed based on provided bundle tag) with same digests
 	// then update the bundle with new values (image name, digest)
-	for i, invImg := range bundleRef.Definition.InvocationImages {
-		newImgName, err := getNewImageNameFromBundleReference(invImg.Image, opts.Reference)
+	for _, invImg := range bundleRef.Definition.InvocationImages {
+		relocMap, err := p.updateRelocationMapping(bundleRef.RelocationMap, layout, invImg.Image, opts.Reference)
 		if err != nil {
 			return err
 		}
 
-		origImg := invImg.Image
-		if relocatedImage, ok := bundleRef.RelocationMap[invImg.Image]; ok {
-			origImg = relocatedImage
-		}
-		digest, err := pushUpdatedImage(layout, origImg, newImgName)
-		if err != nil {
-			return err
-		}
-
-		err = p.updateBundleWithNewImage(bundleRef.Definition, newImgName, digest, i)
-		if err != nil {
-			return err
-		}
+		bundleRef.RelocationMap = relocMap
 	}
-	for name, img := range bundleRef.Definition.Images {
-		newImgName, err := getNewImageNameFromBundleReference(img.Image, opts.Reference)
+	for _, img := range bundleRef.Definition.Images {
+		relocMap, err := p.updateRelocationMapping(bundleRef.RelocationMap, layout, img.Image, opts.Reference)
 		if err != nil {
 			return err
 		}
 
-		origImg := img.Image
-		if relocatedImage, ok := bundleRef.RelocationMap[img.Image]; ok {
-			origImg = relocatedImage
-		}
-		digest, err := pushUpdatedImage(layout, origImg, newImgName)
-		if err != nil {
-			return err
-		}
-
-		err = p.updateBundleWithNewImage(bundleRef.Definition, newImgName, digest, name)
-		if err != nil {
-			return err
-		}
+		bundleRef.RelocationMap = relocMap
 	}
 
 	bundleRef, err = p.Registry.PushBundle(ctx, bundleRef, opts.InsecureRegistry)
@@ -337,36 +308,6 @@ func pushUpdatedImage(layout registry.Layout, origImg string, newImgName image.N
 	return digest, nil
 }
 
-// updateBundleWithNewImage updates a bundle with a new image (with digest) at the provided index
-func (p *Porter) updateBundleWithNewImage(bun cnab.ExtendedBundle, newImg image.Name, digest image.Digest, index interface{}) error {
-	taggedImage, err := p.rewriteImageWithDigest(newImg.String(), digest.String())
-	if err != nil {
-		return fmt.Errorf("unable to update image reference for %s: %w", newImg.String(), err)
-	}
-
-	// update bundle with new image
-	switch v := index.(type) {
-	case int: // invocation images is a slice, indexed by an integer
-		i := index.(int)
-		origImg := bun.InvocationImages[i]
-		updatedImg := origImg.DeepCopy()
-		updatedImg.Image = taggedImage
-		updatedImg.Digest = digest.String()
-		bun.InvocationImages[i] = *updatedImg
-	case string: // images is a map, indexed by a string
-		i := index.(string)
-		origImg := bun.Images[i]
-		updatedImg := origImg.DeepCopy()
-		updatedImg.Image = taggedImage
-		updatedImg.Digest = digest.String()
-		bun.Images[i] = *updatedImg
-	default:
-		return fmt.Errorf("unknown image index type: %v", v)
-	}
-
-	return nil
-}
-
 // getNewImageNameFromBundleReference derives a new image.Name object from the provided original
 // image (string) using the provided bundleTag to clean registry/org/etc.
 func getNewImageNameFromBundleReference(origImg, bundleTag string) (image.Name, error) {
@@ -416,6 +357,31 @@ func (p *Porter) rewriteBundleWithInvocationImageDigest(ctx context.Context, m *
 	return bun, nil
 }
 
+func (p *Porter) updateRelocationMapping(relocationMap relocation.ImageRelocationMap, layout registry.Layout, originImg string, newReference string) (relocation.ImageRelocationMap, error) {
+	newImgName, err := getNewImageNameFromBundleReference(originImg, newReference)
+	if err != nil {
+		return nil, err
+	}
+
+	originImgRef := originImg
+	if relocatedImage, ok := relocationMap[originImg]; ok {
+		originImgRef = relocatedImage
+	}
+	digest, err := pushUpdatedImage(layout, originImgRef, newImgName)
+	if err != nil {
+		return nil, err
+	}
+
+	taggedImage, err := p.rewriteImageWithDigest(newImgName.String(), digest.String())
+	if err != nil {
+		return nil, fmt.Errorf("unable to update image reference for %s: %w", newImgName.String(), err)
+	}
+
+	// update relocation map
+	relocationMap[originImg] = taggedImage
+	return relocationMap, nil
+}
+
 func (p *Porter) rewriteImageWithDigest(InvocationImage string, imgDigest string) (string, error) {
 	ref, err := cnab.ParseOCIReference(InvocationImage)
 	if err != nil {

pkg/porter/publish_test.go

@@ -8,7 +8,9 @@ import (
 	"get.porter.sh/porter/pkg/cache"
 	"get.porter.sh/porter/pkg/cnab"
 	"github.com/cnabio/cnab-go/bundle"
+	"github.com/cnabio/cnab-to-oci/relocation"
 	"github.com/pivotal/image-relocation/pkg/image"
+	"github.com/pivotal/image-relocation/pkg/registry"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -33,7 +35,7 @@ func TestPublish_Validate_PorterYamlDoesNotExist(t *testing.T) {
 	assert.EqualError(
 		t,
 		err,
-		"could not find porter.yaml in the current directory, make sure you are in the right directory or specify the porter manifest with --file",
+		"could not find porter.yaml in the current directory /, make sure you are in the right directory or specify the porter manifest with --file",
 		"porter.yaml not present so should have failed validation",
 	)
 }
@@ -145,51 +147,56 @@ func TestPublish_getNewImageNameFromBundleReference(t *testing.T) {
 	})
 }
 
-func TestPublish_UpdateBundleWithNewImage(t *testing.T) {
+func TestPublish_UpdateRelocationMapping(t *testing.T) {
 	p := NewTestPorter(t)
 	defer p.Close()
 
-	bun := cnab.NewBundle(bundle.Bundle{
-		Name: "mybuns",
-		InvocationImages: []bundle.InvocationImage{
-			{
-				BaseImage: bundle.BaseImage{
-					Image:  "myorg/myinvimg",
-					Digest: "abc",
-				},
-			},
-		},
-		Images: map[string]bundle.Image{
-			"myimg": {
-				BaseImage: bundle.BaseImage{
-					Image:  "myorg/myimg",
-					Digest: "abc",
-				},
-			},
-		},
-	})
+	originImg := "myorg/myinvimg"
 	tag := "myneworg/mynewbuns"
-
 	digest, err := image.NewDigest("sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687")
 	require.NoError(t, err, "should have successfully created a digest")
 
-	// update invocation image
-	newInvImgName, err := getNewImageNameFromBundleReference(bun.InvocationImages[0].Image, tag)
-	require.NoError(t, err, "should have successfully derived new image name from bundle tag")
+	testcases := []struct {
+		description   string
+		relocationMap relocation.ImageRelocationMap
+		layout        registry.Layout
+		hasError      bool
+	}{
+		{description: "has relocation mapping defined", relocationMap: relocation.ImageRelocationMap{"myorg/myinvimg": "private/myinvimg"}, layout: mockRegistryLayout{expectedDigest: digest}},
+		{description: "empty relocation map", relocationMap: relocation.ImageRelocationMap{}, layout: mockRegistryLayout{expectedDigest: digest}},
+		{description: "failed to update", relocationMap: relocation.ImageRelocationMap{"myorg/myinvimg": "private/myinvimg"}, layout: mockRegistryLayout{hasError: true}, hasError: true},
+	}
 
-	err = p.updateBundleWithNewImage(bun, newInvImgName, digest, 0)
-	require.NoError(t, err, "updating bundle with new image should not have failed")
-	require.Equal(t, tag+":535ae3fd0b6a46c169fd3d38b486a8a2@sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687", bun.InvocationImages[0].Image)
-	require.Equal(t, "sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687", bun.InvocationImages[0].Digest)
+	for _, tc := range testcases {
+		tc := tc
+		t.Run(tc.description, func(t *testing.T) {
+			newMap, err := p.updateRelocationMapping(tc.relocationMap, tc.layout, originImg, tag)
+			require.Equal(t, tc.hasError, err != nil)
+			if !tc.hasError {
+				require.Equal(t, tag+":535ae3fd0b6a46c169fd3d38b486a8a2@sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687", newMap[originImg])
+			}
+		})
+	}
+}
 
-	// update image
-	newImgName, err := getNewImageNameFromBundleReference(bun.Images["myimg"].Image, tag)
-	require.NoError(t, err, "should have successfully derived new image name from bundle tag")
+type mockRegistryLayout struct {
+	hasError       bool
+	expectedDigest image.Digest
+}
+
+func (m mockRegistryLayout) Add(name image.Name) (image.Digest, error) {
+	if m.hasError {
+		return image.EmptyDigest, errors.New("failed to add image")
+	}
+	return m.expectedDigest, nil
+}
+
+func (m mockRegistryLayout) Push(digest image.Digest, name image.Name) error {
+	return nil
+}
 
-	err = p.updateBundleWithNewImage(bun, newImgName, digest, "myimg")
-	require.NoError(t, err, "updating bundle with new image should not have failed")
-	require.Equal(t, tag+":2055d740dd5c7ec245f0ce739ef7ab57@sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687", bun.Images["myimg"].Image)
-	require.Equal(t, "sha256:6b5a28ccbb76f12ce771a23757880c6083234255c5ba191fca1c5db1f71c1687", bun.Images["myimg"].Digest)
+func (m mockRegistryLayout) Find(n image.Name) (image.Digest, error) {
+	return m.expectedDigest, nil
 }
 
 func TestPublish_RefreshCachedBundle(t *testing.T) {

tests/integration/archive_test.go

@@ -77,6 +77,8 @@ func TestArchive_StableDigest(t *testing.T) {
 
 	// Use a fixed bundle to work with so that we can rely on the registry and layer digests
 	const reference = "ghcr.io/getporter/examples/whalegap:v0.2.0"
+	const referencedImg = "carolynvs/whalesayd@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f"
+	const invocationImg = "ghcr.io/getporter/examples/whalegap:2bed6d7bf087c159835ddfac5838fd34@sha256:5ada057d9b24c443d9fb0240b49c13a5a36a11d5f4dda8adaaa2ec74e39c0d24"
 
 	// Archive bundle
 	archive1Opts := porter.ArchiveOptions{}
@@ -91,7 +93,9 @@ func TestArchive_StableDigest(t *testing.T) {
 	info, err := p.FileSystem.Stat(archiveFile1)
 	require.NoError(p.T(), err)
 	tests.AssertFilePermissionsEqual(t, archiveFile1, pkg.FileModeWritable, info.Mode())
-	containsRequiredMetadata(p, archiveFile1)
+	relocMap := getRelocationMap(p, archiveFile1)
+	require.Equal(p.T(), relocMap[referencedImg], "ghcr.io/getporter/examples/whalegap@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f", relocMap)
+	require.Equal(p.T(), relocMap[invocationImg], "ghcr.io/getporter/examples/whalegap@sha256:5ada057d9b24c443d9fb0240b49c13a5a36a11d5f4dda8adaaa2ec74e39c0d24", relocMap)
 
 	hash1 := getHash(p, archiveFile1)
 
@@ -108,20 +112,36 @@ func TestArchive_StableDigest(t *testing.T) {
 	err = p.Archive(ctx, archive2Opts)
 	require.NoError(t, err, "Second archive failed")
 	assert.Equal(p.T(), hash1, getHash(p, archiveFile2), "shasum of archive did not stay the same on the second call to archive")
-	containsRequiredMetadata(p, archiveFile2)
+	relocMap = getRelocationMap(p, archiveFile1)
+	require.Equal(p.T(), relocMap[referencedImg], "ghcr.io/getporter/examples/whalegap@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f", relocMap)
+	require.Equal(p.T(), relocMap[invocationImg], "ghcr.io/getporter/examples/whalegap@sha256:5ada057d9b24c443d9fb0240b49c13a5a36a11d5f4dda8adaaa2ec74e39c0d24", relocMap)
 
 	// Publish bundle from archive, with new reference
+	localReference := "localhost:5000/archived-whalegap:v0.2.0"
 	publishFromArchiveOpts := porter.PublishOptions{
 		ArchiveFile: archiveFile1,
 		BundlePullOptions: porter.BundlePullOptions{
-			Reference: fmt.Sprintf("localhost:5000/archived-whalegap:v0.2.0"),
+			Reference: localReference,
 		},
 	}
 	err = publishFromArchiveOpts.Validate(p.Context)
 	require.NoError(p.T(), err, "validation of publish opts for bundle failed")
 
 	err = p.Publish(ctx, publishFromArchiveOpts)
 	require.NoError(p.T(), err, "publish of bundle from archive failed")
+
+	// Archive from the newly published bundle in local registry
+	archive3Opts := porter.ArchiveOptions{}
+	archive3Opts.Reference = localReference
+	archiveFile3 := "mybuns3.tgz"
+	err = archive3Opts.Validate(ctx, []string{archiveFile3}, p.Porter)
+	require.NoError(p.T(), err, "validation of archive opts for bundle failed")
+	err = p.Archive(ctx, archive3Opts)
+	require.NoError(t, err, "archive from the published bundle in local registry failed")
+	// make sure the relocation map has updated to use local registry for referenced images
+	relocMap = getRelocationMap(p, archiveFile3)
+	require.Equal(p.T(), relocMap[referencedImg], "localhost:5000/archived-whalegap@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f", relocMap)
+	require.Equal(p.T(), relocMap[invocationImg], "localhost:5000/archived-whalegap@sha256:5ada057d9b24c443d9fb0240b49c13a5a36a11d5f4dda8adaaa2ec74e39c0d24", relocMap)
 }
 
 func getHash(p *porter.TestPorter, path string) string {
@@ -136,26 +156,24 @@ func getHash(p *porter.TestPorter, path string) string {
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
-func containsRequiredMetadata(p *porter.TestPorter, path string) {
+func getRelocationMap(p *porter.TestPorter, archiveFilePath string) relocation.ImageRelocationMap {
 	tmpDir, err := p.FileSystem.TempDir("", "porter-integration-tests")
 	require.NoError(p.T(), err)
 	defer p.FileSystem.RemoveAll(tmpDir)
 
-	source := p.FileSystem.Abs(path)
+	source := p.FileSystem.Abs(archiveFilePath)
 	l := loader.NewLoader()
 	imp := packager.NewImporter(source, tmpDir, l)
 	err = imp.Import()
 	require.NoError(p.T(), err, "opening archive failed")
 
+	_, err = p.FileSystem.Stat(filepath.Join(tmpDir, strings.TrimSuffix(filepath.Base(source), ".tgz"), "bundle.json"))
+	require.NoError(p.T(), err)
 	relocMapBytes, err := p.FileSystem.ReadFile(filepath.Join(tmpDir, strings.TrimSuffix(filepath.Base(source), ".tgz"), "relocation-mapping.json"))
 	require.NoError(p.T(), err)
 
 	// make sure the relocation map contains the expected image
 	relocMap := relocation.ImageRelocationMap{}
 	require.NoError(p.T(), json.Unmarshal(relocMapBytes, &relocMap))
-	require.Equal(p.T(), relocMap["carolynvs/whalesayd@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f"], "ghcr.io/getporter/examples/whalegap@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f", relocMap)
-	require.Equal(p.T(), relocMap["ghcr.io/getporter/examples/whalegap:2bed6d7bf087c159835ddfac5838fd34@sha256:5ada057d9b24c443d9fb0240b49c13a5a36a11d5f4dda8adaaa2ec74e39c0d24"], "ghcr.io/getporter/examples/whalegap@sha256:5ada057d9b24c443d9fb0240b49c13a5a36a11d5f4dda8adaaa2ec74e39c0d24", relocMap)
-
-	_, err = p.FileSystem.Stat(filepath.Join(tmpDir, strings.TrimSuffix(filepath.Base(source), ".tgz"), "bundle.json"))
-	require.NoError(p.T(), err)
+	return relocMap
 }

tests/tester/main.go

@@ -194,5 +194,5 @@ func (t *Tester) createPorterHome(configFilePath string) error {
 
 func (t Tester) Chdir(dir string) {
 	t.TestContext.Chdir(dir)
-	os.Chdir(dir)
+	require.NoError(t.T, os.Chdir(dir))
 }