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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

martijndegouw
Copy link
Contributor

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.

@smortex
Copy link
Member

smortex commented Jun 27, 2023

Looks like REFERENCE.md is outdated. From your working directory:

$ bundle update
$ bundle exec rake strings:generate:reference
$ git add REFERENCE.md
$ git commit --amend
$ git push --force

This will re-run the test suite.

@martijndegouw martijndegouw force-pushed the feature/keytype branch 2 times, most recently from 391a875 to 81e9c5d Compare June 28, 2023 18:46
Copy link
Member

@smortex smortex left a 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.

@martijndegouw
Copy link
Contributor Author

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?

@smortex
Copy link
Member

smortex commented Jun 28, 2023

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
@kenyon kenyon added the enhancement New feature or request label Dec 3, 2023
@kenyon
Copy link
Member

kenyon commented Dec 3, 2023

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

@kenyon kenyon added the needs-work not ready to merge just yet label Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-work not ready to merge just yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants