-
Notifications
You must be signed in to change notification settings - Fork 94
major code cleanup #8
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
Conversation
|
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. |
SuperQ
left a comment
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.
LGTM
|
It's perfectly fine to stick with systemd unit files in |
| dest: "{{ minio_client_bin }}" | ||
| owner: "{{ minio_user }}" | ||
| group: "{{ minio_group }}" | ||
| owner: "root" |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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