Skip to content

upgrade cryptography sub-dependency#4525

Closed
reinvantveer wants to merge 1 commit intooperator-framework:masterfrom
reinvantveer:upgrade_cryptography
Closed

upgrade cryptography sub-dependency#4525
reinvantveer wants to merge 1 commit intooperator-framework:masterfrom
reinvantveer:upgrade_cryptography

Conversation

@reinvantveer
Copy link
Copy Markdown
Contributor

@reinvantveer reinvantveer commented Feb 13, 2021

Signed-off-by: Rein van 't Veer reinvantveer@gmail.com

This PR is part of #4237 to work towards a more user-friendly way of doing reproducible local builds

Description of the change:
Upgrade the cryptography package in such a way that future updates can be installed.

Motivation for the change:
The cryptography package saw some discussion recently when it switched to a toolchain that included a Rust compiler to install the package. This broke quite a few builds, also ones involving Ansible since it includes the cryptography package as a subdependency.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

Signed-off-by: Rein van 't Veer <reinvantveer@gmail.com>
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 13, 2021
@reinvantveer reinvantveer changed the title upgrade sub-dependencies upgrade cryptography sub-dependency Feb 13, 2021
@reinvantveer reinvantveer marked this pull request as ready for review February 13, 2021 20:40
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 13, 2021
@reinvantveer
Copy link
Copy Markdown
Contributor Author

@estroz It "just installed": https://github.com/operator-framework/operator-sdk/runs/1894851979?check_suite_focus=true#step:5:422. No installation of Rust compiler was needed. This is a bit puzzling, seeing #4510 (comment). I think this one requires a bit more investigation.

@reinvantveer
Copy link
Copy Markdown
Contributor Author

reinvantveer commented Feb 13, 2021

I ran a quick smoke test by inserting a RUN pip3 list, which looks totally fine:

Step 7/14 : RUN pip3 list
 ---> Running in 7469f3a969df
Package             Version
------------------- ----------
ansible             2.9.15
ansible-runner      1.4.6
ansible-runner-http 1.0.0
appdirs             1.4.4
cachetools          4.2.1
certifi             2020.12.5
cffi                1.14.5
chardet             4.0.0
cryptography        3.4.4
distlib             0.3.1
docutils            0.16
filelock            3.0.12
google-auth         1.26.1
idna                2.10
ipaddress           1.0.23
Jinja2              2.11.3
jmespath            0.10.0
kubernetes          11.0.0
lockfile            0.12.2
MarkupSafe          1.1.1
oauthlib            3.1.0
openshift           0.11.2
pexpect             4.8.0
pip                 19.3.1
pipenv              2020.11.15
psutil              5.8.0
ptyprocess          0.7.0
pyasn1              0.4.8
pyasn1-modules      0.2.8
pycparser           2.20
python-daemon       2.2.4
python-dateutil     2.8.1
python-string-utils 1.0.0
PyYAML              5.4.1
requests            2.25.1
requests-oauthlib   1.3.0
requests-unixsocket 0.2.0
rsa                 4.7
ruamel.yaml         0.16.12
ruamel.yaml.clib    0.2.2
setuptools          41.6.0
six                 1.15.0
urllib3             1.26.3
virtualenv          20.4.2
virtualenv-clone    0.5.4
websocket-client    0.57.0

I admit I'm a little confused. Did v3.4.4 of cryptography get rewritten?

@reinvantveer
Copy link
Copy Markdown
Contributor Author

reinvantveer commented Feb 13, 2021

Ah, now I see, this explains it: https://cryptography.io/en/latest/installation.html#building-cryptography-on-linux

In our case, we just install a pre-built binary wheel, since we use an up-to-date pip!

Edit: As the docs in the link above suggest, cryptography requires pip >=19.0 to install the pre-built wheel

@openshift-ci-robot
Copy link
Copy Markdown

@reinvantveer: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 20, 2021
@reinvantveer
Copy link
Copy Markdown
Contributor Author

@estroz Closing as this PR is unlikely to merge well with expected upcoming updates on the pipenv story

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants