-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add dest option to systemd_creds_encrypt #10549
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
base: main
Are you sure you want to change the base?
Changes from all commits
84780db
c6193e0
8aa1262
f6e67fe
b34d0fd
ce4116b
451c2e3
74e2aa1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| minor_changes: | ||
| - systemd_creds_encrypt - add ``dest`` option to control path to the credential file to be created (https://github.com/ansible-collections/community.general/pull/10549). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,38 +32,43 @@ | |
| name: | ||
| description: | ||
| - The credential name to embed in the encrypted credential data. | ||
| When using the O(dest) option, you can set this to an empty | ||
| string ("") to prevent C(systemd-creds encrypt) from using | ||
| filename from O(dest) as the credential name. Conversely, | ||
| leave out the O(name) option to let C(systemd-creds encrypt) | ||
| use the filename from O(dest) as the credential name. | ||
| type: str | ||
| required: false | ||
| not_after: | ||
| description: | ||
| - The time when the credential shall not be used anymore. | ||
| - Takes a timestamp specification in the format described in V(systemd.time(7\)). | ||
| type: str | ||
| required: false | ||
| pretty: | ||
| description: | ||
| - Pretty print the output so that it may be pasted directly into a unit file. | ||
| type: bool | ||
| required: false | ||
| default: false | ||
| secret: | ||
| description: | ||
| - The secret to encrypt. | ||
| type: str | ||
| required: true | ||
| dest: | ||
| description: | ||
| - The destination for the credential file. | ||
| type: path | ||
| version_added: 11.3.0 | ||
| timestamp: | ||
| description: | ||
| - The timestamp to embed into the encrypted credential. | ||
| - Takes a timestamp specification in the format described in V(systemd.time(7\)). | ||
| type: str | ||
| required: false | ||
| user: | ||
| description: | ||
| - A user name or numeric UID to encrypt the credential for. | ||
| - If set to the special string V(self) it sets the user to the user of the calling process. | ||
| - Requires C(systemd) 256 or later. | ||
| type: str | ||
| required: false | ||
| notes: | ||
| - C(systemd-creds) requires C(systemd) 250 or later. | ||
| """ | ||
|
|
@@ -80,6 +85,13 @@ | |
| - name: Print the encrypted secret | ||
| ansible.builtin.debug: | ||
| msg: "{{ encrypted_secret }}" | ||
|
|
||
| - name: Create a credential file | ||
| become: true | ||
| community.general.systemd_creds_encrypt: | ||
| name: db | ||
| secret: access_token | ||
| dest: /etc/credstore.encrypted/db.cred | ||
| """ | ||
|
|
||
| RETURN = r""" | ||
|
|
@@ -91,6 +103,7 @@ | |
| """ | ||
|
|
||
| from ansible.module_utils.basic import AnsibleModule | ||
| import os | ||
|
|
||
|
|
||
| def main(): | ||
|
|
@@ -103,8 +116,12 @@ def main(): | |
| secret=dict(type="str", required=True, no_log=True), | ||
| timestamp=dict(type="str"), | ||
| user=dict(type="str"), | ||
| dest=dict(type="path"), | ||
| ), | ||
| supports_check_mode=True, | ||
| mutually_exclusive=[ | ||
| ['pretty', 'dest'] | ||
| ] | ||
| ) | ||
|
|
||
| cmd = module.get_bin_path("systemd-creds", required=True) | ||
|
|
@@ -115,11 +132,37 @@ def main(): | |
| secret = module.params["secret"] | ||
| timestamp = module.params["timestamp"] | ||
| user = module.params["user"] | ||
| dest = module.params["dest"] | ||
|
|
||
| if dest and os.path.isfile(dest): | ||
| decrypt_cmd = [cmd, "decrypt"] | ||
| if name: | ||
| decrypt_cmd.append("--name=" + name) | ||
|
|
||
| decrypt_cmd.append(dest) | ||
| decrypt_cmd.append("-") | ||
|
|
||
| rc, stdout, stderr = module.run_command(decrypt_cmd) | ||
| if rc != 0: | ||
| return module.fail_json( | ||
| "The credential file already exists; Couldn't decrypt to verify if it contains the same secret", | ||
| changed=False, | ||
| value=None, | ||
| rc=rc, | ||
| stderr=stderr, | ||
| ) | ||
| elif stdout == secret: | ||
| return module.exit_json( | ||
| changed=False, | ||
| value=None, | ||
| rc=rc, | ||
| stderr=stderr, | ||
| ) | ||
|
|
||
| encrypt_cmd = [cmd, "encrypt"] | ||
| if name: | ||
| if name is not None: | ||
| encrypt_cmd.append("--name=" + name) | ||
| else: | ||
| elif not dest: | ||
| encrypt_cmd.append("--name=") | ||
|
Comment on lines
+163
to
166
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be interesting to add one (or more) testcase that expose what happens when |
||
| if not_after: | ||
| encrypt_cmd.append("--not-after=" + not_after) | ||
|
|
@@ -129,13 +172,19 @@ def main(): | |
| encrypt_cmd.append("--timestamp=" + timestamp) | ||
| if user: | ||
| encrypt_cmd.append("--uid=" + user) | ||
| encrypt_cmd.extend(["-", "-"]) | ||
|
|
||
| encrypt_cmd.append("-") | ||
|
|
||
| if dest and not module.check_mode: | ||
| encrypt_cmd.append(dest) | ||
| else: | ||
| encrypt_cmd.append("-") | ||
|
|
||
| rc, stdout, stderr = module.run_command(encrypt_cmd, data=secret, binary_data=True) | ||
|
|
||
| module.exit_json( | ||
| changed=False, | ||
| value=stdout, | ||
| changed=bool(dest), | ||
| value=stdout if not dest else None, | ||
| rc=rc, | ||
| stderr=stderr, | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| # Copyright (c) Ansible Project | ||
| # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) | ||
| # SPDX-License-Identifier: GPL-3.0-or-later | ||
|
|
||
| dependencies: | ||
| - setup_remote_tmp_dir |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,3 +53,59 @@ | |
| - '"SetCredentialEncrypted=web: " in pretty_encrypted_secret.value' | ||
| fail_msg: "SetCredentialEncrypted is not in the output" | ||
| success_msg: "SetCredentialEncrypted is in the output" | ||
|
|
||
| - name: Ensure the dest file doesn't already exist | ||
| ansible.builtin.file: | ||
| path: "{{ remote_tmp_dir }}/systemd_cred_encrypt_test_secret.cred" | ||
| state: absent | ||
|
|
||
| - name: Encrypt secret into the dest file | ||
| community.general.systemd_creds_encrypt: | ||
| secret: token | ||
| dest: "{{ remote_tmp_dir }}/systemd_cred_encrypt_test_secret.cred" | ||
| register: encrypted_dest | ||
|
|
||
| - name: Assert that the task changes when no secret was present | ||
| ansible.builtin.assert: | ||
| that: | ||
| - "encrypted_dest.changed" | ||
| fail_msg: "The task didn't change, but should have" | ||
| success_msg: "The task changed, as it should have" | ||
|
|
||
| - name: Check if the dest file exists | ||
| stat: | ||
| path: "{{ remote_tmp_dir }}/systemd_cred_encrypt_test_secret.cred" | ||
| register: dest_file | ||
|
|
||
| - name: Assert that the dest file exists | ||
| ansible.builtin.assert: | ||
| that: | ||
| - "dest_file.stat.exists" | ||
| fail_msg: "The dest file doesn't exist" | ||
| success_msg: "The dest file does exist" | ||
|
|
||
| - name: Encrypt the same secret into the same dest file as before | ||
| community.general.systemd_creds_encrypt: | ||
| secret: token | ||
| dest: "{{ remote_tmp_dir }}/systemd_cred_encrypt_test_secret.cred" | ||
| register: encrypted_dest_again | ||
|
|
||
| - name: Assert that the task doesn't change with the same secret | ||
| ansible.builtin.assert: | ||
| that: | ||
| - "not encrypted_dest_again.changed" | ||
| fail_msg: "The task changed, but shouldn't have" | ||
| success_msg: "The task didn't change, as it should have" | ||
|
|
||
| - name: Encrypt a different secret into the same dest file as before | ||
| community.general.systemd_creds_encrypt: | ||
| secret: another token | ||
| dest: "{{ remote_tmp_dir }}/systemd_cred_encrypt_test_secret.cred" | ||
| register: encrypted_dest_again | ||
|
|
||
| - name: Assert that the task changes with a different secret | ||
| ansible.builtin.assert: | ||
| that: | ||
| - "encrypted_dest_again.changed" | ||
| fail_msg: "The task didn't change, but should have" | ||
| success_msg: "The task changed, as it should have" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps add an decryption test/options as well? That's https://github.com/ansible-collections/community.general/blob/main/plugins/modules/systemd_creds_decrypt.py on the other hand
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
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.
I think this note might be better placed in
destbelow - both cases described required thatdestis set.Also, maybe a more affirmative wording might help. Something like (inserted be low in dest's description):
Or similar - take this with a pinch of salt.
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.
What do you think?