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.relocateImage(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.relocateImage(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) relocateImage(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, fmt.Errorf("unable to push updated image: %w", 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,57 @@ func TestPublish_getNewImageNameFromBundleReference(t *testing.T) {
 	})
 }
 
-func TestPublish_UpdateBundleWithNewImage(t *testing.T) {
+func TestPublish_RelocateImage(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
+		wantErr       error
+	}{
+		{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}, wantErr: errors.New("unable to push updated image")},
+	}
 
-	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.relocateImage(tc.relocationMap, tc.layout, originImg, tag)
+			if tc.wantErr != nil {
+				require.ErrorContains(t, err, tc.wantErr.Error())
+				return
+			}
+			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) {
+	return image.EmptyDigest, nil
+}
+
+func (m mockRegistryLayout) Push(digest image.Digest, name image.Name) error {
+	if m.hasError {
+		return errors.New("failed to add image")
+	}
+	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

@@ -47,10 +47,15 @@ func TestArchive_AirgappedEnvironment(t *testing.T) {
 	require.NoError(t, err, "Could not get the published port of the temporary registry")
 
 	// Publish referenced bundle to one location
-	localRegRef := fmt.Sprintf("localhost:%s/whalesayd:latest", tempRegistryPort)
-	require.NoError(t, shx.RunE("docker", "pull", "carolynvs/whalesayd@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f"))
-	require.NoError(t, shx.RunE("docker", "tag", "carolynvs/whalesayd@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f", localRegRef))
-	require.NoError(t, shx.RunE("docker", "push", localRegRef))
+	referencedImg := "carolynvs/whalesayd@sha256:8b92b7269f59e3ed824e811a1ff1ee64f0d44c0218efefada57a4bebc2d7ef6f"
+	localRegRepo := fmt.Sprintf("localhost:%s/whalesayd", tempRegistryPort)
+	localRegRef := localRegRepo + ":latest"
+	require.NoError(t, shx.RunE("docker", "pull", referencedImg))
+	require.NoError(t, shx.RunE("docker", "tag", referencedImg, localRegRef))
+	output, err := shx.OutputE("docker", "push", localRegRef)
+	require.NoError(t, err)
+	digest := getDigestFromDockerOutput(test.T, output)
+	localRefWithDigest := fmt.Sprintf("%s@%s", localRegRepo, digest)
 
 	// publish a test bundle that reference the image from the temp registry
 	originTestBun := filepath.Join(test.RepoRoot, fmt.Sprintf("tests/testdata/%s/porter.yaml", testdata.MyBunsWithImgReference))
@@ -64,6 +69,16 @@ func TestArchive_AirgappedEnvironment(t *testing.T) {
 
 	archiveFilePath := filepath.Join(test.TestDir, "archive-test.tgz")
 	test.RequirePorter("archive", archiveFilePath, "--reference", testdata.MyBunsWithImgReferenceRef)
+	relocMap := getRelocationMap(test, archiveFilePath)
+	require.Equal(test.T, relocMap[localRefWithDigest], "localhost:5000/mybun-with-img-reference@sha256:499f71eec2e3bd78f26c268bbf5b2a65f73b96216fac4a89b86b5ebf115527b6", relocMap)
+
+	// publish from the archived bundle
+	newRef := fmt.Sprintf("localhost:5000/%s-%s:%s", testdata.MyBunsWithImgReference, "second", "v0.2.0")
+	test.RequirePorter("publish", "--archive", archiveFilePath, "-r", newRef)
+	archiveFilePath2 := filepath.Join(test.TestDir, "archive-test2.tgz")
+	test.RequirePorter("archive", archiveFilePath2, "--reference", newRef)
+	relocMap2 := getRelocationMap(test, archiveFilePath2)
+	require.Equal(test.T, relocMap2[localRefWithDigest], "localhost:5000/mybun-with-img-reference-second@sha256:499f71eec2e3bd78f26c268bbf5b2a65f73b96216fac4a89b86b5ebf115527b6", relocMap)
 }
 
 // Validate that archiving a bundle twice results in the same digest
@@ -91,7 +106,6 @@ 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)
 
 	hash1 := getHash(p, archiveFile1)
 
@@ -108,20 +122,28 @@ 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)
-
 	// 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")
 }
 
 func getHash(p *porter.TestPorter, path string) string {
@@ -136,26 +158,29 @@ func getHash(p *porter.TestPorter, path string) string {
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
-func containsRequiredMetadata(p *porter.TestPorter, path string) {
-	tmpDir, err := p.FileSystem.TempDir("", "porter-integration-tests")
-	require.NoError(p.T(), err)
-	defer p.FileSystem.RemoveAll(tmpDir)
-
-	source := p.FileSystem.Abs(path)
+func getRelocationMap(test tester.Tester, archiveFilePath string) relocation.ImageRelocationMap {
 	l := loader.NewLoader()
-	imp := packager.NewImporter(source, tmpDir, l)
-	err = imp.Import()
-	require.NoError(p.T(), err, "opening archive failed")
+	imp := packager.NewImporter(archiveFilePath, test.TestDir, l)
+	err := imp.Import()
+	require.NoError(test.T, err, "opening archive failed")
 
-	relocMapBytes, err := p.FileSystem.ReadFile(filepath.Join(tmpDir, strings.TrimSuffix(filepath.Base(source), ".tgz"), "relocation-mapping.json"))
-	require.NoError(p.T(), err)
+	_, err = test.TestContext.FileSystem.Stat(filepath.Join(test.TestDir, strings.TrimSuffix(filepath.Base(archiveFilePath), ".tgz"), "bundle.json"))
+	require.NoError(test.T, err)
+	relocMapBytes, err := test.TestContext.FileSystem.ReadFile(filepath.Join(test.TestDir, strings.TrimSuffix(filepath.Base(archiveFilePath), ".tgz"), "relocation-mapping.json"))
+	require.NoError(test.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)
+	require.NoError(test.T, json.Unmarshal(relocMapBytes, &relocMap))
+	return relocMap
+}
 
-	_, err = p.FileSystem.Stat(filepath.Join(tmpDir, strings.TrimSuffix(filepath.Base(source), ".tgz"), "bundle.json"))
-	require.NoError(p.T(), err)
+func getDigestFromDockerOutput(t *testing.T, output string) string {
+	_, after, found := strings.Cut(output, "digest: ")
+	require.True(t, found)
+	results := strings.Split(after, " ")
+	require.Greater(t, len(results), 1)
+	require.Contains(t, results[0], "sha256")
+
+	return results[0]
 }

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