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

x509_crl: prepare releasing the mode option for AnsibleModule's use #596

Merged
merged 2 commits into from
Apr 29, 2023
Merged

x509_crl: prepare releasing the mode option for AnsibleModule's use #596

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8fabd3
Prepare releasing the mode option for AnsibleModule's use.
felixfontein Apr 28, 2023
6666baf
Update docs.
felixfontein Apr 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelogs/fragments/596-x509_crl-mode.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
bugfixes:
- "x509_crl - remove problem with ansible-core 2.16 due to ``AnsibleModule`` is now validating the ``mode`` parameter's values (https://github.com/ansible-collections/community.crypto/issues/596)."
minor_changes:
- "x509_crl - the ``crl_mode`` option has been added to replace the existing ``mode`` option (https://github.com/ansible-collections/community.crypto/issues/596)."
deprecated_features:
- "x509_crl - the ``mode`` option is deprecated; use ``crl_mode`` instead. The ``mode`` option will change its meaning in community.crypto 3.0.0, and will refer to the CRL file's mode instead (https://github.com/ansible-collections/community.crypto/issues/596)."
36 changes: 32 additions & 4 deletions plugins/modules/x509_crl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
default: present
choices: [ absent, present ]

mode:
crl_mode:
description:
- Defines how to process entries of existing CRLs.
- If set to C(generate), makes sure that the CRL has the exact set of revoked certificates
Expand All @@ -51,8 +51,17 @@
I(revoked_certificates), but can also contain other revoked certificates. If the CRL file
already exists, all entries from the existing CRL will also be included in the new CRL.
When using C(update), you might be interested in setting I(ignore_timestamps) to C(true).
- The default value is C(generate).
felixfontein marked this conversation as resolved.
Show resolved Hide resolved
type: str
# default: generate
choices: [ generate, update ]
version_added: 2.13.0
mode:
description:
- This parameter is deprecated and will be removed in community.crypto 3.0.0. Use I(crl_mode) instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this option is the new crl_mode option, as defined in line 45, but it's being set at deprecated with a message to use crl_mode "instead". Am I misreading something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes; basically I renamed mode to crl_mode, but instead of just adding mode as an alias to crl_mode I have to keep it as a separate option, since otherwise mode as an option would show up from AnsibleModule's add_file_common_args=True.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about changing the documentation to:

Suggested change
- This parameter is deprecated and will be removed in community.crypto 3.0.0. Use I(crl_mode) instead.
- This parameter has been renamed to I(crl_mode). The old name I(mode) is now deprecated and will be removed in community.crypto 3.0.0.
Simply replace usage of this parameter with I(crl_mode).

Copy link
Contributor

Choose a reason for hiding this comment

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

I see my mistake, I completely missed line 59. What had me confused was thinking that 61 was still part of crl_mode 🤦

With that, the old description was fine. The new description is good too; only thing I'd avoid is use of the word "simply" but maybe that's my own feeling only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed 'simply', see 6666baf.

- Note that from community.crypto 3.0.0 on, I(mode) will be used for the CRL file's mode.
type: str
default: generate
# default: generate
choices: [ generate, update ]

force:
Expand Down Expand Up @@ -479,7 +488,7 @@ def __init__(self, module):

self.format = module.params['format']

self.update = module.params['mode'] == 'update'
self.update = module.params['crl_mode'] == 'update'
self.ignore_timestamps = module.params['ignore_timestamps']
self.return_content = module.params['return_content']
self.name_encoding = module.params['name_encoding']
Expand Down Expand Up @@ -827,7 +836,18 @@ def main():
module = AnsibleModule(
argument_spec=dict(
state=dict(type='str', default='present', choices=['present', 'absent']),
mode=dict(type='str', default='generate', choices=['generate', 'update']),
crl_mode=dict(
type='str',
# default='generate',
choices=['generate', 'update'],
),
mode=dict(
type='str',
# default='generate',
choices=['generate', 'update'],
removed_in_version='3.0.0',
removed_from_collection='community.crypto',
),
force=dict(type='bool', default=False),
backup=dict(type='bool', default=False),
path=dict(type='path', required=True),
Expand Down Expand Up @@ -882,6 +902,14 @@ def main():
add_file_common_args=True,
)

if module.params['mode']:
if module.params['crl_mode']:
module.fail_json('You cannot use both `mode` and `crl_mode`. Use `crl_mode`.')
module.params['crl_mode'] = module.params['mode']
# TODO: in 3.0.0, once the option `mode` has been removed, remove this:
module.params.pop('mode', None)
# From then on, `mode` will be the file mode of the CRL file

if not CRYPTOGRAPHY_FOUND:
module.fail_json(msg=missing_required_lib('cryptography >= {0}'.format(MINIMAL_CRYPTOGRAPHY_VERSION)),
exception=CRYPTOGRAPHY_IMP_ERR)
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/targets/filter_x509_crl_info/tasks/impl.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
reason_critical: true
invalidity_date: 20191012000000Z
ignore_timestamps: false
mode: update
crl_mode: update
return_content: true
register: crl_2_change

Expand All @@ -156,7 +156,7 @@
reason_critical: true
invalidity_date: 20191012000000Z
ignore_timestamps: true
mode: update
crl_mode: update
return_content: true
register: crl_2_change_order

Expand Down
16 changes: 8 additions & 8 deletions tests/integration/targets/x509_crl/tasks/impl.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@
revoked_certificates:
- serial_number: 1235
ignore_timestamps: true
mode: update
crl_mode: update
check_mode: true
register: crl_2_idem_update_change_check

Expand All @@ -378,7 +378,7 @@
revoked_certificates:
- serial_number: 1235
ignore_timestamps: true
mode: update
crl_mode: update
register: crl_2_idem_update_change

- name: Create CRL 2 (idempotent update, check mode)
Expand All @@ -398,7 +398,7 @@
reason_critical: true
invalidity_date: 20191012000000Z
ignore_timestamps: true
mode: update
crl_mode: update
check_mode: true
register: crl_2_idem_update_check

Expand All @@ -419,7 +419,7 @@
reason_critical: true
invalidity_date: 20191012000000Z
ignore_timestamps: true
mode: update
crl_mode: update
register: crl_2_idem_update

- name: Create CRL 2 (changed timestamps, check mode)
Expand All @@ -439,7 +439,7 @@
reason_critical: true
invalidity_date: 20191012000000Z
ignore_timestamps: false
mode: update
crl_mode: update
check_mode: true
register: crl_2_change_check

Expand All @@ -460,7 +460,7 @@
reason_critical: true
invalidity_date: 20191012000000Z
ignore_timestamps: false
mode: update
crl_mode: update
return_content: true
register: crl_2_change

Expand Down Expand Up @@ -493,7 +493,7 @@
reason_critical: true
invalidity_date: 20191012000000Z
ignore_timestamps: true
mode: update
crl_mode: update
return_content: true
register: crl_2_change_order_ignore

Expand All @@ -514,7 +514,7 @@
reason_critical: true
invalidity_date: 20191012000000Z
ignore_timestamps: true
mode: update
crl_mode: update
return_content: true
register: crl_2_change_order

Expand Down