Only upload sigstore signatures to the sigstore tag schema#2717
Merged
mtrmac merged 1 commit intocontainers:mainfrom Jun 2, 2025
Merged
Only upload sigstore signatures to the sigstore tag schema#2717mtrmac merged 1 commit intocontainers:mainfrom
mtrmac merged 1 commit intocontainers:mainfrom
Conversation
a05acfb to
ead1cfa
Compare
8e32049 to
f884987
Compare
0a17c1f to
61906b4
Compare
9e1dd58 to
50ebc82
Compare
Collaborator
Author
Member
|
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. |
Collaborator
Author
|
Note to self: See the final outcome of https://issues.redhat.com/browse/CLOUDDST-26881 . |
aguidirh
reviewed
May 26, 2025
aguidirh
left a comment
There was a problem hiding this comment.
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
Collaborator
Author
|
See documented impact in https://issues.redhat.com/browse/RUN-2504 / https://issues.redhat.com/browse/CLOUDDST-26881 . @giuseppe PTAL. |
... 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č <mitr@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
... 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.