-
Notifications
You must be signed in to change notification settings - Fork 95
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
kbs-types and sigstore updates #408
Conversation
Signed-off-by: Mikko Ylinen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -38,7 +38,7 @@ serde = { workspace = true, features = ["serde_derive", "rc"] } | |||
serde_json.workspace = true | |||
serde_yaml = { version = "0.9", optional = true } | |||
sha2.workspace = true | |||
sigstore = { git = "https://github.com/sigstore/sigstore-rs.git", rev = "69e8f33", default-features = false, optional = true} | |||
sigstore = { git = "https://github.com/sigstore/sigstore-rs.git", tag = "v0.7.2", default-features = false, optional = true} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Seems that we have done duplicated things 39eec75
but it is ok for me to get this merged first and I'll take a rebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, I had totally missed this. There was no PR with the title indicating the overlap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea. No problem. I once thought that no one care about this : )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like fa01b06 is another overlap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea. No problem. I once thought that no one care about this : )
I'm doing "housekeeping" every once in a while. I personally think we should be doing version upgrades more systematically and be sure we're always using the latest when making new releases. Staying always up-to-date helps if some sudden CVE comes and we have to update quickly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Xynnn007 I added signed-of-bys from both of us to the sigstore-rs commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Let's wait another maintainer to check again and get this merged.
Update to a more recent sigstore-rs version that brings fixes to what image-rs had to workaround earlier. It also pulls in never RustCrypto version which triggered cosign PublibKeyVerifier errors due to changes in data structures. Signed-off-by: Mikko Ylinen <[email protected]> Signed-off-by: Xynnn007 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.