Pin libraries to current releases #298
Conversation
|
I was originally going to open an issue about the fact that the curl demo couldn't build/run on macOS without the |
ajbozarth
added
bug
|
@ajbozarth Thanks for the fix to the |
|
The curl/README.md should be updated with any new arguments, and the change in defaults |
|
Noticed that text in README.md refers to how to build the image, and provides a few examples. But the tag varies. Once we get to USAGE.md it states to run It could be helpful to at least have the default build example using the correct tag perhaps |
@baentsch After our discussion during this week's sync up I took another look and realized I had only investigated the GitHub Actions and not circleci, leading me to think I would have to write tests from scratch. Which I evaluated would be better done in a follow up PR, but upon finding the circleci test configs I will make updates there. Though as #294 says, we should move our circleci automation to gh actions in a future PR.
@planetf1 I will take a look through the docs, and make sure the tag it uses (docker hub or local build) matches the context of the section it's in. If it doesn't or is unclear I will made the edit or add clarification to the surrounding text |
|
I just pushed an update addressing @planetf1 inline comments, and adding a couple lines to the circleCI to build and test the curl image using main/master libs as well as the pinned ones. @planetf1 I looked into the docs for inconsistencies in the build image name and found that is very consistent, using the local image name in the README and using the docker hub image in USAGE. It also includes a line at the top of USAGE clarifying that it uses the public image name throughout that doc.
I updated the README to include the new ARGs and removed direct reference to versions so as any future updates to the pinned version would not require updates to the docs Lastly, on the CI update, what I pushed was a simple addition since we should add more through tests when we move testing to GH Actions, which currently only does builds, and only for one demo. (covered by #284 IIUC) |
ajbozarth
changed the title
|
As promised in #284 (comment) I have just pushed my update for the nginx demo and added a task list to the top description for tracking my progress in updating demos. I will also be updating the circleCI for the 3 jobs currently being run (openssl3, nginx, and httpd). As I will be pushing updated here periodically as I finish them, I will ping again for review when I have finished. Feel free to either wait until then or review as I go at your leisure. |
There was a problem hiding this comment.
Thanks for these updates @ajbozarth, FYI - I've deployed the nginx test-server update at test.openquantumsafe.org and all tests pass.
Status UpdateFully updated demos
Updated demo (with issues)
Still to be worked on
Personal notesI believe the work here is getting close to done. I plan to bring up some of my notes above at next Tuesday's meeting to discuss what "done" is here. After the discussion with the team I plan to start looking at #294 as a follow up to this work. |
|
ngtcp2 should be able to build with boringssl |
Supporting building with boringssl is a great idea, but the current demo doesn't support it. So I would consider that a new feature that should be handled in a separate PR |
|
Thanks for the summary, @ajbozarth . Regarding haproxy, may I suggest @pi-314159 and @davidkel sync? IMO all that's needed there is a link to the haproxy documentation as per #273 (comment). Unless of course you want to keep supporting a docker image despite the upstream apparently already supporting oqs (?). Sounds a bit like wasted effort, no? |
I get what you mean, I'll discuss with @davidkel and see if he still wants to update if it's just "putting the doc steps into a Dockerfile" I'll also raise the topic of just dropping it in Tuesday's meeting (I already planned on discussing dropping at least unbound since it's out of date and there was no interest in it). |
|
Sharing plans based on todays discussion on the OQS call: Demo specific updates:
Follow up work:
|
|
FYI X11 is still working for me on mac sequoia (I have XQuartz installed) and Fedora core 41 .. will try demo |
"Unsupported" but CI-supported -- sounds weird. Can I suggest the terms |
This is much better, thank you
We did not discuss what to do with already published docker images for deprecated demos, In my opinion as long as those published docker images still run as expected there no reason to delete them.
I had intended to run all of the against main/master, but when I get to the CI work we can discuss if we only want to test the maintained demos and not unmaintained against main/master |
|
I tried epiphany on fedora workstation 41 (where I can run X11 apps) I noticed the launch invocation seems incorrect in that it does not pass $DISPLAY. When I do so I get: which is some issue relating to the accessibility bus. The main app window starts to draw, then the app exits. However I don't think this is necessarily worth pursuing |
In that case I'll take some time today to go through the steps I listed above to deprecate epiphany. I also noticed CI complaining about my lack of commit sign-offs for my more recent commits, I'll address that at the same time |
|
I've taken the steps to deprecate epiphany, and updated the relevant docs and GitHub issues. With this, the PR is now in a state of final review. I have no more outstanding work items or review items to address prior to merging. The only caveat is the DCO CI failing due to my forgetting to sign off on my later commits. Remediating this would be messy due to multiple merges with master. I could do a squash and force push if it is an actual blocker, but afaik every PR is merged with a "Squash and merge" so is it necessary? If so I will need to create a new branch to hold the epiphany commit that's referenced in its deprecation message since a squash and force would delete the referenced url. @baentsch you usually handle merges in this repo. |
|
As an FYI in case it's useful in the future for @ajbozarth or others, you can automate signing off on commits for a repo by following the (very well hidden) instructions in |
Thanks, I knew I could automate it, but this is the first project I've worked on that required sign offs so I hadn't taken the time to figure out how to do so |
|
A few updates per discussion in todays call:
After this PR has final approval I will run a giant git squash with sign-off to fix the DCO failures as per todays discussion (unless @ryjones you have better solution?) |
You could to a |
There was a problem hiding this comment.
Thanks for this work @ajbozarth -- this is a substantial improvement over the previous state of things. One thing did deteriorate, though, namely the breadth of algorithms implicitly tested by the different integrations: It was intentional to not always use kyber768 but to also use other algs. Please consider bringing back this diversity -- unless there are strong reasons to do something different.
Could point out the specific cases where this changed? I had not intended to touch the algorithms used unless it was breaking the build, like switching to mldsa in the openssh demo. On a quick reread it does look like @davidkel did touch algorithms in his changes (haproxy and mosquitto). @baentsch If you want me to try and address that I can otherwise I'll do the squats and force push previously discussed later today |
Updates demos to use a pinned release version rather than main/master Updates demo builds to support both linux/amd64 and linux/arm64 Deprecates demos that were unable to be updated due to any reason haproxy and mosquitto demo updates provided by David Kelsey Co-authored-by: Dave Kelsey <[email protected]> Signed-off-by: Alex Bozarth <[email protected]>
|
I have pushed the giant squashed commit with an updated commit message. I also copied the previous git history into a new branch on my fork and protected it like I did with the epiphany and openlitespeed updates. This will allow at least some easy reference for future devs looking at the PR for help. As discussed with @baentsch in todays OQS call I will be making sure that my test cases I add in my CI work include more algorithm diversity that was lost in this update (specifically haproxy and mosquitto) This PR should be good to merge now, just double check the Issues it will be automatically closing is correct first. |
|
@ajbozarth haproxy and mosquitto were modeled on the pattern that was used for openvpn which defined a KEM_LIST with those algorithms which was why they changed although haproxy and mosquitto only had 3 kems listed originally so they weren't diverse much anyway. Given google's push for X25519MLKEM768 and also a recent discussion I had with @bhess around the oqs test server maybe the suggested list for all demos could be which is what Basil has enabled on the oqs test server |
@davidkel can you please explain what you mean by this statement? I have developed the oqs test server many years ago as a testbed for all algorithms as an aid for the PQC FOSS community -- are you/@bhess now aiming to drop this property? Or are you referring only to the default algs the server accepts at port 443? More generally, we are about to be dropping Kyber support, so the list above may need revising (either now or in the very near future). |
|
Hi @baentsch, I believe @davidkel is specifically referring to the DEFAULT_GROUPS supported on the ports marked with a *. These ports have always supported a limited set of algorithms, as detailed on the test.openquantumsafe.org page. The change here extends the DEFAULT_GROUPS to include the standardized algorithms. Of course, all algorithms remain fully testable through the various individual ports available on the test server. |
|
@baentsch is there anything else blocking this from merge? I'm just waiting for you or another maintainer to merge (since I'm not a maintainer) |
@ajbozarth Nope. Merged: Thanks for the work (also to @davidkel )! |
This fixes a typo in the httpd job and disables the failing openvpn, ngtcp2, and h2load jobs that were re-enabled in open-quantum-safe#298 Fixes open-quantum-safe#318 Signed-off-by: Alex Bozarth <[email protected]>
This fixes a typo in the httpd job and disables the failing openvpn, ngtcp2, and h2load jobs that were re-enabled in #298 Fixes #318 Signed-off-by: Alex Bozarth <[email protected]>
Signed-off-by: Michael Baentsch <[email protected]>
* correct dockerhub secrets location Signed-off-by: Michael Baentsch <[email protected]> * fixing new test added by #298 Signed-off-by: Michael Baentsch <[email protected]> --------- Signed-off-by: Michael Baentsch <[email protected]>
Signed-off-by: Michael Baentsch <[email protected]>
Signed-off-by: Michael Baentsch <[email protected]>
Sets the default tag for openssl, liboqs, oqs-provider to the current latest relese instead of main/master. Also updates curl to the latest release.
Inlcudes some fixes to support multi-platform builds, now supporting both linux/amd64 and linux/arm64, where only linux/amd64 worked before.
Related Issues: #230 #213 #182
Fixes #230 Fixes #182 Fixes #273 Fixes #226 Fixes #171
Demos updated (ordered by interest):
chromiumN/AquicN/A