Skip to content
This repository has been archived by the owner on Jul 2, 2023. It is now read-only.

aes: fix build errors #17

Merged
merged 4 commits into from Jan 27, 2022
Merged

Conversation

stevenhorsman
Copy link
Member

Updates the hmac version requirements and code to fix the build errors

Fixes: confidential-containers/guest-components#263
Signed-off-by: stevenhorsman steven@uk.ibm.com

Updates the hmac version requirements and code to fix the build errors

Fixes: #16
Signed-off-by: stevenhorsman <steven@uk.ibm.com>
@stevenhorsman
Copy link
Member Author

Hey @arronwy - this issue is causing problems in the kata-agent CCv0 build due to the dependency on this repo for the rustified pull image (e.g. the static checks failing in kata-containers/kata-containers#3377). I may have got the fix wrong here, but it would be great if you could have a look and let me know. Thanks!

@stevenhorsman
Copy link
Member Author

H @sameo - it looks like you might also have access to this repo, so if you are available to take a look that would be helpful. Thanks!

@sameo
Copy link
Member

sameo commented Jan 5, 2022

Hey @stevenhorsman . Thanks for the PR...Running the GH actions now, let's see.

- Fix nightly clippy linting errors
Signed-off-by: stevenhorsman <steven@uk.ibm.com>
@stevenhorsman
Copy link
Member Author

stevenhorsman commented Jan 5, 2022

@sameo - thanks for kicking off the workflow. There were a bunch of nightly clippy errors. None of it in code I changed, but I've fixed up most of them. There was one error in src/utils/keyprovider.rs which I couldn't fix as I think that file is generated and I've not got much experience with proto buffers to know how to add annotations.

@stevenhorsman
Copy link
Member Author

Hi @arronwy - are you able to help with the final linting problem? Thanks.

@arronwy
Copy link
Member

arronwy commented Jan 10, 2022

Hi @arronwy - are you able to help with the final linting problem? Thanks.

Sure, I'll be back to office tomorrow and check this issue with @pravinrajr9

@pravinrajr9
Copy link
Contributor

pravinrajr9 commented Jan 12, 2022

Hi @arronwy - are you able to help with the final linting problem? Thanks.

Sure, I'll be back to office tomorrow and check this issue with @pravinrajr9

utils/keyprovider.rs is auto generated code, there is a issue opened here hyperium/tonic#876 has an open issue
we might to wait or we might need to stop generating this code from build.rs and fix manually with the patch below

diff --git a/src/utils/keyprovider.rs b/src/utils/keyprovider.rs
index d12ce43..c481f8e 100644
--- a/src/utils/keyprovider.rs
+++ b/src/utils/keyprovider.rs
@@ -59,11 +59,13 @@ pub mod key_provider_service_client {
         #[doc = r""]
         #[doc = r" This requires the server to support it otherwise it might respond with an"]
         #[doc = r" error."]
+        #[must_use]
         pub fn send_gzip(mut self) -> Self {
             self.inner = self.inner.send_gzip();
             self
         }
         #[doc = r" Enable decompressing responses with `gzip`."]
+        #[must_use]
         pub fn accept_gzip(mut self) -> Self {
             self.inner = self.inner.accept_gzip();
             self

@stevenhorsman
Copy link
Member Author

@pravinrajr9 - thanks for the information and link. Given that the tonic issue hasn't been looked at for 12 days and this PR has been holding up work in kata-containers for 8 days it would be great to get a workaround in with the patch you suggested.

@arronwy
Copy link
Member

arronwy commented Jan 21, 2022

@sameo Since the clippy error happens on nightly build aganist auto generated code, can we merged this PR first and wait tonic upstream to fix the issue to avoid the temp fix in our repo?

@sameo
Copy link
Member

sameo commented Jan 21, 2022

@sameo Since the clippy error happens on nightly build aganist auto generated code, can we merged this PR first and wait tonic upstream to fix the issue to avoid the temp fix in our repo?

@arronwy I'd prefer if we carry @pravinrajr9 patch and disable the autogenerated code until hyperium/tonic#892 is merged.

Samuel Ortiz added 2 commits January 26, 2022 17:09
We need 0.10+ or else we hit that error:

error[E0277]: the trait bound `sha2::Sha256: CoreProxy` is not satisfied
  --> src/blockcipher/aes_ctr.rs:25:22
   |
25 |     pub hmac: Option<HmacSha256>,
   |                      ^^^^^^^^^^ the trait `CoreProxy` is not implemented for `sha2::Sha256`
   |
   = note: required because of the requirements on the impl of `BlockSizeUser` for `HmacCore<sha2::Sha256>`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `ocicrypt-rs` due to previous error

Signed-off-by: Samuel Ortiz <s.ortiz@apple.com>
Do not autogenerate keyprovider.rs and patch it until
hyperium/tonic#892 is merged

Signed-off-by: Samuel Ortiz <s.ortiz@apple.com>
@sameo
Copy link
Member

sameo commented Jan 26, 2022

@arronwy @stevenhorsman @pravinrajr9 Build is fixed with a couple of fixes. Are you ok with merging it?

Copy link
Member

@arronwy arronwy left a comment

Choose a reason for hiding this comment

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

lgtm, thanks @stevenhorsman @sameo

Copy link
Contributor

@pravinrajr9 pravinrajr9 left a comment

Choose a reason for hiding this comment

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

LGTM

@sameo sameo merged commit c4e1505 into confidential-containers:main Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build error: in compatibility with hmac change
4 participants