Record layer diff id only

Signed-off-by: Javier Romero <root@jromero.codes> Signed-off-by: Andrew Meyer <ameyer@pivotal.io> Signed-off-by: Micah Young <myoung@pivotal.io>
mdelillo · Oct 21, 2019 · 71b076d · 71b076d
1 parent d2b3633
commit 71b076d
Show file tree

Hide file tree

Showing 11 changed files with 74 additions and 55 deletions.
diff --git a/builder/builder.go b/builder/builder.go
@@ -3,8 +3,6 @@ package builder
 import (
 	"archive/tar"
 	"bytes"
-	"crypto/sha256"
-	"encoding/hex"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -257,12 +255,20 @@ func (b *Builder) Save(logger logging.Logger) error {
 		}
 
 		if err := b.image.AddLayer(bpLayerTar); err != nil {
-			return errors.Wrapf(err, "adding layer tar for buildpack %s:%s", style.Symbol(bp.Descriptor().Info.ID), style.Symbol(bp.Descriptor().Info.Version))
+			return errors.Wrapf(err,
+				"adding layer tar for buildpack %s:%s",
+				style.Symbol(bp.Descriptor().Info.ID),
+				style.Symbol(bp.Descriptor().Info.Version),
+			)
 		}
 
-		sha, err := sha256ForFile(bpLayerTar)
+		diffID, err := dist.LayerDiffID(bpLayerTar)
 		if err != nil {
-			return errors.Wrapf(err, "generating sha for %s", style.Symbol(bpLayerTar))
+			return errors.Wrapf(err,
+				"getting content hashes for buildpack %s:%s",
+				style.Symbol(bp.Descriptor().Info.ID),
+				style.Symbol(bp.Descriptor().Info.Version),
+			)
 		}
 
 		bpInfo := bp.Descriptor().Info
@@ -278,7 +284,7 @@ func (b *Builder) Save(logger logging.Logger) error {
 		}
 
 		bpLayers[bpInfo.ID][bpInfo.Version] = BuildpackLayerInfo{
-			LayerDigest: "sha256:" + sha,
+			LayerDiffID: diffID.String(),
 			Order:       bp.Descriptor().Order,
 		}
 	}
@@ -342,19 +348,6 @@ func (b *Builder) Save(logger logging.Logger) error {
 	return b.image.Save()
 }
 
-func sha256ForFile(path string) (string, error) {
-	file, err := os.Open(path)
-	if err != nil {
-		return "", errors.Wrap(err, "failed to open file")
-	}
-	hasher := sha256.New()
-	if _, err := io.Copy(hasher, file); err != nil {
-		return "", errors.Wrap(err, "failed to copy file to hasher")
-	}
-
-	return hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))), nil
-}
-
 func processOrder(buildpacks []BuildpackMetadata, order dist.Order) (dist.Order, error) {
 	resolvedOrder := dist.Order{}
 

diff --git a/builder/builder_test.go b/builder/builder_test.go
@@ -45,12 +45,12 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 		bp1v2          dist.Buildpack
 		bp2v1          dist.Buildpack
 		bpOrder        dist.Buildpack
-		buf            bytes.Buffer
+		outBuf         bytes.Buffer
 		logger         logging.Logger
 	)
 
 	it.Before(func() {
-		logger = ifakes.NewFakeLogger(&buf)
+		logger = ifakes.NewFakeLogger(&outBuf)
 		baseImage = fakes.NewImage("base/image", "", nil)
 		mockController = gomock.NewController(t)
 		mockLifecycle = testmocks.NewMockLifecycle(mockController)
@@ -610,13 +610,6 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 				h.AssertEq(t, len(layers["buildpack-1-id"]), 2)
 				h.AssertEq(t, len(layers["buildpack-2-id"]), 1)
 
-				h.AssertUnique(t,
-					layers["buildpack-1-id"]["buildpack-1-version-1"].LayerDigest,
-					layers["buildpack-1-id"]["buildpack-1-version-2"].LayerDigest,
-					layers["buildpack-2-id"]["buildpack-2-version-1"].LayerDigest,
-					layers["order-buildpack-id"]["order-buildpack-version"].LayerDigest,
-				)
-
 				h.AssertEq(t, len(layers["buildpack-1-id"]["buildpack-1-version-1"].Order), 0)
 				h.AssertEq(t, len(layers["buildpack-1-id"]["buildpack-1-version-2"].Order), 0)
 				h.AssertEq(t, len(layers["buildpack-2-id"]["buildpack-2-version-1"].Order), 0)
@@ -632,9 +625,24 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 
 			when("base image already has buildpack layers label", func() {
 				it.Before(func() {
+					var mdJSON bytes.Buffer
+					h.AssertNil(t, json.Compact(
+						&mdJSON,
+						[]byte(`{
+  "buildpack-1-id": {
+    "buildpack-1-version-1": {
+      "layerDiffID": "sha256:buildpack-1-version-1-diff-id"
+    },
+    "buildpack-1-version-2": {
+      "layerDiffID": "sha256:buildpack-1-version-2-diff-id"
+    }
+  }
+}
+`)))
+
 					h.AssertNil(t, baseImage.SetLabel(
 						"io.buildpacks.buildpack.layers",
-						`{ "buildpack-1-id": { "buildpack-1-version-1": { "layerDigest": "sha256:buildpack-1-version-1-sha" }, "buildpack-1-version-2": { "layerDigest": "sha256:buildpack-1-version-2-orig-sha" } } }`,
+						mdJSON.String(),
 					))
 
 					var err error
@@ -660,17 +668,14 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 					h.AssertEq(t, len(layers["buildpack-1-id"]), 2)
 					h.AssertEq(t, len(layers["buildpack-2-id"]), 1)
 
-					h.AssertEq(t, layers["buildpack-1-id"]["buildpack-1-version-1"].LayerDigest, "sha256:buildpack-1-version-1-sha")
+					h.AssertEq(t, layers["buildpack-1-id"]["buildpack-1-version-1"].LayerDiffID, "sha256:buildpack-1-version-1-diff-id")
+
 					h.AssertUnique(t,
-						layers["buildpack-1-id"]["buildpack-1-version-1"].LayerDigest,
-						layers["buildpack-1-id"]["buildpack-1-version-2"].LayerDigest,
-						layers["buildpack-2-id"]["buildpack-2-version-1"].LayerDigest,
+						layers["buildpack-1-id"]["buildpack-1-version-1"].LayerDiffID,
+						layers["buildpack-1-id"]["buildpack-1-version-2"].LayerDiffID,
+						layers["buildpack-2-id"]["buildpack-2-version-1"].LayerDiffID,
 					)
 
-					h.AssertMatch(t, layers["buildpack-1-id"]["buildpack-1-version-1"].LayerDigest, "^sha256:.*")
-					h.AssertMatch(t, layers["buildpack-1-id"]["buildpack-1-version-2"].LayerDigest, "^sha256:.*")
-					h.AssertMatch(t, layers["buildpack-2-id"]["buildpack-2-version-1"].LayerDigest, "^sha256:.*")
-
 					h.AssertEq(t, len(layers["buildpack-1-id"]["buildpack-1-version-1"].Order), 0)
 					h.AssertEq(t, len(layers["buildpack-1-id"]["buildpack-1-version-2"].Order), 0)
 					h.AssertEq(t, len(layers["buildpack-2-id"]["buildpack-2-version-1"].Order), 0)
@@ -683,8 +688,8 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) {
 					var layers builder.BuildpackLayers
 					h.AssertNil(t, json.Unmarshal([]byte(label), &layers))
 
-					h.AssertContains(t, buf.String(), "Warning: buildpack 'buildpack-1-id@buildpack-1-version-2' already exists on builder and will be overridden")
-					h.AssertNotContains(t, layers["buildpack-1-id"]["buildpack-1-version-2"].LayerDigest, "buildpack-1-version-2-orig-sha")
+					h.AssertContains(t, outBuf.String(), "Warning: buildpack 'buildpack-1-id@buildpack-1-version-2' already exists on builder and will be overridden")
+					h.AssertNotContains(t, layers["buildpack-1-id"]["buildpack-1-version-2"].LayerDiffID, "buildpack-1-version-2-diff-id")
 				})
 			})
 

diff --git a/builder/metadata.go b/builder/metadata.go
@@ -8,7 +8,7 @@ const BuildpackLayersLabel = "io.buildpacks.buildpack.layers"
 type BuildpackLayers map[string]map[string]BuildpackLayerInfo
 
 type BuildpackLayerInfo struct {
-	LayerDigest string     `json:"layerDigest"`
+	LayerDiffID string     `json:"layerDiffID"`
 	Order       dist.Order `json:"order,omitempty"`
 }
 

diff --git a/cache/image_cache.go b/cache/image_cache.go
@@ -16,7 +16,7 @@ type ImageCache struct {
 }
 
 func NewImageCache(imageRef name.Reference, dockerClient *client.Client) *ImageCache {
-	sum := sha256.Sum256([]byte(imageRef.String()))
+	sum := sha256.Sum256([]byte(imageRef.Name()))
 	return &ImageCache{
 		image:  fmt.Sprintf("pack-cache-%x", sum[:6]),
 		docker: dockerClient,

diff --git a/cache/image_cache_test.go b/cache/image_cache_test.go
@@ -76,12 +76,12 @@ func testImageCache(t *testing.T, when spec.G, it spec.S) {
 			ref, err := name.ParseReference("my/repo:latest", name.WeakValidation)
 			h.AssertNil(t, err)
 			subject := cache.NewImageCache(ref, dockerClient)
+
 			ref, err = name.ParseReference("my/repo", name.WeakValidation)
 			h.AssertNil(t, err)
 			expected := cache.NewImageCache(ref, dockerClient)
-			if subject.Name() != expected.Name() {
-				t.Fatalf("The same repo name should result in the same image")
-			}
+
+			h.AssertEq(t, subject.Name(), expected.Name())
 		})
 
 		it("resolves implied registry", func() {

diff --git a/cache/volume_cache.go b/cache/volume_cache.go
@@ -15,7 +15,7 @@ type VolumeCache struct {
 }
 
 func NewVolumeCache(imageRef name.Reference, suffix string, dockerClient *client.Client) *VolumeCache {
-	sum := sha256.Sum256([]byte(imageRef.String()))
+	sum := sha256.Sum256([]byte(imageRef.Name()))
 	return &VolumeCache{
 		volume: fmt.Sprintf("pack-cache-%x.%s", sum[:6], suffix),
 		docker: dockerClient,

diff --git a/cache/volume_cache_test.go b/cache/volume_cache_test.go
@@ -85,12 +85,12 @@ func testCache(t *testing.T, when spec.G, it spec.S) {
 			ref, err := name.ParseReference("my/repo:latest", name.WeakValidation)
 			h.AssertNil(t, err)
 			subject := cache.NewVolumeCache(ref, "some-suffix", dockerClient)
+
 			ref, err = name.ParseReference("my/repo", name.WeakValidation)
 			h.AssertNil(t, err)
 			expected := cache.NewVolumeCache(ref, "some-suffix", dockerClient)
-			if subject.Name() != expected.Name() {
-				t.Fatalf("The same repo name should result in the same volume")
-			}
+
+			h.AssertEq(t, subject.Name(), expected.Name())
 		})
 
 		it("resolves implied registry", func() {
@@ -100,9 +100,8 @@ func testCache(t *testing.T, when spec.G, it spec.S) {
 			ref, err = name.ParseReference("my/repo", name.WeakValidation)
 			h.AssertNil(t, err)
 			expected := cache.NewVolumeCache(ref, "some-suffix", dockerClient)
-			if subject.Name() != expected.Name() {
-				t.Fatalf("The same repo name should result in the same volume")
-			}
+
+			h.AssertEq(t, subject.Name(), expected.Name())
 		})
 	})
 

diff --git a/create_builder.go b/create_builder.go
@@ -32,7 +32,7 @@ func (c *Client) CreateBuilder(ctx context.Context, opts CreateBuilderOptions) e
 
 	baseImage, err := c.imageFetcher.Fetch(ctx, opts.BuilderConfig.Stack.BuildImage, !opts.Publish, !opts.NoPull)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "fetch build image")
 	}
 
 	c.logger.Debugf("Creating builder %s from build-image %s", style.Symbol(opts.BuilderName), style.Symbol(baseImage.Name()))

diff --git a/dist/layers.go b/dist/layers.go
@@ -9,6 +9,8 @@ import (
 	"path"
 	"path/filepath"
 
+	v1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/tarball"
 	"github.com/pkg/errors"
 
 	"github.com/buildpack/pack/internal/archive"
@@ -109,3 +111,23 @@ func embedBuildpackTar(tw *tar.Writer, uid, gid int, bp Buildpack, baseTarDir st
 
 	return nil
 }
+
+func LayerDiffID(layerTarPath string) (v1.Hash, error) {
+	fh, err := os.Open(layerTarPath)
+	if err != nil {
+		return v1.Hash{}, errors.Wrap(err, "opening tar file")
+	}
+	defer fh.Close()
+
+	layer, err := tarball.LayerFromFile(layerTarPath)
+	if err != nil {
+		return v1.Hash{}, errors.Wrap(err, "reading layer tar")
+	}
+
+	hash, err := layer.DiffID()
+	if err != nil {
+		return v1.Hash{}, errors.Wrap(err, "generating diff id")
+	}
+
+	return hash, nil
+}
diff --git a/go.sum b/go.sum
@@ -235,4 +235,4 @@ gotest.tools v2.1.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81
 gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=
 gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
 honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
-honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
+honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
diff --git a/image/fetcher.go b/image/fetcher.go
@@ -77,12 +77,12 @@ func (f *Fetcher) fetchDaemonImage(name string) (imgutil.Image, error) {
 }
 
 func (f *Fetcher) pullImage(ctx context.Context, imageID string) error {
-	auth, err := registryAuth(imageID)
+	regAuth, err := registryAuth(imageID)
 	if err != nil {
 		return err
 	}
 	rc, err := f.docker.ImagePull(ctx, imageID, types.ImagePullOptions{
-		RegistryAuth: auth,
+		RegistryAuth: regAuth,
 	})
 	if err != nil {
 		return err