Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add custom label metadata to packaged buildpacks #1877

Merged
merged 4 commits into from
Sep 6, 2023
Merged

Add custom label metadata to packaged buildpacks #1877

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9454706
Add custom label metadata to packaged buildpacks
colincasey Aug 28, 2023
2214603
Added test for failing image label
colincasey Aug 31, 2023
13cb3ac
Run `make tidy`
colincasey Aug 31, 2023
9c328bd
Merge branch 'main' into add_image_labels_to_buildpack
jkutner Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions internal/commands/buildpack_package.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type BuildpackPackageFlags struct {
BuildpackRegistry string
Path string
FlattenExclude []string
Label map[string]string
Publish bool
Flatten bool
Depth int
Expand Down Expand Up @@ -111,6 +112,7 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa
Flatten: flags.Flatten,
FlattenExclude: flags.FlattenExclude,
Depth: flags.Depth,
Labels: flags.Label,
}); err != nil {
return err
}
Expand Down Expand Up @@ -138,6 +140,7 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa
cmd.Flags().BoolVar(&flags.Flatten, "flatten", false, "Flatten the buildpack into a single layer")
cmd.Flags().StringSliceVarP(&flags.FlattenExclude, "flatten-exclude", "e", nil, "Buildpacks to exclude from flattening, in the form of '<buildpack-id>@<buildpack-version>'")
cmd.Flags().IntVar(&flags.Depth, "depth", -1, "Max depth to flatten.\nOmission of this flag or values < 0 will flatten the entire tree.")
cmd.Flags().StringToStringVarP(&flags.Label, "label", "l", nil, "Labels to add to packaged Buildpack, in the form of '<name>=<value>'")
if !cfg.Experimental {
cmd.Flags().MarkHidden("flatten")
cmd.Flags().MarkHidden("depth")
Expand Down
14 changes: 14 additions & 0 deletions internal/commands/buildpack_package_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,20 @@ func testPackageCommand(t *testing.T, when spec.G, it spec.S) {
h.AssertError(t, cmd.Execute(), "parsing pull policy")
})
})

when("--label cannot be parsed", func() {
it("errors with a descriptive message", func() {
cmd := packageCommand()
cmd.SetArgs([]string{
"some-image-name", "--config", "/path/to/some/file",
"--label", "name+value",
})

err := cmd.Execute()
h.AssertNotNil(t, err)
h.AssertError(t, err, "invalid argument \"name+value\" for \"-l, --label\" flag: name+value must be formatted as key=value")
})
})
})
}

Expand Down
19 changes: 17 additions & 2 deletions pkg/buildpack/builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@
return stacks
}

func (b *PackageBuilder) SaveAsFile(path, imageOS string) error {
func (b *PackageBuilder) SaveAsFile(path, imageOS string, labels map[string]string) error {
if err := b.validate(); err != nil {
return err
}
Expand All @@ -348,6 +348,13 @@
return errors.Wrap(err, "creating layout image")
}

for labelKey, labelValue := range labels {
err = layoutImage.SetLabel(labelKey, labelValue)
if err != nil {
return errors.Wrapf(err, "adding label %s=%s", labelKey, labelValue)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is possible to also add the test case for this one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this was added in 2214603

}

Check warning on line 355 in pkg/buildpack/builder.go

View check run for this annotation

Codecov / codecov/patch

pkg/buildpack/builder.go#L354-L355 

Added lines #L354 - L355 were not covered by tests
}

tempDirName := ""
if b.buildpack != nil {
tempDirName = "package-buildpack"
Expand Down Expand Up @@ -430,7 +437,7 @@
return &layoutImage{Image: i}, nil
}

func (b *PackageBuilder) SaveAsImage(repoName string, publish bool, imageOS string) (imgutil.Image, error) {
func (b *PackageBuilder) SaveAsImage(repoName string, publish bool, imageOS string, labels map[string]string) (imgutil.Image, error) {
if err := b.validate(); err != nil {
return nil, err
}
Expand All @@ -439,6 +446,14 @@
if err != nil {
return nil, errors.Wrapf(err, "creating image")
}

for labelKey, labelValue := range labels {
err = image.SetLabel(labelKey, labelValue)
if err != nil {
return nil, errors.Wrapf(err, "adding label %s=%s", labelKey, labelValue)
}
}

tempDirName := ""
if b.buildpack != nil {
tempDirName = "package-buildpack"
Expand Down
97 changes: 80 additions & 17 deletions pkg/buildpack/builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"path/filepath"
"testing"

"github.com/pkg/errors"

"github.com/buildpacks/imgutil/fakes"
"github.com/buildpacks/imgutil/layer"
"github.com/buildpacks/lifecycle/api"
Expand Down Expand Up @@ -76,18 +78,18 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
fn func(*buildpack.PackageBuilder) error
}{
{name: "SaveAsImage", expectedImageOS: "linux", fn: func(builder *buildpack.PackageBuilder) error {
_, err := builder.SaveAsImage("some/package", false, "linux")
_, err := builder.SaveAsImage("some/package", false, "linux", map[string]string{})
return err
}},
{name: "SaveAsImage", expectedImageOS: "windows", fn: func(builder *buildpack.PackageBuilder) error {
_, err := builder.SaveAsImage("some/package", false, "windows")
_, err := builder.SaveAsImage("some/package", false, "windows", map[string]string{})
return err
}},
{name: "SaveAsFile", expectedImageOS: "linux", fn: func(builder *buildpack.PackageBuilder) error {
return builder.SaveAsFile(path.Join(tmpDir, "package.cnb"), "linux")
return builder.SaveAsFile(path.Join(tmpDir, "package.cnb"), "linux", map[string]string{})
}},
{name: "SaveAsFile", expectedImageOS: "windows", fn: func(builder *buildpack.PackageBuilder) error {
return builder.SaveAsFile(path.Join(tmpDir, "package.cnb"), "windows")
return builder.SaveAsFile(path.Join(tmpDir, "package.cnb"), "windows", map[string]string{})
}},
} {
// always use copies to avoid stale refs
Expand Down Expand Up @@ -391,7 +393,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
h.AssertNil(t, err)
builder.AddDependency(dependency2)

img, err := builder.SaveAsImage("some/package", false, expectedImageOS)
img, err := builder.SaveAsImage("some/package", false, expectedImageOS, map[string]string{})
h.AssertNil(t, err)

metadata := buildpack.Metadata{}
Expand Down Expand Up @@ -453,7 +455,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
h.AssertNil(t, err)
builder.AddDependency(dependency2)

img, err := builder.SaveAsImage("some/package", false, expectedImageOS)
img, err := builder.SaveAsImage("some/package", false, expectedImageOS, map[string]string{})
h.AssertNil(t, err)

metadata := buildpack.Metadata{}
Expand Down Expand Up @@ -518,7 +520,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {

builder.AddDependency(dependencyNestedNested)

img, err := builder.SaveAsImage("some/package", false, expectedImageOS)
img, err := builder.SaveAsImage("some/package", false, expectedImageOS, map[string]string{})
h.AssertNil(t, err)

metadata := buildpack.Metadata{}
Expand Down Expand Up @@ -563,7 +565,9 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
builder := buildpack.NewBuilder(mockImageFactory("linux"))
builder.SetBuildpack(buildpack1)

packageImage, err := builder.SaveAsImage("some/package", false, "linux")
var customLabels = map[string]string{"test.label.one": "1", "test.label.two": "2"}

packageImage, err := builder.SaveAsImage("some/package", false, "linux", customLabels)
h.AssertNil(t, err)

labelData, err := packageImage.Label("io.buildpacks.buildpackage.metadata")
Expand All @@ -586,6 +590,11 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
osVal, err := packageImage.OS()
h.AssertNil(t, err)
h.AssertEq(t, osVal, "linux")

imageLabels, err := packageImage.Labels()
h.AssertNil(t, err)
h.AssertEq(t, imageLabels["test.label.one"], "1")
h.AssertEq(t, imageLabels["test.label.two"], "2")
})

it("sets extension metadata", func() {
Expand All @@ -609,7 +618,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
h.AssertNil(t, err)
builder := buildpack.NewBuilder(mockImageFactory("linux"))
builder.SetExtension(extension1)
packageImage, err := builder.SaveAsImage("some/package", false, "linux")
packageImage, err := builder.SaveAsImage("some/package", false, "linux", map[string]string{})
h.AssertNil(t, err)
labelData, err := packageImage.Label("io.buildpacks.buildpackage.metadata")
h.AssertNil(t, err)
Expand Down Expand Up @@ -642,7 +651,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
builder := buildpack.NewBuilder(mockImageFactory("linux"))
builder.SetBuildpack(buildpack1)

packageImage, err := builder.SaveAsImage("some/package", false, "linux")
packageImage, err := builder.SaveAsImage("some/package", false, "linux", map[string]string{})
h.AssertNil(t, err)

var bpLayers dist.ModuleLayers
Expand All @@ -666,7 +675,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
builder := buildpack.NewBuilder(mockImageFactory("linux"))
builder.SetBuildpack(buildpack1)

packageImage, err := builder.SaveAsImage("some/package", false, "linux")
packageImage, err := builder.SaveAsImage("some/package", false, "linux", map[string]string{})
h.AssertNil(t, err)

buildpackExists := func(name, version string) {
Expand Down Expand Up @@ -713,10 +722,51 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
builder := buildpack.NewBuilder(mockImageFactory("windows"))
builder.SetBuildpack(buildpack1)

_, err = builder.SaveAsImage("some/package", false, "windows")
_, err = builder.SaveAsImage("some/package", false, "windows", map[string]string{})
h.AssertNil(t, err)
})

it("should report an error when custom label cannot be set", func() {
mockImageFactory = func(expectedImageOS string) *testmocks.MockImageFactory {
var imageWithLabelError = &imageWithLabelError{Image: fakes.NewImage("some/package", "", nil)}
imageFactory := testmocks.NewMockImageFactory(mockController)
imageFactory.EXPECT().NewImage("some/package", true, expectedImageOS).Return(imageWithLabelError, nil).MaxTimes(1)
return imageFactory
}

buildpack1, err := ifakes.NewFakeBuildpack(dist.BuildpackDescriptor{
WithAPI: api.MustParse("0.2"),
WithInfo: dist.ModuleInfo{
ID: "bp.1.id",
Version: "bp.1.version",
Name: "One",
Description: "some description",
Homepage: "https://example.com/homepage",
Keywords: []string{"some-keyword"},
Licenses: []dist.License{
{
Type: "MIT",
URI: "https://example.com/license",
},
},
},
WithStacks: []dist.Stack{
{ID: "stack.id.1"},
{ID: "stack.id.2"},
},
WithOrder: nil,
}, 0644)
h.AssertNil(t, err)

builder := buildpack.NewBuilder(mockImageFactory("linux"))
builder.SetBuildpack(buildpack1)

var customLabels = map[string]string{"test.label.fail": "true"}

_, err = builder.SaveAsImage("some/package", false, "linux", customLabels)
h.AssertError(t, err, "adding label test.label.fail=true")
})

when("flatten is set", func() {
var (
buildpack1 buildpack.BuildModule
Expand Down Expand Up @@ -841,7 +891,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
builder.AddDependencies(bp1, nil)
builder.AddDependencies(compositeBP2, []buildpack.BuildModule{bp21, bp22, compositeBP3, bp31})

packageImage, err := builder.SaveAsImage("some/package", false, "linux")
packageImage, err := builder.SaveAsImage("some/package", false, "linux", map[string]string{})
h.AssertNil(t, err)

fakePackageImage := packageImage.(*fakes.Image)
Expand All @@ -864,7 +914,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
builder.AddDependencies(bp1, nil)
builder.AddDependencies(compositeBP2, []buildpack.BuildModule{bp21, bp22, compositeBP3, bp31})

packageImage, err := builder.SaveAsImage("some/package", false, "linux")
packageImage, err := builder.SaveAsImage("some/package", false, "linux", map[string]string{})
h.AssertNil(t, err)

fakePackageImage := packageImage.(*fakes.Image)
Expand All @@ -888,8 +938,10 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
builder := buildpack.NewBuilder(mockImageFactory(""))
builder.SetBuildpack(buildpack1)

var customLabels = map[string]string{"test.label.one": "1", "test.label.two": "2"}

outputFile := filepath.Join(tmpDir, fmt.Sprintf("package-%s.cnb", h.RandString(10)))
h.AssertNil(t, builder.SaveAsFile(outputFile, "linux"))
h.AssertNil(t, builder.SaveAsFile(outputFile, "linux", customLabels))

withContents := func(fn func(data []byte)) h.TarEntryAssertion {
return func(t *testing.T, header *tar.Header, data []byte) {
Expand Down Expand Up @@ -928,6 +980,9 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
h.ContentContains(`"io.buildpacks.buildpack.layers":"{\"bp.1.id\":{\"bp.1.version\":{\"api\":\"0.2\",\"stacks\":[{\"id\":\"stack.id.1\"},{\"id\":\"stack.id.2\"}],\"layerDiffID\":\"sha256:44447e95b06b73496d1891de5afb01936e9999b97ea03dad6337d9f5610807a7\"}}`),
// image os
h.ContentContains(`"os":"linux"`),
// custom labels
h.ContentContains(`"test.label.one":"1"`),
h.ContentContains(`"test.label.two":"2"`),
)
}))
}))
Expand All @@ -946,7 +1001,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
builder.SetBuildpack(buildpack1)

outputFile := filepath.Join(tmpDir, fmt.Sprintf("package-%s.cnb", h.RandString(10)))
h.AssertNil(t, builder.SaveAsFile(outputFile, "linux"))
h.AssertNil(t, builder.SaveAsFile(outputFile, "linux", map[string]string{}))

h.AssertOnTarEntry(t, outputFile, "/blobs",
h.IsDirectory(),
Expand Down Expand Up @@ -996,7 +1051,7 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) {
builder.SetBuildpack(buildpack1)

outputFile := filepath.Join(tmpDir, fmt.Sprintf("package-%s.cnb", h.RandString(10)))
h.AssertNil(t, builder.SaveAsFile(outputFile, "windows"))
h.AssertNil(t, builder.SaveAsFile(outputFile, "windows", map[string]string{}))

// Windows baselayer content is constant
expectedBaseLayerReader, err := layer.WindowsBaseLayer()
Expand Down Expand Up @@ -1047,3 +1102,11 @@ func computeLayerSHA(reader io.ReadCloser) (string, error) {

return digest.Hex, nil
}

type imageWithLabelError struct {
*fakes.Image
}

func (i *imageWithLabelError) SetLabel(string, string) error {
return errors.New("Label could not be set")
}
7 changes: 5 additions & 2 deletions pkg/client/package_buildpack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ type PackageBuildpackOptions struct {

// List of buildpack images to exclude from the package been flatten.
FlattenExclude []string

// Map of labels to add to the Buildpack
Labels map[string]string
}

// PackageBuildpack packages buildpack(s) into either an image or file.
Expand Down Expand Up @@ -124,9 +127,9 @@ func (c *Client) PackageBuildpack(ctx context.Context, opts PackageBuildpackOpti

switch opts.Format {
case FormatFile:
return packageBuilder.SaveAsFile(opts.Name, opts.Config.Platform.OS)
return packageBuilder.SaveAsFile(opts.Name, opts.Config.Platform.OS, opts.Labels)
case FormatImage:
_, err = packageBuilder.SaveAsImage(opts.Name, opts.Publish, opts.Config.Platform.OS)
_, err = packageBuilder.SaveAsImage(opts.Name, opts.Publish, opts.Config.Platform.OS, opts.Labels)
return errors.Wrapf(err, "saving image")
default:
return errors.Errorf("unknown format: %s", style.Symbol(opts.Format))
Expand Down
4 changes: 2 additions & 2 deletions pkg/client/package_extension.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ func (c *Client) PackageExtension(ctx context.Context, opts PackageBuildpackOpti

switch opts.Format {
case FormatFile:
return packageBuilder.SaveAsFile(opts.Name, opts.Config.Platform.OS)
return packageBuilder.SaveAsFile(opts.Name, opts.Config.Platform.OS, map[string]string{})
case FormatImage:
_, err = packageBuilder.SaveAsImage(opts.Name, opts.Publish, opts.Config.Platform.OS)
_, err = packageBuilder.SaveAsImage(opts.Name, opts.Publish, opts.Config.Platform.OS, map[string]string{})
return errors.Wrapf(err, "saving image")
default:
return errors.Errorf("unknown format: %s", style.Symbol(opts.Format))
Expand Down
Loading