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.

Sign up for GitHub

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

Jump to bottom

openssl_privatekey: add support for format option #60388

Merged
merged 14 commits into from
Oct 17, 2019
Merged

openssl_privatekey: add support for format option #60388

merged 14 commits into from
Oct 17, 2019

Conversation

felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Aug 11, 2019 •
edited

SUMMARY

Fixes #59253.

WIP since some things are missing:

  • idempotency
  • integration tests
ISSUE TYPE
  • New Feature Pull Request
COMPONENT NAME

openssl_privatekey

@ansibot
Copy link
Contributor

ansibot commented Aug 11, 2019

cc @MarkusTeufelberger @Shaps @Spredzy @Xyon @puiterwijk @resmo
click here for bot help

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.9 This issue/PR affects Ansible v2.9 crypto Crypto community (ACME, openssl, letsencrypt) module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_plugin This PR includes a new plugin. owner_pr This PR is made by the module's maintainer. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. and removed new_plugin This PR includes a new plugin. owner_pr This PR is made by the module's maintainer. labels Aug 11, 2019
@felixfontein
Copy link
Contributor Author

What's missing is a proper detection what the current format of a PEM key is (https://github.com/ansible/ansible/pull/60388/files#diff-f47facfb3118315473f7ea281891f42fR649). Everything else is there.

@felixfontein felixfontein mentioned this pull request Aug 15, 2019
Closed
@felixfontein
Copy link
Contributor Author

felixfontein commented Aug 18, 2019 •
edited

(During rebase, I squashed to commits so far.)

I probably won't have time to work on this before 2.9 feature freeze, so if anyone is interested in getting this into Ansible 2.9, please help with the key format identification part :)

(Also, raw format support might not work properly since loading keys assumes PEM.)

@felixfontein felixfontein mentioned this pull request Aug 18, 2019
Closed
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Aug 30, 2019
@felixfontein
Copy link
Contributor Author

I now added a proper key format identification (based on the OpenSSL code, thanks @bkmeneguello for the pointer), and some more tests.

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 12, 2019
@ansibot
Copy link
Contributor

ansibot commented Oct 12, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/crypto/openssl_privatekey.py:0:0: option-incorrect-version-added: version_added for new option (format) should be '2.10'. Currently StrictVersion ('2.9')

The test ansible-test sanity --test yamllint [explain] failed with 2 errors:

test/integration/targets/openssl_privatekey/tasks/impl.yml:388:94: error: syntax error: expected <block end>, but found '<scalar>'
test/integration/targets/openssl_privatekey/tests/validate.yml:186:94: error: syntax error: expected <block end>, but found '<scalar>'

click here for bot help

@bkmeneguello
Copy link
Contributor

Amazing work, I'll review ASAP

@felixfontein
Copy link
Contributor Author

I'm right now adding another option format_mismatch (default value regenerate, also offers convert) which allows to convert keys to other formats (if possible) instead of regenerating them.

@felixfontein felixfontein changed the title [WIP] openssl_privatekey: add support for format option openssl_privatekey: add support for format option Oct 12, 2019
@felixfontein
Copy link
Contributor Author

If sanity checks don't have any surprises, I should be done for now. At least locally, the tests ran through :)

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. labels Oct 12, 2019
@felixfontein
Copy link
Contributor Author

Tests pass. \o/

ready_for_review

@felixfontein felixfontein mentioned this pull request Oct 15, 2019
Closed
@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Oct 16, 2019
@Shaps
Copy link
Contributor

Shaps commented Oct 17, 2019

shipit

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Oct 17, 2019
@felixfontein felixfontein merged commit d00d0c8 into ansible:devel Oct 17, 2019
@felixfontein felixfontein deleted the openssl_privatekey-format branch October 17, 2019 08:40
@felixfontein
Copy link
Contributor Author

@bkmeneguello @Shaps thanks a lot for reviewing this! :)

@ansible ansible locked and limited conversation to collaborators Nov 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Shaps Shaps Shaps left review comments

@bkmeneguello bkmeneguello bkmeneguello approved these changes

Assignees
No one assigned
Labels
affects_2.9 This issue/PR affects Ansible v2.9 crypto Crypto community (ACME, openssl, letsencrypt) feature This issue/PR relates to a feature request. has_issue module This issue/PR relates to a module. new_plugin This PR includes a new plugin. shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow to select openssl_privatekey format
4 participants
@felixfontein @ansibot @bkmeneguello @Shaps