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

fix do-in-docker args #498

Merged
merged 3 commits into from
Jul 27, 2023
Merged

fix do-in-docker args #498

merged 3 commits into from
Jul 27, 2023

Conversation

Zerocool56
Copy link
Contributor

Add :z flag to volumes. It's fix permission problems on hosts with SELinux (like Fedora 38)
Add HOME env. It's fix permission problems with creation ~/.npm dir

@amezin
Copy link
Member

amezin commented Jul 23, 2023

So it happens only with do-in-docker.sh because CI runs everything as root, and podman maps host uid to container's root.
But do-in-docker.sh keeps host uid, which has no corresponding user in the container (no home, no passwd entry, etc).

Storing HOME content directly in the working directory doesn't seem like a good idea - at least because that content won't be ignored by git, eslint, and would be inconvenient to clean up. Also, the script can be invoked from a subdirectory - which makes things even worse.

Unless there's a specific reason to keep the content between do-in-docker.sh invocations, I'd prefer HOME to be a temporary directory (it will match do-in-podman.sh behavior). Otherwise, it should be all kept in a subdirectory (for example, ${SCRIPT_DIR}/.container-home), and that directory should be added to .gitignore. In this case, do-in-podman.sh should also be modified accordingly.

Also, could you explain SELinux issue in more detail?

@Zerocool56
Copy link
Contributor Author

Zerocool56 commented Jul 25, 2023

About selinux in docker
https://docs.docker.com/storage/bind-mounts/#configure-the-selinux-label
As said in official docs, if we mount dir into the container, we should use z option to update selinux labels for correct permissions.
On my fedora38 without this flag i've got a permission error on npm install step.

About podman we have similar article
https://www.redhat.com/sysadmin/user-namespaces-selinux-rootless-containers

@amezin
Copy link
Member

amezin commented Jul 27, 2023

I don't like the idea of changing permissions (or other attributes) of the files. At least when it could be avoided.

With podman, --security-opt label=disable works without :z. Since it's not some public production service accessed from an untrusted network, I think --security-opt label=disable should be fine.

I can't reproduce the issue with Docker though. Installed as in https://developer.fedoraproject.org/tools/docker/docker-installation.html Have you enabled SELinux support in Docker manually?

Had to manually enable SELinux in /etc/docker/daemon.json to reproduce the issue with Docker. Anyway, --security-opt label=disable seems to work there too.

do-in-docker.sh Outdated Show resolved Hide resolved
do-in-podman.sh Outdated Show resolved Hide resolved
@Zerocool56
Copy link
Contributor Author

I just install docker from the base repos of Fedora, simply run dnf install docker but I suppose it's not necessary.
And SELinux was enabled by default.

About security opt, maybe it's more secure way and we can use it in both situations

@amezin
Copy link
Member

amezin commented Jul 27, 2023

About security opt, maybe it's more secure way

It's "less secure", effectively disabling SELinux. Not disabling completely, just not limiting permissions additionally.

@amezin amezin merged commit 7370040 into ddterm:master Jul 27, 2023
9 of 11 checks passed
@Zerocool56
Copy link
Contributor Author

I meant more secure than relabeling dirs and files.

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.

None yet

2 participants