geerlingguy / ansible-role-repo-epel

Ansible Role - EPEL Repository for RHEL/CentOS
https://galaxy.ansible.com/geerlingguy/repo-epel/
MIT License
186 stars 148 forks source link

Allow nonroot user to execute #21

Closed suzuki-shunsuke closed 7 years ago

suzuki-shunsuke commented 7 years ago

This PR resolves the problem that if the remote user is not root he can't execute this role due to permission error. This PR adds the variable "epel_nonroot", whose value is true if the remote user is not root and otherwise false, and sets become: "{{ epel_nonroot }}" to tasks which need root priviledges.

wookietreiber commented 7 years ago

@suzuki-shunsuke I haven't seen this practice before. Is this somewhere documented as a best practice or where does it come from?

suzuki-shunsuke commented 7 years ago

@wookietreiber

Thanks for good question. The answer is no. I have come up with it by myself. I have ever made some ansible roles and often used this practice but I have never seen that anyone other than me has used it.

But I believe we should use this practice in ansible tasks which require the root's priviledge, because the value of the become option depends on whether the remote user is root or not.

How do you think?

suzuki-shunsuke commented 7 years ago

Here I show a simple example. We will think about an ansible task to install vim on Debian or Ubuntu.

If we install vim on Vagrant(bento/ubuntu-16.04) and the remote user is vagrant, the task will be following.

- name: Install vim
  apt:
    name: vim
    update_cache: yes
  become: yes

If the value of become option is 'no', the permission error will occur.

On the other hand, if we install vim on docker container (python:2.7.13) and the remote user is root, the value of the become option should be 'no', because if the value of the become option is 'yes', the following error will occur.

# check docker and ansible version
$ docker --version
Docker version 17.03.1-ce, build c6d412e
$ ansible --version
ansible 2.3.1.0
  config file =
  configured module search path = Default w/o overrides
  python version = 2.7.13 (default, Apr  3 2017, 18:16:45) [GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.38)]

# check inventory file and playbook
$ cat hosts 
[default]
python

$ cat playbook.yml 
---
- hosts: default
  connection: docker
  tasks:
  - apt:
      name: vim
      update_cache: yes
    become: yes

# run docker container
$ docker run -ti --name python python:2.7.13 bash

# run ansible playbook in another terminal
$ ansible-playbook -i hosts playbook.yml

PLAY [default] *****************************************************************

TASK [Gathering Facts] *********************************************************
ok: [python]

TASK [apt] *********************************************************************
fatal: [python]: FAILED! => {"changed": false, "failed": true, "module_stderr": "/bin/sh: 1: sudo: not found\n", "module_stdout": "", "msg"
: "MODULE FAILURE", "rc": 127}
        to retry, use: --limit @/Users/jp21722/Documents/ansible-connection-docker-example/playbook.retry

PLAY RECAP *********************************************************************
python                     : ok=1    changed=0    unreachable=0    failed=1

By the way, on some docker container the environment variable "USER" is undefined, so in that case we have to judge whether the remote user is root by the environment variable "HOME".

docker run -ti --name python python:2.7.13 bash                              
root@8da1ef77cd3f:/# echo $USER   

root@8da1ef77cd3f:/# echo $HOME
/root
geerlingguy commented 7 years ago

This is quite confusing to me... I generally run roles like this with become: yes and let it use the sudo/root user by default.

I'd rather not introduce a variable that makes it difficult to find out what user is actually being used (unless that's required to get the role to work).

I kind of understand what you're attempting, but I'd rather you just use become on the role level however you like to, e.g.

- hosts: all
  roles:
    - role: geerlingguy.repo-epel
      become: {{ whatever_you_want_here }}
suzuki-shunsuke commented 7 years ago

@geerlingguy

I'm sorry for confusing you, and thank you for your opinion about my practice. I think the point of this PR is when we make general ansible roles we should set the "become" option per tasks or not. I have ever believed that we should set "become" options per tasks because it is potentially dangerous to execute command with root privilege unnecessarily. But this time I have checked some popular ansible roles and have found that they don't set the "become" option per tasks. So I will rethink about this topic.

Thank you again.