ansible / receptor-collection

Apache License 2.0
14 stars 15 forks source link

receptor_replace_tls should be true by default #71

Open kurokobo opened 8 months ago

kurokobo commented 8 months ago

receptor_replace_tls is introduced by #39 and its default value is false, but I believe this should be true by default. Or in the first place, I think there is no need to allow this parameter to be changed. Keeping module default (true) and removing receptor_replace_tls should be better.

I assume the reason for the default value (false) is to avolid handler to be triggered every time by the task with ansible.builtin.copy with force: true, which results always changed.

However, force: true for the ansible.builtin.copy module does not break idempotency, as documented below.

If true, the remote file will be replaced when contents are different than the source. If false, the file will only be transferred if the destination does not exist. https://docs.ansible.com/ansible/latest/collections/ansible/builtin/copy_module.html

Any certificates should be copied if the file contents are different, even if the file already exists. Also, for worksign cert, force for copy module is left at the default true: https://github.com/ansible/receptor-collection/blob/be929596f0598e0ac3956f3430bbae8603d3448f/roles/setup/tasks/worksign_local.yml

@fosterseth Could you please tell me if there is any particular reason why you set the default to false in #39?

kurokobo commented 8 months ago

This can cause problems with certificates not installing properly by following steps:

In this case, receptor CA has been changed, but install bundle does not replace CA since receptor_replace_tls is false by default. This causes the exec node to not be in ready state.

I thought about creating a PR to set receptor_replace_tls to true in group_vars in install bundle on the AWX side, but it didn't seem necessary to have receptor_replace_tls: false in the first place, so I created an issue in this repository.