-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
Module to generate Diffie-Hellman parameters #32620
Conversation
The test
The test
The test
The test
The test
|
I'm directly calling An alternative would be using the |
As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
Looks nice already! 👍 Please still use pyopenssl, if at all possible. You can call functions not offered by pyopenssl via the In the documentation of return values you refer to private keys. |
Using the pyopenssl-api appears to be quite complicated. I'm trying to follow https://wiki.openssl.org/index.php/Diffie_Hellman#Using_the_Low_Level_APIs but having trouble caling Using the pyca/cryptography module would be much more convenient (possibly also for the other modules)... but brings in new dependencies. I've fixed the mentions of private keys. |
It may actually be desirable to refactor all modules to use the cryptography library; it'd make implementing #32626 a lot easier as well. |
I fully agree, however it was a hard requirement for my colleague and me when writing openssl_certificate and refatoring openssl_csr that it has to run on Debian Jessie. We absolutely need openssl_privatekey, openssl_csr and openssl_certificate to be fully functional on Jessie hosts. This realistically only leaves a relatively old version of pyopenssl. Once Jessie is out of support (April 2020) I'm planning to take another look at cryptography. |
Won't this just work: https://packages.debian.org/jessie/python-cryptography ? I haven't tried it, but it seems (some) version of cryptography is available. |
Not surprisingly, since cryptography is a dependency of pyopenssl. :-) Unfortunately, no, this won't just work. Certificate and CSR handling were implemented after 0.6 and reworked a few times later. |
As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
Well, since pyopenssl does not seem like an option for now and I have no need for that module anyways, I'd just like you to use Essentially just calling out to Lastly it would be really great if you also added at least some small integration tests (take a look at the existing ones) in a future PR, this stuff has a tendency of being a bit difficult on various platforms and having at least some basic assertion that something works as intended somewhere is always helpful. |
As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
4 similar comments
As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add |
shipit |
shipit |
Though you need one from @Spredzy too I think. ;-) Also it probably needs to be manually merged since it contains changes to several files in different folders and ansibot doesn't merge that automatically afair. |
According to the workflow documentation new modules always need a core team member to review them. |
Highly annoying to have to do this again and again and again as the rules change during the game
Ansible prefers dummy
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.
LGTM shipit
shipit |
bot_status |
Componentslib/ansible/modules/crypto/openssl_dhparam.py test/integration/targets/openssl_dhparam/aliases test/integration/targets/openssl_dhparam/tasks/main.yml test/integration/targets/openssl_dhparam/tests/validate.yml Metadatawaiting_on: maintainer |
rebuild_merge |
shipit |
@gundalow: it looks like one of the CI processes is hanging (https://app.shippable.com/github/ansible/ansible/runs/52245/45/console) |
bot_status |
Componentslib/ansible/modules/crypto/openssl_dhparam.py test/integration/targets/openssl_dhparam/aliases test/integration/targets/openssl_dhparam/tasks/main.yml test/integration/targets/openssl_dhparam/tests/validate.yml Metadatawaiting_on: maintainer |
* Module to generate Diffie-Hellman parameters Implements ansible#32577 * Add integration tests for openssl_dhparam * Slightly refactor check to prevent unnecessary regeneration * Fix code smell in tests Highly annoying to have to do this again and again and again as the rules change during the game * Using module.run_command() and module.atomic_move() from a tempfile. * Remove underscore variable Ansible prefers dummy
Fixes #32577
SUMMARY
Implements diffie-hellman parameter generation to complement the openssl crypto modules. Directly calls openssl due to pyopenssl not having a dhparam api.
ISSUE TYPE
COMPONENT NAME
openssl_dhparam
ADDITIONAL INFORMATION