Skip to content

Commit

Permalink
Merge pull request #2123 from buildpacks/fix/bp-new
Browse files Browse the repository at this point in the history 
Fix pack buildpack new --targets
  • Loading branch information
@jjbustamante
jjbustamante authored Apr 19, 2024
2 parents aef63b5 + e397773 commit 12b7d24
Show file tree
Hide file tree
Showing 14 changed files with 269 additions and 247 deletions.
29 changes: 16 additions & 13 deletions internal/commands/buildpack_new_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:[email protected]@16.04"})
command.SetArgs([]string{"--path", path, "example/targets", "--targets", "linux/arm/v6:[email protected]"})

err := command.Execute()
h.AssertNil(t, err)
Expand All @@ -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)
Expand All @@ -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",
Expand All @@ -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",
},
},
},
Expand All @@ -160,16 +163,16 @@ 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",
},
},
},
},
}).Return(nil).MaxTimes(1)

path := filepath.Join(tmpDir, "targets")
command.SetArgs([]string{"--path", path, "example/targets", "--targets", "linux/arm/v6:[email protected]@16.04;[email protected]@10.9", "-t", "windows/amd64:[email protected]"})
command.SetArgs([]string{"--path", path, "example/targets", "--targets", "linux/arm/v6:[email protected];[email protected]", "-t", "windows/amd64:[email protected]"})

err := command.Execute()
h.AssertNil(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/layer/writer_factory_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package layer_test

import (
"archive/tar"
"archive/tar" //nolint
"testing"

ilayer "github.com/buildpacks/imgutil/layer"
Expand Down
11 changes: 8 additions & 3 deletions internal/target/parse.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package target

import (
"fmt"
"strings"

"github.com/pkg/errors"
Expand Down Expand Up @@ -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
}

Expand Down
48 changes: 34 additions & 14 deletions internal/target/parse_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,14 @@ 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))
h.AssertNotNil(t, err)
})
it("should parse targets as expected", func() {
output, err := target.ParseTargets([]string{"linux/arm/v6", "linux/amd64:[email protected];[email protected]@10.06"}, logging.NewLogWithWriters(&outBuf, &outBuf))
output, err := target.ParseTargets([]string{"linux/arm/v6", "linux/amd64:[email protected];[email protected];debian@10.06"}, logging.NewLogWithWriters(&outBuf, &outBuf))
h.AssertNil(t, err)
h.AssertEq(t, output, []dist.Target{
{
Expand All @@ -80,47 +81,66 @@ 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",
Version: "8.10",
},
{
Name: "debian",
Versions: []string{"8.10", "10.06"},
Name: "debian",
Version: "10.06",
},
},
},
})
})
})

when("target#ParseDistro", func() {
it("should parse distro as expected", func() {
output, err := target.ParseDistro("[email protected]@20.08", logging.NewLogWithWriters(&outBuf, &outBuf))
output, err := target.ParseDistro("[email protected]", 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)
})
it("should return an error", func() {
it("should return an error when name is missing", func() {
_, err := target.ParseDistro("@[email protected]", logging.NewLogWithWriters(&outBuf, &outBuf))
h.AssertNotNil(t, err)
})
it("should return an error when there are two versions", func() {
_, err := target.ParseDistro("[email protected]@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("[email protected]@20.08;[email protected]@10.06", logging.NewLogWithWriters(&outBuf, &outBuf))
output, err := target.ParseDistros("[email protected];ubuntu@20.08;[email protected];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)
Expand Down
33 changes: 23 additions & 10 deletions pkg/buildpack/buildpack.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
undecodedKeys, err := readDescriptor(KindBuildpack, &descriptor, blob)
if err != nil {
return nil, err
}
if len(undecodedKeys) > 0 {
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
}
Expand All @@ -89,38 +93,47 @@ 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 {
undecodedKeys, err := readDescriptor(KindExtension, &descriptor, blob)
if err != nil {
return nil, err
}
if len(undecodedKeys) > 0 {
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
}
return buildpackFrom(&descriptor, blob, layerWriterFactory)
}

func readDescriptor(kind string, descriptor interface{}, blob Blob) 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()

descriptorFile := kind + ".toml"

_, buf, err := archive.ReadTarEntry(rc, descriptorFile)
if err != nil {
return errors.Wrapf(err, "reading %s", descriptorFile)
return undecodedKeys, 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)
return undecodedKeys, errors.Wrapf(err, "decoding %s", descriptorFile)
}

return nil
undecoded := md.Undecoded()
for _, k := range undecoded {
undecodedKeys = append(undecodedKeys, k.String())
}

return undecodedKeys, nil
}

func detectPlatformSpecificValues(descriptor *dist.BuildpackDescriptor, blob Blob) error {
Expand Down
Loading

0 comments on commit 12b7d24

Please sign in to comment.