Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update openssl3 in pypi #1489

Merged
merged 1 commit into from May 16, 2024
Merged

Update openssl3 in pypi #1489

merged 1 commit into from May 16, 2024

Conversation

phoebusm
Copy link
Collaborator

@phoebusm phoebusm commented Apr 11, 2024 •
edited

Reference Issues/PRs

#1491

What does this implement or fix?

Update openssl to v3

Any other comments?

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

@phoebusm phoebusm force-pushed the feature/update_openssl3_in_pypi branch 2 times, most recently from 73a88d2 to 0accd0c Compare April 12, 2024 08:16
@phoebusm phoebusm self-assigned this Apr 12, 2024
@phoebusm phoebusm force-pushed the feature/update_openssl3_in_pypi branch from a9c062b to 0551881 Compare April 12, 2024 08:25
@phoebusm phoebusm marked this pull request as ready for review April 12, 2024 08:25
poodlewars
poodlewars reviewed Apr 12, 2024
View reviewed changes
build_tooling/build_many_linux_image.sh
@@ -44,7 +44,7 @@ RUN rpmkeys --import 'https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x3F
s/#?exclude.*/exclude=.edu/' /etc/yum/pluginconf.d/fastestmirror.conf
ADD sccache /usr/local/bin/
RUN yum update -y && \
yum install -y zip jq less devtoolset-11-gdb \
yum install -y zip jq less devtoolset-11-gdb perl-IPC-Cmd \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need perl-IPC-Cmd ?

Copy link
Collaborator Author

@phoebusm phoebusm Apr 12, 2024 •
edited

Choose a reason for hiding this comment

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

It is required by v3 compilation:
https://github.com/man-group/ArcticDB/actions/runs/8649691810/job/23716646005

Can't locate IPC/Cmd.pm in @INC (@INC contains: /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 .) at -e line 1.
  BEGIN failed--compilation aborted at -e line 1.
  CMake Error at buildtrees/versioning_/versions/openssl/53fb72a19981a3439cd569f58b722ca6cd26e23d/unix/portfile.cmake:27 (message):
    
  
    Perl cannot find IPC::Cmd.  Please install it through your system package
    manager.

Underlying reason is the configure script of lastest openssl is a perl script:
openssl/openssl#19740 (comment)

@phoebusm phoebusm linked an issue Apr 12, 2024 that may be closed by this pull request
Update openssl version pin for pypi build #1491
Closed
@poodlewars
Copy link
Collaborator

Change looks fine. What test coverage do we have for our SSL interaction? Do the persistent storage tests use HTTPS? Worth running them if so, or doing a quick manual test if not.

@phoebusm phoebusm force-pushed the feature/update_openssl3_in_pypi branch from 0551881 to 397f2ba Compare April 15, 2024 08:47
@phoebusm
Copy link
Collaborator Author

phoebusm commented Apr 15, 2024 •
edited

Change looks fine. What test coverage do we have for our SSL interaction?

#1338
S3 auto ca path PR has already been merged. It contains the https related tests.

Do the persistent storage tests use HTTPS? Worth running them if so, or doing a quick manual test if not.

Good point. Let me turn it on

@phoebusm
Copy link
Collaborator Author

Do the persistent storage tests use HTTPS? Worth running them if so, or doing a quick manual test if not.

@poodlewars It is on now.

@phoebusm phoebusm force-pushed the feature/update_openssl3_in_pypi branch 2 times, most recently from 304e974 to 5f75a0e Compare May 1, 2024 12:30
@phoebusm phoebusm linked an issue May 2, 2024 that may be closed by this pull request
Bad way to add prefix to the library on the testing pipeline #1550
Closed
@phoebusm phoebusm force-pushed the feature/update_openssl3_in_pypi branch from 6e14e26 to 4f7bd59 Compare May 3, 2024 14:16
@poodlewars
Copy link
Collaborator

Do the persistent storage tests use HTTPS? Worth running them if so, or doing a quick manual test if not.

@poodlewars It is on now.

@phoebusm can you share a link showing it passing please?

@phoebusm
Copy link
Collaborator Author

@poodlewars

root@XPS:/tmp# cat /etc/*-release | grep PRETTY_NAME
PRETTY_NAME="Ubuntu 22.04.4 LTS"
root@XPS:/tmp# python -m venv venv
root@XPS:/tmp# source venv/bin/activate
(venv) root@XPS:/tmp# pip install arcticdb-4.1.0.dev0-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl 
(venv) root@XPS:/tmp# python
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import arcticdb as adb
>>> arctic = adb.Arctic("s3s://s3-eu-north-1.amazonaws.com:XXXXXXXXXXXX?aws_auth=true&ssl=True")
>>> arctic.list_libraries()
[]
>>> quit()
(venv) root@XPS:/tmp# ls -al /etc/pki/ca-trust/extracted/pem/
ls: cannot access '/etc/pki/ca-trust/extracted/pem/': No such file or directory
(venv) root@XPS:/tmp# ls -al /etc/ssl/certs/ca-certificates.crt
-rw-r--r-- 1 root root 208567 Nov 27 14:45 /etc/ssl/certs/ca-certificates.crt

@phoebusm phoebusm force-pushed the feature/update_openssl3_in_pypi branch from 8b6ed09 to 1256b6a Compare May 15, 2024 08:47
@phoebusm phoebusm merged commit b2c820c into master May 16, 2024
114 checks passed
@phoebusm phoebusm deleted the feature/update_openssl3_in_pypi branch May 16, 2024 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@poodlewars poodlewars poodlewars approved these changes

@alexowens90 alexowens90 Awaiting requested review from alexowens90 alexowens90 is a code owner

@willdealtry willdealtry Awaiting requested review from willdealtry willdealtry is a code owner

Assignees

@phoebusm phoebusm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bad way to add prefix to the library on the testing pipeline Update openssl version pin for pypi build
2 participants
@phoebusm @poodlewars