SSH is reloaded by default even if sshd_manage_service is set to false

[Ouack23]

Hello,

The documentation states that :

* `sshd_allow_reload`

If set to *false*, a reload of sshd wont happen on change. This can help with
troubleshooting. You'll need to manually reload sshd if you want to apply the
changed configuration. Defaults to the same value as `sshd_manage_service`.
(Except on AIX, where `sshd_manage_service` is default *false*, but
`sshd_allow_reload` is default *true*)

but the default variables set sshd_allow_reload to true : https://github.com/willshersystems/ansible-sshd/blob/master/defaults/main.yml#L14

So either the documentation is wrong and should be corrected, or the defaults file should be changed like this:

# If the below is false, don't reload the ssh daemon on change
- sshd_allow_reload: true
+ sshd_allow_reload: "{{ sshd_manage_service }}"

Thank you for reading :)

[Jakuje]

When this was introduced in bcd864f, the documentation was matching the code. But it is "broken" since 43ed7c1 which dropped the assignment.

I am worried that the problem with such assignment might have issues with the variable precedence (would have to double-check though). Do you want to open a PR with this change, ideally also with a reproducer tests to verify that it works as expected in the testing environment (probably only by checking variables as they are based on containers and do not have services running)?

[Jakuje]

@mattwillsher the change wont be that easy as proposed in the issue. I think the defaults/main.yml are evaluated before the user variables are considered so it might be easier to update documentation ...

[richm]

@mattwillsher the change wont be that easy as proposed in the issue. I think the defaults/main.yml are evaluated before the user variables are considered so it might be easier to update documentation ...

It depends - variables defined in defaults/main.yml, and in vars (which includes vars/main.yml as well as inline vars:) are lazily evaluated - they are only evaluated when the value is actually needed - and defaults/main.yml has the lowest precedence https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html#ansible-variable-precedence - so it might work

[mattwillsher]

I was thinking update the docs, which seems to be the original ask?

Could the logic be moved into the when clause for the AIX reload?
Or shift it to an __sshd_allow_reload in vars as others are handled? That a bigger change though.

[richm]

I was thinking update the docs, which seems to be the original ask?

Ah, ok. Works for me.

Could the logic be moved into the when clause for the AIX reload? Or shift it to an __sshd_allow_reload in vars as others are handled? That a bigger change though.

[Jakuje]

so it might work

it does not sounds very convincing :)

In any case, I guess we would need some test coverage.

On the other hand, the semantics of defaults depending on other variable sounds awkward. And given that this really did not work for last 7 years, adjusting the documentation to match reality sounds like the best.

Added to #303.