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

samples: remove nginxhttps sample #247

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

Redent0r
Copy link

@Redent0r Redent0r commented Nov 8, 2024

Merge Checklist
  • Followed patch format from upstream recommendation: https://github.com/kata-containers/community/blob/main/CONTRIBUTING.md#patch-format
    • Included a single commit in a given PR - at least unless there are related commits and each makes sense as a change on its own.
  • Aware about the PR to be merged using "create a merge commit" rather than "squash and merge" (or similar)
  • The upstream/missing label (or upstream/not-needed) has been set on the PR.
Summary

This sample was meant to show support for pulling images with v1 manifest schema versions.

The nginxhttps image has been modified in https://hub.docker.com/r/ymqytw/nginxhttps/tags such that we are no longer able to pull it:

$ docker pull ymqytw/nginxhttps:1.5
Error response from daemon: missing signature key

We may remove this sample since schema version 1 manifests are deprecated per https://docs.docker.com/engine/deprecated/#pushing-and-pulling-with-image-manifest-v2-schema-1 : "These legacy formats should no longer be used, and users are recommended to update images to use current formats, or to upgrade to more current images". This schema version was used by old docker versions. Further OCI spec https://github.com/opencontainers/image-spec/blob/main/manifest.md#image-manifest-property-descriptions only supports schema version 2.

Test Methodology

test run: https://dev.azure.com/mariner-org/mariner/_build/results?buildId=672238&view=results

@Redent0r
Copy link
Author

Redent0r commented Nov 8, 2024

A couple of more points to remove this image:

  • Schema v1 only seems to be present in some very old images (>8 years old)
  • Haven't been able to find another image with v1 schema in https://hub.docker.com/search?q=nginxhttps
  • We'll still be testing containerd pull in our test pipeline. We test all images twice: with default pull and with containerd pull
  • If a client reports v1 image broke, we may add that image back to our test suite. Although they may also consider using a newer image, since it's deprecated by docker anyways

This sample was meant to show support for pulling images with v1 manifest schema versions.

The nginxhttps image has been modified in https://hub.docker.com/r/ymqytw/nginxhttps/tags such that we are no longer able to pull it:

$ docker pull ymqytw/nginxhttps:1.5
Error response from daemon: missing signature key

We may remove this sample since schema version 1 manifests are deprecated per
https://docs.docker.com/engine/deprecated/#pushing-and-pulling-with-image-manifest-v2-schema-1 :
"These legacy formats should no longer be used, and users are recommended to update images to use current formats, or to upgrade to more
current images". This schema version was used by old docker versions. Further OCI spec
https://github.com/opencontainers/image-spec/blob/main/manifest.md#image-manifest-property-descriptions only supports schema version 2.

Signed-off-by: Saul Paredes <[email protected]>
@Redent0r Redent0r force-pushed the saulparedes/remove_nginxhttps_sample branch from 93993f2 to 715cae1 Compare November 8, 2024 19:24
@Redent0r Redent0r marked this pull request as ready for review November 11, 2024 16:41
@Redent0r Redent0r requested review from a team as code owners November 11, 2024 16:41
@sprt sprt added the upstream/not-needed PRs that will not be upstreamed (e.g. internal) label Nov 11, 2024
@Redent0r Redent0r merged commit 52039cb into msft-main Nov 11, 2024
84 of 106 checks passed
@Redent0r Redent0r deleted the saulparedes/remove_nginxhttps_sample branch November 11, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream/not-needed PRs that will not be upstreamed (e.g. internal)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants