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

Distinguish or merge java_cert and java_keystore module #53867

Closed
maunzCache opened this issue Mar 15, 2019 · 9 comments
Closed

Distinguish or merge java_cert and java_keystore module #53867

maunzCache opened this issue Mar 15, 2019 · 9 comments
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bot_closed collection:community.general collection Related to Ansible Collections work docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md support:community This issue/PR relates to code supported by the Ansible community. system System category

Comments

@maunzCache
Copy link
Contributor

SUMMARY

There are two modules: java_cert and java_keystore
From the documentation it is not entierly clear what the difference there is in behavior and functionality.

It seems to me that this is a duplicated module so it can either be merged or it must be clear by its documentation what it does different and maybe have a clear statement when to use the other module.

ISSUE TYPE
  • Documentation Report
COMPONENT NAME
  • modules/system/java_cert.py
  • modules/system/java_keystore.py
ANSIBLE VERSION

2.7.x

@ansibot
Copy link
Contributor

ansibot commented Mar 15, 2019

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Mar 15, 2019

cc @Mogztter @haad
click here for bot help

@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. system System category labels Mar 15, 2019
@ggrossetie
Copy link
Contributor

There are two modules: java_cert and java_keystore
From the documentation it is not entierly clear what the difference there is in behavior and functionality.

  • java_keystore: Create or delete a Java keystore in JKS format.
  • java_cert: Use keytool to import/remove key from java keystore (cacerts)

So the first one will create a keystore (from a private key) and the second one will import or remove certificates from an existing keystore.

It seems to me that this is a duplicated module

It's not.

so it can either be merged

I'm not sure about this one because you don't necessarily want to create a keystore from a private key. Also java_keystore is usually run a single time, as opposed to java_cert (where you usually want to add one or more certificates).
It will also make the declaration hard to write and to read because you will define a certificat to create the keystore and one or more certificates to add/remove from the created keystore. So in my opinion it will be unclear what the module is doing. I also prefer to compose modules rather than having a "big" module.

or it must be clear by its documentation what it does different and maybe have a clear statement when to use the other module.

Feel free to improve the documentation. For reference here's the pull request where I describe what java_keystore is doing: #35273

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Mar 15, 2019
@ggrossetie
Copy link
Contributor

ggrossetie commented Mar 16, 2019

So the first one will create a keystore (from a private key) and the second one will import or remove certificates from an existing keystore.

My bad, I wasn't not aware that java_cert can create a truststore if it does not exist.
Usually you will create a truststore to store AC and trusted certificates and a keystore to store a public and private key.

java_cert was designed to create and manage a truststore where java_keystore was designed to create a keystore.
Maybe java_cert should be renamed java_truststore ?

I think the confusion the confusion comes from the fact that you can use keystore as a truststore if you add AC and trusted certificates to it. At least that's my understanding 😉

@ilmarh
Copy link

ilmarh commented Mar 16, 2019

Me maybe wrong, but my understanding is that they are identical.
We generate host keys with keytool (which in turn creates keystore), then generate csr from this keystore and sign it with CA. Then store certificates in places.

So i think that merge is a good idea if this module will provide access to all keytool operations.

@maunzCache
Copy link
Contributor Author

I think the confusion the confusion comes from the fact that you can use keystore as a truststore if you add AC and trusted certificates to it. At least that's my understanding

Now you got me really confused. I haven't been digging deeper on the words truststore and keystore. Just noticed that one of my colleagues just used java_cert but found the documentation for the other one.

From what I understand is that a truststore actually is a keystore but without the private key part so actually both modules do the same: Store a cert in a .jks file.
Technical it would be possible to ensure that when a private key is given, the cert goes into the KeyStore if not it's - or it should be - a TrustStore item. That would confirm @ilmarh

Don't want to question your implementation here @Mogztter just asking if there is a need for two almost similar modules regarding behavior. Haven't had a look into the code yet.

@iinuwa
Copy link
Contributor

iinuwa commented May 6, 2019

If I may add my two cents as a new user. It is confusing to have both modules that are closely related but do not share similar syntax.
For instance, java_keystore accepts a string for the certificate, but java_cert requires a file path (cert_path) or certificate url (cert_url). java_keystore accepts adding private keys, but java_cert does not.
I agree with @getJack that in Java terminal a keystore is a JKS with a cert/private key pair or chain stored and a truststore is a JKS that has certificates without a private key pair. (Unfortunately, the term keystore is overloaded.) If the contents of the JKS determine whether it is a keystore, does the Ansible module need to care whether the result is a keystore or a truststore? For example, the theoretical playbook below shows examples of creating keystores/truststores based on whether the user provides a private key, and appending to or replacing the keystore using the force flag:

Playbook Example
- name: ensure that only this trusted certificate exists in this keystore
- java_keystore:
    certificate: |
      -----BEGIN CERTIFICATE-----
      ...
      -----END CERTIFICATE-----
    # private_key: None (default)
    password: keystore_password
    dest: /path/to/truststore.jks
    force: true

- name: ensure that this trusted certificate also exists in this keystore
- java_keystore:
    certificate: |
      -----BEGIN CERTIFICATE-----
      ...
      -----END CERTIFICATE-----
    password: keystore_password
    # private_key: None (default)
    dest: /path/to/truststore.jks
    # force: false (default)

- name: ensure that this key/cert pair exists in this keystore
  java_keystore:
    name: key1
    certificate: |
      -----BEGIN CERTIFICATE-----
      ...
      -----END CERTIFICATE-----
    private_key: |
      -----BEGIN RSA PRIVATE KEY-----
      ...
      -----END RSA PRIVATE KEY-----
    password: keystore_password
    dest: /path/to/keystore.jks
    # force: false (default)

- name: ensure that only this key/cert pair exists in this keystore
  java_keystore:
    name: key2
    certificate: |
      -----BEGIN CERTIFICATE-----
      ...
      -----END CERTIFICATE-----
    private_key: |
      -----BEGIN RSA PRIVATE KEY-----
      ...
      -----END RSA PRIVATE KEY-----
    password: keystore_password
    dest: /path/to/keystore.jks
    force: false (default)

Admittedly, although this makes sense to me, it might not make sense to all users, especially because of the confusion between keystore and truststore. If combining the modules isn't a great idea, then I like the idea of renaming java_cert to java_truststore, and perhaps there could be a way to abstract the common methods in the two modules (creating keystores, looking up the keytool path ( #56122 ), etc.) for easier maintainability? Again, I'm a new user, so maybe I'm not seeing a bigger picture, but just my thoughts.

(Also want to give a thank you to everyone who is working on this project; I think it will wind up being a huge part of our infrastructure in my organization!)

@ansibot ansibot added collection Related to Ansible Collections work collection:community.general needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md labels Apr 29, 2020
@Justin-DynamicD
Copy link

Adding my vote to this. It seems (to me) that java_cert is the more immature of the two, not supporting owner or group settings when creating a new file, either. I'd vote for removing it and adding the modicum of flexibility needed to allow java_keystore to handle both key and trust stores.

@ansibot
Copy link
Contributor

ansibot commented Aug 16, 2020

Thank you very much for your interest in Ansible. Ansible has migrated much of the content into separate repositories to allow for more rapid, independent development. We are closing this issue/PR because this content has been moved to one or more collection repositories.

For further information, please see:
https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md

@ansibot ansibot closed this as completed Aug 16, 2020
@ansible ansible locked and limited conversation to collaborators Sep 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 bot_closed collection:community.general collection Related to Ansible Collections work docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. needs_collection_redirect https://github.com/ansible/ansibullbot/blob/master/docs/collection_migration.md support:community This issue/PR relates to code supported by the Ansible community. system System category
Projects
None yet
Development

No branches or pull requests

6 participants