-
Notifications
You must be signed in to change notification settings - Fork 16
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
Fix to use the latest Islet SDK #245
Conversation
Thanks for pulling this forward, @bitboom ! I see that CI tests ran clean with your 1st commit, but then when you tried to re-enable the failing (previously commented) ISLET build-and-test tests, they failed in this build. I took a quick look and the failiure is here:
It was not immediately obvious to me if this is a failure due to (a) Islet-APIs changing or (b) if it's an error with driver If it is (b), let me know. I can look at it later. I was hoping to get the previous PR #243 integrated first, where failing tests are commented out. Then, you can layer your fix on top of that. Am waiting for Ye & you to sign-off on my PR which basically comments out failing tests. |
It is assumed that there might be a problem with the Because Islet API was successful on CI and my environment too.
The certificate error did not occur in my environment but occurred in the CI as you said. run_example.sh: simple_app_under_islet: Running run_app_by_name_as_client_make_trusted_request
Running App as client
Client peer id is Measured-6190eb90b293886c172ec644dafb7e33ee2cea6541abe15300d96380df525bf9
SSL client read: Hi from your secret server
` |
Hi, @bitboom, Re: It is assumed that there might be a problem with the openssl certificate depending on the environment. ... The CI
We will need to debug this in CI's env and on your machine, to compare the OpenSSL library versions. That is a bit of painful effort. What you will need to do is to edit out all the test-methods (after install) in Check-in this to your dev-branch so that the CI-roundtrip is shortened to just this set of ISLET related failing test-methods.
And add some commands in the I don't know what / how to report the OpenSSL version string. But if you can add that, we can compare the version string that CI installs v/s what's on your dev-box. Once we find out what the differences are ... we can tweak the I think this troubleshooting / debugging will be a slow exercise. I'll leave it up to you to figure out if this is the best way to troubleshoot, or you have any better ideas to figure out what the OpenSSL version issue is (or might be). |
0d5f33c
to
68b39ab
Compare
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 for getting this clean-up done, @bitboom .
Summary seems to be that the failures are not related to Ubuntu latest v22.xx upgrade but something else related to build / infra-config around Islet.
For future reference, I suggest you please explain the source of the failure and the fix, and point out in your commit message this is not an Ubuntu-upgrade related issue.
@bitboom -- This Islet-cleanup change is good. You can go in first, if you wish ... but you won't be able to merge as the simulated-SEV related tests are still failing. But my PR #243 is still waiting for approval from @yelvmw . I will need to check-in above PR before you can merge. Ye said he will finish the review by today. Let's wait. |
Nice work, @bitboom ! |
Signed-off-by: Sangwan Kwon <[email protected]>
Signed-off-by: Sangwan Kwon <[email protected]>
68b39ab
to
d417918
Compare
This PR addresses build errors in the Islet SDK. (Fixed #244)
Issues and Resolutions:
Issue: Compatibility issues due to the upgrade of a third-party library used by the Islet SDK.
Fixing: Upgraded to a compatible version and enforced explicit versioning to prevent automatic upgrades of the third-party library. (Add Cargo.lock for dependency consistency islet-project/islet#315)
Issue: Upgrade of the MRENCLAVE value for the simulated version of Islet SDK targeting x86_64.
Fixing: Updated to the upgraded value and added comments on how to obtain it. Although a shell script should ideally be written to fetch the actual value systematically, frequent changes are not anticipated, so only a description has been added for now.