From 1bb5ff183801d8ffc10f1c067a8051e49a097dbf Mon Sep 17 00:00:00 2001 From: dwillist Date: Fri, 19 Feb 2021 11:01:21 -0500 Subject: [PATCH] update command name, formatting changes Signed-off-by: dwillist --- cmd/cmd.go | 2 +- create_asset_cache.go | 10 +- create_asset_cache_test.go | 65 +++--- internal/blob/blob.go | 12 +- internal/blob/downloader.go | 18 +- internal/blob/downloader_test.go | 220 ++++++++++-------- internal/builder/testmocks/mock_lifecycle.go | 6 +- .../buildpackage/testmocks/mockPackage.go | 3 +- internal/commands/asset-cache.go | 21 ++ internal/commands/create_asset_cache.go | 19 +- internal/commands/create_asset_cache_test.go | 23 +- .../mock_inspect_image_writer_factory.go | 6 +- .../commands/testmocks/mock_pack_client.go | 6 +- internal/dist/asset.go | 4 - internal/dist/asset_cache.go | 12 +- internal/dist/asset_cache_test.go | 22 +- internal/dist/asset_map.go | 24 +- internal/dist/asset_map_test.go | 7 +- internal/dist/asset_test.go | 6 +- internal/dist/blobMap.go | 6 +- internal/dist/buildpack.go | 5 +- internal/dist/buildpack_descriptor.go | 2 +- internal/dist/testmocks/mock_buildpack.go | 6 +- logging/default_logger_test.go | 3 +- package_buildpack.go | 1 + package_buildpack_test.go | 10 +- testmocks/mock_docker_client.go | 11 +- testmocks/mock_downloader.go | 6 +- testmocks/mock_image_factory.go | 3 +- testmocks/mock_image_fetcher.go | 6 +- 30 files changed, 299 insertions(+), 246 deletions(-) create mode 100644 internal/commands/asset-cache.go diff --git a/cmd/cmd.go b/cmd/cmd.go index ee3957be33..999f8e4c63 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -93,7 +93,7 @@ func NewPackCommand(logger ConfigurableLogger) (*cobra.Command, error) { rootCmd.AddCommand(commands.SetDefaultRegistry(logger, cfg, cfgPath)) rootCmd.AddCommand(commands.RemoveRegistry(logger, cfg, cfgPath)) rootCmd.AddCommand(commands.YankBuildpack(logger, cfg, &packClient)) - rootCmd.AddCommand(commands.CreateAssetCache(logger, cfg, &packClient)) + rootCmd.AddCommand(commands.NewAssetCacheCommand(logger, cfg, &packClient)) } packHome, err := config.PackHome() diff --git a/create_asset_cache.go b/create_asset_cache.go index 22c9388295..374792473a 100644 --- a/create_asset_cache.go +++ b/create_asset_cache.go @@ -3,15 +3,17 @@ package pack import ( "context" "fmt" + + "github.com/google/go-containerregistry/pkg/name" + "github.com/buildpacks/pack/internal/blob" "github.com/buildpacks/pack/internal/dist" - "github.com/google/go-containerregistry/pkg/name" ) type CreateAssetCacheOptions struct { ImageName string Assets dist.Assets - Publish bool + Publish bool } func (c *Client) CreateAssetCache(ctx context.Context, opts CreateAssetCacheOptions) error { @@ -48,7 +50,7 @@ func (c *Client) downloadAssets(assets []dist.Asset) (dist.BlobMap, error) { if err != nil { return dist.BlobMap{}, err } - result[asset.Sha256] = dist.NewBlobAssetPair(b,asset.ToAssetValue("")) + result[asset.Sha256] = dist.NewBlobAssetPair(b, asset.ToAssetValue("")) } return result, nil } @@ -61,4 +63,4 @@ func validateConfig(cfg CreateAssetCacheOptions) (CreateAssetCacheOptions, error return CreateAssetCacheOptions{ ImageName: tag.String(), }, nil -} \ No newline at end of file +} diff --git a/create_asset_cache_test.go b/create_asset_cache_test.go index 63d6996045..eefbe52dd1 100644 --- a/create_asset_cache_test.go +++ b/create_asset_cache_test.go @@ -4,7 +4,19 @@ import ( "bytes" "context" "encoding/json" + "io/ioutil" + "os" + "path/filepath" + "strings" + "testing" + "github.com/buildpacks/imgutil/fakes" + "github.com/golang/mock/gomock" + "github.com/google/go-containerregistry/pkg/name" + "github.com/pkg/errors" + "github.com/sclevine/spec" + "github.com/sclevine/spec/report" + "github.com/buildpacks/pack" "github.com/buildpacks/pack/internal/blob" "github.com/buildpacks/pack/internal/dist" @@ -13,16 +25,6 @@ import ( "github.com/buildpacks/pack/pkg/archive" h "github.com/buildpacks/pack/testhelpers" "github.com/buildpacks/pack/testmocks" - "github.com/golang/mock/gomock" - "github.com/google/go-containerregistry/pkg/name" - "github.com/pkg/errors" - "github.com/sclevine/spec" - "github.com/sclevine/spec/report" - "io/ioutil" - "os" - "path/filepath" - "strings" - "testing" ) func TestCreateAssetCacheCommand(t *testing.T) { @@ -66,8 +68,8 @@ func testCreateAssetCacheCommand(t *testing.T, when spec.G, it spec.S) { when("#CreateAssetCache", func() { when("using a local buildpackage", func() { var ( - firstAssetBlob blob.Blob - secondAssetBlob blob.Blob + firstAssetBlob blob.Blob + secondAssetBlob blob.Blob ) it.Before(func() { @@ -94,9 +96,8 @@ second-asset-blob-contents. mockDownloader.EXPECT().Download(gomock.Any(), "https://first-asset-uri", gomock.Any()).Return(firstAssetBlob, nil) mockDownloader.EXPECT().Download(gomock.Any(), "https://second-asset-uri", gomock.Any()).Return(secondAssetBlob, nil) - assert.Succeeds(client.CreateAssetCache(context.Background(), pack.CreateAssetCacheOptions{ - ImageName: imageName, + ImageName: imageName, Assets: []dist.Asset{ { ID: "first-asset", @@ -136,20 +137,20 @@ second-asset-blob-contents. var assetMap dist.AssetMap assert.Succeeds(json.NewDecoder(strings.NewReader(layersLabel)).Decode(&assetMap)) assert.Equal(assetMap, dist.AssetMap{ - "first-sha256": dist.AssetValue { - ID: "first-asset", - Name: "First Asset", + "first-sha256": dist.AssetValue{ + ID: "first-asset", + Name: "First Asset", LayerDiffID: "sha256:edde92682d3bc9b299b52a0af4a3934ae6742e0eb90bc7168e81af5ab6241722", - Stacks: []string{"io.buildpacks.stacks.bionic"}, - URI: "https://first-asset-uri", - Version: "1.2.3", + Stacks: []string{"io.buildpacks.stacks.bionic"}, + URI: "https://first-asset-uri", + Version: "1.2.3", }, "second-sha256": dist.AssetValue{ - ID: "second-asset", - Name: "Second Asset", + ID: "second-asset", + Name: "Second Asset", LayerDiffID: "sha256:46e2287266ceafd2cd4f580566f2b9f504f7b78d472bb3401de18f2410ad1614", - Stacks: []string{"io.buildpacks.stacks.bionic"}, - URI: "https://second-asset-uri", - Version: "4.5.6", + Stacks: []string{"io.buildpacks.stacks.bionic"}, + URI: "https://second-asset-uri", + Version: "4.5.6", }, }) @@ -164,7 +165,6 @@ second-asset-blob-contents. assert.Nil(err) assert.Contains(string(b), "first-asset-blob-contents.") - secondLayerName, err := fakeImage.FindLayerWithPath("/cnb/assets/second-sha256") assert.Nil(err) @@ -189,9 +189,9 @@ second-asset-blob-contents. mockImageFactory.EXPECT().NewImage(imageName, false).Return(fakeImage, nil) assert.Succeeds(client.CreateAssetCache(context.Background(), pack.CreateAssetCacheOptions{ - ImageName: imageName, - Assets: []dist.Asset{}, - Publish: true, + ImageName: imageName, + Assets: []dist.Asset{}, + Publish: true, })) assert.Equal(fakeImage.IsSaved(), true) @@ -204,7 +204,7 @@ second-asset-blob-contents. it("fails with an error message", func() { imageName := "::::" err := client.CreateAssetCache(context.Background(), pack.CreateAssetCacheOptions{ - ImageName: imageName, + ImageName: imageName, }) assert.ErrorContains(err, "invalid asset cache image name: ") }) @@ -215,7 +215,7 @@ second-asset-blob-contents. mockImageFactory.EXPECT().NewImage(imageName, true).Return(nil, errors.New("image fetch error")) err := client.CreateAssetCache(context.Background(), pack.CreateAssetCacheOptions{ - ImageName: imageName, + ImageName: imageName, }) assert.ErrorContains(err, "unable to create asset cache image:") @@ -232,9 +232,8 @@ second-asset-blob-contents. mockImageFactory.EXPECT().NewImage(imageName, true).Return(fakeImage, nil) mockDownloader.EXPECT().Download(gomock.Any(), "https://first-asset-uri", gomock.Any(), gomock.Any()).Return(nil, errors.New("blob download error")) - err = client.CreateAssetCache(context.Background(), pack.CreateAssetCacheOptions{ - ImageName: imageName, + ImageName: imageName, Assets: []dist.Asset{ { ID: "first-asset", diff --git a/internal/blob/blob.go b/internal/blob/blob.go index eda8edd116..d3306ab5f4 100644 --- a/internal/blob/blob.go +++ b/internal/blob/blob.go @@ -18,21 +18,19 @@ type Blob interface { type blob struct { path string - raw bool - + raw bool } -type BlobOption func(*blob) *blob +type Option func(*blob) -func RawOption(b *blob) *blob { +func RawOption(b *blob) { b.raw = true - return b } -func NewBlob(path string, opts ...BlobOption) Blob { +func NewBlob(path string, opts ...Option) Blob { result := &blob{path: path} for _, opt := range opts { - result = opt(result) + opt(result) } return result } diff --git a/internal/blob/downloader.go b/internal/blob/downloader.go index c4c1d9683f..22f2bdc202 100644 --- a/internal/blob/downloader.go +++ b/internal/blob/downloader.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" "fmt" - v1 "github.com/google/go-containerregistry/pkg/v1" "io" "io/ioutil" "net/http" @@ -12,6 +11,8 @@ import ( "os" "path/filepath" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/mitchellh/ioprogress" "github.com/pkg/errors" @@ -26,9 +27,9 @@ const ( ) type downloader struct { - logger logging.Logger - baseCacheDir string - blobOptions []BlobOption + logger logging.Logger + baseCacheDir string + blobOptions []Option validationSha256 string } @@ -64,8 +65,7 @@ func (d downloader) Download(ctx context.Context, pathOrURI string, options ...D return nil, err } - - if err := validateBlobSha( NewBlob(path, d.blobOptions...),d.validationSha256); err != nil { + if err := validateBlobSha(NewBlob(path, d.blobOptions...), d.validationSha256); err != nil { return nil, err } return NewBlob(path, d.blobOptions...), nil @@ -73,7 +73,7 @@ func (d downloader) Download(ctx context.Context, pathOrURI string, options ...D path := d.handleFile(pathOrURI) - if err := validateBlobSha( NewBlob(path, d.blobOptions...),d.validationSha256); err != nil { + if err := validateBlobSha(NewBlob(path, d.blobOptions...), d.validationSha256); err != nil { return nil, err } return NewBlob(path, d.blobOptions...), nil @@ -223,7 +223,9 @@ func validateBlobSha(b Blob, expectedSha256 string) error { if err != nil { return err } - h,_, err := v1.SHA256(r) + defer r.Close() + + h, _, err := v1.SHA256(r) if err != nil { return err } diff --git a/internal/blob/downloader_test.go b/internal/blob/downloader_test.go index d7a2328118..cc0d090f29 100644 --- a/internal/blob/downloader_test.go +++ b/internal/blob/downloader_test.go @@ -3,6 +3,8 @@ package blob_test import ( "compress/gzip" "context" + "crypto/rand" + "fmt" "io" "io/ioutil" "net/http" @@ -38,7 +40,11 @@ func testDownloader(t *testing.T, when spec.G, it spec.S) { ) it.Before(func() { - cacheDir, err = ioutil.TempDir("", "cache") + randPostfix := make([]byte, 4) + _, err := rand.Read(randPostfix) + h.AssertNil(t, err) + + cacheDir, err = ioutil.TempDir("", fmt.Sprintf("cache-%x", randPostfix)) h.AssertNil(t, err) subject = blob.NewDownloader(logging.New(ioutil.Discard), cacheDir) }) @@ -137,25 +143,31 @@ func testDownloader(t *testing.T, when spec.G, it spec.S) { assertBlob(t, b) }) }) + }) - when("Options are used", func() { - when("RawDownload option", func() { - var tgz string + when("Options are used", func() { + when("RawDownload option", func() { + var tgz string + it.Before(func() { + tgz = h.CreateTGZ(t, filepath.Join("testdata", "blob"), "./", 0777) + }) + + it.After(func() { + os.Remove(tgz) + }) + when("uri", func() { + var ( + server *ghttp.Server + uri string + ) it.Before(func() { - tgz = h.CreateTGZ(t, filepath.Join("testdata", "blob"), "./", 0777) + server = ghttp.NewServer() }) - it.After(func() { - os.Remove(tgz) + server.Close() }) - when("uri", func() { - var ( - server *ghttp.Server - uri string - ) - + when("valid uri", func() { it.Before(func() { - server = ghttp.NewServer() uri = server.URL() + "/downloader/somefile.tgz" server.AppendHandlers(func(w http.ResponseWriter, r *http.Request) { @@ -167,9 +179,6 @@ func testDownloader(t *testing.T, when spec.G, it spec.S) { w.WriteHeader(304) }) }) - it.After(func() { - server.Close() - }) it("downloads and reads URI contents as raw bytes", func() { b, err := subject.Download(context.TODO(), uri, blob.RawDownload) @@ -177,7 +186,6 @@ func testDownloader(t *testing.T, when spec.G, it spec.S) { // validate by checking that blob contents are in gzip format. assertBlob(t, b, hasGzip) - }) it("downloads and reads cached request as raw bytes", func() { @@ -195,117 +203,123 @@ func testDownloader(t *testing.T, when spec.G, it spec.S) { }) }) - when("file", func() { - it("opens and reads raw bytes", func() { - absPath, err := filepath.Abs(tgz) - h.AssertNil(t, err) - - b, err := subject.Download(context.TODO(), absPath, blob.RawDownload) - h.AssertNil(t, err) + when("uri is invalid", func() { + when("uri file is not found", func() { + it.Before(func() { + server.AppendHandlers(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(404) + }) + }) - // validate by checking that blob contents are in gzip format. - assertBlob(t, b, hasGzip) + it("should return error", func() { + uri = server.URL() + "/bad/download" + r, err := subject.Download(context.TODO(), uri) + h.AssertError(t, err, "could not download") + h.AssertError(t, err, "http status '404'") + fmt.Print(r) + }) }) + when("uri is unsupported", func() { + it("should return error", func() { + _, err := subject.Download(context.TODO(), "not-supported://file.tgz") + h.AssertError(t, err, "unsupported protocol 'not-supported'") + }) + }) }) - when("followed by non-raw download", func() { - it("does not preform a second raw download", func() { - absPath, err := filepath.Abs(tgz) - h.AssertNil(t, err) + }) - b, err := subject.Download(context.TODO(), absPath, blob.RawDownload) - h.AssertNil(t, err) + when("file", func() { + it("opens and reads raw bytes", func() { + absPath, err := filepath.Abs(tgz) + h.AssertNil(t, err) - // validate by checking that blob contents are in gzip format. - assertBlob(t, b, hasGzip) + b, err := subject.Download(context.TODO(), absPath, blob.RawDownload) + h.AssertNil(t, err) - // second non-raw download - b, err = subject.Download(context.TODO(), absPath) - h.AssertNil(t, err) - assertBlob(t, b) //is not raw! - }) + // validate by checking that blob contents are in gzip format. + assertBlob(t, b, hasGzip) }) }) - when("ValidateDownload option", func() { - when("file", func() { - var absPath string - it.Before(func() { - absPath, err = filepath.Abs(tgz) - h.AssertNil(t, err) - }) - it("validates file sha256's match", func() { - _, err = subject.Download(context.TODO(), absPath, blob.ValidateDownload("f6d3b9d05f1a56deb4830eaf6529c91e92423f984bf5451f04765123e5ece6e6")) - h.AssertNil(t, err) - }) - when("sh256 values do not match", func() { - it("returns an error", func() { - _, err = subject.Download(context.TODO(), absPath, blob.ValidateDownload("bad-sha256")) - h.AssertError(t, err, `validation failed, expected "sha256:bad-sha256", got "sha256:f6d3b9d05f1a56deb4830eaf6529c91e92423f984bf5451f04765123e5ece6e6"`) - }) - }) - }) - - when("uri", func() { - var ( - server *ghttp.Server - uri string - ) + when("followed by non-raw download", func() { + it("does not preform a second raw download", func() { + absPath, err := filepath.Abs(tgz) + h.AssertNil(t, err) - it.Before(func() { - server = ghttp.NewServer() - uri = server.URL() + "/downloader/somefile.tgz" + b, err := subject.Download(context.TODO(), absPath, blob.RawDownload) + h.AssertNil(t, err) - server.AppendHandlers(func(w http.ResponseWriter, r *http.Request) { - w.Header().Add("ETag", "A") - http.ServeFile(w, r, tgz) - }) + // validate by checking that blob contents are in gzip format. + assertBlob(t, b, hasGzip) - server.AppendHandlers(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(304) - }) - }) - it.After(func() { - server.Close() - }) - it("validates file sha256's match", func() { - _, err := subject.Download(context.TODO(), uri, blob.ValidateDownload("f6d3b9d05f1a56deb4830eaf6529c91e92423f984bf5451f04765123e5ece6e6")) - h.AssertNil(t, err) - }) - when("sha256 values to not match", func() { - it("returns an error", func() { - _, err := subject.Download(context.TODO(), uri, blob.ValidateDownload("bad-sha256")) - h.AssertError(t, err, `validation failed, expected "sha256:bad-sha256", got "sha256:f6d3b9d05f1a56deb4830eaf6529c91e92423f984bf5451f04765123e5ece6e6"`) - }) - }) + // second non-raw download + b, err = subject.Download(context.TODO(), absPath) + h.AssertNil(t, err) + assertBlob(t, b) //is not raw! }) - when("sha256 values do not match", func() { - when("uri", func() { + }) + }) + when("ValidateDownload option", func() { + var tgz string + it.Before(func() { + tgz = h.CreateTGZ(t, filepath.Join("testdata", "blob"), "./", 0777) + }) + when("file", func() { + var absPath string + it.Before(func() { + absPath, err = filepath.Abs(tgz) + h.AssertNil(t, err) + }) + it("validates file sha256's match", func() { + _, err = subject.Download(context.TODO(), absPath, blob.ValidateDownload("f6d3b9d05f1a56deb4830eaf6529c91e92423f984bf5451f04765123e5ece6e6")) + h.AssertNil(t, err) + }) + when("sh256 values do not match", func() { + it("returns an error", func() { + _, err = subject.Download(context.TODO(), absPath, blob.ValidateDownload("bad-sha256")) + h.AssertError(t, err, `validation failed, expected "sha256:bad-sha256", got "sha256:f6d3b9d05f1a56deb4830eaf6529c91e92423f984bf5451f04765123e5ece6e6"`) }) }) }) - }) - when("uri is invalid", func() { - when("uri file is not found", func() { + when("uri", func() { + var ( + server *ghttp.Server + uri string + ) + it.Before(func() { + server = ghttp.NewServer() + uri = server.URL() + "/downloader/somefile.tgz" + server.AppendHandlers(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(404) + w.Header().Add("ETag", "A") + http.ServeFile(w, r, tgz) }) - }) - it("should return error", func() { - _, err := subject.Download(context.TODO(), uri) - h.AssertError(t, err, "could not download") - h.AssertError(t, err, "http status '404'") + server.AppendHandlers(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(304) + }) + }) + it.After(func() { + server.Close() + }) + it("validates file sha256's match", func() { + _, err := subject.Download(context.TODO(), uri, blob.ValidateDownload("f6d3b9d05f1a56deb4830eaf6529c91e92423f984bf5451f04765123e5ece6e6")) + h.AssertNil(t, err) + }) + when("sha256 values to not match", func() { + it("returns an error", func() { + _, err := subject.Download(context.TODO(), uri, blob.ValidateDownload("bad-sha256")) + h.AssertError(t, err, `validation failed, expected "sha256:bad-sha256", got "sha256:f6d3b9d05f1a56deb4830eaf6529c91e92423f984bf5451f04765123e5ece6e6"`) + }) }) }) + when("sha256 values do not match", func() { + when("uri", func() { - when("uri is unsupported", func() { - it("should return error", func() { - _, err := subject.Download(context.TODO(), "not-supported://file.tgz") - h.AssertError(t, err, "unsupported protocol 'not-supported'") }) }) }) diff --git a/internal/builder/testmocks/mock_lifecycle.go b/internal/builder/testmocks/mock_lifecycle.go index 258909b72e..c0753bd846 100644 --- a/internal/builder/testmocks/mock_lifecycle.go +++ b/internal/builder/testmocks/mock_lifecycle.go @@ -5,10 +5,12 @@ package testmocks import ( - builder "github.com/buildpacks/pack/internal/builder" - gomock "github.com/golang/mock/gomock" io "io" reflect "reflect" + + gomock "github.com/golang/mock/gomock" + + builder "github.com/buildpacks/pack/internal/builder" ) // MockLifecycle is a mock of Lifecycle interface diff --git a/internal/buildpackage/testmocks/mockPackage.go b/internal/buildpackage/testmocks/mockPackage.go index d632b6a7bf..2939926153 100644 --- a/internal/buildpackage/testmocks/mockPackage.go +++ b/internal/buildpackage/testmocks/mockPackage.go @@ -5,9 +5,10 @@ package testmocks import ( - gomock "github.com/golang/mock/gomock" io "io" reflect "reflect" + + gomock "github.com/golang/mock/gomock" ) // MockPackage is a mock of Package interface diff --git a/internal/commands/asset-cache.go b/internal/commands/asset-cache.go new file mode 100644 index 0000000000..09e2be18d8 --- /dev/null +++ b/internal/commands/asset-cache.go @@ -0,0 +1,21 @@ +package commands + +import ( + "github.com/spf13/cobra" + + "github.com/buildpacks/pack/internal/config" + "github.com/buildpacks/pack/logging" +) + +func NewAssetCacheCommand(logger logging.Logger, cfg config.Config, client PackClient) *cobra.Command { + cmd := &cobra.Command{ + Use: "asset-cache", + Aliases: []string{}, + Short: "Interact with asset caches", + RunE: nil, + } + + cmd.AddCommand(CreateAssetCache(logger, cfg, client)) + AddHelpFlag(cmd, "asset-cache") + return cmd +} diff --git a/internal/commands/create_asset_cache.go b/internal/commands/create_asset_cache.go index bb307d5d6f..968395674d 100644 --- a/internal/commands/create_asset_cache.go +++ b/internal/commands/create_asset_cache.go @@ -2,15 +2,17 @@ package commands import ( "fmt" + "sort" + + "github.com/pkg/errors" + "github.com/spf13/cobra" + "github.com/buildpacks/pack" pubcfg "github.com/buildpacks/pack/config" "github.com/buildpacks/pack/internal/config" "github.com/buildpacks/pack/internal/dist" "github.com/buildpacks/pack/internal/image" "github.com/buildpacks/pack/logging" - "github.com/pkg/errors" - "github.com/spf13/cobra" - "sort" ) type CreateAssetCacheFlags struct { @@ -42,13 +44,12 @@ func CreateAssetCache(logger logging.Logger, cfg config.Config, client PackClien var flags CreateAssetCacheFlags cmd := &cobra.Command{ - Use: "create-asset-cache cache-name", + Use: "create cache-name", Hidden: false, Args: cobra.ExactArgs(1), Short: "Yank build an asset cache using the specified buildpack", Example: "pack create-asset-cache /path/to/buildpack/root", RunE: logError(logger, func(cmd *cobra.Command, args []string) error { - // pull policy should indicate preceedence of daemon flags if err := validateAssetCacheFlags(&flags); err != nil { return err @@ -69,7 +70,6 @@ func CreateAssetCache(logger logging.Logger, cfg config.Config, client PackClien buildpackInfo, err := tryInspect(client, inspectOptions) if err != nil { - return errors.New("buildpack not found") } @@ -80,7 +80,7 @@ func CreateAssetCache(logger logging.Logger, cfg config.Config, client PackClien if err := client.CreateAssetCache(cmd.Context(), pack.CreateAssetCacheOptions{ ImageName: args[0], Assets: assets, - Publish: flags.Publish, + Publish: flags.Publish, }); err != nil { return errors.Wrap(err, "error, unable to create asset cache") } @@ -100,7 +100,7 @@ func CreateAssetCache(logger logging.Logger, cfg config.Config, client PackClien } func tryInspect(c PackClient, inspectOptions []pack.InspectBuildpackOptions) (*pack.BuildpackInfo, error) { - var buildpackInfo *pack.BuildpackInfo = nil + var buildpackInfo *pack.BuildpackInfo var err error for _, inspectOption := range inspectOptions { buildpackInfo, err = c.InspectBuildpack(inspectOption) @@ -125,7 +125,6 @@ func validateAssetCacheFlags(flags *CreateAssetCacheFlags) error { } func getAssets(info *pack.BuildpackInfo) ([]dist.Asset, error) { - result := []dist.Asset{} assetMap := map[string]dist.Asset{} @@ -144,7 +143,7 @@ func getAssets(info *pack.BuildpackInfo) ([]dist.Asset, error) { } sort.Slice(result, func(i, j int) bool { - return result[i].ID < result[j].ID + return result[i].Sha256 < result[j].Sha256 }) return result, nil diff --git a/internal/commands/create_asset_cache_test.go b/internal/commands/create_asset_cache_test.go index 2e272e85d6..6bcc9f7725 100644 --- a/internal/commands/create_asset_cache_test.go +++ b/internal/commands/create_asset_cache_test.go @@ -3,7 +3,16 @@ package commands_test import ( "bytes" "fmt" + "testing" + "github.com/buildpacks/lifecycle/api" + "github.com/golang/mock/gomock" + "github.com/heroku/color" + "github.com/pkg/errors" + "github.com/sclevine/spec" + "github.com/sclevine/spec/report" + "github.com/spf13/cobra" + "github.com/buildpacks/pack" "github.com/buildpacks/pack/internal/buildpackage" "github.com/buildpacks/pack/internal/commands" @@ -14,13 +23,6 @@ import ( ilogging "github.com/buildpacks/pack/internal/logging" "github.com/buildpacks/pack/logging" h "github.com/buildpacks/pack/testhelpers" - "github.com/golang/mock/gomock" - "github.com/heroku/color" - "github.com/pkg/errors" - "github.com/sclevine/spec" - "github.com/sclevine/spec/report" - "github.com/spf13/cobra" - "testing" ) func TestCreateAssetCache(t *testing.T) { @@ -103,7 +105,6 @@ func testCreateAssetCache(t *testing.T, when spec.G, it spec.S) { mockClient = testmocks.NewMockPackClient(mockController) logger = ilogging.NewLogWithWriters(&outBuf, &outBuf) command = commands.CreateAssetCache(logger, cfg, mockClient) - }) it.After(func() { @@ -184,7 +185,6 @@ func testCreateAssetCache(t *testing.T, when spec.G, it spec.S) { assert.Nil(command.Execute()) assert.Equal(daemonValues, []bool{true}) - }) }) @@ -325,7 +325,7 @@ func testCreateAssetCache(t *testing.T, when spec.G, it spec.S) { mockClient.EXPECT().CreateAssetCache(gomock.Any(), pack.CreateAssetCacheOptions{ ImageName: "some/asset-cache", Assets: []dist.Asset{firstAsset, secondAsset}, - Publish: true, + Publish: true, }) assert.Succeeds(command.Execute()) @@ -420,13 +420,12 @@ func testCreateAssetCache(t *testing.T, when spec.G, it spec.S) { mockClient.EXPECT().CreateAssetCache(gomock.Any(), pack.CreateAssetCacheOptions{ ImageName: "some/asset-cache", Assets: []dist.Asset{firstAsset, secondAsset}, - Publish: true, + Publish: true, }).Return(errors.New("asset-cache-creation-error")) err := command.Execute() assert.ErrorContains(err, "error, unable to create asset cache") assert.ErrorContains(err, "asset-cache-creation-error") - }) }) }) diff --git a/internal/commands/testmocks/mock_inspect_image_writer_factory.go b/internal/commands/testmocks/mock_inspect_image_writer_factory.go index 6f887e2e87..2b931aa84c 100644 --- a/internal/commands/testmocks/mock_inspect_image_writer_factory.go +++ b/internal/commands/testmocks/mock_inspect_image_writer_factory.go @@ -5,9 +5,11 @@ package testmocks import ( - writer "github.com/buildpacks/pack/internal/inspectimage/writer" - gomock "github.com/golang/mock/gomock" reflect "reflect" + + gomock "github.com/golang/mock/gomock" + + writer "github.com/buildpacks/pack/internal/inspectimage/writer" ) // MockInspectImageWriterFactory is a mock of InspectImageWriterFactory interface diff --git a/internal/commands/testmocks/mock_pack_client.go b/internal/commands/testmocks/mock_pack_client.go index 5b4f1f4e8b..1bcc0061d3 100644 --- a/internal/commands/testmocks/mock_pack_client.go +++ b/internal/commands/testmocks/mock_pack_client.go @@ -6,9 +6,11 @@ package testmocks import ( context "context" - pack "github.com/buildpacks/pack" - gomock "github.com/golang/mock/gomock" reflect "reflect" + + gomock "github.com/golang/mock/gomock" + + pack "github.com/buildpacks/pack" ) // MockPackClient is a mock of PackClient interface diff --git a/internal/dist/asset.go b/internal/dist/asset.go index 2cfbed19a3..9f530315c3 100644 --- a/internal/dist/asset.go +++ b/internal/dist/asset.go @@ -38,7 +38,3 @@ func (a *Assets) ToIncompleteAssetMap() AssetMap { return result } - - - - diff --git a/internal/dist/asset_cache.go b/internal/dist/asset_cache.go index a005950c7f..7c6eb2f30d 100644 --- a/internal/dist/asset_cache.go +++ b/internal/dist/asset_cache.go @@ -5,16 +5,18 @@ import ( "bytes" "encoding/json" "fmt" - "github.com/buildpacks/imgutil" - "github.com/buildpacks/pack/internal/blob" - "github.com/buildpacks/pack/pkg/archive" - v1 "github.com/google/go-containerregistry/pkg/v1" - "github.com/pkg/errors" "io" "io/ioutil" "os" "path" "path/filepath" + + "github.com/buildpacks/imgutil" + v1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/pkg/errors" + + "github.com/buildpacks/pack/internal/blob" + "github.com/buildpacks/pack/pkg/archive" ) const AssetCacheLayersLabel = "io.buildpacks.asset.layers" diff --git a/internal/dist/asset_cache_test.go b/internal/dist/asset_cache_test.go index a96dbc858f..fbda5b6088 100644 --- a/internal/dist/asset_cache_test.go +++ b/internal/dist/asset_cache_test.go @@ -2,18 +2,20 @@ package dist_test import ( "encoding/json" - "github.com/buildpacks/imgutil/fakes" - "github.com/buildpacks/pack/internal/blob" - "github.com/buildpacks/pack/internal/dist" - "github.com/buildpacks/pack/pkg/archive" - "github.com/buildpacks/pack/testhelpers" - "github.com/google/go-containerregistry/pkg/name" - "github.com/sclevine/spec" "io/ioutil" "os" "path/filepath" "strings" "testing" + + "github.com/buildpacks/imgutil/fakes" + "github.com/google/go-containerregistry/pkg/name" + "github.com/sclevine/spec" + + "github.com/buildpacks/pack/internal/blob" + "github.com/buildpacks/pack/internal/dist" + "github.com/buildpacks/pack/pkg/archive" + "github.com/buildpacks/pack/testhelpers" ) func TestAssetCache(t *testing.T) { @@ -43,15 +45,15 @@ first-asset-blob-contents. second-asset-blob-contents. `), os.ModePerm)) secondAssetBlob = blob.NewBlob(secondAssetBlobPath) - }) + when("#Save", func() { it("saves the cache", func() { tag, err := name.NewTag("asset-cache-image") assert.Nil(err) fakeImage := fakes.NewImage("asset-cache-image", "some-top-level-sha", tag) blobMap := dist.BlobMap{ - "first-sha256": dist.NewBlobAssetPair(firstAssetBlob, dist.AssetValue{ + "first-sha256": dist.NewBlobAssetPair(firstAssetBlob, dist.AssetValue{ ID: "first-asset", Name: "First Asset", Stacks: []string{"io.buildpacks.stacks.bionic"}, @@ -139,7 +141,7 @@ second-asset-blob-contents. fakeImage := fakes.NewImage("asset-cache-image", "some-top-level-sha", tag) blobMap := dist.BlobMap{ - "first-sha256": dist.NewBlobAssetPair(invalidBlob, dist.AssetValue{ + "first-sha256": dist.NewBlobAssetPair(invalidBlob, dist.AssetValue{ ID: "first-asset", Name: "First Asset", Stacks: []string{"io.buildpacks.stacks.bionic"}, diff --git a/internal/dist/asset_map.go b/internal/dist/asset_map.go index 1ddc88b303..42b1b6c01d 100644 --- a/internal/dist/asset_map.go +++ b/internal/dist/asset_map.go @@ -21,21 +21,21 @@ type AssetValue struct { func (a *AssetValue) ToAsset(sha256 string) Asset { return Asset{ - Sha256: sha256, - ID: a.ID, - Version: a.Version, - Name: a.Name, - URI: a.URI, - Licenses: a.Licenses, + Sha256: sha256, + ID: a.ID, + Version: a.Version, + Name: a.Name, + URI: a.URI, + Licenses: a.Licenses, Description: a.Description, - Homepage: a.Homepage, - Stacks: a.Stacks, - Metadata: a.Metadata, + Homepage: a.Homepage, + Stacks: a.Stacks, + Metadata: a.Metadata, } } func (a *AssetMap) ToAssets() Assets { - result := make(Assets,0) + result := make(Assets, 0) for hash, assetVal := range *a { result = append(result, assetVal.ToAsset(hash)) } @@ -48,7 +48,6 @@ func (a *AssetMap) ToAssets() Assets { return result } - func (a *AssetMap) Keys() []string { result := make([]string, len(*a)) idx := 0 @@ -67,7 +66,7 @@ func (a *AssetMap) Filter(keepKeys []string) { // both strings are sorted we can compare using indices i := 0 j := 0 - for ;j < len(allKeys); { + for j < len(allKeys) { switch { case i == len(keepKeys) || allKeys[j] < keepKeys[i]: delete(*a, allKeys[j]) @@ -80,4 +79,3 @@ func (a *AssetMap) Filter(keepKeys []string) { } } } - diff --git a/internal/dist/asset_map_test.go b/internal/dist/asset_map_test.go index a7cd5bc946..d3e0ecf21c 100644 --- a/internal/dist/asset_map_test.go +++ b/internal/dist/asset_map_test.go @@ -1,10 +1,12 @@ package dist_test import ( + "testing" + + "github.com/sclevine/spec" + "github.com/buildpacks/pack/internal/dist" "github.com/buildpacks/pack/testhelpers" - "github.com/sclevine/spec" - "testing" ) func TestAssetMap(t *testing.T) { @@ -139,7 +141,6 @@ func testAssetMap(t *testing.T, when spec.G, it spec.S) { when("AssetMap#Filter", func() { it("filters out all but the sepecified keys", func() { - a := dist.AssetMap{ "A-sha256": dist.AssetValue{}, "B-sha256": dist.AssetValue{}, diff --git a/internal/dist/asset_test.go b/internal/dist/asset_test.go index d72c3a1dba..30d331195f 100644 --- a/internal/dist/asset_test.go +++ b/internal/dist/asset_test.go @@ -1,10 +1,12 @@ package dist_test import ( + "testing" + + "github.com/sclevine/spec" + "github.com/buildpacks/pack/internal/dist" "github.com/buildpacks/pack/testhelpers" - "github.com/sclevine/spec" - "testing" ) func TestAsset(t *testing.T) { diff --git a/internal/dist/blobMap.go b/internal/dist/blobMap.go index 730d4dd1e5..70f94ec776 100644 --- a/internal/dist/blobMap.go +++ b/internal/dist/blobMap.go @@ -1,10 +1,10 @@ package dist import ( - "github.com/buildpacks/pack/internal/blob" "sort" -) + "github.com/buildpacks/pack/internal/blob" +) type BlobAssetPair struct { Blob blob.Blob @@ -13,7 +13,7 @@ type BlobAssetPair struct { func NewBlobAssetPair(b blob.Blob, aVal AssetValue) BlobAssetPair { return BlobAssetPair{ - Blob: b, + Blob: b, AssetVal: aVal, } } diff --git a/internal/dist/buildpack.go b/internal/dist/buildpack.go index 3b99cd04bd..95762a088a 100644 --- a/internal/dist/buildpack.go +++ b/internal/dist/buildpack.go @@ -2,11 +2,12 @@ package dist import ( "archive/tar" + "io" + "path" + "github.com/BurntSushi/toml" "github.com/buildpacks/lifecycle/api" "github.com/pkg/errors" - "io" - "path" "github.com/buildpacks/pack/internal/style" "github.com/buildpacks/pack/pkg/archive" diff --git a/internal/dist/buildpack_descriptor.go b/internal/dist/buildpack_descriptor.go index c7fae036c0..bad5b04a78 100644 --- a/internal/dist/buildpack_descriptor.go +++ b/internal/dist/buildpack_descriptor.go @@ -16,7 +16,7 @@ type BuildpackDescriptor struct { Info BuildpackInfo `toml:"buildpack"` Stacks []Stack `toml:"stacks"` Order Order `toml:"order"` - Assets Assets `toml:"assets"` + Assets Assets `toml:"assets"` } func (b *BuildpackDescriptor) EscapedID() string { diff --git a/internal/dist/testmocks/mock_buildpack.go b/internal/dist/testmocks/mock_buildpack.go index efa16e5841..e8f8acbc9a 100644 --- a/internal/dist/testmocks/mock_buildpack.go +++ b/internal/dist/testmocks/mock_buildpack.go @@ -5,10 +5,12 @@ package testmocks import ( - dist "github.com/buildpacks/pack/internal/dist" - gomock "github.com/golang/mock/gomock" io "io" reflect "reflect" + + gomock "github.com/golang/mock/gomock" + + dist "github.com/buildpacks/pack/internal/dist" ) // MockBuildpack is a mock of Buildpack interface diff --git a/logging/default_logger_test.go b/logging/default_logger_test.go index 05bfb7b014..92654b7486 100644 --- a/logging/default_logger_test.go +++ b/logging/default_logger_test.go @@ -2,9 +2,10 @@ package logging_test import ( "bytes" - "github.com/buildpacks/pack/logging" "testing" + "github.com/buildpacks/pack/logging" + "github.com/sclevine/spec" h "github.com/buildpacks/pack/testhelpers" diff --git a/package_buildpack.go b/package_buildpack.go index edf8b55f09..c7c6a2b6b7 100644 --- a/package_buildpack.go +++ b/package_buildpack.go @@ -2,6 +2,7 @@ package pack import ( "context" + "github.com/pkg/errors" pubbldpkg "github.com/buildpacks/pack/buildpackage" diff --git a/package_buildpack_test.go b/package_buildpack_test.go index fa86b9e1a9..4e6a5d5a0e 100644 --- a/package_buildpack_test.go +++ b/package_buildpack_test.go @@ -469,11 +469,11 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { packageDescriptor dist.BuildpackDescriptor tmpDir string err error - childAssets = dist.Assets{{ - ID: "child-asset", - Name: "child-asset-name", - Sha256: "child-asset-sha256", - Stacks: []string{"some.stack.id"}, + childAssets = dist.Assets{{ + ID: "child-asset", + Name: "child-asset-name", + Sha256: "child-asset-sha256", + Stacks: []string{"some.stack.id"}, Version: "1.2.3", }} ) diff --git a/testmocks/mock_docker_client.go b/testmocks/mock_docker_client.go index 504c8d8355..72b57b0887 100644 --- a/testmocks/mock_docker_client.go +++ b/testmocks/mock_docker_client.go @@ -6,6 +6,12 @@ package testmocks import ( context "context" + io "io" + net "net" + http "net/http" + reflect "reflect" + time "time" + types "github.com/docker/docker/api/types" container "github.com/docker/docker/api/types/container" events "github.com/docker/docker/api/types/events" @@ -17,11 +23,6 @@ import ( volume "github.com/docker/docker/api/types/volume" gomock "github.com/golang/mock/gomock" v1 "github.com/opencontainers/image-spec/specs-go/v1" - io "io" - net "net" - http "net/http" - reflect "reflect" - time "time" ) // MockCommonAPIClient is a mock of CommonAPIClient interface diff --git a/testmocks/mock_downloader.go b/testmocks/mock_downloader.go index ca20e557da..8ff0499f09 100644 --- a/testmocks/mock_downloader.go +++ b/testmocks/mock_downloader.go @@ -6,9 +6,11 @@ package testmocks import ( context "context" - blob "github.com/buildpacks/pack/internal/blob" - gomock "github.com/golang/mock/gomock" reflect "reflect" + + gomock "github.com/golang/mock/gomock" + + blob "github.com/buildpacks/pack/internal/blob" ) // MockDownloader is a mock of Downloader interface diff --git a/testmocks/mock_image_factory.go b/testmocks/mock_image_factory.go index 2de5f597ce..c377e8086d 100644 --- a/testmocks/mock_image_factory.go +++ b/testmocks/mock_image_factory.go @@ -5,9 +5,10 @@ package testmocks import ( + reflect "reflect" + imgutil "github.com/buildpacks/imgutil" gomock "github.com/golang/mock/gomock" - reflect "reflect" ) // MockImageFactory is a mock of ImageFactory interface diff --git a/testmocks/mock_image_fetcher.go b/testmocks/mock_image_fetcher.go index fd1b3c8a13..c49a95e73b 100644 --- a/testmocks/mock_image_fetcher.go +++ b/testmocks/mock_image_fetcher.go @@ -6,10 +6,12 @@ package testmocks import ( context "context" + reflect "reflect" + imgutil "github.com/buildpacks/imgutil" - config "github.com/buildpacks/pack/config" gomock "github.com/golang/mock/gomock" - reflect "reflect" + + config "github.com/buildpacks/pack/config" ) // MockImageFetcher is a mock of ImageFetcher interface