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

RFC for OCI Image Annotations on Buildpacks #314

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

candrews
Copy link

The buildpack tool should set OCI annotations on the OCI artifacts it produces providing users of these buildpacks with a consistent, standard mechanism for gathering information about the buildpack, including how to find its documentation and what version it is. The annotation values can be gathered from existing data sources (such as buildpack.toml) therefore not requiring any additional effort on the part of users of the buildpack tool.

Originally proposed to Paketo at paketo-buildpacks/rfcs#307 where it was pointed out that this change would best be made in the buildpack tool, see paketo-buildpacks/rfcs#307 (comment)

@buildpack-bot
Copy link
Member

Maintainers,

As you review this RFC please queue up issues to be created using the following commands:

/queue-issue <repo> "<title>" [labels]...
/unqueue-issue <uid>

Issues

(none)


When building the buildpack, the `buidpack` tool can get the values for the `org.opencontainers.image.source` and `org.opencontainers.image.revision` annotations from git. `org.opencontainers.image.source` is derived from the git origin and `org.opencontainers.image.revision` is the git commit ref.

The other annotation values come from `buildpack.toml` mapped to OCI annotations as follows:
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when an app has more than one buildpack? The proposed schema doesn't support more than one entry.

Copy link
Author

Choose a reason for hiding this comment

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

The RFC is for the buildpacks themselves - not for the apps built by the buildpacks.

For example, it would apply to the Paketo Java buildpack, gcr.io/paketo-buildpacks/java, not to the application it builds.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok. I think the RFC wording might need adjusting to make that clearer.

For example here:

The buildpack tool should set OCI annotations on the OCI artifacts it produces

"buildpack tool" isn't a phrase that's really used by this project, and read to me as meaning "the buildpack".

So it then read as:

"The buildpack should set OCI annotations on the OCI artifacts it produces"

...and the only OCI artifacts created when the buildpack runs is the app OCI image.

It sounds like what you're really proposing is a change to the buildpack distribution specification:
https://github.com/buildpacks/spec/blob/main/distribution.md

Copy link
Contributor

Choose a reason for hiding this comment

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

It sounds like what you're really proposing is a change to the buildpack distribution specification:

Combined with proposing that the Pack CLI implement these changes, so that when a buildpack maintainer runs pack buildpack package to package their buildpack into the OCI image form of distribution it has the proposed labels set.

Copy link
Author

@candrews candrews Jun 26, 2024

Choose a reason for hiding this comment

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

Instead of:

The buildpack tool should set...

I've updated the RFC to say:

pack buildpack package should set

Does that eliminate the confusion?

It sounds like what you're really proposing is a change to the buildpack distribution specification:
https://github.com/buildpacks/spec/blob/main/distribution.md

I believe so - is there something I should do in this RFC to indicate as much?

Copy link
Contributor

Choose a reason for hiding this comment

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

Does that eliminate the confusion?

Yeah that does thank you.

I believe so - is there something I should do in this RFC to indicate as much?

So I think this comes down to whether this RFC is proposing only a change to the Pack CLI's own implementation, or a change to the specification which then the Pack CLI (plus any other implementations) would implement. If the latter, than the spec change could be described under the "spec change" section of the RFC template.

Copy link
Author

Choose a reason for hiding this comment

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

I've updated this MR to indicate that the Distribution spec would also be updated.

@candrews candrews force-pushed the oci-image-annotations branch 3 times, most recently from 3147747 to a9a4863 Compare June 26, 2024 20:43
Signed-off-by: Craig Andrews <candrews@integralblue.com>
@natalieparellano
Copy link
Member

natalieparellano commented Jun 27, 2024

Awesome to see this! I believe this supersedes #196

Edit: I am wrong, this is about buildpacks themselves, not the artifacts they produce!

Co-authored-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Craig Andrews <candrews@integralblue.com>
@shresthaujjwal
Copy link

Thanks for opening this RFC. May I recommend extending this RFC not just to buildpack but also to builders?

@candrews
Copy link
Author

Thanks for opening this RFC. May I recommend extending this RFC not just to buildpack but also to builders?

That sounds like a good idea to me!

# How it Works
[how-it-works]: #how-it-works

When building the buildpack, the `buildpack` tool can get the values for the `org.opencontainers.image.source` and `org.opencontainers.image.revision` annotations from git. `org.opencontainers.image.source` is derived from the git origin and `org.opencontainers.image.revision` is the git commit ref.
Copy link
Member

Choose a reason for hiding this comment

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

git commit ref

Should this be the Git commit sha instead of the ref?

Copy link
Author

Choose a reason for hiding this comment

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

You are correct - that should be the sha, not the commit ref.

candrews and others added 2 commits July 12, 2024 11:33
Co-authored-by: Joe Kutner <jpkutner@gmail.com>
Signed-off-by: Craig Andrews <candrews@integralblue.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants