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 OCI recommended annotations #196

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

Add OCI recommended annotations #196

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 78 additions & 0 deletions text/0000-annotations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Meta
[meta]: #meta
- Name: OCI Image annotations
- Start Date: 2021-12-03
- Author(s): [@samj1912](https://github.com/samj1912)
- Status: Draft <!-- Acceptable values: Draft, Approved, On Hold, Superseded -->
- RFC Pull Request: (leave blank)
- CNB Pull Request: (leave blank)
- CNB Issue: (leave blank)
- Supersedes: (put "N/A" unless this replaces an existing RFC, then link to that RFC)

# Summary
[summary]: #summary

Add OCI recommended image annotations to output images.

# Definitions
[definitions]: #definitions

N/A

# Motivation
[motivation]: #motivation

Certain OCI image scanning and debugging tools take into account standard annotations on the images as defined by [OCI Spec - Pre-defined Annotation keys](https://github.com/opencontainers/image-spec/blob/main/annotations.md#pre-defined-annotation-keys). Buildpacks already records some of this information in the `project.toml` and other labels defined at https://github.com/buildpacks/spec/blob/main/platform.md#labels. We should also preserve this information in the output image in appropriate annotations as recommended by the OCI spec.

# What it is
[what-it-is]: #what-it-is

Adds OCI recommended annotations to the output image.

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

Optionally adds the following annotations where applicable when lifecycle is used to publish images directly to the registry -

- `org.opencontainers.image.base.name`: Run image tag name. Currently stored in `io.buildpacks.lifecycle.metadata.run-image.reference`
Copy link
Member

@natalieparellano natalieparellano Jan 3, 2022 •
edited

Choose a reason for hiding this comment

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

I think this is actually io.buildpacks.lifecycle.metadata.stack.runImage.image

- `org.opencontainers.image.base.digest`: Run image digest. Currently stored in `io.buildpacks.lifecycle.metadata.run-image.reference`
Copy link
Member

Choose a reason for hiding this comment

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

Does the run-image.reference data include both the mutable tag reference and the immutable digest?

Copy link
Member

Choose a reason for hiding this comment

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

Based on the spec this would be the immutable digest:

A digest reference to a manifest stored in an OCI image registry

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `org.opencontainers.image.base.digest`: Run image digest. Currently stored in `io.buildpacks.lifecycle.metadata.run-image.reference`
- `org.opencontainers.image.base.digest`: Run image digest. Currently stored in `io.buildpacks.lifecycle.metadata.runImage.reference`


The platform may also provide appropriate keys inside `project-metadata.toml` to the lifecycle so that it exports the appropriate annotations listed below.
Copy link
Member

Choose a reason for hiding this comment

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

In the ideal world I think these project metadata annotation would be better contributed by a buildpack vs by the lifecycle for the following reasons:

  1. Buildpack contributed behavior more easily upgradable accross a variable of platforms
  2. Keeping the surface area of the platform API as small as possible makes the spec and the conditional logic in lifecycle less complicated, thus making our lives easier and enabling us to actually make progress shipping things.

The way I imagine it:

  • step 1 remove daemon
  • step 2 allow buildpacks to contribute aribtrary annotations
  • step 3 provide a (utility?) buildpack that contributes these annotations based off the values in project.toml

Copy link
Member

Choose a reason for hiding this comment

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

That ordering makes sense to me. Is there a rough plan/timeline for step 1?

Copy link
Member

Choose a reason for hiding this comment

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

It ties into this RFC issue: #197

@jjbustamante is on it!

Copy link
Member

@jromero jromero Jan 20, 2022 •
edited

Choose a reason for hiding this comment

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

Based on my comment here, I'm back to say that I don't think step 1 is necessary.

steps 2 and 3 can be an alternative approach to this RFC without step 1. I'd love to see it comparatively in the Alternatives section.

Copy link
Member

Choose a reason for hiding this comment

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

#196 (comment)


- `org.opencontainers.image.source`: URL to the image source if the source is a source controlled repository, URL to a blob etc. Sourced from `source.metadata.repository`.
- `org.opencontainers.image.revision`: Source controlled revision of application source code. Sourced from `source.version.commit`.
- `org.opencontainers.image.authors`: Authors of hte image. Sourced from `source.metadata.authors`.
- `org.opencontainers.image.version`: Authors of hte image. Sourced from `source.metadata.version`.
Comment on lines +44 to +45
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- `org.opencontainers.image.authors`: Authors of hte image. Sourced from `source.metadata.authors`.
- `org.opencontainers.image.version`: Authors of hte image. Sourced from `source.metadata.version`.
- `org.opencontainers.image.authors`: Authors of the image. Sourced from `source.metadata.authors`.
- `org.opencontainers.image.version`: Version of the image. Sourced from `source.metadata.version`.

- `org.opencontainers.image.documentation`: URL to find more information on the image. Sourced from `source.metadata.documentation-url`.
- `org.opencontainers.image.title`: Human-readable title of the image. Sourced from `source.metadata.name`.
- `org.opencontainers.image.licenses`: License(s) under which contained software is distributed as an SPDX License Expression. Sourced from `source.metadata.licenses`.

A platform may choose to map the above fields to the fields defined in a project descriptor as noted at https://github.com/buildpacks/spec/blob/extensions/project-descriptor%2F0.2/extensions/project-descriptor.md#non-_-tables.

Everything under `source.metadata.<key>` is mapped from `_.<key>` from the project descriptor in this case.

# Drawbacks
[drawbacks]: #drawbacks

In case of the daemon mode, the annotations cannot be applied and will be ignored.
Copy link
Member

Choose a reason for hiding this comment

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

Up until this point we have endeavored to make the output image identical regardless of export target (registry vs daemon). I am leary of introducing behavior differences between the daemon and remote cases. IMO it would be much better to remove the daemon as an export target first before continuing with this RFC.

Copy link
Member

@jromero jromero Jan 20, 2022 •
edited

Choose a reason for hiding this comment

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

I would agree if the registry and daemon cases were identical to begin with but they are already not. Things like io.buildpacks.lifecycle.metadata.runImage.reference change depending on whether it's built for daemon or registry. The same inputs for an image don't survive round-tripping because of that and the difference in compression of layers. In other words, lifecycle -> daemon -> registry != lifecycle -> registry. IMO, it doesn't sound reasonable to block this feature on the eventual hope that we'll get rid of the daemon. At the end of the day, the core concern (discrepancy) will always exist but be pushed onto the platform.

Copy link
Member

Choose a reason for hiding this comment

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

IMO, it doesn't sound reasonable to block this feature on the eventual hope that we'll get rid of the daemon.

In order to get rid of the daemon instead of merely hoping it goes away I think we need to hold the line somewhere and actually prioritize getting rid of it. Adding remote-only features seems like the right place to draw the line to me.

Yes there is a path forward without removing the daemon first, but it is uglier and requires more compromises in the lifecycle and imgutil and caveats in our docs and specs. Instead we could invest in removing the daemon and clean up some of the uglier parts of the project.


# Alternatives
[alternatives]: #alternatives

N/A

# Prior Art
[prior-art]: #prior-art

- https://github.com/buildpacks/rfcs/blob/main/text/0054-project-descriptor-schema.md
- https://github.com/buildpacks/rfcs/blob/main/text/0084-project-descriptor-domains.md

# Unresolved Questions
[unresolved-questions]: #unresolved-questions

N/A

# Spec. Changes (OPTIONAL)
[spec-changes]: #spec-changes

Per above.