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

Skip subgid check #51

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Skip subgid check #51

wants to merge 3 commits into from

Conversation

jonppe
Copy link

@jonppe jonppe commented Dec 15, 2021

Disable setting subuid and subgid by default.
Add minor comment about alternatives to include_role.

@ikke-t
Copy link
Owner

ikke-t commented Dec 16, 2021

Thanks I like the idea. Nowadays it's rather given that user has the sub{g,u}id set. Where as at the time I wrote the playbooks, it typically wasn't there.

I'd like to ask a slight change. I still believe the check is in place, because if not, things won't work. So Instead of not including the task, please allow it to check it always the files, but just make the update conditional. And fail if the line is missing and update is disabled.

Johannes Aalto added 3 commits December 16, 2021 11:58
Adding subgids and subguids is not needed anymore since useradd
adds them automatically.

The addition implemented here only works for the first user (id 1000 with
subids starting from 100000).
For other users it leads to overlapping subuid/subgid ranges
which might lead to very confusing issues when running containers.

The addition is left here in case someone is using it or wants
to override default ids with some specific values.

Fixes ikke-t#34
It feels appropriate to mention this alternative since
the author of the anti-pattern article seems to propose this approach too
(in the more recent comments of the article).
"find" module only checks that files exist in a directory.
In this case it did not fail since the path was a file but there was
a warning:

  TASK [ikke_t.podman_container_systemd : check if user is in subuid file] ***********************
  [WARNING]: Skipped '/etc/subuid' path due to this access issue: '/etc/subuid' is not a directory
  ok: [hostname]
@jonppe
Copy link
Author

jonppe commented Dec 16, 2021

I changed the implementation as proposed.
Also noticed that the check wasn't actually working.

find:
path: /etc/subuid
contains: '^{{ container_run_as_user }}:.*$'
shell: "grep -i '^{{ container_run_as_user}}:.*' /etc/subuid"
Copy link
Owner

Choose a reason for hiding this comment

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

I'd rather not drop to shell for things ansible can handle natively

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