Skip to content
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

sudo: properly allow wheel group to use sudo via visudo #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ljmf00
Copy link
Member

@ljmf00 ljmf00 commented Nov 3, 2020

Signed-off-by: Luís Ferreira [email protected]


The same approach on Arch Linux infrastructure https://git.archlinux.org/infrastructure.git/tree/roles/sudo/tasks/main.yml as sudoers file already has entries for what is done here previously. Also it's better to use visudo to edit /etc/sudoers.

This approach will unsure that running ansible twice won't break /etc/sudoers file.

@ljmf00 ljmf00 requested a review from FFY00 November 3, 2020 13:53
Copy link
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep sudores.d support and just install a new file there with your config.

Copy link
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry.

@ljmf00
Copy link
Member Author

ljmf00 commented Nov 3, 2020

Please keep sudores.d support and just install a new file there with your config.

The sudoers.d support is enabled by default on arch. The previous method mess some things up when running the ansible playbook.

This is what the default configuration looks like:


## Read drop-in files from /etc/sudoers.d
@includedir /etc/sudoers.d

So instead of modifying the sudoers, you propose to add the changes on /etc/sudoers.d seperate files?

@FFY00
Copy link
Member

FFY00 commented Nov 3, 2020

The sudoers.d support is enabled by default on arch.

Yes, but that can change anytime.

The previous method mess some things up when running the ansible playbook.

Could you elaborate?

So instead of modifying the sudoers, you propose to add the changes on /etc/sudoers.d seperate files?

Yes, that avoids all this regex complexity. Also, the current regex is easily breakable, if we go with the regex approach we should use something a bit better.

@ljmf00
Copy link
Member Author

ljmf00 commented Nov 3, 2020

Yes, but that can change anytime.

Sure, then I propose to have our own sudoers template, instead.

Could you elaborate?

The problem I faced is because /etc/sudoers changed on a new update, and by adding includedir like this:

- name: enable sudoers.d support
  lineinfile:
    path: /etc/sudoers
    line: '#includedir /etc/sudoers.d'

With the new configuration, it will write something like:


## Read drop-in files from /etc/sudoers.d
@includedir /etc/sudoers.d
#includedir /etc/sudoers.d

And will include the files inside sudoers.d twice. See the error when running something with sudo:

/etc/sudoers.d/archbuild:5: Alias "ARCHBUILD" already defined
                       /usr/bin/makerepropkg

Yes, that avoids all this regex complexity. Also, the current regex is easily breakable, if we go with the regex approach we should use something a bit better.

Yeah agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants