From 0bba81c160767a44d9e030e25003d3f65c50d9b7 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Tue, 9 Apr 2024 13:03:45 -0400 Subject: [PATCH 1/4] Fix https://github.com/buildpacks/pack/issues/2120 Signed-off-by: Natalie Arellano --- internal/commands/buildpack_new_test.go | 29 ++++++++++--------- internal/layer/writer_factory_test.go | 2 +- internal/target/parse.go | 11 +++++-- internal/target/parse_test.go | 38 ++++++++++++++++--------- pkg/dist/buildmodule.go | 6 ++-- pkg/dist/buildpack_descriptor.go | 33 +++++++++------------ pkg/dist/buildpack_descriptor_test.go | 18 ++++++------ 7 files changed, 75 insertions(+), 62 deletions(-) diff --git a/internal/commands/buildpack_new_test.go b/internal/commands/buildpack_new_test.go index 4a988672df..05a76f42b4 100644 --- a/internal/commands/buildpack_new_test.go +++ b/internal/commands/buildpack_new_test.go @@ -97,14 +97,14 @@ func testBuildpackNewCommand(t *testing.T, when spec.G, it spec.S) { Arch: "arm", ArchVariant: "v6", Distributions: []dist.Distribution{{ - Name: "ubuntu", - Versions: []string{"14.04", "16.04"}, + Name: "ubuntu", + Version: "14.04", }}, }}, }).Return(nil).MaxTimes(1) path := filepath.Join(tmpDir, "targets") - command.SetArgs([]string{"--path", path, "example/targets", "--targets", "linux/arm/v6:ubuntu@14.04@16.04"}) + command.SetArgs([]string{"--path", path, "example/targets", "--targets", "linux/arm/v6:ubuntu@14.04"}) err := command.Execute() h.AssertNil(t, err) @@ -120,8 +120,11 @@ func testBuildpackNewCommand(t *testing.T, when spec.G, it spec.S) { Arch: "arm", ArchVariant: "v6", Distributions: []dist.Distribution{{ - Name: "ubuntu", - Versions: []string{"14.04", "16.04"}, + Name: "ubuntu", + Version: "14.04", + }, { + Name: "ubuntu", + Version: "16.04", }}, }}, }).Return(nil).MaxTimes(1) @@ -133,7 +136,7 @@ func testBuildpackNewCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNotNil(t, err) }) when("it should", func() { - it("support format [os][/arch][/variant]:[name@version@version2];[some-name@version@version2]", func() { + it("support format [os][/arch][/variant]:[name@version];[some-name@version]", func() { mockClient.EXPECT().NewBuildpack(gomock.Any(), client.NewBuildpackOptions{ API: "0.8", ID: "example/targets", @@ -146,12 +149,12 @@ func testBuildpackNewCommand(t *testing.T, when spec.G, it spec.S) { ArchVariant: "v6", Distributions: []dist.Distribution{ { - Name: "ubuntu", - Versions: []string{"14.04", "16.04"}, + Name: "ubuntu", + Version: "14.04", }, { - Name: "debian", - Versions: []string{"8.10", "10.9"}, + Name: "debian", + Version: "8.10", }, }, }, @@ -160,8 +163,8 @@ func testBuildpackNewCommand(t *testing.T, when spec.G, it spec.S) { Arch: "amd64", Distributions: []dist.Distribution{ { - Name: "windows-nano", - Versions: []string{"10.0.19041.1415"}, + Name: "windows-nano", + Version: "10.0.19041.1415", }, }, }, @@ -169,7 +172,7 @@ func testBuildpackNewCommand(t *testing.T, when spec.G, it spec.S) { }).Return(nil).MaxTimes(1) path := filepath.Join(tmpDir, "targets") - command.SetArgs([]string{"--path", path, "example/targets", "--targets", "linux/arm/v6:ubuntu@14.04@16.04;debian@8.10@10.9", "-t", "windows/amd64:windows-nano@10.0.19041.1415"}) + command.SetArgs([]string{"--path", path, "example/targets", "--targets", "linux/arm/v6:ubuntu@14.04;debian@8.10", "-t", "windows/amd64:windows-nano@10.0.19041.1415"}) err := command.Execute() h.AssertNil(t, err) diff --git a/internal/layer/writer_factory_test.go b/internal/layer/writer_factory_test.go index af9c5cfc59..d39dff77a2 100644 --- a/internal/layer/writer_factory_test.go +++ b/internal/layer/writer_factory_test.go @@ -1,7 +1,7 @@ package layer_test import ( - "archive/tar" + "archive/tar" //nolint "testing" ilayer "github.com/buildpacks/imgutil/layer" diff --git a/internal/target/parse.go b/internal/target/parse.go index f5ea955892..61a6f4da05 100644 --- a/internal/target/parse.go +++ b/internal/target/parse.go @@ -1,6 +1,7 @@ package target import ( + "fmt" "strings" "github.com/pkg/errors" @@ -66,11 +67,15 @@ func ParseDistro(distroString string, logger logging.Logger) (distro dist.Distri if d[0] == "" || len(d) == 0 { return distro, errors.Errorf("distro's versions %s cannot be specified without distro's name", style.Symbol("@"+strings.Join(d[1:], "@"))) } - if len(d) <= 2 && (strings.Contains(strings.Join(d[1:], ""), "") || d[1] == "") { + distro.Name = d[0] + if len(d) < 2 { logger.Warnf("distro with name %s has no specific version!", style.Symbol(d[0])) + return distro, err } - distro.Name = d[0] - distro.Versions = d[1:] + if len(d) > 2 { + return distro, fmt.Errorf("invalid distro: %s", distroString) + } + distro.Version = d[1] return distro, err } diff --git a/internal/target/parse_test.go b/internal/target/parse_test.go index 030f2c772d..f00c095fb3 100644 --- a/internal/target/parse_test.go +++ b/internal/target/parse_test.go @@ -67,7 +67,7 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { h.AssertNotNil(t, err) }) it("should parse targets as expected", func() { - output, err := target.ParseTargets([]string{"linux/arm/v6", "linux/amd64:ubuntu@22.04;debian@8.10@10.06"}, logging.NewLogWithWriters(&outBuf, &outBuf)) + output, err := target.ParseTargets([]string{"linux/arm/v6", "linux/amd64:ubuntu@22.04;debian@8.10;debian@10.06"}, logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertNil(t, err) h.AssertEq(t, output, []dist.Target{ { @@ -80,12 +80,16 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { Arch: "amd64", Distributions: []dist.Distribution{ { - Name: "ubuntu", - Versions: []string{"22.04"}, + Name: "ubuntu", + Version: "22.04", }, { - Name: "debian", - Versions: []string{"8.10", "10.06"}, + Name: "debian", + Version: "8.10", + }, + { + Name: "debian", + Version: "10.06", }, }, }, @@ -94,10 +98,10 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { }) when("target#ParseDistro", func() { it("should parse distro as expected", func() { - output, err := target.ParseDistro("ubuntu@22.04@20.08", logging.NewLogWithWriters(&outBuf, &outBuf)) + output, err := target.ParseDistro("ubuntu@22.04", logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertEq(t, output, dist.Distribution{ - Name: "ubuntu", - Versions: []string{"22.04", "20.08"}, + Name: "ubuntu", + Version: "22.04", }) h.AssertNil(t, err) }) @@ -112,15 +116,23 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { }) when("target#ParseDistros", func() { it("should parse distros as expected", func() { - output, err := target.ParseDistros("ubuntu@22.04@20.08;debian@8.10@10.06", logging.NewLogWithWriters(&outBuf, &outBuf)) + output, err := target.ParseDistros("ubuntu@22.04;ubuntu@20.08;debian@8.10;debian@10.06", logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertEq(t, output, []dist.Distribution{ { - Name: "ubuntu", - Versions: []string{"22.04", "20.08"}, + Name: "ubuntu", + Version: "22.04", + }, + { + Name: "ubuntu", + Version: "20.08", + }, + { + Name: "debian", + Version: "8.10", }, { - Name: "debian", - Versions: []string{"8.10", "10.06"}, + Name: "debian", + Version: "10.06", }, }) h.AssertNil(t, err) diff --git a/pkg/dist/buildmodule.go b/pkg/dist/buildmodule.go index 5126d988e0..ea34cb68ca 100644 --- a/pkg/dist/buildmodule.go +++ b/pkg/dist/buildmodule.go @@ -56,10 +56,10 @@ type Target struct { OS string `json:"os" toml:"os"` Arch string `json:"arch" toml:"arch"` ArchVariant string `json:"variant,omitempty" toml:"variant,omitempty"` - Distributions []Distribution `json:"distributions,omitempty" toml:"distributions,omitempty"` + Distributions []Distribution `json:"distros,omitempty" toml:"distros,omitempty"` } type Distribution struct { - Name string `json:"name,omitempty" toml:"name,omitempty"` - Versions []string `json:"versions,omitempty" toml:"versions,omitempty"` + Name string `json:"name,omitempty" toml:"name,omitempty"` + Version string `json:"version,omitempty" toml:"version,omitempty"` } diff --git a/pkg/dist/buildpack_descriptor.go b/pkg/dist/buildpack_descriptor.go index cf93daa2a2..77ed9028d4 100644 --- a/pkg/dist/buildpack_descriptor.go +++ b/pkg/dist/buildpack_descriptor.go @@ -54,32 +54,25 @@ func (b *BuildpackDescriptor) EnsureStackSupport(stackID string, providedMixins return nil } -func (b *BuildpackDescriptor) EnsureTargetSupport(os, arch, distroName, distroVersion string) error { +func (b *BuildpackDescriptor) EnsureTargetSupport(givenOS, givenArch, givenDistroName, givenDistroVersion string) error { if len(b.Targets()) == 0 { if (!b.WithLinuxBuild && !b.WithWindowsBuild) || len(b.Stacks()) > 0 { // nolint return nil // Order buildpack or stack buildpack, no validation required - } else if b.WithLinuxBuild && os == DefaultTargetOSLinux && arch == DefaultTargetArch { + } else if b.WithLinuxBuild && givenOS == DefaultTargetOSLinux && givenArch == DefaultTargetArch { return nil - } else if b.WithWindowsBuild && os == DefaultTargetOSWindows && arch == DefaultTargetArch { + } else if b.WithWindowsBuild && givenOS == DefaultTargetOSWindows && givenArch == DefaultTargetArch { return nil } } - for _, target := range b.Targets() { - if target.OS == os { - if target.Arch == "" || arch == "" || target.Arch == arch { - if len(target.Distributions) == 0 || distroName == "" || distroVersion == "" { + for _, bpTarget := range b.Targets() { + if bpTarget.OS == givenOS { + if bpTarget.Arch == "" || givenArch == "" || bpTarget.Arch == givenArch { + if len(bpTarget.Distributions) == 0 || givenDistroName == "" || givenDistroVersion == "" { return nil } - for _, distro := range target.Distributions { - if distro.Name == distroName { - if len(distro.Versions) == 0 { - return nil - } - for _, version := range distro.Versions { - if version == distroVersion { - return nil - } - } + for _, bpDistro := range bpTarget.Distributions { + if bpDistro.Name == givenDistroName && bpDistro.Version == givenDistroVersion { + return nil } } } @@ -97,9 +90,9 @@ func (b *BuildpackDescriptor) EnsureTargetSupport(os, arch, distroName, distroVe return fmt.Errorf( "unable to satisfy target os/arch constraints; build image: %s, buildpack %s: %s", toJSONMaybe(target{ - OS: os, - Arch: arch, - Distribution: osDistribution{Name: distroName, Version: distroVersion}, + OS: givenOS, + Arch: givenArch, + Distribution: osDistribution{Name: givenDistroName, Version: givenDistroVersion}, }), style.Symbol(b.Info().FullName()), toJSONMaybe(b.Targets()), diff --git a/pkg/dist/buildpack_descriptor_test.go b/pkg/dist/buildpack_descriptor_test.go index 1f755edb5d..99f2e91124 100644 --- a/pkg/dist/buildpack_descriptor_test.go +++ b/pkg/dist/buildpack_descriptor_test.go @@ -221,12 +221,12 @@ func testBuildpackDescriptor(t *testing.T, when spec.G, it spec.S) { Arch: "fake-arch", Distributions: []dist.Distribution{ { - Name: "fake-distro", - Versions: []string{"0.1"}, + Name: "fake-distro", + Version: "0.1", }, { - Name: "another-distro", - Versions: []string{"0.22"}, + Name: "another-distro", + Version: "0.22", }, }, }}, @@ -247,12 +247,12 @@ func testBuildpackDescriptor(t *testing.T, when spec.G, it spec.S) { Arch: "fake-arch", Distributions: []dist.Distribution{ { - Name: "fake-distro", - Versions: []string{"0.1"}, + Name: "fake-distro", + Version: "0.1", }, { - Name: "another-distro", - Versions: []string{"0.22"}, + Name: "another-distro", + Version: "0.22", }, }, }}, @@ -260,7 +260,7 @@ func testBuildpackDescriptor(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, bp.EnsureStackSupport("some.stack.id", []string{}, true)) h.AssertError(t, bp.EnsureTargetSupport("some-other-os", "fake-arch", "fake-distro", "0.0"), - `unable to satisfy target os/arch constraints; build image: {"os":"some-other-os","arch":"fake-arch","distribution":{"name":"fake-distro","version":"0.0"}}, buildpack 'some.buildpack.id@some.buildpack.version': [{"os":"fake-os","arch":"fake-arch","distributions":[{"name":"fake-distro","versions":["0.1"]},{"name":"another-distro","versions":["0.22"]}]}]`) + `unable to satisfy target os/arch constraints; build image: {"os":"some-other-os","arch":"fake-arch","distribution":{"name":"fake-distro","version":"0.0"}}, buildpack 'some.buildpack.id@some.buildpack.version': [{"os":"fake-os","arch":"fake-arch","distros":[{"name":"fake-distro","version":"0.1"},{"name":"another-distro","version":"0.22"}]}]`) }) it("succeeds with missing arch", func() { From b9cccb713e19b2adc28eb9905be46db9f23f1f24 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 19 Apr 2024 15:04:33 -0400 Subject: [PATCH 2/4] Add warning when we detect wrong things in buildpack.toml or extension.toml Signed-off-by: Natalie Arellano --- pkg/buildpack/buildpack.go | 25 ++- pkg/buildpack/buildpack_test.go | 252 +++++++++++---------------- pkg/buildpack/downloader.go | 8 +- pkg/client/create_builder_test.go | 28 +-- pkg/client/package_buildpack.go | 2 +- pkg/client/package_buildpack_test.go | 8 +- pkg/client/package_extension.go | 2 +- 7 files changed, 145 insertions(+), 180 deletions(-) diff --git a/pkg/buildpack/buildpack.go b/pkg/buildpack/buildpack.go index e658c53e51..9c8e115451 100644 --- a/pkg/buildpack/buildpack.go +++ b/pkg/buildpack/buildpack.go @@ -71,12 +71,16 @@ func FromBlob(descriptor Descriptor, blob Blob) BuildModule { // FromBuildpackRootBlob constructs a buildpack from a blob. It is assumed that the buildpack contents reside at the // root of the blob. The constructed buildpack contents will be structured as per the distribution spec (currently // a tar with contents under '/cnb/buildpacks/{ID}/{version}/*'). -func FromBuildpackRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactory) (BuildModule, error) { +func FromBuildpackRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactory, logger Logger) (BuildModule, error) { descriptor := dist.BuildpackDescriptor{} descriptor.WithAPI = api.MustParse(dist.AssumedBuildpackAPIVersion) - if err := readDescriptor(KindBuildpack, &descriptor, blob); err != nil { + var undecodedKeys []string + if err := readDescriptor(KindBuildpack, &descriptor, blob, undecodedKeys); err != nil { return nil, err } + if len(undecodedKeys) > 0 { + logger.Warnf("Ignoring unexpected key(s) in buildpack descriptor: %s", strings.Join(undecodedKeys, ",")) + } if err := detectPlatformSpecificValues(&descriptor, blob); err != nil { return nil, err } @@ -89,19 +93,23 @@ func FromBuildpackRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactor // FromExtensionRootBlob constructs an extension from a blob. It is assumed that the extension contents reside at the // root of the blob. The constructed extension contents will be structured as per the distribution spec (currently // a tar with contents under '/cnb/extensions/{ID}/{version}/*'). -func FromExtensionRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactory) (BuildModule, error) { +func FromExtensionRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactory, logger Logger) (BuildModule, error) { descriptor := dist.ExtensionDescriptor{} descriptor.WithAPI = api.MustParse(dist.AssumedBuildpackAPIVersion) - if err := readDescriptor(KindExtension, &descriptor, blob); err != nil { + var undecodedKeys []string + if err := readDescriptor(KindExtension, &descriptor, blob, undecodedKeys); err != nil { return nil, err } + if len(undecodedKeys) > 0 { + logger.Warnf("Ignoring unexpected key(s) in extension descriptor: %s", strings.Join(undecodedKeys, ",")) + } if err := validateExtensionDescriptor(descriptor); err != nil { return nil, err } return buildpackFrom(&descriptor, blob, layerWriterFactory) } -func readDescriptor(kind string, descriptor interface{}, blob Blob) error { +func readDescriptor(kind string, descriptor interface{}, blob Blob, undecoded []string) error { rc, err := blob.Open() if err != nil { return errors.Wrapf(err, "open %s", kind) @@ -115,11 +123,16 @@ func readDescriptor(kind string, descriptor interface{}, blob Blob) error { return errors.Wrapf(err, "reading %s", descriptorFile) } - _, err = toml.Decode(string(buf), descriptor) + md, err := toml.Decode(string(buf), descriptor) if err != nil { return errors.Wrapf(err, "decoding %s", descriptorFile) } + undecodedKeys := md.Undecoded() + for _, k := range undecodedKeys { + undecoded = append(undecoded, k.String()) + } + return nil } diff --git a/pkg/buildpack/buildpack_test.go b/pkg/buildpack/buildpack_test.go index 4536a5e79e..b44bbc94e9 100644 --- a/pkg/buildpack/buildpack_test.go +++ b/pkg/buildpack/buildpack_test.go @@ -54,11 +54,10 @@ func testBuildpack(t *testing.T, when spec.G, it spec.S) { when("#BuildpackFromRootBlob", func() { it("parses the descriptor file", func() { - bp, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` + bp, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` api = "0.3" [buildpack] @@ -69,11 +68,9 @@ homepage = "http://geocities.com/cool-bp" [[stacks]] id = "some.stack.id" `)) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) h.AssertEq(t, bp.Descriptor().API().String(), "0.3") @@ -84,11 +81,10 @@ id = "some.stack.id" }) it("translates blob to distribution format", func() { - bp, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` + bp, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` api = "0.3" [buildpack] @@ -99,14 +95,12 @@ version = "1.2.3" id = "some.stack.id" `)) - tarBuilder.AddDir("bin", 0700, time.Now()) - tarBuilder.AddFile("bin/detect", 0700, time.Now(), []byte("detect-contents")) - tarBuilder.AddFile("bin/build", 0700, time.Now(), []byte("build-contents")) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + tarBuilder.AddDir("bin", 0700, time.Now()) + tarBuilder.AddFile("bin/detect", 0700, time.Now(), []byte("detect-contents")) + tarBuilder.AddFile("bin/build", 0700, time.Now(), []byte("build-contents")) + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) h.AssertNil(t, bp.Descriptor().EnsureTargetSupport(dist.DefaultTargetOSLinux, dist.DefaultTargetArch, "", "")) @@ -151,11 +145,10 @@ id = "some.stack.id" }) it("translates blob to windows bat distribution format", func() { - bp, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` + bp, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` api = "0.9" [buildpack] @@ -163,14 +156,12 @@ id = "bp.one" version = "1.2.3" `)) - tarBuilder.AddDir("bin", 0700, time.Now()) - tarBuilder.AddFile("bin/detect", 0700, time.Now(), []byte("detect-contents")) - tarBuilder.AddFile("bin/build.bat", 0700, time.Now(), []byte("build-contents")) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + tarBuilder.AddDir("bin", 0700, time.Now()) + tarBuilder.AddFile("bin/detect", 0700, time.Now(), []byte("detect-contents")) + tarBuilder.AddFile("bin/build.bat", 0700, time.Now(), []byte("build-contents")) + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) bpDescriptor := bp.Descriptor().(*dist.BuildpackDescriptor) @@ -189,11 +180,10 @@ version = "1.2.3" }) it("translates blob to windows exe distribution format", func() { - bp, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` + bp, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` api = "0.3" [buildpack] @@ -201,14 +191,12 @@ id = "bp.one" version = "1.2.3" `)) - tarBuilder.AddDir("bin", 0700, time.Now()) - tarBuilder.AddFile("bin/detect", 0700, time.Now(), []byte("detect-contents")) - tarBuilder.AddFile("bin/build.exe", 0700, time.Now(), []byte("build-contents")) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + tarBuilder.AddDir("bin", 0700, time.Now()) + tarBuilder.AddFile("bin/detect", 0700, time.Now(), []byte("detect-contents")) + tarBuilder.AddFile("bin/build.exe", 0700, time.Now(), []byte("build-contents")) + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) bpDescriptor := bp.Descriptor().(*dist.BuildpackDescriptor) @@ -244,13 +232,10 @@ id = "some.stack.id" }, } - bp, err := buildpack.FromBuildpackRootBlob( - &errorBlob{ - realBlob: realBlob, - limit: 4, - }, - archive.DefaultTarWriterFactory(), - ) + bp, err := buildpack.FromBuildpackRootBlob(&errorBlob{ + realBlob: realBlob, + limit: 4, + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) bpReader, err := bp.Open() @@ -274,17 +259,14 @@ id = "some.stack.id" when("no exec bits set", func() { it("sets to 0755 if directory", func() { - bp, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(bpTOMLData)) - tarBuilder.AddDir("some-dir", 0600, time.Now()) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + bp, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(bpTOMLData)) + tarBuilder.AddDir("some-dir", 0600, time.Now()) + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) tarPath := writeBlobToFile(bp) @@ -299,18 +281,15 @@ id = "some.stack.id" when("no exec bits set", func() { it("sets to 0755 if 'bin/detect' or 'bin/build'", func() { - bp, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(bpTOMLData)) - tarBuilder.AddFile("bin/detect", 0600, time.Now(), []byte("detect-contents")) - tarBuilder.AddFile("bin/build", 0600, time.Now(), []byte("build-contents")) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + bp, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(bpTOMLData)) + tarBuilder.AddFile("bin/detect", 0600, time.Now(), []byte("detect-contents")) + tarBuilder.AddFile("bin/build", 0600, time.Now(), []byte("build-contents")) + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) bpDescriptor := bp.Descriptor().(*dist.BuildpackDescriptor) @@ -334,17 +313,14 @@ id = "some.stack.id" when("not directory, 'bin/detect', or 'bin/build'", func() { it("sets to 0755 if ANY exec bit is set", func() { - bp, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(bpTOMLData)) - tarBuilder.AddFile("some-file", 0700, time.Now(), []byte("some-data")) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + bp, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(bpTOMLData)) + tarBuilder.AddFile("some-file", 0700, time.Now(), []byte("some-data")) + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) tarPath := writeBlobToFile(bp) @@ -359,17 +335,14 @@ id = "some.stack.id" when("not directory, 'bin/detect', or 'bin/build'", func() { it("sets to 0644 if NO exec bits set", func() { - bp, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(bpTOMLData)) - tarBuilder.AddFile("some-file", 0600, time.Now(), []byte("some-data")) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + bp, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(bpTOMLData)) + tarBuilder.AddFile("some-file", 0600, time.Now(), []byte("some-data")) + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) tarPath := writeBlobToFile(bp) @@ -385,37 +358,31 @@ id = "some.stack.id" when("there is no descriptor file", func() { it("returns error", func() { - _, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + _, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertError(t, err, "could not find entry path 'buildpack.toml'") }) }) when("there is no api field", func() { it("assumes an api version", func() { - bp, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` + bp, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` [buildpack] id = "bp.one" version = "1.2.3" [[stacks]] id = "some.stack.id"`)) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) h.AssertEq(t, bp.Descriptor().API().String(), "0.1") }) @@ -423,55 +390,48 @@ id = "some.stack.id"`)) when("there is no id", func() { it("returns error", func() { - _, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` + _, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` [buildpack] id = "" version = "1.2.3" [[stacks]] id = "some.stack.id"`)) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertError(t, err, "'buildpack.id' is required") }) }) when("there is no version", func() { it("returns error", func() { - _, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` + _, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` [buildpack] id = "bp.one" version = "" [[stacks]] id = "some.stack.id"`)) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertError(t, err, "'buildpack.version' is required") }) }) when("both stacks and order are present", func() { it("returns error", func() { - _, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` + _, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` [buildpack] id = "bp.one" version = "1.2.3" @@ -484,31 +444,26 @@ id = "some.stack.id" id = "bp.nested" version = "bp.nested.version" `)) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertError(t, err, "cannot have both 'targets'/'stacks' and an 'order' defined") }) }) when("missing stacks and order", func() { it("does not return an error", func() { - _, err := buildpack.FromBuildpackRootBlob( - &readerBlob{ - openFn: func() io.ReadCloser { - tarBuilder := archive.TarBuilder{} - tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` + _, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` [buildpack] id = "bp.one" version = "1.2.3" `)) - return tarBuilder.Reader(archive.DefaultTarWriterFactory()) - }, + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) }, - archive.DefaultTarWriterFactory(), - ) + }, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) }) }) @@ -528,10 +483,7 @@ version = "1.2.3" }) it("hardlink is preserved in the output tar file", func() { - bp, err := buildpack.FromBuildpackRootBlob( - blob.NewBlob(bpRootFolder), - archive.DefaultTarWriterFactory(), - ) + bp, err := buildpack.FromBuildpackRootBlob(blob.NewBlob(bpRootFolder), archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) tarPath := writeBlobToFile(bp) diff --git a/pkg/buildpack/downloader.go b/pkg/buildpack/downloader.go index 454afda0cb..d3a5eaea2c 100644 --- a/pkg/buildpack/downloader.go +++ b/pkg/buildpack/downloader.go @@ -141,7 +141,7 @@ func (c *buildpackDownloader) Download(ctx context.Context, moduleURI string, op return nil, nil, errors.Wrapf(err, "downloading %s from %s", kind, style.Symbol(moduleURI)) } - mainBP, depBPs, err = decomposeBlob(blob, kind, opts.ImageOS) + mainBP, depBPs, err = decomposeBlob(blob, kind, opts.ImageOS, c.logger) if err != nil { return nil, nil, errors.Wrapf(err, "extracting from %s", style.Symbol(moduleURI)) } @@ -153,7 +153,7 @@ func (c *buildpackDownloader) Download(ctx context.Context, moduleURI string, op // decomposeBlob decomposes a buildpack or extension blob into the main module (order buildpack or extension) and // (for buildpack blobs) its dependent buildpacks. -func decomposeBlob(blob blob.Blob, kind string, imageOS string) (mainModule BuildModule, depModules []BuildModule, err error) { +func decomposeBlob(blob blob.Blob, kind string, imageOS string, logger Logger) (mainModule BuildModule, depModules []BuildModule, err error) { isOCILayout, err := IsOCILayoutBlob(blob) if err != nil { return mainModule, depModules, errors.Wrapf(err, "inspecting %s blob", kind) @@ -171,9 +171,9 @@ func decomposeBlob(blob blob.Blob, kind string, imageOS string) (mainModule Buil } if kind == KindExtension { - mainModule, err = FromExtensionRootBlob(blob, layerWriterFactory) + mainModule, err = FromExtensionRootBlob(blob, layerWriterFactory, logger) } else { - mainModule, err = FromBuildpackRootBlob(blob, layerWriterFactory) + mainModule, err = FromBuildpackRootBlob(blob, layerWriterFactory, logger) } if err != nil { return mainModule, depModules, errors.Wrapf(err, "reading %s", kind) diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index 7c51fa3090..88e888d9c6 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -112,10 +112,10 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { mockDownloader.EXPECT().Download(gomock.Any(), "file:///some-lifecycle").Return(blob.NewBlob(filepath.Join("testdata", "lifecycle", "platform-0.4")), nil).AnyTimes() mockDownloader.EXPECT().Download(gomock.Any(), "file:///some-lifecycle-platform-0-1").Return(blob.NewBlob(filepath.Join("testdata", "lifecycle-platform-0.1")), nil).AnyTimes() - bp, err := buildpack.FromBuildpackRootBlob(exampleBuildpackBlob, archive.DefaultTarWriterFactory()) + bp, err := buildpack.FromBuildpackRootBlob(exampleBuildpackBlob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/bp-one.tgz", gomock.Any()).Return(bp, nil, nil).AnyTimes() - ext, err := buildpack.FromExtensionRootBlob(exampleExtensionBlob, archive.DefaultTarWriterFactory()) + ext, err := buildpack.FromExtensionRootBlob(exampleExtensionBlob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/ext-one.tgz", gomock.Any()).Return(ext, nil, nil).AnyTimes() @@ -776,12 +776,12 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { opts.Config.Extensions[0].URI = "https://example.fake/ext-one-with-api-9.tgz" buildpackBlob := blob.NewBlob(filepath.Join("testdata", "buildpack-api-0.4")) - bp, err := buildpack.FromBuildpackRootBlob(buildpackBlob, archive.DefaultTarWriterFactory()) + bp, err := buildpack.FromBuildpackRootBlob(buildpackBlob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/bp-one-with-api-4.tgz", gomock.Any()).Return(bp, nil, nil) extensionBlob := blob.NewBlob(filepath.Join("testdata", "extension-api-0.9")) - extension, err := buildpack.FromExtensionRootBlob(extensionBlob, archive.DefaultTarWriterFactory()) + extension, err := buildpack.FromExtensionRootBlob(extensionBlob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/ext-one-with-api-9.tgz", gomock.Any()).Return(extension, nil, nil) @@ -824,16 +824,16 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { bp2v1Blob := blob.NewBlob(filepath.Join("testdata", "buildpack-non-deterministic", "buildpack-2-version-1")) bp2v2Blob := blob.NewBlob(filepath.Join("testdata", "buildpack-non-deterministic", "buildpack-2-version-2")) - bp1v1, err = buildpack.FromBuildpackRootBlob(bp1v1Blob, archive.DefaultTarWriterFactory()) + bp1v1, err = buildpack.FromBuildpackRootBlob(bp1v1Blob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) - bp1v2, err = buildpack.FromBuildpackRootBlob(bp1v2Blob, archive.DefaultTarWriterFactory()) + bp1v2, err = buildpack.FromBuildpackRootBlob(bp1v2Blob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) - bp2v1, err = buildpack.FromBuildpackRootBlob(bp2v1Blob, archive.DefaultTarWriterFactory()) + bp2v1, err = buildpack.FromBuildpackRootBlob(bp2v1Blob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) - bp2v2, err = buildpack.FromBuildpackRootBlob(bp2v2Blob, archive.DefaultTarWriterFactory()) + bp2v2, err = buildpack.FromBuildpackRootBlob(bp2v2Blob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) return []buildpack.BuildModule{bp2v2, bp2v1, bp1v1, bp1v2} @@ -855,7 +855,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { bpDependencies := prepareBuildpackDependencies() buildpackBlob := blob.NewBlob(filepath.Join("testdata", "buildpack-api-0.4")) - bp, err := buildpack.FromBuildpackRootBlob(buildpackBlob, archive.DefaultTarWriterFactory()) + bp, err := buildpack.FromBuildpackRootBlob(buildpackBlob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/bp-one-with-api-4.tgz", gomock.Any()).DoAndReturn( func(ctx context.Context, buildpackURI string, opts buildpack.DownloadOptions) (buildpack.BuildModule, []buildpack.BuildModule, error) { @@ -865,7 +865,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { }) extensionBlob := blob.NewBlob(filepath.Join("testdata", "extension-api-0.9")) - extension, err := buildpack.FromExtensionRootBlob(extensionBlob, archive.DefaultTarWriterFactory()) + extension, err := buildpack.FromExtensionRootBlob(extensionBlob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/ext-one-with-api-9.tgz", gomock.Any()).DoAndReturn( func(ctx context.Context, buildpackURI string, opts buildpack.DownloadOptions) (buildpack.BuildModule, []buildpack.BuildModule, error) { @@ -898,7 +898,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { opts.Config.Buildpacks[0].URI = directoryPath buildpackBlob := blob.NewBlob(directoryPath) - buildpack, err := buildpack.FromBuildpackRootBlob(buildpackBlob, archive.DefaultTarWriterFactory()) + buildpack, err := buildpack.FromBuildpackRootBlob(buildpackBlob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), directoryPath, gomock.Any()).Return(buildpack, nil, nil) @@ -914,7 +914,7 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { opts.Config.Extensions[0].URI = directoryPath extensionBlob := blob.NewBlob(directoryPath) - extension, err := buildpack.FromExtensionRootBlob(extensionBlob, archive.DefaultTarWriterFactory()) + extension, err := buildpack.FromExtensionRootBlob(extensionBlob, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), directoryPath, gomock.Any()).Return(extension, nil, nil) @@ -1030,13 +1030,13 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { blob1 := blob.NewBlob(filepath.Join("testdata", "buildpack-flatten", "buildpack-1")) for i := 2; i <= 7; i++ { b := blob.NewBlob(filepath.Join("testdata", "buildpack-flatten", fmt.Sprintf("buildpack-%d", i))) - bp, err := buildpack.FromBuildpackRootBlob(b, archive.DefaultTarWriterFactory()) + bp, err := buildpack.FromBuildpackRootBlob(b, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) depBPs = append(depBPs, bp) } mockDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/flatten-bp-1.tgz").Return(blob1, nil).AnyTimes() - bp, err := buildpack.FromBuildpackRootBlob(blob1, archive.DefaultTarWriterFactory()) + bp, err := buildpack.FromBuildpackRootBlob(blob1, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/flatten-bp-1.tgz", gomock.Any()).Return(bp, depBPs, nil).AnyTimes() diff --git a/pkg/client/package_buildpack.go b/pkg/client/package_buildpack.go index 49ca63882a..2cdcddb9ec 100644 --- a/pkg/client/package_buildpack.go +++ b/pkg/client/package_buildpack.go @@ -98,7 +98,7 @@ func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOpti return err } - bp, err := buildpack.FromBuildpackRootBlob(mainBlob, writerFactory) + bp, err := buildpack.FromBuildpackRootBlob(mainBlob, writerFactory, c.logger) if err != nil { return errors.Wrapf(err, "creating buildpack from %s", style.Symbol(bpURI)) } diff --git a/pkg/client/package_buildpack_test.go b/pkg/client/package_buildpack_test.go index b94662c7de..1810eb418a 100644 --- a/pkg/client/package_buildpack_test.go +++ b/pkg/client/package_buildpack_test.go @@ -462,25 +462,25 @@ func testPackageBuildpack(t *testing.T, when spec.G, it spec.S) { blob1 := blob.NewBlob(filepath.Join("testdata", "buildpack-flatten", "buildpack-1")) mockDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/flatten-bp-1.tgz").Return(blob1, nil).AnyTimes() - bp, err := buildpack.FromBuildpackRootBlob(blob1, archive.DefaultTarWriterFactory()) + bp, err := buildpack.FromBuildpackRootBlob(blob1, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/flatten-bp-1.tgz", gomock.Any()).Return(bp, nil, nil).AnyTimes() // flatten buildpack 2 blob2 := blob.NewBlob(filepath.Join("testdata", "buildpack-flatten", "buildpack-2")) - bp2, err := buildpack.FromBuildpackRootBlob(blob2, archive.DefaultTarWriterFactory()) + bp2, err := buildpack.FromBuildpackRootBlob(blob2, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/flatten-bp-2.tgz", gomock.Any()).Return(bp2, nil, nil).AnyTimes() // flatten buildpack 3 blob3 := blob.NewBlob(filepath.Join("testdata", "buildpack-flatten", "buildpack-3")) - bp3, err := buildpack.FromBuildpackRootBlob(blob3, archive.DefaultTarWriterFactory()) + bp3, err := buildpack.FromBuildpackRootBlob(blob3, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) var depBPs []buildpack.BuildModule for i := 4; i <= 7; i++ { b := blob.NewBlob(filepath.Join("testdata", "buildpack-flatten", fmt.Sprintf("buildpack-%d", i))) - bp, err := buildpack.FromBuildpackRootBlob(b, archive.DefaultTarWriterFactory()) + bp, err := buildpack.FromBuildpackRootBlob(b, archive.DefaultTarWriterFactory(), nil) h.AssertNil(t, err) depBPs = append(depBPs, bp) } diff --git a/pkg/client/package_extension.go b/pkg/client/package_extension.go index 0e2c0e7317..690d12afba 100644 --- a/pkg/client/package_extension.go +++ b/pkg/client/package_extension.go @@ -42,7 +42,7 @@ func (c *Client) PackageExtension(ctx context.Context, opts PackageBuildpackOpti return err } - ex, err := buildpack.FromExtensionRootBlob(mainBlob, writerFactory) + ex, err := buildpack.FromExtensionRootBlob(mainBlob, writerFactory, c.logger) if err != nil { return errors.Wrapf(err, "creating extension from %s", style.Symbol(exURI)) } From 39fecde57621747181487ed0d564e405e651086c Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 19 Apr 2024 15:17:57 -0400 Subject: [PATCH 3/4] Add test Signed-off-by: Natalie Arellano --- pkg/buildpack/buildpack.go | 28 ++++++++++++------------- pkg/buildpack/buildpack_test.go | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/pkg/buildpack/buildpack.go b/pkg/buildpack/buildpack.go index 9c8e115451..c6f9778204 100644 --- a/pkg/buildpack/buildpack.go +++ b/pkg/buildpack/buildpack.go @@ -74,12 +74,12 @@ func FromBlob(descriptor Descriptor, blob Blob) BuildModule { func FromBuildpackRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactory, logger Logger) (BuildModule, error) { descriptor := dist.BuildpackDescriptor{} descriptor.WithAPI = api.MustParse(dist.AssumedBuildpackAPIVersion) - var undecodedKeys []string - if err := readDescriptor(KindBuildpack, &descriptor, blob, undecodedKeys); err != nil { + undecodedKeys, err := readDescriptor(KindBuildpack, &descriptor, blob) + if err != nil { return nil, err } if len(undecodedKeys) > 0 { - logger.Warnf("Ignoring unexpected key(s) in buildpack descriptor: %s", strings.Join(undecodedKeys, ",")) + logger.Warnf("Ignoring unexpected key(s) in descriptor for buildpack %s: %s", descriptor.EscapedID(), strings.Join(undecodedKeys, ",")) } if err := detectPlatformSpecificValues(&descriptor, blob); err != nil { return nil, err @@ -96,12 +96,12 @@ func FromBuildpackRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactor func FromExtensionRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactory, logger Logger) (BuildModule, error) { descriptor := dist.ExtensionDescriptor{} descriptor.WithAPI = api.MustParse(dist.AssumedBuildpackAPIVersion) - var undecodedKeys []string - if err := readDescriptor(KindExtension, &descriptor, blob, undecodedKeys); err != nil { + undecodedKeys, err := readDescriptor(KindExtension, &descriptor, blob) + if err != nil { return nil, err } if len(undecodedKeys) > 0 { - logger.Warnf("Ignoring unexpected key(s) in extension descriptor: %s", strings.Join(undecodedKeys, ",")) + logger.Warnf("Ignoring unexpected key(s) in descriptor for extension %s: %s", descriptor.EscapedID(), strings.Join(undecodedKeys, ",")) } if err := validateExtensionDescriptor(descriptor); err != nil { return nil, err @@ -109,10 +109,10 @@ func FromExtensionRootBlob(blob Blob, layerWriterFactory archive.TarWriterFactor return buildpackFrom(&descriptor, blob, layerWriterFactory) } -func readDescriptor(kind string, descriptor interface{}, blob Blob, undecoded []string) error { +func readDescriptor(kind string, descriptor interface{}, blob Blob) (undecodedKeys []string, err error) { rc, err := blob.Open() if err != nil { - return errors.Wrapf(err, "open %s", kind) + return undecodedKeys, errors.Wrapf(err, "open %s", kind) } defer rc.Close() @@ -120,20 +120,20 @@ func readDescriptor(kind string, descriptor interface{}, blob Blob, undecoded [] _, buf, err := archive.ReadTarEntry(rc, descriptorFile) if err != nil { - return errors.Wrapf(err, "reading %s", descriptorFile) + return undecodedKeys, errors.Wrapf(err, "reading %s", descriptorFile) } md, err := toml.Decode(string(buf), descriptor) if err != nil { - return errors.Wrapf(err, "decoding %s", descriptorFile) + return undecodedKeys, errors.Wrapf(err, "decoding %s", descriptorFile) } - undecodedKeys := md.Undecoded() - for _, k := range undecodedKeys { - undecoded = append(undecoded, k.String()) + undecoded := md.Undecoded() + for _, k := range undecoded { + undecodedKeys = append(undecodedKeys, k.String()) } - return nil + return undecodedKeys, nil } func detectPlatformSpecificValues(descriptor *dist.BuildpackDescriptor, blob Blob) error { diff --git a/pkg/buildpack/buildpack_test.go b/pkg/buildpack/buildpack_test.go index b44bbc94e9..2de8975630 100644 --- a/pkg/buildpack/buildpack_test.go +++ b/pkg/buildpack/buildpack_test.go @@ -1,6 +1,7 @@ package buildpack_test import ( + "bytes" "fmt" "io" "os" @@ -20,6 +21,7 @@ import ( "github.com/buildpacks/pack/pkg/blob" "github.com/buildpacks/pack/pkg/buildpack" "github.com/buildpacks/pack/pkg/dist" + "github.com/buildpacks/pack/pkg/logging" h "github.com/buildpacks/pack/testhelpers" ) @@ -496,6 +498,40 @@ version = "1.2.3" ) }) }) + + when("there are wrong things in the file", func() { + it("warns", func() { + outBuf := bytes.Buffer{} + logger := logging.NewLogWithWriters(&outBuf, &outBuf) + _, err := buildpack.FromBuildpackRootBlob(&readerBlob{ + openFn: func() io.ReadCloser { + tarBuilder := archive.TarBuilder{} + tarBuilder.AddFile("buildpack.toml", 0700, time.Now(), []byte(` +api = "0.3" + +[buildpack] +id = "bp.one" +version = "1.2.3" +homepage = "http://geocities.com/cool-bp" + +[[targets]] +os = "some-os" +arch = "some-arch" +variant = "some-arch-variant" +[[targets.distributions]] +name = "some-distro-name" +version = "some-distro-version" +[[targets.distros]] +name = "some-distro-name" +versions = ["some-distro-version"] +`)) + return tarBuilder.Reader(archive.DefaultTarWriterFactory()) + }, + }, archive.DefaultTarWriterFactory(), logger) + h.AssertNil(t, err) + h.AssertContains(t, outBuf.String(), "Warning: Ignoring unexpected key(s) in descriptor for buildpack bp.one: targets.distributions,targets.distributions.name,targets.distributions.version,targets.distros.versions") + }) + }) }) when("#Match", func() { From 7949931b0438a1a929f94c4ca8eb82fc631731a2 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 19 Apr 2024 15:22:10 -0400 Subject: [PATCH 4/4] Add distro test Signed-off-by: Natalie Arellano --- internal/target/parse_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/target/parse_test.go b/internal/target/parse_test.go index f00c095fb3..61b2c6e39e 100644 --- a/internal/target/parse_test.go +++ b/internal/target/parse_test.go @@ -61,6 +61,7 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { h.AssertNotEq(t, outBuf.String(), "") }) }) + when("target#ParseTargets", func() { it("should throw an error when atleast one target throws error", func() { _, err := target.ParseTargets([]string{"linux/arm/v6", ":distro@version"}, logging.NewLogWithWriters(&outBuf, &outBuf)) @@ -96,6 +97,7 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { }) }) }) + when("target#ParseDistro", func() { it("should parse distro as expected", func() { output, err := target.ParseDistro("ubuntu@22.04", logging.NewLogWithWriters(&outBuf, &outBuf)) @@ -105,15 +107,21 @@ func testParseTargets(t *testing.T, when spec.G, it spec.S) { }) h.AssertNil(t, err) }) - it("should return an error", func() { + it("should return an error when name is missing", func() { _, err := target.ParseDistro("@22.04@20.08", logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertNotNil(t, err) }) + it("should return an error when there are two versions", func() { + _, err := target.ParseDistro("some-distro@22.04@20.08", logging.NewLogWithWriters(&outBuf, &outBuf)) + h.AssertNotNil(t, err) + h.AssertError(t, err, "invalid distro") + }) it("should warn when distro version is not specified", func() { target.ParseDistro("ubuntu", logging.NewLogWithWriters(&outBuf, &outBuf)) h.AssertNotEq(t, outBuf.String(), "") }) }) + when("target#ParseDistros", func() { it("should parse distros as expected", func() { output, err := target.ParseDistros("ubuntu@22.04;ubuntu@20.08;debian@8.10;debian@10.06", logging.NewLogWithWriters(&outBuf, &outBuf))