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

[WIP] OpenSSL modules: rename to x509_* #63801

Closed
wants to merge 2 commits into from

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Renames the openssl_* modules to x509_*. Fixes #33715.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

openssl_certificate
openssl_certificate_info
openssl_csr
openssl_csr_info
openssl_dhparam
openssl_pkcs12
openssl_privatekey
openssl_privatekey_info
openssl_publickey

@ansibot
Copy link
Contributor

ansibot commented Oct 22, 2019

@felixfontein This PR was evaluated as a potentially problematic PR for the following reasons:

  • More than 50 changed files.

Such PR can only be merged by human. Contact a Core team member to review this PR on IRC: #ansible-devel on irc.freenode.net

click here for bot help

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 feature This issue/PR relates to a feature request. has_issue needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. labels Oct 22, 2019
@felixfontein
Copy link
Contributor Author

@MarkusTeufelberger
Copy link
Contributor

MarkusTeufelberger commented Oct 22, 2019

Afaik CSRs are not actually covered by X.509 (https://tools.ietf.org/html/rfc5280), probably public/private keys neither. I'm still not sure what would be a good compromise naming-wise though.

@felixfontein
Copy link
Contributor Author

@MarkusTeufelberger that's a very good point. How about pki instead of x509? And maybe even pki_x509_ for actual X.509 objects. The latter would result in (I included not yet existing CRL modules):

  • pki_csr
  • pki_csr_info
  • pki_dhparam
  • pki_pkcs12
  • pki_privatekey
  • pki_privatekey_info
  • pki_publickey
  • pki_x509_certificate
  • pki_x509_certificate_info
  • pki_x509_crl
  • pki_x509_crl_info

And without the extra x509_:

  • pki_certificate
  • pki_certificate_info
  • pki_crl
  • pki_crl_info
  • pki_csr
  • pki_csr_info
  • pki_dhparam
  • pki_pkcs12
  • pki_privatekey
  • pki_privatekey_info
  • pki_publickey

@felixfontein felixfontein changed the title OpenSSL modules: rename to x509_* [WIP] OpenSSL modules: rename to x509_* Oct 23, 2019
@ansibot ansibot added the WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. label Oct 23, 2019
Copy link
Contributor

@gundalow gundalow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to feature creep this, thought could you please move the modules into a subdir ie modules/crypto/x509/
BOTMETA.yml will need updating to reflect this.
This will help when we move to Collections

@MarkusTeufelberger
Copy link
Contributor

If this is to be removed from upstream and into its own crypto collection, the namespacing there might make most prefixes unneccessary or redundant.

@gundalow
Copy link
Contributor

gundalow commented Nov 6, 2019

If this is to be removed from upstream and into its own crypto collection, the namespacing there might make most prefixes unneccessary or redundant.

That is a fair point. At the moment we don't know if the collection will be called
ansible.community_collection (everything) (Layout 1)
ansible.crypto_community (directory level) (Layout 3)
ansible.openssl_community (sub-directory level) (Layout 4)

For the potential layouts see gist.github.com/gundalow/81884926ff744755e2d2f46ef1ba1ebb

If you have an ideas on this, I'd welcome your input, via IRC or add comments on the gist.

@MarkusTeufelberger
Copy link
Contributor

Most solutions involving collections I've seen so far look strictly worse in the user experience (more stuff to type, more things to take care of upon installation, more fractured test procedures leading to decreased quality in modules, still no plan for documentation), so I'll probably not donate my time towards maintaining a crypto, openssl/x509/pki or community collection since it is probably going to be spent on reworking my existing code. As such, my opinion on that topic doesn't really count I think.

I would call the collection(s) crypto.x509 etc. (similar to layout 4), no need to have ansible or commmunity in their name at all. Such a collection would no longer be "ansible" anyways and thus also the "community" in the name is a bit redundant.

@felixfontein
Copy link
Contributor Author

I guess that before talking about collections and subdirectories, we should first figure out how to actually name these modules :) Any opinions on #63801 (comment), or other suggestions?

Also, I'm still hoping that the transition to collections will make it possible to upgrade to the next version(s) of Ansible with minimal (or even no) changes to existing roles and/or playbooks. (Maybe like adding something to playbooks a la "allow to use stuff from all community collections without having to explicitly add the namespaces somewhere".)

@felixfontein
Copy link
Contributor Author

(I rebased to get rid of the conflicts. No content-wise changes, except that I squashed both commits into one.)

@felixfontein
Copy link
Contributor Author

Any more ideas on how we can actually name the modules?

@felixfontein
Copy link
Contributor Author

Minimal version continued in ansible-collections/community.crypto#7.

@felixfontein felixfontein deleted the openssl-x509-rename branch March 30, 2020 19:12
@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Mar 30, 2020
@ansible ansible locked and limited conversation to collaborators Apr 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.10 This issue/PR affects Ansible v2.10 feature This issue/PR relates to a feature request. has_issue needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The naming of the openssl_certificate-module
5 participants