readthedocs / readthedocs-docker-images

Docker image definitions used by Read the Docs
115 stars 70 forks source link

libssl and header files version on readthedocs/build:7.0 are too low #176

Closed 0xaead closed 2 years ago

0xaead commented 2 years ago

I found this issue when building docs for pyca/cryptography with readthedocs/build:7.0. It is initially tracked down through build error in https://readthedocs.org/projects/cryptography/builds/14446988/.

build/temp.linux-x86_64-3.9/_openssl.c: At top level: build/temp.linux-x86_64-3.9/_openssl.c:1320:8: error: redefinition of ‘struct ocsp_responder_id_st’ struct ocsp_responder_id_st { ^~~~~~~~ In file included from /usr/include/openssl/crypto.h:131:0, from /usr/include/openssl/bio.h:69, from /usr/include/openssl/asn1.h:65, from build/temp.linux-x86_64-3.9/_openssl.c:622: /usr/include/openssl/ossl_typ.h:208:16: note: originally defined here typedef struct ocsp_responder_id_st OCSP_RESPID; ^~~~~~~~

In 7.0 container, I found /usr/include/openssl/opensslv.h:

# define OPENSSL_VERSION_NUMBER 0x100020efL # ifdef OPENSSL_FIPS # define OPENSSL_VERSION_TEXT "OpenSSL 1.0.2n-fips 7 Dec 2017" # else # define OPENSSL_VERSION_TEXT "OpenSSL 1.0.2n 7 Dec 2017" # endif

Yet the openssl application is 1.1.1.

$ openssl version OpenSSL 1.1.1 11 Sep 2018

Further check found there are mixed 2 versions of libssl installed. $ find /usr -name libssl* /usr/lib/x86_64-linux-gnu/pkgconfig/libssl.pc /usr/lib/x86_64-linux-gnu/libssl.a /usr/lib/x86_64-linux-gnu/libssl.so /usr/lib/x86_64-linux-gnu/libssl3.so /usr/lib/x86_64-linux-gnu/libssl.so.1.0.0 /usr/lib/x86_64-linux-gnu/libssl.so.1.1 /usr/share/doc/libssl1.0-dev /usr/share/doc/libssl1.0.0 /usr/share/doc/libssl1.1 /usr/share/lintian/overrides/libssl1.0.0

Could you unify the openssl to 1.1.1 in build 7.0?

humitos commented 2 years ago

Hi! Thanks for reporting this issue.

We are moving away from 5.0.x, 6.0.x and 7.0.x images because they are hard to maintain and easy to break when re-building them. We are working on a new implementation at (https://github.com/readthedocs/readthedocs.org/pull/8447). The new images will have Ubuntu 20.04 LTS and this would be the output of that command:

$ docker run -it readthedocs/build:ubuntu20 /bin/bash
docs@f89fa32c78f5:~$ find /usr -name libssl*
/usr/share/doc/libssl-dev
/usr/share/doc/libssl1.1
/usr/lib/x86_64-linux-gnu/pkgconfig/libssl.pc
/usr/lib/x86_64-linux-gnu/libssl3.so
/usr/lib/x86_64-linux-gnu/libssl.a
/usr/lib/x86_64-linux-gnu/libssl.so
/usr/lib/x86_64-linux-gnu/libssl.so.1.1
docs@f89fa32c78f5:~$ exit

and

docs@a07edb506298:~$ openssl version
OpenSSL 1.1.1f  31 Mar 2020

(the image will be generated by the Dockerfile from this PR https://github.com/readthedocs/readthedocs-docker-images/pull/166/)

I understand this will be correct for your use case. Is that correct?

0xaead commented 2 years ago

Yeah, literally it would be nice to have header files in place with same version 1.1.1f.

So when PR #116 will be officially released so users could take it into use in their product line confidently? Thanks.

humitos commented 2 years ago

So when PR #116 will be officially released so users could take it into use in their product line confidently? Thanks.

We are working on this currently. However, we don't have an ETA we can compromise to. I'd expect that we can deploy these changes (in beta probably) in a month.

0xaead commented 2 years ago

we can deploy these changes (in beta probably) in a month.

That is quite promising. I am hoping to keep this issue open till build 7 was removed/replaced by said one, in case any other people encounter similar issue... What is your opinion?

humitos commented 2 years ago

Yeah. Sounds good. This is something I may use myself to QA our work probably.

humitos commented 2 years ago

@0xaead Hi! I did some tests today locally with my branch that implements the new images and support for Ubuntu 20.04. The build did work without any issue. I use this diff in your project (branched from main)

diff --git a/.readthedocs.yml b/.readthedocs.yml
index 04d5d1a4..fd0e4e2c 100644
--- a/.readthedocs.yml
+++ b/.readthedocs.yml
@@ -6,6 +6,7 @@ sphinx:
   builder: dirhtml

 build:
-  # First RTD build env which includes a rust toolchain
-  image: "7.0"
-
+  os: "ubuntu-20.04"
+  tools:
+    python: "3.8"
+    rust: "1.55"

NOTE that this won't work on current Read the Docs production yet. Hopefully, next week or the other it will work 🤞🏼

I wanted to ask you if there is anything that I should check besides that the build succeed to make sure that the problem reported is solved in the new images.

0xaead commented 2 years ago

@humitos Hi, if the build of pyca/cryptography/docs is successful, then I am not aware of any other issue to be noticed. I will ping owner of the project to shift to said image onward.

0xaead commented 2 years ago

@humitos p.s. please ping me once said release is finalized.

NOTE that this won't work on current Read the Docs production yet. Hopefully, next week or the other it will work 🤞🏼

I will initiate changes on pyca/cryptography side.

humitos commented 2 years ago

Hi @0xaead! In #166 we implemented the ability to use Ubuntu 20.04 LTS --which is still in beta. Would you like to give it a try and let us know if it works? You need to add something like this to your .readthedocs.yaml file:

build:
  os: "ubuntu-20.04"
  tools:
    python: "3.9"

Note that if you are already defining the Python version via python.version you have to remove it from there.

Thanks!

0xaead commented 2 years ago

@humitos Sure. DO you want me to run ubuntu-20.04-2021.09.23 from https://hub.docker.com/r/readthedocs/build/tags?page=1&ordering=last_updated? Or build container from current master HEAD? I will try the 1st one as a handy choice first, tell me if I still have to go 2nd one ;)

humitos commented 2 years ago

(on the phone)

Nope, just run your build on read the docs (readthedocs.org) with the chunk of code I shared in my previous comment.

0xaead commented 2 years ago

@humitos OK, server build is ongoing now.

P.S. Is virtualenv installed in said image? At least my test on ubuntu-20.04-2021.09.23 ends up of missing that module....

My quick script to build docs inside the container here:

git clone --no-single-branch --depth 50 https://github.com/pyca/cryptography .
git fetch origin --force --tags --prune --prune-tags --depth 50 pull/6207/head:external-6207
git checkout --force 92020fea7581522c734cf745f34789c92085688d
git clean -d -f -f
python3.9 -mvirtualenv /home/docs/checkouts/readthedocs.org/user_builds/cryptography/envs/6207
/home/docs/checkouts/readthedocs.org/user_builds/cryptography/envs/6207/bin/python -m pip install --upgrade --no-cache-dir pip setuptools
/home/docs/checkouts/readthedocs.org/user_builds/cryptography/envs/6207/bin/python -m pip install --upgrade --no-cache-dir mock==1.0.1 pillow==5.4.1 "alabaster>=0.7,<0.8,!=0.7.5" commonmark==0.8.1 recommonmark==0.5.0 "sphinx<2" "sphinx-rtd-theme<0.5" "readthedocs-sphinx-ext<2.2"

Stuck here:

python3.9 -mvirtualenv /home/docs/checkouts/readthedocs.org/user_builds/cryptography/envs/6207

astrojuanlu commented 2 years ago

xref: https://github.com/pyca/cryptography/pull/6331

humitos commented 2 years ago

(on the phone)

No, virtualenv is not installed, nor Python, nor the other tools either. This is done at build time using a cache.

Note that these images are not meant to be used outside Read the Docs. That's why I was saying to just try this in readthedocs.org directly.

Let me know how it goes! ☺️

humitos commented 2 years ago

(on the phone)

Oh! Note that if you need Rust, you have to define it as:

build:
  tools:
    rust: "1.55"
0xaead commented 2 years ago

Let me know how it goes!

Looking good, and pyca/cryptography shifted to 20.04 already.

P.S. Thanks for your guys' fantastic works to readthedocs. Keep going~

humitos commented 2 years ago

Excellent! Thanks for reporting back! 😎