-
-
Notifications
You must be signed in to change notification settings - Fork 956
Detect rootless mode #1484
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
Detect rootless mode #1484
Changes from 8 commits
7c16c3e
5c1cd81
a8887d2
4416c3d
818a909
79528ec
b0fc24f
da85e73
12a40fd
dff7d60
9d4fe3a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import functools | ||
| import hashlib | ||
| import os | ||
| from typing import Sequence | ||
|
|
@@ -9,6 +10,7 @@ | |
| from pre_commit.prefix import Prefix | ||
| from pre_commit.util import CalledProcessError | ||
| from pre_commit.util import clean_path_on_failure | ||
| from pre_commit.util import cmd_output | ||
| from pre_commit.util import cmd_output_b | ||
|
|
||
| ENVIRONMENT_DIR = 'docker' | ||
|
|
@@ -76,7 +78,24 @@ def install_environment( | |
| os.mkdir(directory) | ||
|
|
||
|
|
||
| @functools.lru_cache(maxsize=1) | ||
| def docker_is_rootless() -> bool: | ||
| returncode, stdout, stderr = cmd_output( | ||
| ('docker', 'system', 'info'), | ||
| ) | ||
| for line in stdout.splitlines(): | ||
| # rootless docker has "rootless" | ||
| # rootless podman has "rootless: true" | ||
| if line.strip().startswith('rootless'): | ||
| if 'false' not in line: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: Technically, the case where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be useful as a code comment
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that what is causing the coverage error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is weird. If tests include example outputs containing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @asottile any ideas about this?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's no test for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would probably not catch docker's behavior. Better to just improve the test matrix |
||
| return True | ||
| break | ||
|
themr0c marked this conversation as resolved.
|
||
| return False | ||
|
|
||
|
|
||
| def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover | ||
| if: docker_is_rootless(): | ||
|
themr0c marked this conversation as resolved.
Outdated
|
||
| return () | ||
| try: | ||
| return ('-u', f'{os.getuid()}:{os.getgid()}') | ||
| except AttributeError: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be even faster and simplify both the logic below and the mock output used in testing to only query Docker/Podman for the field we care about. From the rootless-docker GitHub Action's README,
docker info --format "{{ .ClientInfo.Context }}"outputsrootlessin Docker when in rootless mode, and other strings (most commonlydefault, I believe) otherwise. I suspect, based on Podman's documentation, thatpodman info --format "{{ .host.security.rootless }}"outputstruewhen in rootless mode andfalseotherwise. Go templates are quite powerful, so we can say:docker info --format '{{ if .ClientInfo.Context }} {{ eq .ClientInfo.Context "rootless" }} {{ else }} {{ .host.security.rootless }} {{ end }}'. This command outputstruewhen running in rootless mode andfalseotherwise in both Docker and Podman, although I have only tested it on Docker. I don't have Podman installed, so hopefully someone who does can straighten me out if I'm mistaken. Disclosure: I am the author of the rootless-docker GitHub Action.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here on my machine (Linux Mint 19.3, Docker version 20.10.16, build aa7e414) your suggestion returns
defaultfor rootless docker, asrootlessseems to be part of theSecurity OptionsunderServer.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for letting me know. Bummer that the behavior isn't consistent across our machines even on the exact same build of Docker. I am on Ubuntu 22.04, but I wonder whether the difference has more to do with how we installed rootless Docker? It's possible to detect rootless mode under
SecurityOptionstoo (e.g.,docker info --format '{{ range .SecurityOptions }}{{ if eq . "name=rootless" }}true{{ end }}{{ end }}'), FWIW.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC I followed the official instructions for Ubuntu. Maybe you chose to set the client context explicitly where I went for the
DOCKER_HOSTenvironment variable? Both options are listed in the instructions: https://docs.docker.com/engine/security/rootless/#clientThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you are correct. I suppose one could check the
DOCKER_HOSTenvironment variable before runningdocker info.