From 945470694ca86236478cc9376f354a3a307ca9fa Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Mon, 28 Aug 2023 11:30:34 -0300 Subject: [PATCH 1/3] Add custom label metadata to packaged buildpacks Introduces a new flag `--label` to the `pack buildpack package` command which: * can be used to add labels one at a time (e.g.; `--label a=1 --label b=2`) * can be used to add multiple labels in CSV format (e.g.; `--label a=1,b=2`) The labels are parsed into a `map[string]string` and added as label metadata to the packaged OCI or `.cnb` file. Signed-off-by: Colin Casey --- internal/commands/buildpack_package.go | 3 ++ internal/commands/buildpack_package_test.go | 14 +++++++ pkg/buildpack/builder.go | 19 ++++++++- pkg/buildpack/builder_test.go | 46 +++++++++++++-------- pkg/client/package_buildpack.go | 7 +++- pkg/client/package_extension.go | 4 +- 6 files changed, 70 insertions(+), 23 deletions(-) diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index f33107a63..6cac0ad74 100644 --- a/internal/commands/buildpack_package.go +++ b/internal/commands/buildpack_package.go @@ -24,6 +24,7 @@ type BuildpackPackageFlags struct { BuildpackRegistry string Path string FlattenExclude []string + Label map[string]string Publish bool Flatten bool Depth int @@ -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 } @@ -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 '@'") 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 '='") if !cfg.Experimental { cmd.Flags().MarkHidden("flatten") cmd.Flags().MarkHidden("depth") diff --git a/internal/commands/buildpack_package_test.go b/internal/commands/buildpack_package_test.go index 838c7a6dd..f527c01d0 100644 --- a/internal/commands/buildpack_package_test.go +++ b/internal/commands/buildpack_package_test.go @@ -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") + }) + }) }) } diff --git a/pkg/buildpack/builder.go b/pkg/buildpack/builder.go index 222f1e577..93724a3e6 100644 --- a/pkg/buildpack/builder.go +++ b/pkg/buildpack/builder.go @@ -338,7 +338,7 @@ func (b *PackageBuilder) resolvedStacks() []dist.Stack { 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 } @@ -348,6 +348,13 @@ func (b *PackageBuilder) SaveAsFile(path, imageOS string) error { 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) + } + } + tempDirName := "" if b.buildpack != nil { tempDirName = "package-buildpack" @@ -430,7 +437,7 @@ func newLayoutImage(imageOS string) (*layoutImage, error) { 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 } @@ -439,6 +446,14 @@ func (b *PackageBuilder) SaveAsImage(repoName string, publish bool, imageOS stri 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" diff --git a/pkg/buildpack/builder_test.go b/pkg/buildpack/builder_test.go index 1f2066c83..2eb4cac06 100644 --- a/pkg/buildpack/builder_test.go +++ b/pkg/buildpack/builder_test.go @@ -76,18 +76,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 @@ -391,7 +391,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{} @@ -453,7 +453,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{} @@ -518,7 +518,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{} @@ -563,7 +563,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") @@ -586,6 +588,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() { @@ -609,7 +616,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) @@ -642,7 +649,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 @@ -666,7 +673,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) { @@ -713,7 +720,7 @@ 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) }) @@ -841,7 +848,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) @@ -864,7 +871,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) @@ -888,8 +895,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) { @@ -928,6 +937,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"`), ) })) })) @@ -946,7 +958,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(), @@ -996,7 +1008,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() diff --git a/pkg/client/package_buildpack.go b/pkg/client/package_buildpack.go index 17463007b..502564d2f 100644 --- a/pkg/client/package_buildpack.go +++ b/pkg/client/package_buildpack.go @@ -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. @@ -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)) diff --git a/pkg/client/package_extension.go b/pkg/client/package_extension.go index 93aed9d41..0e2c0e731 100644 --- a/pkg/client/package_extension.go +++ b/pkg/client/package_extension.go @@ -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)) From 2214603309e82599d241ca522e5af460ca236a3b Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Thu, 31 Aug 2023 09:37:50 -0300 Subject: [PATCH 2/3] Added test for failing image label Signed-off-by: Colin Casey --- pkg/buildpack/builder_test.go | 50 +++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/pkg/buildpack/builder_test.go b/pkg/buildpack/builder_test.go index 2eb4cac06..9ca2156f1 100644 --- a/pkg/buildpack/builder_test.go +++ b/pkg/buildpack/builder_test.go @@ -6,6 +6,7 @@ import ( "compress/gzip" "encoding/json" "fmt" + "github.com/pkg/errors" "io" "os" "path" @@ -724,6 +725,47 @@ func testPackageBuilder(t *testing.T, when spec.G, it spec.S) { 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 @@ -1059,3 +1101,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") +} From 13cb3acf6f22c11129d3a0a50d08b86ed4fb1ab3 Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Thu, 31 Aug 2023 09:40:26 -0300 Subject: [PATCH 3/3] Run `make tidy` Signed-off-by: Colin Casey --- pkg/buildpack/builder_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/buildpack/builder_test.go b/pkg/buildpack/builder_test.go index 9ca2156f1..7181e08bc 100644 --- a/pkg/buildpack/builder_test.go +++ b/pkg/buildpack/builder_test.go @@ -6,13 +6,14 @@ import ( "compress/gzip" "encoding/json" "fmt" - "github.com/pkg/errors" "io" "os" "path" "path/filepath" "testing" + "github.com/pkg/errors" + "github.com/buildpacks/imgutil/fakes" "github.com/buildpacks/imgutil/layer" "github.com/buildpacks/lifecycle/api"