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

rules: Drop architecture and build-date from required container labels #100

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

cgwalters
Copy link
Contributor

(OCI/Docker) containers have a long history, and Red Hat was there in the early days, before things were standardized as OCI even - and especially before things like manifest lists and also the standard annotation keys: https://github.com/opencontainers/image-spec/blob/main/annotations.md#pre-defined-annotation-keys

There are two labels this policy requires that duplicate standard metadata.

  • architecture: This is already handled via manifest listing and is part of the config
  • build-date: There's a standard Created field

As these are basically legacy from old Red Hat build systems we are just cargo culting forward for as far as I know no good reason, let's stop doing that.

Signed-off-by: Colin Walters [email protected]

…abels

(OCI/Docker) containers have a long history, and Red Hat was there in the early days, before things were standardized as OCI even - and especially before things like manifest lists and also the standard annotation keys: https://github.com/opencontainers/image-spec/blob/main/annotations.md#pre-defined-annotation-keys

There are two labels this policy requires that duplicate standard metadata.

- `architecture`: This is already handled via manifest listing and is part of the config
- `build-date`: There's a standard `Created` field

As these are basically legacy from old Red Hat build systems we are just cargo culting forward for as far as I know no good reason, let's stop doing that.
@cgwalters
Copy link
Contributor Author

I've said this elsewhere but IMO the OpenSUSE base image has a good set of labels, including only using the OCI standard ones, and a well-defined namespacing for their custom ones.

The labels on many Red Hat containers today fall short of this:

skopeo inspect -n docker://docker.io/opensuse/leap | jq .Labels
{
  "org.openbuildservice.disturl": "obs://build.opensuse.org/openSUSE:Leap:15.6:Images/images/25049cab88eae9e94c1aeac7a8f0aee7-opensuse-leap-image:docker",
  "org.opencontainers.image.created": "2024-09-19T19:48:27.536230874Z",
  "org.opencontainers.image.description": "Image containing a minimal environment for containers based on openSUSE Leap 15.6.",
  "org.opencontainers.image.source": "https://build.opensuse.org/package/show/openSUSE:Leap:15.6:Images/opensuse-leap-image?rev=25049cab88eae9e94c1aeac7a8f0aee7",
  "org.opencontainers.image.title": "openSUSE Leap 15.6 Base Container",
  "org.opencontainers.image.url": "https://www.opensuse.org/",
  "org.opencontainers.image.vendor": "openSUSE Project",
  "org.opencontainers.image.version": "15.6.5.643",
  "org.opensuse.base.created": "2024-09-19T19:48:27.536230874Z",
  "org.opensuse.base.description": "Image containing a minimal environment for containers based on openSUSE Leap 15.6.",
  "org.opensuse.base.disturl": "obs://build.opensuse.org/openSUSE:Leap:15.6:Images/images/25049cab88eae9e94c1aeac7a8f0aee7-opensuse-leap-image:docker",
  "org.opensuse.base.reference": "registry.opensuse.org/opensuse/leap:15.6.5.643",
  "org.opensuse.base.source": "https://build.opensuse.org/package/show/openSUSE:Leap:15.6:Images/opensuse-leap-image?rev=25049cab88eae9e94c1aeac7a8f0aee7",
  "org.opensuse.base.title": "openSUSE Leap 15.6 Base Container",
  "org.opensuse.base.url": "https://www.opensuse.org/",
  "org.opensuse.base.vendor": "openSUSE Project",
  "org.opensuse.base.version": "15.6.5.643",
  "org.opensuse.reference": "registry.opensuse.org/opensuse/leap:15.6.5.643"
}
``

@lcarva
Copy link
Collaborator

lcarva commented Jan 9, 2025

+1 to moving towards the standard labels. I think dropping those two is probably ok. But we also want to make sure nothing else relies on these labels. Not sure what's the safest way to go about that.

There's also url, vcs-ref, vcs-type, and probably others that we can drop.

I vaguely recall that the many io.k8s.* labels we require were to support certain product features. But, of course, we should revisit all of those as well.

@cgwalters
Copy link
Contributor Author

But we also want to make sure nothing else relies on these labels. Not sure what's the safest way to go about that.

@cgwalters
Copy link
Contributor Author

There's also url, vcs-ref, vcs-type, and probably others that we can drop.

I don't think we should drop those, but we should switch to requiring the OCI standard versions.

@cgwalters
Copy link
Contributor Author

IOW let's treat discussion of keys that are fully redundant and can just be deleted (the ones here) from ones that may need to change separately.

Copy link
Collaborator

@lcarva lcarva left a comment

Choose a reason for hiding this comment

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

@ralphbean, @arewm, @rhartman93 could you ptal?

@arewm
Copy link
Contributor

arewm commented Jan 9, 2025

I am generally in favor of moving towards accepted standards. I tried to take a look at our requirements in the past to reassess what we should require, but I think that the changes were too large to be easily prioritized: https://docs.google.com/document/d/1MB-b7ldO6J0de_3IajiseUA0aoLma4r9DJtkeogt-BQ/edit?tab=t.0 (internal document).

In that, I proposed using org.opencontainers.image.created and ultimately dropping the architecture label.

@lcarva
Copy link
Collaborator

lcarva commented Jan 9, 2025

I'll merge this on Monday if there are no objections (or sooner if I get additional approvals.)

- name: architecture
description: Architecture the software in the image should target.
- name: build-date
description: Date/Time image was built as RFC 3339 date-time.
Copy link

Choose a reason for hiding this comment

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

Why do we want to remove build date?

It helps with idempotency in the release process.

removing architecture make a sense due to to multiarch (manifest v list)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why do we want to remove build date?

I wrote in the commit message - because it's redundant with existing standard metadata. The replacement for it is the created field in https://github.com/opencontainers/image-spec/blob/main/config.md

Copy link
Contributor Author

@cgwalters cgwalters Jan 9, 2025

Choose a reason for hiding this comment

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

$ skopeo inspect -n docker://registry.access.redhat.com/ubi9/ubi:latest
{
...
    "Created": "2025-01-09T06:37:11.783912772Z",
    "Labels": {
...
        "build-date": "2025-01-09T06:27:16Z",
...

In ubi9 for example the values are off by about 10 seconds (edit: minutes wow...why?) just because they're generated at slightly different times. There's no value in the duplication.
(In fact, if one cares about reproducible builds these types of random timestamps are The Enemy and need to be carefully controlled and considered)

@ralphbean
Copy link
Member

No objection to dropping this from the required list.

Note, I do see some special occurrences of build-date in konflux-ci/build-definitions that we'll want to deal with too, although that doesn't block removing this as a requirement. There, the buildah task is setting the build-date and architecture on all images. That's the next thing to modernize.

@ralphbean ralphbean merged commit 85dab38 into release-engineering:main Jan 28, 2025
1 check passed
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.

5 participants