kata-containers: CC | dependency error on kata-agent for s390x

Description of problem

Since #5542 was merged on CCv0, CI jobs (e.g. http://jenkins.katacontainers.io/job/kata-containers-CCv0-ubuntu-20.04-s390x-PR/229/consoleText) for s390x has been failing due to the following build error on agent:

Compiling ed25519 v1.5.2
error: failed to run custom build command for `ring v0.16.20`

Caused by:
  process didn't exit successfully: `/kata-containers/src/agent/target/release/build/ring-117466d433d50a09/build-script-build` (exit status: 101)
  --- stderr
  thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /root/.cargo/registry/src/github.com-eae4ba8cbf2ce1c7/ring-0.16.20/build.rs:358:10
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...
make: *** [Makefile:113: target/s390x-unknown-linux-gnu/release/kata-agent] Error 101

On the image-rs side, https://github.com/confidential-containers/image-rs/pull/47 made a difference for s390x, which introduced sigstore-rs → openidconnect → ring dependency chain (https://github.com/sigstore/sigstore-rs/blob/main/Cargo.toml#L25).

A conversation for the s390x support on ring started with a PR(https://github.com/briansmith/ring/pull/1297) last year, but not yet merged. We need to fix this issue on image-rs or sigstore-rs like what has been made for kata-ctl (https://github.com/kata-containers/kata-containers/blob/main/src/tools/kata-ctl/Cargo.toml#L23) while we are waiting for having the PR for ring merged.

[target.'cfg(target_arch = "s390x")'.dependencies]
reqwest = { version = "0.11", default-features = false, features = ["json", "blocking", "native-tls"] }

[target.'cfg(not(target_arch = "s390x"))'.dependencies]
reqwest = { version = "0.11", default-features = false, features = ["json", "blocking", "rustls-tls"] }

cc: @stevenhorsman, @arronwy, @hbrueckner

About this issue

Original URL
State: open
Created 2 years ago
Comments: 16 (15 by maintainers)

Commits related to this issue

CC: Make agent build differently for image-rs This is just to keep the support for s390x without the cosign verification while looking for a solution for #5582. Fixes: #5599 Signed-off-by: Hyounggy... — committed to BbolroC/kata-containers by BbolroC 2 years ago
CC: Make agent build differently for image-rs on s390x This is just to keep the support for s390x without the cosign verification while looking for a solution for #5582. Fixes: #5599 Signed-off-by:... — committed to BbolroC/kata-containers by BbolroC 2 years ago
CC: Make agent build differently for image-rs on s390x This is just to keep the support for s390x without the cosign verification while looking for a solution for #5582. Fixes: #5599 Signed-off-by:... — committed to BbolroC/kata-containers by BbolroC 2 years ago
CC: Make agent build differently for image-rs on s390x This is just to keep the support for s390x without the cosign verification while looking for a solution for #5582. Fixes: #5599 Signed-off-by:... — committed to BbolroC/kata-containers by BbolroC 2 years ago
CC: Make agent build differently for image-rs on s390x This is just to keep the support for s390x without the cosign verification while looking for a solution for #5582. Fixes: #5599 Signed-off-by:... — committed to BbolroC/kata-containers by BbolroC 2 years ago
CC: Make agent build differently for image-rs on s390x This is just to keep the support for s390x without the cosign verification while looking for a solution for #5582. Fixes: #5599 Signed-off-by:... — committed to BbolroC/kata-containers by BbolroC 2 years ago
Exclude image-rs cosign feature from when building for s390x Exclude the image-rs cosign feature when the build target is the s390x architecture. Fixes: kata-containers#5582 Signed-off-by: Matthew ... — committed to mattarnoatibm/kata-containers by mattarnoatibm 2 years ago
Exclude image-rs cosign feature from when building for s390x Exclude the image-rs cosign feature when the build target is the s390x architecture. Change Cargo to use workspace resolver 2 so that con... — committed to mattarnoatibm/kata-containers by mattarnoatibm 2 years ago
agent: Fix s390x agent build Exclude the image-rs cosign feature when the build target is the s390x architecture. Change Cargo to use workspace resolver 2 so that conditional include for the image-r... — committed to mattarnoatibm/kata-containers by mattarnoatibm 2 years ago
agent: Fix s390x agent build Exclude the image-rs cosign feature when the build target is the s390x architecture. Change Cargo to use workspace resolver 2 so that conditional include for the image-r... — committed to mattarnoatibm/kata-containers by mattarnoatibm 2 years ago
agent: Fix s390x agent build Exclude the image-rs cosign feature when the build target is the s390x architecture. Change Cargo to use workspace resolver 2 so that conditional include for the image-r... — committed to mattarnoatibm/kata-containers by mattarnoatibm 2 years ago

Most upvoted comments

BTW - This PR is working add RustCrypto support, which can get rid of ring. https://github.com/ramosbugs/openidconnect-rs/pull/58

Xynnn007 on Nov 7, 2022

Looks like @briansmith is working on https://github.com/briansmith/ring/issues/1555, when the issue be finished, maybe openidconnect can run on s390x platform successfully with new released ring version.

liudalibj on Nov 10, 2022

UPDATE

Investigation and test results

The code changes below made for kata-ctl cannot solve the problem:

[target.'cfg(target_arch = "s390x")'.dependencies]
reqwest = { version = "0.11", default-features = false, features = ["json", "blocking", "native-tls"] }

[target.'cfg(not(target_arch = "s390x"))'.dependencies]
reqwest = { version = "0.11", default-features = false, features = ["json", "blocking", "rustls-tls"] }

The reqwest has features where we could selectively configure either of them for the crate (even none of them).

rustls-tls refers to hyper-rustls -> rustls -> ring
native-tls does not have any chain to ring.

The problem is different for the sigstore case (Cargo.toml at the commit referred by image-rs: https://github.com/sigstore/sigstore-rs/blob/v0.3.3/Cargo.toml) , native-tls is configured by default. but the problem comes from openidconnect, where ring is hard-coded as dependency (https://github.com/ramosbugs/openidconnect-rs/blob/main/Cargo.toml#L41)

Conclusion

Let sigstore not use openidconnect - not good
Let image-rs not use sigstore - not good
Get the PR for ring merged - ideal, but no guarantee for when

I look forward to feedback from @uweigand. Thanks @Xynnn007 and @huoqifeng for the discussion

BbolroC on Nov 4, 2022

FYI, recentlyopenidconnect replaced ring with Rust Cryptos. I’m going to test in the sigstore crate and replace that with the new version, which may help s390x to support cosign.

Xynnn007 on Feb 7, 2023

While we wait for the fix in the ring create to be progressed and completed we’re going to attempt a workaround conditionally compiling image-rs without the sigstore-rs crate and without support for cosign signing when its compiled for s390x: https://github.com/confidential-containers/image-rs/issues/80.

mattarnoatibm on Nov 25, 2022

Plus, unlike the PR for ring (titled s390x support), it might be really hard to convince an author of openidconnect (https://github.com/ramosbugs) to switch to sha2 just for the issue we are facing.

Yep - that makes sense and not unexpected, we just thought we’d throw in the idea. 🤞 we can get the ring PR merged soon.

stevenhorsman on Nov 7, 2022

@Xynnn007 - thanks for the update and for testing it out. I thought Gerry mentioned that the latest sigstore clashed with another dependency, but hopefully that is resolved/resolvable now?

stevenhorsman on Feb 7, 2023

@Xynnn007

arronwy on Nov 4, 2022