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.

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

Add custom label metadata to packaged buildpacks #1877

Merged
merged 4 commits into from
Sep 6, 2023

Conversation

colincasey
Copy link
Contributor

@colincasey colincasey commented Aug 28, 2023

Summary

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.

Output

The before/after from pack buildpack package --help is shown below.

Before

buildpack package allows users to package (a) buildpack(s) into OCI format, which can then to be hosted in image repositories or persisted on disk as a '.cnb' file. You can also package a number of buildpacks together, to enable easier distribution of a set of buildpacks. Packaged buildpacks can be used as inputs to `pack build` (using the `--buildpack` flag), and they can be included in the configs used in `pack builder create` and `pack buildpack package`. For more on how to package a buildpack, see: https://buildpacks.io/docs/buildpack-author-guide/package-a-buildpack/.

Usage:
  pack buildpack package <name> --config <config-path> [flags]

Examples:
pack buildpack package my-buildpack --config ./package.toml
pack buildpack package my-buildpack.cnb --config ./package.toml --f file

Flags:
  -r, --buildpack-registry string   Buildpack Registry name
  -c, --config string               Path to package TOML config
  -f, --format string               Format to save package as ("image" or "file")
  -h, --help                        Help for 'package'
  -p, --path string                 Path to the Buildpack that needs to be packaged
      --publish                     Publish to registry (applies to "--format=image" only)
      --pull-policy string          Pull policy to use. Accepted values are always, never, and if-not-present. The default is always

Global Flags:
      --no-color     Disable color output
  -q, --quiet        Show less output
      --timestamps   Enable timestamps in output
  -v, --verbose      Show more output

After

buildpack package allows users to package (a) buildpack(s) into OCI format, which can then to be hosted in image repositories or persisted on disk as a '.cnb' file. You can also package a number of buildpacks together, to enable easier distribution of a set of buildpacks. Packaged buildpacks can be used as inputs to `pack build` (using the `--buildpack` flag), and they can be included in the configs used in `pack builder create` and `pack buildpack package`. For more on how to package a buildpack, see: https://buildpacks.io/docs/buildpack-author-guide/package-a-buildpack/.

Usage:
  pack buildpack package <name> --config <config-path> [flags]

Examples:
pack buildpack package my-buildpack --config ./package.toml
pack buildpack package my-buildpack.cnb --config ./package.toml --f file

Flags:
  -r, --buildpack-registry string   Buildpack Registry name
  -c, --config string               Path to package TOML config
  -f, --format string               Format to save package as ("image" or "file")
  -h, --help                        Help for 'package'
  -l, --label stringToString        Labels to add to packaged Buildpack, in the form of '<name>=<value>' (default [])
  -p, --path string                 Path to the Buildpack that needs to be packaged
      --publish                     Publish to registry (applies to "--format=image" only)
      --pull-policy string          Pull policy to use. Accepted values are always, never, and if-not-present. The default is always

Global Flags:
      --no-color     Disable color output
  -q, --quiet        Show less output
      --timestamps   Enable timestamps in output
  -v, --verbose      Show more output

Documentation

  • Should this change be documented?
    • Yes, see #___
    • No

Related

Resolves #1867

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 <[email protected]>
@colincasey colincasey requested review from a team as code owners August 28, 2023 15:02
@github-actions github-actions bot added this to the 0.31.0 milestone Aug 28, 2023
@github-actions github-actions bot added the type/enhancement Issue that requests a new feature or improvement. label Aug 28, 2023
@jjbustamante
Copy link
Member

@colincasey Thanks for this PR!! ❤️

@dlion
Copy link
Member

dlion commented Aug 30, 2023

Thank you for this PR @colincasey !
During the next working group we're gonna discuss about the necessity or not of having an RFC behind this flag.
Meanwhile I think it would be important to document this new flag at least in our documentation, can you take care of that as well?

@colincasey
Copy link
Contributor Author

I don't mind documenting the flag @dlion but I'm not sure how. When I looked at buildpacks/docs it seems like the pack CLI pages are auto-generated by the pack-docs-update make target. In that case, won't the pack buildpack package page get updated with the new --label flag since it appears in pack buildpack package --help?

Also, I wanted to address the failing codecov/patch check but I couldn't figure out how to simulate an error while setting a label on the fakes.Image in the unit tests. None of the other error cases in SaveAsImage or SaveAsFile have code coverage either which leads me to believe it may not possible to simulate these types of errors in the current tests. Any suggestions?

@jjbustamante
Copy link
Member

Hi @colincasey

My suggestion will be the following:

Yo can extend the fake.Image as follow:

type imageLabelError struct {
	*fakes.Image
	error bool
}

func (i *imageLabelError) SetLabel(k string, v string) error {
	if i.error {
		return errors.New("Put some fancy error here...")
	}
	return i.Image.SetLabel(k, v)
}

Then we have the function on line 49 that is mocking the image return by the factory, you can create a new fake instance configure to fail when SetLabel is called, maybe based on the imageName or something like that.

it.Before(func() {
		mockController = gomock.NewController(t)
		mockImageFactory = func(expectedImageOS string) *testmocks.MockImageFactory {
			imageFactory := testmocks.NewMockImageFactory(mockController)

			if expectedImageOS != "" {
				fakePackageImage := fakes.NewImage("some/package", "", nil)

                                  // You could add the logic to return a 
                                 // &imageLabelError{Image: fakePackageImage, error: true}
                                 // whenever you want the setLabel to fail

				imageFactory.EXPECT().NewImage("some/package", true, expectedImageOS).Return(fakePackageImage, nil).MaxTimes(1)
			}

			return imageFactory
		}

	})

@dlion
Copy link
Member

dlion commented Aug 31, 2023

In that case, won't the pack buildpack package page get updated with the new --label flag since it appears in pack buildpack package --help?

You are totally correct! I was thinking of adding it to sections like https://buildpacks.io/docs/buildpack-author-guide/package-a-buildpack/ but then yes, I recalled that the label would be automatically documented here https://buildpacks.io/docs/tools/pack/cli/pack_buildpack_package/, thanks for checking it out!

Also, I wanted to address the failing #1877 check but I couldn't figure out how to simulate an error while setting a label on the fakes.Image in the unit tests.

As @jjbustamante mentioned you are passing to the Builder a mockImageFactory, for example:
builder := buildpack.NewBuilder(mockImageFactory(expectedImageOS)), mockImageFactory(expectedImageOS) returns a *testmocks.MockImageFactory which can be programmed to return whatever you need

@colincasey
Copy link
Contributor Author

Thanks @jjbustamante & @dlion. Coding in Go is still very new to me. TIL I learned about struct embedding 😄

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

Copy link
Member

@jjbustamante jjbustamante left a comment

Choose a reason for hiding this comment

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

I downloaded the branch, and ran a quick test against our sample repo. I ran pack buildpack package create and added a custom label. I saved the package as image on my docker daemon and as file.

For the OCI image on disk, after checking the manifest and config file, my custom label was there.

> cat blobs/sha256/3467d7313119104e71d494587e9966b6f26d8a093b2b8ed4a98d495891207e8b | jq '.config.Labels | ."my.label" '
"my.value"

Also inspecting the image saved in the daemon

> docker inspect my-buildpack-label | jq '.[] | .Config.Labels | ."my.label" '
"my.value"

Show my label correctly.

This looks good to me! Thank you @colincasey for this awesome contribution!! it is going to be very helpful for the community

@jkutner jkutner merged commit 6329e89 into buildpacks:main Sep 6, 2023
15 checks passed
@colincasey colincasey deleted the add_image_labels_to_buildpack branch September 8, 2023 18:13
@jjbustamante jjbustamante modified the milestones: 0.31.0, 0.30.1 Sep 12, 2023
@jjbustamante jjbustamante modified the milestones: 0.32.0, 0.31.0 Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to add image labels to buildpacks during pack buildpack package
4 participants