ansible-collection-hardening
ansible-collection-hardening copied to clipboard
Use distribution version as default when major version isn't defined
OpenBSD doesn't have an ansible_facts.distribution_major_version
fact; I think SmartOS is the same way, but haven't confirmed. For example, here are all the ansible_distribution*
facts on OpenBSD:
{
"ansible_distribution": "OpenBSD",
"ansible_distribution_release": "release",
"ansible_distribution_version": "6.8"
}
In this PR I've changed the distribution version to be used as a fallback default when trying to load variables. Otherwise, on these platforms Ansible will fail with a cryptic message about first_found
lookup failing (because it can't template its arguments).
Nice catch, unfortunately we currently have no CI testing on OpenBSD so this didn't come up sooner.
However I have some problems with your patch. Using the fact ansible_facts.distribution_version
is plainly wrong for all other use-cases we want to cover here. :(
To make this clear, we use this task to load OS specific variables. In this particular case we have different variables for RHEL and RHEL8 distributions. I know, that your logic will not really disturb this, but it will make it very hard to reason about which files are loaded.
I'm thinking about some better solution, for the time being, could you try to "fake" this fact via an Ansible task in your playbook:
- name: fake distribution_major_version fact
set_fact:
ansible_facts: "{{ ansible_facts | combine({'distribution_major_version':ansible_facts.distribution_version}) }}"
- name: include role
include_role:
name: ssh_hardening
now I had a talk with @rndmh3ro and we both agreed, that we don't want to complicate the os variable selection any further. And we both also agreed, that inserting the above task to "fake" the ansible_major_version
fact in the ssh_hardening role might be a good workaround. But we should only do that for a very limited number of operatingsystems, namely OpenBSD and SmartOS.
@jbronn are you up to do that?
sorry to let this slip, but a different solution was already merged with #597