search

confidential-containers / guest-components

Confidential Containers Guest Tools and Components
Apache License 2.0
71 stars 76 forks source link

image-rs: bump dependencies #573

Closed Xynnn007 closed 3 weeks ago

Xynnn007 commented 3 weeks ago

This PR updates some dependencies version.

There will be a lint error for rust toolchain v1.72.0 if we use sigstore v0.9.0 upstream. At the meanwhile, the latest toolchain accepts this so this test passes.

I once thought we could allow this in guest-components but soon found that there is clippy check using v1.72.0 rust toolchain to check clippy in kata side.

I think it is hard and not reasonable for upstream sigstore for this to fix a historical version toolchain lint error. Thus I created a forked version of sigstore v0.9.0 with only one extra patch to fix this. This is why the sigstore's rev points to a personal repo. Hopefully this workaround could be acceptable.

For a long time, I wonder how we update rust toolchain to higher level in kata-side. The frequency?

stevenhorsman commented 3 weeks ago

I think the reqwest bump has broken the signature verification test (I backed down to 0.11.27 and it passed):

---- signature_verification stdout ----
cdh_path: /home/runner/work/guest-components/guest-components/image-rs/scripts/ttrpc/confidential-data-hub
script_dir: /home/runner/work/guest-components/guest-components/image-rs/scripts
thread 'signature_verification' panicked at image-rs/tests/signature_verification.rs:214:13:
assertion `left == right` failed: Test: Allow pulling a unencrypted signed image from a protected registry., Signing scheme: Simple Signing, Err(Security validate failed: Validate image failed: Failed to query extensions supported by registry v2 endpoint

Caused by:
    0: error sending request for url (https://quay.io/v2/)
    1: client error (Connect)
    2: invalid URL, scheme is not http)
  left: false
 right: true

The code is failing in https://github.com/confidential-containers/guest-components/blob/0b28991babc7d5c064b904bef1d318882fe6e976/image-rs/src/signature/mechanism/simple/xrss.rs#L102-L107

Interesting I think this is the negative test as quay.io doesn't support xrss, so we expect the headers to be missing, but it obviously shouldn't fail like this, so I'll try and debug it more.

stevenhorsman commented 3 weeks ago

I think the reqwest bump has broken the signature verification test (I backed down to 0.11.27 and it passed)

After too much local debugging and trial and error I found the fix for this. Adding the "default-tls" feature to reqwests.

Xynnn007 commented 3 weeks ago

Let me also update ehsm-client and oci-distribution dep in this PR.

Xynnn007 commented 3 weeks ago

Rebased the code.

LGTM, but we probably want some clarity on kata msrv before merging.

What would be the trigger of this?

stevenhorsman commented 3 weeks ago

LGTM, but we probably want some clarity on kata msrv before merging.

What would be the trigger of this?

The conversation in the Kata AC meeting I guess.

I ran cargo msrv on this code and it looks like the required version is 1.72.1, so we are super close with 1.72.0 in kata-containers, so I don't anticipate any issues bumping to that at the minimum as Zvonko is planning to move to move us to 1.74 as part of the CDI work, so my personal view is that we don't need to wait to merge this.