Skip to content

Only upload sigstore signatures to the sigstore tag schema #2717

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

Merged
merged 1 commit into from
Jun 2, 2025

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Feb 13, 2025

... not to registries with X-R-S-S (that would immediately fail, when uploading to such a registry, with no opt-out), and not to
lookaside (that would work if users set up lookaside).

Before this PR, if the image had any non-sigstore signatures, we would upload both the non-sigstore and sigstore signatures to lookaside.

@mheon @TomSweeneyRedHat PTAL. The bug has been that way for 2.5 years. I think the combination of required circumstances (an image must have both kinds of signatures, and the user must have configured lookaside, not use of sigstore tags, while somehow requiring sigstore signatures) makes it unlikely that users would rely on it, but I can’t quite rule it out.

@mtrmac mtrmac force-pushed the sigstore-only branch 2 times, most recently from a05acfb to ead1cfa Compare March 4, 2025 18:47
@mtrmac mtrmac added the kind/bug A defect in an existing functionality (or a PR fixing it) label Mar 4, 2025
@mtrmac mtrmac force-pushed the sigstore-only branch 2 times, most recently from 8e32049 to f884987 Compare March 19, 2025 17:16
@mtrmac mtrmac force-pushed the sigstore-only branch 3 times, most recently from 0a17c1f to 61906b4 Compare April 1, 2025 18:11
@mtrmac mtrmac force-pushed the sigstore-only branch 2 times, most recently from 9e1dd58 to 50ebc82 Compare April 7, 2025 20:30
@mtrmac
Copy link
Collaborator Author

mtrmac commented May 14, 2025

@mheon WDYT? @giuseppe PTAL

@mheon
Copy link
Member

mheon commented May 14, 2025

I kind of wish we'd gotten this one in for 5.5 so it could soak in Fedora, but the number of people using sigstore on Fedora is likely miniscule compared to the number on RHEL so that might not have helped us any.

I'll give it some thought but I think fixing is the best course of action from what I see now.

@mtrmac
Copy link
Collaborator Author

mtrmac commented May 21, 2025

Note to self: See the final outcome of https://issues.redhat.com/browse/CLOUDDST-26881 .

Copy link

@aguidirh aguidirh left a comment

Choose a reason for hiding this comment

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

Hi @mtrmac

I tested with the code from this PR and I confirm it fixes the problem.

I ran oc-mirror v2 three times with the current containers/image dependency v5.35.0 and as you can see the logs below the signatures were always increasing in number:

== with current containers/image release v5.35.0 ==

aguidi@fedora:~$ tree /home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
/home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
├── signature-1
├── signature-10
├── signature-11
├── signature-12
├── signature-13
├── signature-14
├── signature-15
├── signature-16
├── signature-17
├── signature-18
├── signature-19
├── signature-2
├── signature-20
├── signature-21
├── signature-22
├── signature-23
├── signature-24
├── signature-25
├── signature-26
├── signature-27
├── signature-28
├── signature-3
├── signature-4
├── signature-5
├── signature-6
├── signature-7
├── signature-8
└── signature-9

1 directory, 28 files
aguidi@fedora:~$ tree /home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
/home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
├── signature-1
├── signature-10
├── signature-11
├── signature-12
├── signature-13
├── signature-14
├── signature-15
├── signature-16
├── signature-17
├── signature-18
├── signature-19
├── signature-2
├── signature-20
├── signature-21
├── signature-22
├── signature-23
├── signature-24
├── signature-25
├── signature-26
├── signature-27
├── signature-28
├── signature-29
├── signature-3
├── signature-30
├── signature-31
├── signature-4
├── signature-5
├── signature-6
├── signature-7
├── signature-8
└── signature-9

1 directory, 31 files

aguidi@fedora:~$ tree /home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
/home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
├── signature-1
├── signature-10
├── signature-11
├── signature-12
├── signature-13
├── signature-14
├── signature-15
├── signature-16
├── signature-17
├── signature-18
├── signature-19
├── signature-2
├── signature-20
├── signature-21
├── signature-22
├── signature-23
├── signature-24
├── signature-25
├── signature-26
├── signature-27
├── signature-28
├── signature-29
├── signature-3
├── signature-30
├── signature-31
├── signature-32
├── signature-33
├── signature-34
├── signature-4
├── signature-5
├── signature-6
├── signature-7
├── signature-8
└── signature-9

1 directory, 34 files

On the other hand, when I built oc-mirror v2 changing the containers/image dependency to the one from this PR, the signatures were reduced to only 4 and even running oc-mirror 2 times, it always kept only 4 signatures (as you can see in the logs below):

== with code from PR https://github.com/containers/image/pull/2717 ==
== replace github.com/containers/image/v5 => github.com/mtrmac/image/v5 v5.0.0-20250514190842-8137112c374e ==

aguidi@fedora:~$ tree /home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
/home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
├── signature-1
├── signature-2
├── signature-3
└── signature-4

1 directory, 4 files

aguidi@fedora:~$ tree /home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
/home/aguidi/.local/share/containers/sigstore/albo/aws-load-balancer-controller-rhel8@sha256=1aae3a6a1487932216159759a98d60630fe4885736c0232a4d6d276820ece333
├── signature-1
├── signature-2
├── signature-3
└── signature-4

1 directory, 4 files

/lgtm

@mtrmac
Copy link
Collaborator Author

mtrmac commented May 26, 2025

Copy link
Member

@giuseppe giuseppe left a comment

Choose a reason for hiding this comment

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

LGTM

... not to registries with X-R-S-S (that would immediately fail,
when uploading to such a registry, with no opt-out), and not for
 lookaside (that would _work_ if users set up lookaside).

Signed-off-by: Miloslav Trmač <[email protected]>
@mtrmac mtrmac merged commit 6f44ecc into containers:main Jun 2, 2025
10 checks passed
@mtrmac mtrmac deleted the sigstore-only branch June 2, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A defect in an existing functionality (or a PR fixing it)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants