diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index aba817cfd7..373464e2bc 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1089,12 +1089,6 @@ func testAcceptance( var packageImageName string it.Before(func() { - h.SkipIf(t, - !pack.Supports("package-buildpack"), - "--buildpack does not accept buildpackage unless package-buildpack is supported", - ) - h.SkipIf(t, dockerHostOS() == "windows", "These tests are not yet compatible with Windows-based containers") - packageImageName = packageBuildpackAsImage(t, assert, pack, @@ -1134,12 +1128,6 @@ func testAcceptance( ) it.Before(func() { - h.SkipIf(t, - !pack.Supports("package-buildpack --format"), - "--buildpack does not accept buildpackage file unless package-buildpack with --format is supported", - ) - h.SkipIf(t, dockerHostOS() == "windows", "These tests are not yet compatible with Windows-based containers") - var err error tmpDir, err = ioutil.TempDir("", "package-file") assert.Nil(err) diff --git a/build.go b/build.go index db3792e32b..f4eb3e2f97 100644 --- a/build.go +++ b/build.go @@ -531,7 +531,11 @@ func (c *Client) processBuildpacks(ctx context.Context, builderImage imgutil.Ima return fetchedBPs, order, errors.Wrapf(err, "extracting buildpacks from %s", style.Symbol(bp)) } } else { - layerWriterFactory, err := layer.NewWriterFactory(builderImage) + imageOS, err := builderImage.OS() + if err != nil { + return fetchedBPs, order, errors.Wrap(err, "getting image OS") + } + layerWriterFactory, err := layer.NewWriterFactory(imageOS) if err != nil { return fetchedBPs, order, errors.Wrapf(err, "get tar writer factory for image %s", style.Symbol(builderImage.Name())) } diff --git a/create_builder.go b/create_builder.go index b10c38fcf1..b6feafef03 100644 --- a/create_builder.go +++ b/create_builder.go @@ -249,7 +249,11 @@ func (c *Client) addBuildpacksToBuilder(ctx context.Context, opts CreateBuilderO return errors.Wrapf(err, "extracting buildpacks from %s", style.Symbol(b.ID)) } } else { - layerWriterFactory, err := layer.NewWriterFactory(bldr.Image()) + imageOS, err := bldr.Image().OS() + if err != nil { + return errors.Wrap(err, "getting image OS") + } + layerWriterFactory, err := layer.NewWriterFactory(imageOS) if err != nil { return errors.Wrapf(err, "get tar writer factory for image %s", style.Symbol(bldr.Name())) } diff --git a/create_builder_test.go b/create_builder_test.go index 6854f9be08..5ea0ac4992 100644 --- a/create_builder_test.go +++ b/create_builder_test.go @@ -10,10 +10,9 @@ import ( "runtime" "testing" - "github.com/buildpacks/pack/config" - "github.com/buildpacks/imgutil/fakes" "github.com/buildpacks/lifecycle/api" + "github.com/docker/docker/api/types" "github.com/golang/mock/gomock" "github.com/heroku/color" "github.com/pkg/errors" @@ -23,6 +22,7 @@ import ( "github.com/buildpacks/pack" pubbldr "github.com/buildpacks/pack/builder" pubbldpkg "github.com/buildpacks/pack/buildpackage" + "github.com/buildpacks/pack/config" "github.com/buildpacks/pack/internal/blob" "github.com/buildpacks/pack/internal/builder" "github.com/buildpacks/pack/internal/dist" @@ -47,6 +47,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { mockDownloader *testmocks.MockDownloader mockImageFactory *testmocks.MockImageFactory mockImageFetcher *testmocks.MockImageFetcher + mockDockerClient *testmocks.MockCommonAPIClient fakeBuildImage *fakes.Image fakeRunImage *fakes.Image fakeRunImageMirror *fakes.Image @@ -63,6 +64,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { mockDownloader = testmocks.NewMockDownloader(mockController) mockImageFetcher = testmocks.NewMockImageFetcher(mockController) mockImageFactory = testmocks.NewMockImageFactory(mockController) + mockDockerClient = testmocks.NewMockCommonAPIClient(mockController) fakeBuildImage = fakes.NewImage("some/build-image", "", nil) h.AssertNil(t, fakeBuildImage.SetLabel("io.buildpacks.stack.id", "some.stack.id")) @@ -87,9 +89,12 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { pack.WithDownloader(mockDownloader), pack.WithImageFactory(mockImageFactory), pack.WithFetcher(mockImageFetcher), + pack.WithDockerClient(mockDockerClient), ) h.AssertNil(t, err) + mockDockerClient.EXPECT().Info(context.TODO()).Return(types.Info{OSType: "linux"}, nil).AnyTimes() + opts = pack.CreateBuilderOptions{ BuilderName: "some/builder", Config: pubbldr.Config{ @@ -562,9 +567,9 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { bldr := successfullyCreateBuilder() h.AssertEq(t, bldr.LifecycleDescriptor().Info.Version.String(), "0.0.0") - //nolint:staticcheck + // nolint:staticcheck h.AssertEq(t, bldr.LifecycleDescriptor().API.BuildpackVersion.String(), "0.2") - //nolint:staticcheck + // nolint:staticcheck h.AssertEq(t, bldr.LifecycleDescriptor().API.PlatformVersion.String(), "0.2") h.AssertEq(t, bldr.LifecycleDescriptor().APIs.Buildpack.Deprecated.AsStrings(), []string{}) h.AssertEq(t, bldr.LifecycleDescriptor().APIs.Buildpack.Supported.AsStrings(), []string{"0.2", "0.3", "0.4"}) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 2afbbc863b..c73bd7628a 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -89,7 +89,11 @@ func New(baseImage imgutil.Image, name string) (*Builder, error) { } func constructBuilder(img imgutil.Image, newName string, metadata Metadata) (*Builder, error) { - layerWriterFactory, err := layer.NewWriterFactory(img) + imageOS, err := img.OS() + if err != nil { + return nil, errors.Wrap(err, "getting image OS") + } + layerWriterFactory, err := layer.NewWriterFactory(imageOS) if err != nil { return nil, err } diff --git a/internal/buildpackage/builder.go b/internal/buildpackage/builder.go index 43bddb19e1..2b10c19f04 100644 --- a/internal/buildpackage/builder.go +++ b/internal/buildpackage/builder.go @@ -3,6 +3,7 @@ package buildpackage import ( "archive/tar" "compress/gzip" + "context" "io/ioutil" "os" @@ -14,16 +15,23 @@ import ( "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/pkg/errors" + pubcfg "github.com/buildpacks/pack/config" "github.com/buildpacks/pack/internal/archive" "github.com/buildpacks/pack/internal/dist" "github.com/buildpacks/pack/internal/stack" "github.com/buildpacks/pack/internal/style" ) +const windowsPackageBase = "mcr.microsoft.com/windows/nanoserver:1809-amd64" // TODO: Should this be hard-coded? + type ImageFactory interface { NewImage(repoName string, local bool) (imgutil.Image, error) } +type ImageFetcher interface { + Fetch(ctx context.Context, name string, daemon bool, pullPolicy pubcfg.PullPolicy) (imgutil.Image, error) +} + type WorkableImage interface { SetLabel(string, string) error AddLayerWithDiffID(path, diffID string) error @@ -62,12 +70,16 @@ func (i *layoutImage) AddLayerWithDiffID(path, _ string) error { type PackageBuilder struct { buildpack dist.Buildpack dependencies []dist.Buildpack + imageOS string imageFactory ImageFactory + imageFetcher ImageFetcher } -func NewBuilder(imageFactory ImageFactory) *PackageBuilder { +func NewBuilder(imageOS string, imageFactory ImageFactory, imageFetcher ImageFetcher) *PackageBuilder { return &PackageBuilder{ + imageOS: imageOS, imageFactory: imageFactory, + imageFetcher: imageFetcher, } } @@ -192,14 +204,26 @@ func (b *PackageBuilder) SaveAsFile(path string) error { return archive.WriteDirToTar(tw, layoutDir, "/", 0, 0, 0755, true, nil) } -func (b *PackageBuilder) SaveAsImage(repoName string, publish bool) (imgutil.Image, error) { +func (b *PackageBuilder) SaveAsImage(ctx context.Context, repoName string, publish bool, pullPolicy pubcfg.PullPolicy) (imgutil.Image, error) { if err := b.validate(); err != nil { return nil, err } - image, err := b.imageFactory.NewImage(repoName, !publish) - if err != nil { - return nil, errors.Wrapf(err, "creating image") + var ( + image imgutil.Image + err error + ) + if b.imageOS == "windows" { + image, err = b.imageFetcher.Fetch(ctx, windowsPackageBase, !publish, pullPolicy) + if err != nil { + return nil, errors.Wrapf(err, "fetching base image") + } + image.Rename(repoName) + } else { + image, err = b.imageFactory.NewImage(repoName, !publish) + if err != nil { + return nil, errors.Wrapf(err, "creating image") + } } tmpDir, err := ioutil.TempDir("", "package-buildpack") diff --git a/internal/buildpackage/builder_test.go b/internal/buildpackage/builder_test.go index a7b5dbe4c2..1d3d58017f 100644 --- a/internal/buildpackage/builder_test.go +++ b/internal/buildpackage/builder_test.go @@ -3,6 +3,7 @@ package buildpackage_test import ( "archive/tar" "compress/gzip" + "context" "encoding/json" "fmt" "io" @@ -21,6 +22,7 @@ import ( "github.com/sclevine/spec" "github.com/sclevine/spec/report" + "github.com/buildpacks/pack/config" "github.com/buildpacks/pack/internal/buildpackage" "github.com/buildpacks/pack/internal/dist" ifakes "github.com/buildpacks/pack/internal/fakes" @@ -36,7 +38,6 @@ func TestPackageBuilder(t *testing.T) { func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { var ( - fakePackageImage *fakes.Image mockController *gomock.Controller mockImageFactory *testmocks.MockImageFactory subject *buildpackage.PackageBuilder @@ -47,10 +48,10 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { mockController = gomock.NewController(t) mockImageFactory = testmocks.NewMockImageFactory(mockController) - fakePackageImage = fakes.NewImage("some/package", "", nil) + fakePackageImage := fakes.NewImage("some/package", "", nil) mockImageFactory.EXPECT().NewImage("some/package", true).Return(fakePackageImage, nil).AnyTimes() - subject = buildpackage.NewBuilder(mockImageFactory) + subject = buildpackage.NewBuilder("linux", mockImageFactory, nil) var err error tmpDir, err = ioutil.TempDir("", "package_builder_tests") @@ -68,7 +69,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { fn func() error }{ {name: "SaveAsImage", fn: func() error { - _, err := subject.SaveAsImage(fakePackageImage.Name(), false) + _, err := subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) return err }}, {name: "SaveAsFile", fn: func() error { @@ -269,7 +270,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) subject.AddDependency(dependency2) - _, err = subject.SaveAsImage("some/package", false) + _, err = subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) h.AssertError(t, err, "no compatible stacks among provided buildpacks") }) }) @@ -324,7 +325,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) subject.AddDependency(dependency2) - img, err := subject.SaveAsImage("some/package", false) + img, err := subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) h.AssertNil(t, err) metadata := buildpackage.Metadata{} @@ -388,7 +389,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { subject.AddDependency(dependencyNestedNested) - img, err := subject.SaveAsImage("some/package", false) + img, err := subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) h.AssertNil(t, err) metadata := buildpackage.Metadata{} @@ -404,97 +405,203 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { }) when("#SaveAsImage", func() { - it("sets metadata", func() { - buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ - API: api.MustParse("0.2"), - Info: dist.BuildpackInfo{ - ID: "bp.1.id", - Version: "bp.1.version", - }, - Stacks: []dist.Stack{ - {ID: "stack.id.1"}, - {ID: "stack.id.2"}, - }, - Order: nil, - }, 0644) - h.AssertNil(t, err) + when("creating linux package", func() { + it("sets metadata", func() { + buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "bp.1.id", + Version: "bp.1.version", + }, + Stacks: []dist.Stack{ + {ID: "stack.id.1"}, + {ID: "stack.id.2"}, + }, + Order: nil, + }, 0644) + h.AssertNil(t, err) - subject.SetBuildpack(buildpack1) + subject.SetBuildpack(buildpack1) - packageImage, err := subject.SaveAsImage(fakePackageImage.Name(), false) - h.AssertNil(t, err) + packageImage, err := subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) + h.AssertNil(t, err) - labelData, err := packageImage.Label("io.buildpacks.buildpackage.metadata") - h.AssertNil(t, err) - var md buildpackage.Metadata - h.AssertNil(t, json.Unmarshal([]byte(labelData), &md)) - - h.AssertEq(t, md.ID, "bp.1.id") - h.AssertEq(t, md.Version, "bp.1.version") - h.AssertEq(t, len(md.Stacks), 2) - h.AssertEq(t, md.Stacks[0].ID, "stack.id.1") - h.AssertEq(t, md.Stacks[1].ID, "stack.id.2") - }) + labelData, err := packageImage.Label("io.buildpacks.buildpackage.metadata") + h.AssertNil(t, err) + var md buildpackage.Metadata + h.AssertNil(t, json.Unmarshal([]byte(labelData), &md)) + + h.AssertEq(t, md.ID, "bp.1.id") + h.AssertEq(t, md.Version, "bp.1.version") + h.AssertEq(t, len(md.Stacks), 2) + h.AssertEq(t, md.Stacks[0].ID, "stack.id.1") + h.AssertEq(t, md.Stacks[1].ID, "stack.id.2") + }) - it("sets buildpack layers label", func() { - buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ - API: api.MustParse("0.2"), - Info: dist.BuildpackInfo{ID: "bp.1.id", Version: "bp.1.version"}, - Stacks: []dist.Stack{{ID: "stack.id.1"}, {ID: "stack.id.2"}}, - Order: nil, - }, 0644) - h.AssertNil(t, err) - subject.SetBuildpack(buildpack1) + it("sets buildpack layers label", func() { + buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ID: "bp.1.id", Version: "bp.1.version"}, + Stacks: []dist.Stack{{ID: "stack.id.1"}, {ID: "stack.id.2"}}, + Order: nil, + }, 0644) + h.AssertNil(t, err) + subject.SetBuildpack(buildpack1) - _, err = subject.SaveAsImage(fakePackageImage.Name(), false) - h.AssertNil(t, err) + packageImage, err := subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) + h.AssertNil(t, err) - var bpLayers dist.BuildpackLayers - _, err = dist.GetLabel(fakePackageImage, "io.buildpacks.buildpack.layers", &bpLayers) - h.AssertNil(t, err) + var bpLayers dist.BuildpackLayers + _, err = dist.GetLabel(packageImage, "io.buildpacks.buildpack.layers", &bpLayers) + h.AssertNil(t, err) + + bp1Info, ok1 := bpLayers["bp.1.id"]["bp.1.version"] + h.AssertEq(t, ok1, true) + h.AssertEq(t, bp1Info.Stacks, []dist.Stack{{ID: "stack.id.1"}, {ID: "stack.id.2"}}) + }) + + it("adds buildpack layers", func() { + buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ID: "bp.1.id", Version: "bp.1.version"}, + Stacks: []dist.Stack{{ID: "stack.id.1"}, {ID: "stack.id.2"}}, + Order: nil, + }, 0644) + h.AssertNil(t, err) + subject.SetBuildpack(buildpack1) + + packageImage, err := subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) + h.AssertNil(t, err) + + buildpackExists := func(name, version string) { + t.Helper() + dirPath := fmt.Sprintf("/cnb/buildpacks/%s/%s", name, version) + fakePackageImage := packageImage.(*fakes.Image) + layerTar, err := fakePackageImage.FindLayerWithPath(dirPath) + h.AssertNil(t, err) + + h.AssertOnTarEntry(t, layerTar, dirPath, + h.IsDirectory(), + ) + + h.AssertOnTarEntry(t, layerTar, dirPath+"/bin/build", + h.ContentEquals("build-contents"), + h.HasOwnerAndGroup(0, 0), + h.HasFileMode(0644), + ) - bp1Info, ok1 := bpLayers["bp.1.id"]["bp.1.version"] - h.AssertEq(t, ok1, true) - h.AssertEq(t, bp1Info.Stacks, []dist.Stack{{ID: "stack.id.1"}, {ID: "stack.id.2"}}) + h.AssertOnTarEntry(t, layerTar, dirPath+"/bin/detect", + h.ContentEquals("detect-contents"), + h.HasOwnerAndGroup(0, 0), + h.HasFileMode(0644), + ) + } + + buildpackExists("bp.1.id", "bp.1.version") + }) }) - it("adds buildpack layers", func() { - buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ - API: api.MustParse("0.2"), - Info: dist.BuildpackInfo{ID: "bp.1.id", Version: "bp.1.version"}, - Stacks: []dist.Stack{{ID: "stack.id.1"}, {ID: "stack.id.2"}}, - Order: nil, - }, 0644) - h.AssertNil(t, err) - subject.SetBuildpack(buildpack1) + when("creating windows package", func() { + it.Before(func() { + mockImageFetcher := testmocks.NewMockImageFetcher(mockController) + subject = buildpackage.NewBuilder("windows", nil, mockImageFetcher) + fakeBaseImage := fakes.NewImage("", "", nil) + mockImageFetcher.EXPECT().Fetch(context.TODO(), "mcr.microsoft.com/windows/nanoserver:1809-amd64", gomock.Any(), gomock.Any()).Return(fakeBaseImage, nil) + }) - _, err = subject.SaveAsImage(fakePackageImage.Name(), false) - h.AssertNil(t, err) + it("sets metadata", func() { + buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ + ID: "bp.1.id", + Version: "bp.1.version", + }, + Stacks: []dist.Stack{ + {ID: "stack.id.1"}, + {ID: "stack.id.2"}, + }, + Order: nil, + }, 0644) + h.AssertNil(t, err) + + subject.SetBuildpack(buildpack1) - buildpackExists := func(name, version string) { - t.Helper() - dirPath := fmt.Sprintf("/cnb/buildpacks/%s/%s", name, version) - layerTar, err := fakePackageImage.FindLayerWithPath(dirPath) + packageImage, err := subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) h.AssertNil(t, err) - h.AssertOnTarEntry(t, layerTar, dirPath, - h.IsDirectory(), - ) + labelData, err := packageImage.Label("io.buildpacks.buildpackage.metadata") + h.AssertNil(t, err) + var md buildpackage.Metadata + h.AssertNil(t, json.Unmarshal([]byte(labelData), &md)) + + h.AssertEq(t, md.ID, "bp.1.id") + h.AssertEq(t, md.Version, "bp.1.version") + h.AssertEq(t, len(md.Stacks), 2) + h.AssertEq(t, md.Stacks[0].ID, "stack.id.1") + h.AssertEq(t, md.Stacks[1].ID, "stack.id.2") + }) - h.AssertOnTarEntry(t, layerTar, dirPath+"/bin/build", - h.ContentEquals("build-contents"), - h.HasOwnerAndGroup(0, 0), - h.HasFileMode(0644), - ) + it("sets buildpack layers label", func() { + buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ID: "bp.1.id", Version: "bp.1.version"}, + Stacks: []dist.Stack{{ID: "stack.id.1"}, {ID: "stack.id.2"}}, + Order: nil, + }, 0644) + h.AssertNil(t, err) + subject.SetBuildpack(buildpack1) - h.AssertOnTarEntry(t, layerTar, dirPath+"/bin/detect", - h.ContentEquals("detect-contents"), - h.HasOwnerAndGroup(0, 0), - h.HasFileMode(0644), - ) - } + image, err := subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) + h.AssertNil(t, err) + + var bpLayers dist.BuildpackLayers + _, err = dist.GetLabel(image, "io.buildpacks.buildpack.layers", &bpLayers) + h.AssertNil(t, err) + + bp1Info, ok1 := bpLayers["bp.1.id"]["bp.1.version"] + h.AssertEq(t, ok1, true) + h.AssertEq(t, bp1Info.Stacks, []dist.Stack{{ID: "stack.id.1"}, {ID: "stack.id.2"}}) + }) + + it("adds buildpack layers", func() { + buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{ + API: api.MustParse("0.2"), + Info: dist.BuildpackInfo{ID: "bp.1.id", Version: "bp.1.version"}, + Stacks: []dist.Stack{{ID: "stack.id.1"}, {ID: "stack.id.2"}}, + Order: nil, + }, 0644) + h.AssertNil(t, err) + subject.SetBuildpack(buildpack1) + + packageImage, err := subject.SaveAsImage(context.TODO(), "some/package", false, config.PullAlways) + h.AssertNil(t, err) + + buildpackExists := func(name, version string) { + t.Helper() + dirPath := fmt.Sprintf("/cnb/buildpacks/%s/%s", name, version) + fakePackageImage := packageImage.(*fakes.Image) + layerTar, err := fakePackageImage.FindLayerWithPath(dirPath) + h.AssertNil(t, err) + + h.AssertOnTarEntry(t, layerTar, dirPath, + h.IsDirectory(), + ) + + h.AssertOnTarEntry(t, layerTar, dirPath+"/bin/build", + h.ContentEquals("build-contents"), + h.HasOwnerAndGroup(0, 0), + h.HasFileMode(0644), + ) - buildpackExists("bp.1.id", "bp.1.version") + h.AssertOnTarEntry(t, layerTar, dirPath+"/bin/detect", + h.ContentEquals("detect-contents"), + h.HasOwnerAndGroup(0, 0), + h.HasFileMode(0644), + ) + } + + buildpackExists("bp.1.id", "bp.1.version") + }) }) }) diff --git a/internal/layer/writer_factory.go b/internal/layer/writer_factory.go index 4faa116531..72d9c86948 100644 --- a/internal/layer/writer_factory.go +++ b/internal/layer/writer_factory.go @@ -2,9 +2,9 @@ package layer import ( "archive/tar" + "fmt" "io" - "github.com/buildpacks/imgutil" ilayer "github.com/buildpacks/imgutil/layer" "github.com/buildpacks/pack/internal/archive" @@ -14,13 +14,12 @@ type WriterFactory struct { os string } -func NewWriterFactory(image imgutil.Image) (*WriterFactory, error) { - os, err := image.OS() - if err != nil { - return nil, err +func NewWriterFactory(imageOS string) (*WriterFactory, error) { + if imageOS != "linux" && imageOS != "windows" { + return nil, fmt.Errorf("provided image OS '%s' must be either 'linux' or 'windows'", imageOS) } - return &WriterFactory{os: os}, nil + return &WriterFactory{os: imageOS}, nil } func (f *WriterFactory) NewWriter(fileWriter io.Writer) archive.TarWriter { diff --git a/internal/layer/writer_factory_test.go b/internal/layer/writer_factory_test.go index df1074702a..af9c5cfc59 100644 --- a/internal/layer/writer_factory_test.go +++ b/internal/layer/writer_factory_test.go @@ -4,7 +4,6 @@ import ( "archive/tar" "testing" - "github.com/buildpacks/imgutil/fakes" ilayer "github.com/buildpacks/imgutil/layer" "github.com/sclevine/spec" "github.com/sclevine/spec/report" @@ -18,11 +17,16 @@ func TestTarWriterFactory(t *testing.T) { } func testWriterFactory(t *testing.T, when spec.G, it spec.S) { + when("#NewWriterFactory", func() { + it("returns an error for invalid image OS", func() { + _, err := layer.NewWriterFactory("not-an-os") + h.AssertError(t, err, "provided image OS 'not-an-os' must be either 'linux' or 'windows'") + }) + }) + when("#NewWriter", func() { - it("returns a regular tar writer for posix-based images", func() { - image := fakes.NewImage("fake-image", "", nil) - image.SetPlatform("linux", "", "") - factory, err := layer.NewWriterFactory(image) + it("returns a regular tar writer for Linux", func() { + factory, err := layer.NewWriterFactory("linux") h.AssertNil(t, err) _, ok := factory.NewWriter(nil).(*tar.Writer) @@ -31,10 +35,8 @@ func testWriterFactory(t *testing.T, when spec.G, it spec.S) { } }) - it("returns a Windows layer writer for Windows-based images", func() { - image := fakes.NewImage("fake-image", "", nil) - image.SetPlatform("windows", "", "") - factory, err := layer.NewWriterFactory(image) + it("returns a Windows layer writer for Windows", func() { + factory, err := layer.NewWriterFactory("windows") h.AssertNil(t, err) _, ok := factory.NewWriter(nil).(*ilayer.WindowsWriter) diff --git a/package_buildpack.go b/package_buildpack.go index 20b908c446..0abcd9706d 100644 --- a/package_buildpack.go +++ b/package_buildpack.go @@ -4,11 +4,11 @@ import ( "context" "github.com/buildpacks/pack/config" + "github.com/buildpacks/pack/internal/layer" "github.com/pkg/errors" pubbldpkg "github.com/buildpacks/pack/buildpackage" - "github.com/buildpacks/pack/internal/archive" "github.com/buildpacks/pack/internal/buildpackage" "github.com/buildpacks/pack/internal/dist" "github.com/buildpacks/pack/internal/style" @@ -30,7 +30,17 @@ type PackageBuildpackOptions struct { // PackageBuildpack packages buildpack(s) into an image or file func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOptions) error { - packageBuilder := buildpackage.NewBuilder(c.imageFactory) + info, err := c.docker.Info(ctx) + if err != nil { + return errors.Wrap(err, "getting docker info") + } + + writerFactory, err := layer.NewWriterFactory(info.OSType) + if err != nil { + return errors.Wrap(err, "creating layer writer factory") + } + + packageBuilder := buildpackage.NewBuilder(info.OSType, c.imageFactory, c.imageFetcher) if opts.Format == "" { opts.Format = FormatImage @@ -46,7 +56,7 @@ func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOpti return errors.Wrapf(err, "downloading buildpack from %s", style.Symbol(bpURI)) } - bp, err := dist.BuildpackFromRootBlob(blob, archive.DefaultTarWriterFactory()) + bp, err := dist.BuildpackFromRootBlob(blob, writerFactory) if err != nil { return errors.Wrapf(err, "creating buildpack from %s", style.Symbol(bpURI)) } @@ -75,7 +85,7 @@ func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOpti depBPs = append([]dist.Buildpack{mainBP}, deps...) } else { - depBP, err := dist.BuildpackFromRootBlob(blob, archive.DefaultTarWriterFactory()) + depBP, err := dist.BuildpackFromRootBlob(blob, writerFactory) if err != nil { return errors.Wrapf(err, "creating buildpack from %s", style.Symbol(dep.URI)) } @@ -99,7 +109,7 @@ func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOpti case FormatFile: return packageBuilder.SaveAsFile(opts.Name) case FormatImage: - _, err = packageBuilder.SaveAsImage(opts.Name, opts.Publish) + _, err = packageBuilder.SaveAsImage(ctx, opts.Name, opts.Publish, opts.PullPolicy) return errors.Wrapf(err, "saving image") default: return errors.Errorf("unknown format: %s", style.Symbol(opts.Format)) diff --git a/package_buildpack_test.go b/package_buildpack_test.go index 99b8726e62..8c634b682d 100644 --- a/package_buildpack_test.go +++ b/package_buildpack_test.go @@ -9,6 +9,8 @@ import ( "path/filepath" "testing" + "github.com/docker/docker/api/types" + "github.com/buildpacks/pack/config" "github.com/buildpacks/imgutil" @@ -44,6 +46,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { mockDownloader *testmocks.MockDownloader mockImageFactory *testmocks.MockImageFactory mockImageFetcher *testmocks.MockImageFetcher + mockDockerClient *testmocks.MockCommonAPIClient out bytes.Buffer ) @@ -52,6 +55,7 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { mockDownloader = testmocks.NewMockDownloader(mockController) mockImageFactory = testmocks.NewMockImageFactory(mockController) mockImageFetcher = testmocks.NewMockImageFetcher(mockController) + mockDockerClient = testmocks.NewMockCommonAPIClient(mockController) var err error subject, err = pack.NewClient( @@ -59,8 +63,11 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { pack.WithDownloader(mockDownloader), pack.WithImageFactory(mockImageFactory), pack.WithFetcher(mockImageFetcher), + pack.WithDockerClient(mockDockerClient), ) h.AssertNil(t, err) + + mockDockerClient.EXPECT().Info(context.TODO()).Return(types.Info{OSType: "linux"}, nil).AnyTimes() }) it.After(func() {