postgresql
postgresql copied to clipboard
Become root when installing dependencies
as far as I know this whole role is run as root first then changes to the pgsql user when needed then returns as root so this is not needed.
The entire ANXS.postgresql role is supposed to be run as root? If so, isn't that a design bug? Or do you mean something else?
FWIW, this PR was created because I ran ANXS.postgresql against a remote host, where the user used for SSH connections was non-root and had sudo privileges.
I think being able to run as non-root would be an improvement. It would be good if in places where the playbook needs root, it would specifically becomes it there. Not having to specify to run the entire role as root is one less requirement on users. This is backwards compatible too.
I didn't see it specified in the docs that it needs root or not. Either way, that might also be helpful too.
yes, it does. Your doing a lot of system level changes. This means either ssh in as root or as another user that can escalate privileges. So if you look at the tasks, you're installing packages, changing systemd, init files etc. Adding become root/sudo in every tasks is a waste of time and unnecessary. The parts that are not run as root/sudo are the postgres parts where it becomes pgsql to handle pgsql tasks since you can't be root/sudo on postgres. This is also how ansible works.
you don't need to specify anywhere that you need root. You should know what you need by reading the ansible documents and decide if you need to escalate privileges or not. So if you are running this from ansible core or awx, you can easily escalate your privileges when you run the playbook. There is no need to do it on every tasks. When it comes to this PR this is not needed.
@jlozadad I hear you; all of your environments work that way so I can see how this isn't useful to you. I'm a user of this role, and I see value in it.
In terms of documenting the root requirement, I agree the Ansible docs do guide the role author to make a choice for the users if it should be run as root. What I'm confused about is how users could know what choice you made without it being documented.
The PR is posted so if you accept it that would be cool. If you don't accept it that is your choice. Thanks for the role either way. It's really helpful.
Adding become root/sudo in every tasks is a waste of time and unnecessary.
This role already specifies become
in a number of places. If "adding become root/sudo in every tasks is a waste of time and unnecessary," it would be good to drop these directives. Doing so would let the code base be self-consistent, with the possible side-effects of improving user-friendliness and squashing some latent bugs.
$ git grep become:
tasks/configure.yml: become: yes
tasks/configure.yml: become: yes
tasks/configure.yml: become: yes
tasks/configure.yml: become: yes
tasks/configure.yml: become: yes
tasks/configure.yml: become: true
tasks/databases.yml: become: yes
tasks/databases.yml: become: yes
tasks/databases.yml: become: yes
tasks/databases.yml: become: yes
tasks/databases.yml: become: yes
tasks/databases.yml: become: yes
tasks/databases.yml: become: yes
tasks/databases.yml: become: yes
tasks/databases.yml: become: yes
tasks/install_dnf.yml: become: yes
tasks/install_yum.yml: become: yes
tasks/schemas.yml: become: yes
tasks/users.yml: become: yes
tasks/users_privileges.yml: become: yes
tests/docker/site.yml: become: false
tests/docker/site.yml: become: false
tests/playbook.yml: become: yes
That said, I disagree with the notion that explicitly escalating privileges on specific tasks is a harmful design pattern. There are two benefits to escalating privileges only when needed:
- The role better follows the principle of least privilege (PoLP).
- The role is more user-friendly. Users who default to setting
become: true
won't be affected. And users who leavebecome
at its default offalse
(and follow the PoLP) will be able to use this role without stumbling over this roadblock.
Hi @Ichimonji10 , I notice this has stalled... and I'm wondering if there's a fault, or this is just making things safer?
I've always run the role as a standard user which has "sudo" privs, and it seems to work OK.
My goal in filing this issue is to make this role safer. Better to fix an issue before it bites someone than afterwards.
I don't recall whether I was able to run this role as a user who merely had sudo privileges. It's been quite some time since I've had to use this role, as I no longer work on a project which uses this.
This seems like a fine change to me I agree with the sentiments put forward by @Ichimonji10 and @bmbouter
The whole role is meant to be run as non root, that is why there are "become" statemens in other tasks.
In my view, the lack of thereof (hence this pull request) is a bug. Most likely this has not been addressed before as other users that need "yum" probably ran the whole role as root.
In my view, we should merge this into master.
Hi @Ichimonji10 , I notice this has stalled... and I'm wondering if there's a fault, or this is just making things safer?
I've always run the role as a standard user which has "sudo" privs, and it seems to work OK.
Did you run it on Fedora or CentOs? If you, as I, are on Ubuntu or Debian, you would not be affected by the tasks in this pull request.
Did you run it on Fedora or CentOs? If you, as I, are on Ubuntu or Debian, you would not be affected by the tasks in this pull request.
At the time I wrote this pull request, I would have tested it against Fedora and/or RHEL 7.
Sorry Ichimonji10, my question was actually directed to @gclough. I suspect that he has not ran into your elevated rights issue that you are correcting with this pull request, as he might not have been running on Fedora and/or RHEL.
Is this related to issue #434