https://github.com/Tronde/pam_pwd/blob/master/meta/main.yml#L45
please add
- name: Fedora
versions:
- all
unless Fedora is very different and does not work with the code you have.
https://github.com/Tronde/pam_pwd/blob/master/tasks/main.yml
some general issues
- do not use
become:
anywhere - if the user wants to use become
, they can use ansible-playbook --become
, or put it in the inventory, or etc.
- You mostly do not need to use quoting for strings e.g.
name: This is a multi word string that does not require quoting
is valid YAML - you can also use YAML >
and |
for multiline strings for readability e.g.
command: >
this is a
multiline
command
gets passed to the remote shell as this is a multiline command
or
somestring: |
this
is
a
multiline string with the newlines
keeps the embedded newlines
One place you must use quotes is when the value begins with {{
e.g. value: "{{ item }}"
but not value: this is item {{ item }}
-
do not use FQCN module names for builtins - use lineinfile
instead of ansible.builtin.lineinfile
- I know that the Ansible guidelines say to use FQCN everywhere, but there is a lot of debate around this - none of our other roles use FQCN - and for collections we have a tool which converts roles to collection format and can (or can easily be changed to) convert names to FQCN if necessary (https://github.com/linux-system-roles/auto-maintenance/#lsr_role2collectionpy)
-
prefer to not have a lot of conditional code e.g. there are a lot of when: ansible_facts['distribution_major_version'] == "7"
. There are a couple of ways to handle this:
If there are only a few places where conditionals will be used, add some code like this to your tasks/main.yml - https://github.com/linux-system-roles/template/blob/master/tasks/main.yml#L8-L21 (and change __template_*
to __pam_pwd_*
). Then, in the various vars/Platform.yml
and vars/Platform_Version.yml
files, add variables like
__pam_pwd_edit_auth_files: true
for those platforms where you need to update /etc/pam.d/system-auth
and /etc/pam.d/password-auth
- then you can do things like:
- name: "Keep history of the last {{ pam_pwd_history }} passwords used."
lineinfile:
path: "{{ item }}"
insertafter: '^.*pam_pwquality.so.*$'
line: "password requisite pam_pwhistory.so remember={{ pam_pwd_history }} use_authtok"
state: present
backup: yes
with_items:
- /etc/pam.d/system-auth
- /etc/pam.d/password-auth
when:
- item is exists
- __pam_pwd_edit_auth_files | bool
If there a lot of places where the tasks are different, then consider using different task files for different platforms. https://github.com/redhat-cop/automation-good-practices/blob/main/roles/README.adoc#platform-specific-tasks describes a method by which you would have a completely different way to manage passwords on rhel7 vs. rhel8.
https://github.com/Tronde/pam_pwd/blob/master/tasks/main.yml#L52
missing something after the or
?
https://github.com/Tronde/pam_pwd/blob/master/tasks/main.yml#L56
it is better to specify
template:
src: "{{ item }}"
ansible should do the right thing - look in the role templates/
directory
There is a lot of use of command
here - this isn't necessarily a bad thing, but it makes it harder to work in check_mode and makes it harder to be idempotent. https://github.com/redhat-cop/automation-good-practices/blob/main/roles/README.adoc#check-mode-and-idempotency-issues
specifically - use changed_when: false
with command:
if the command does not actually change anything
To make it idempotent, you should first see if the change needs to be made. I'm not sure how authselect works, but something like this:
- name: List profiles {{ pam_pwd_policy_name }}
command: authselect list
register: __pam_pwd_authselect_list
changed_when: false
- name: Select profile {{ pam_pwd_policy_name }}
command: authselect select custom/{{ pam_pwd_policy_name }}
when: not pam_pwd_policy_name is search(__pam_pwd_authselect_list)
that way, Select profile will only be used if it is needed, and the role will report changed: false
if nothing was changed.
- apply changes - typically a handler is used for this - each task which may change the state of the system requiring an authselect apply will call
notify: pam_pwd_authselect_apply
- then in handlers/main.yml
have a task like this:
- name: Apply authselect changes
listen: pam_pwd_authselect_apply
command: authselect apply-changes
Then ansible will call authselect apply-changes, usually at the end of the role or end of the play. This can be problematic though if you want the changes to apply immediately. In that case, you'll need to use register
to keep track of each task that changed something, and call authselect apply-changes
yourself. Something like this:
- name: Select profile {{ pam_pwd_policy_name }}
command: authselect select custom/{{ pam_pwd_policy_name }}
when: not pam_pwd_policy_name is search(__pam_pwd_authselect_list)
register: __pam_pwd_profile
- name: Enable-feature with-faillock
command: authselect enable-feature with-faillock
when: not 'with-faillock' is search(__pam_pwd_get_faillock)
register: __pam_pwd_faillock
- name: Apply changes
command: authselect apply-changes
when: __pam_pwd_profile is changed or __pam_pwd_faillock is changed
see https://docs.ansible.com/ansible/2.9/user_guide/playbooks_tests.html#testing-strings for info about using match
, search
, and regex
https://github.com/Tronde/pam_pwd/blob/master/templates/password-auth and https://github.com/Tronde/pam_pwd/blob/master/templates/system-auth should also have {{ ansible_managed | comment }}
headers (assuming these files can accept comments, and the comment style is #
- https://docs.ansible.com/ansible/latest/user_guide/playbooks_filters.html#adding-comments-to-files