-
Notifications
You must be signed in to change notification settings - Fork 136
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
Add support for specifying key type #320
base: master
Are you sure you want to change the base?
Conversation
Looks like
This will re-run the test suite. |
391a875
to
81e9c5d
Compare
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! I'd like more feedback from module users just in case before merging.
I just quickly checked, but support for —key-type was only added in version 1.10.0 of certbot. Some of the OSses supported by this module, like Debian 10, come with an older version of certbot and will that will not work anymore with this change. We internally use newer versions of the certbot package for Debian 10, so I did not notice this initially. Should this feature take into account the actual version of certbot? |
Ah, that's the kind of tricky things I am not aware of and that can get annoying. Maybe we can add a new fact that get the version of certbot, and does it best to respect the user config: use legacy options for old versions, maybe producing a warning if the user tuned something that cannot be tuned with their version of certbot, and use the bleeding edge options if the installed version is compatible? |
Defaults to rsa for backwards compatibility
81e9c5d
to
ef876ed
Compare
Ubuntu 20.04 still has an old version of certbot, so we can't merge this as is. https://packages.ubuntu.com/focal-updates/certbot |
Pull Request (PR) description
Allow specifying which key type to generate. Certbot changed it's default from rsa to elliptic curve, but I kept the default to rsa for backwards compatibility.