major code cleanup #8
Conversation
paulfantom
requested review from
SuperQ and
atosatto
and removed request for
SuperQ
|
Do we want to add a cleanup for the old unit file location? |
|
That's a good question. Adding this won't change anything in how it operates since files located in /etc/systemd/system takes priority over ones located in lib/systemd/system. |
|
It's perfectly fine to stick with systemd unit files in |
| - name: download minio client | ||
| get_url: | ||
| url: "{{ minio_client_download_url }}" | ||
| dest: "{{ minio_client_bin }}" | ||
| owner: "{{ minio_user }}" | ||
| group: "{{ minio_group }}" | ||
| owner: "root" |
There was a problem hiding this comment.
This was done as a kind of "security measure".
Just not to download binaries from the internet as "root".
But I am not sure it still makes sense.
Thoughts?
There was a problem hiding this comment.
It isn't good to run any binary as a user who can edit this binary. We have a sane default for minio_client_download_url which is an official source, and we are using HTTPS so there shouldn't be any problem with tampering the file while transmitting to server.
I would also consider downloading binary once to local machine, validating checksum and then propagating it to target hosts. This increases security and reduces needed network bandwidth when used with more than 1 target. We are doing the same with most of the roles in @cloudalchemy :)
serversection as those aren't needed when only installing clientwith_indexed_items)ansible_support_packagestominio_ansible_support_packagesas every variable starting fromansible_should come from ansible itself, not from role.python_sni_pip_dependenciesand just list packages in task for better readabilitysystemd_units_dirand always use/etc/systemd/systemvalue as is recommended by systemdinitd_conf_dirvariable and only use its value for better readability