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

Proposal to add OCI Image Annotations on Buildpacks #307

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

candrews
Copy link

Summary

Use Cases

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

Comment on lines +6 to +13
* `org.opencontainers.image.source`
* `org.opencontainers.image.revision`
* `org.opencontainers.image.title`
* `org.opencontainers.image.version`

The following additional annotation are recommended:
* `org.opencontainers.image.url`
* `org.opencontainers.image.description`
Copy link
Member

Choose a reason for hiding this comment

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

according to the spec you sent, would org.opencontainers.image.created, org.opencontainers.image.authors, org.opencontainers.image.documentation, etc be required as well?

Copy link
Author

Choose a reason for hiding this comment

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

I don't believe they would be required, but if they are available, they should be specified. For paketo, I don't see any of that information readily available in buildpack.toml or elsewhere, so I didn't include them in this RFC.

Copy link
Member

Choose a reason for hiding this comment

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

How do you know which fields in the upstream spec you sent are required?

Copy link
Author

Choose a reason for hiding this comment

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

I'm assuming that no fields are required, only that if they are present, they are required to meet the requirements for that field's value.

That's what I see in the wild, at least. I can't find a single image that specifies all of those fields, but I see a number that specify some.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok. The wording of the RFC All Paketo Buildpacks must have at least the following OCI image annotations made me think that this was required to be OCI compliant which was why I was confused.

I think this RFC seems like a good idea, especially if we can pull from the buildpack.toml fields as you suggested

@ForestEckhardt
Copy link
Contributor

As a quick clarifying question this is seeking to add these image labels to the buildpack images we release. Not to add them to the images that are produced by the buildpacks correct?

@candrews
Copy link
Author

As a quick clarifying question this is seeking to add these image labels to the buildpack images we release. Not to add them to the images that are produced by the buildpacks correct?

Correct.

Although I do think that's a good idea, I think it would best be pursued in a separate RFC. https://github.com/paketo-buildpacks/image-labels does most of that already, so the implementation of such an RFC would be to apply it by default with reasonable default values.

@ForestEckhardt
Copy link
Contributor

ForestEckhardt commented May 28, 2024

It this is something that you would like added to the buildpack images themselves that I think it my make sense to try and push this up the stack into pack. We use the pack buildpack package to turn a "formatted" tarball into an OCI image so it may make sense to ask them to add these terms to that tarball by default using the values present in the buildpack.toml.

I would be more than happy to help facilitate an interaction if that is something that you are interested in.

@candrews
Copy link
Author

It this is something that you would like added to the buildpack images themselves that I think it my make sense to try and push this up the stack into pack. We use the pack buildpack package to turn a "formatted" tarball into an OCI image so it may make sense to ask them to add these terms to that tarball by default using the values present in the buildpack.toml.

That makes sense to me! That would benefit more than just paketo too, which is nice.

I would be more than happy to help facilitate an interaction if that is something that you are interested in.

I'm certainly interested, can you please point me in the right direction? Thank you!

@ForestEckhardt
Copy link
Contributor

For sure. The upstream project has an RFCs repository https://github.com/buildpacks/rfcs. Their RFCs have a slightly different format but that is laid out in the repository itself. You are free to open a PR with an RFC in it. Please feel free to link me the RFC when you open it.

@ForestEckhardt
Copy link
Contributor

@candrews Is there anything else that needs to be done on this RFC? Should we close it out in favor of an upstream PR or do you want to modify this one?

@candrews
Copy link
Author

@candrews Is there anything else that needs to be done on this RFC? Should we close it out in favor of an upstream PR or do you want to modify this one?

I submitted the RFC to buildpacks at buildpacks/rfcs#314 - could you please facilitate this process?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants