-
Notifications
You must be signed in to change notification settings - Fork 39
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.
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
RHEL compatibility #27
base: master
Are you sure you want to change the base?
Conversation
defaults/main.yml
Outdated
### Opendkim package configuration | ||
# - Redhat : /etc/opendkim.conf | ||
# - Debian / default configuration default to : /etc/default/opendkim | ||
dkim_default_config_file: "{{ '/etc/opendkim.conf' if ansible_distribution_file_variety == 'RedHat' else '/etc/default/opendkim' }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that /etc/opendkim.conf
is main config file, not defaults
as they are in Debian/Ubuntu. It would be probably better idea make distro conditions to commands working with defaults file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, is ansible_distribution_file_variety
proper var to test? Why you choosed this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback!
I'll make /etc/opendkim.conf as default.
After further testing, I think using ansible_os_family
would be better suited.
ansible_distribution_file_variety
would produce Centos/Alma/Rocky on Redhat derivatives.
Mapping for OS family : https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/facts/system/distribution.py#L511
I don't have much exposure to Debian derivatives so can't comment on that.
name: epel-release | ||
state: latest | ||
when: | ||
- ansible_os_family == 'RedHat' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather have separate install files (install_debian.yml
, install_redhat.yml
) per platform and conditions during include in main.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, the only thing that differs is the epel installation. Would you rather have install_debian_postfix.yml, install_debian_sendmail.yml, install_debian_opendkim.yml and its redhat equivalent?
README.md
Outdated
Ansible role for configuring [Postfix](http://www.postfix.org/) with [OpenDKIM](http://opendkim.org/), an implementation for Linux of [DKIM mail signing](http://dkim.org/). Works on [Debian](https://debian.org) distributions and derived like [Ubuntu](https://ubuntu.com/). | ||
Ansible role for configuring [Postfix](http://www.postfix.org/) with [OpenDKIM](http://opendkim.org/), an implementation for Linux of [DKIM mail signing](http://dkim.org/). | ||
Works on [Debian](https://debian.org) distributions and derived like [Ubuntu](https://ubuntu.com/). | ||
Also works on Redhat 9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add RHEL compatibility