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

feat: Run /docker-entrypoint.d/ scripts as root #4568

Closed
wants to merge 6 commits into from

Conversation

RulerOf
Copy link

@RulerOf RulerOf commented May 15, 2024

what

  • Run the entrypoint script as root

why

  • Installing software during container init (apk add/apt-get install require root)

tests

Containers built as atltest.

Runs CMD as atlantis user:

Debian
╰─❯ docker run -it --rm atltest:debian ps awfux
No files found in /docker-entrypoint.d/, skipping
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root         1  0.0  0.0   2064   768 ?        Ss   19:52   0:00 /usr/bin/dumb-init /bin/sh /usr/local/bin/docker-entrypoint.sh ps awfux
atlantis     6  0.0  0.0   8040  3584 pts/0    Rs+  19:52   0:00 ps awfux
Alpine
╰─❯ docker run -it --rm atltest:alpine ps awfux
No files found in /docker-entrypoint.d/, skipping
PID   USER     TIME  COMMAND
    1 root      0:00 {docker-entrypoi} /usr/bin/dumb-init /bin/sh /usr/local/bin/docker-entrypoint.sh ps awfux
    7 atlantis  0:00 ps awfux

Entrypoint script runs as root and successfully installs software. CMD is run as atlantis user:

╰─❯ cat $PWD/test-entrypoint.sh
apk add --no-cache python3 || true
apt-get update; apt-get install -y python3 || true
Debian
╰─❯ docker run -it --rm -v $PWD/test-entrypoint.sh:/docker-entrypoint.d/test-entrypoint.sh atltest:debian ps awfux
/docker-entrypoint.d/ is not empty, will attempt to perform script execition
Looking for shell scripts in /docker-entrypoint.d/
Launching /docker-entrypoint.d/test-entrypoint.sh
/docker-entrypoint.d/test-entrypoint.sh: 1: apk: not found
Get:1 http://deb.debian.org/debian bookworm InRelease [151 kB]
Get:2 http://deb.debian.org/debian bookworm-updates InRelease [55.4 kB]
Get:3 http://deb.debian.org/debian-security bookworm-security InRelease [48.0 kB]
Get:4 http://deb.debian.org/debian bookworm/main arm64 Packages [8685 kB]
Get:5 http://deb.debian.org/debian bookworm-updates/main arm64 Packages [13.7 kB]
Get:6 http://deb.debian.org/debian-security bookworm-security/main arm64 Packages [153 kB]
Fetched 9106 kB in 1s (6833 kB/s)
Reading package lists... Done
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
The following additional packages will be installed:
  libpython3-stdlib libpython3.11-minimal libpython3.11-stdlib media-types python3-minimal python3.11 python3.11-minimal
Suggested packages:
  python3-doc python3-tk python3-venv python3.11-venv python3.11-doc binutils binfmt-support
The following NEW packages will be installed:
  libpython3-stdlib libpython3.11-minimal libpython3.11-stdlib media-types python3 python3-minimal python3.11 python3.11-minimal
0 upgraded, 8 newly installed, 0 to remove and 0 not upgraded.
Need to get 5072 kB of archives.
After this operation, 23.1 MB of additional disk space will be used.
Get:1 http://deb.debian.org/debian bookworm/main arm64 libpython3.11-minimal arm64 3.11.2-6 [806 kB]
Get:2 http://deb.debian.org/debian bookworm/main arm64 python3.11-minimal arm64 3.11.2-6 [1858 kB]
Get:3 http://deb.debian.org/debian bookworm/main arm64 python3-minimal arm64 3.11.2-1+b1 [26.3 kB]
Get:4 http://deb.debian.org/debian bookworm/main arm64 media-types all 10.0.0 [26.1 kB]
Get:5 http://deb.debian.org/debian bookworm/main arm64 libpython3.11-stdlib arm64 3.11.2-6 [1747 kB]
Get:6 http://deb.debian.org/debian bookworm/main arm64 python3.11 arm64 3.11.2-6 [572 kB]
Get:7 http://deb.debian.org/debian bookworm/main arm64 libpython3-stdlib arm64 3.11.2-1+b1 [9296 B]
Get:8 http://deb.debian.org/debian bookworm/main arm64 python3 arm64 3.11.2-1+b1 [26.3 kB]
Fetched 5072 kB in 0s (18.1 MB/s)
debconf: delaying package configuration, since apt-utils is not installed
Selecting previously unselected package libpython3.11-minimal:arm64.
(Reading database ... 10549 files and directories currently installed.)
Preparing to unpack .../libpython3.11-minimal_3.11.2-6_arm64.deb ...
Unpacking libpython3.11-minimal:arm64 (3.11.2-6) ...
Selecting previously unselected package python3.11-minimal.
Preparing to unpack .../python3.11-minimal_3.11.2-6_arm64.deb ...
Unpacking python3.11-minimal (3.11.2-6) ...
Setting up libpython3.11-minimal:arm64 (3.11.2-6) ...
Setting up python3.11-minimal (3.11.2-6) ...
Selecting previously unselected package python3-minimal.
(Reading database ... 10856 files and directories currently installed.)
Preparing to unpack .../python3-minimal_3.11.2-1+b1_arm64.deb ...
Unpacking python3-minimal (3.11.2-1+b1) ...
Selecting previously unselected package media-types.
Preparing to unpack .../media-types_10.0.0_all.deb ...
Unpacking media-types (10.0.0) ...
Selecting previously unselected package libpython3.11-stdlib:arm64.
Preparing to unpack .../libpython3.11-stdlib_3.11.2-6_arm64.deb ...
Unpacking libpython3.11-stdlib:arm64 (3.11.2-6) ...
Selecting previously unselected package python3.11.
Preparing to unpack .../python3.11_3.11.2-6_arm64.deb ...
Unpacking python3.11 (3.11.2-6) ...
Selecting previously unselected package libpython3-stdlib:arm64.
Preparing to unpack .../libpython3-stdlib_3.11.2-1+b1_arm64.deb ...
Unpacking libpython3-stdlib:arm64 (3.11.2-1+b1) ...
Setting up python3-minimal (3.11.2-1+b1) ...
Selecting previously unselected package python3.
(Reading database ... 11266 files and directories currently installed.)
Preparing to unpack .../python3_3.11.2-1+b1_arm64.deb ...
Unpacking python3 (3.11.2-1+b1) ...
Setting up media-types (10.0.0) ...
Setting up libpython3.11-stdlib:arm64 (3.11.2-6) ...
Setting up libpython3-stdlib:arm64 (3.11.2-1+b1) ...
Setting up python3.11 (3.11.2-6) ...
Setting up python3 (3.11.2-1+b1) ...
running python rtupdate hooks for python3.11...
running python post-rtupdate hooks for python3.11...
Configuration complete; ready for start up
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root         1  0.0  0.0   2064   768 ?        Ss   19:52   0:00 /usr/bin/dumb-init /bin/sh /usr/local/bin/docker-entrypoint.sh ps awfux
atlantis     7  0.0  0.0   8040  3712 pts/0    Rs+  19:52   0:00 ps awfux
Alpine
╰─❯ docker run -it --rm -v $PWD/test-entrypoint.sh:/docker-entrypoint.d/test-entrypoint.sh atltest:alpine ps awfux
/docker-entrypoint.d/ is not empty, will attempt to perform script execition
Looking for shell scripts in /docker-entrypoint.d/
Launching /docker-entrypoint.d/test-entrypoint.sh
fetch https://dl-cdn.alpinelinux.org/alpine/v3.19/main/aarch64/APKINDEX.tar.gz
fetch https://dl-cdn.alpinelinux.org/alpine/v3.19/community/aarch64/APKINDEX.tar.gz
(1/13) Installing libbz2 (1.0.8-r6)
(2/13) Installing libffi (3.4.4-r3)
(3/13) Installing gdbm (1.23-r1)
(4/13) Installing xz-libs (5.4.5-r0)
(5/13) Installing libgcc (13.2.1_git20231014-r0)
(6/13) Installing libstdc++ (13.2.1_git20231014-r0)
(7/13) Installing mpdecimal (2.5.1-r2)
(8/13) Installing libpanelw (6.4_p20231125-r0)
(9/13) Installing sqlite-libs (3.44.2-r0)
(10/13) Installing python3 (3.11.9-r0)
(11/13) Installing python3-pycache-pyc0 (3.11.9-r0)
(12/13) Installing pyc (3.11.9-r0)
(13/13) Installing python3-pyc (3.11.9-r0)
Executing busybox-1.36.1-r15.trigger
OK: 79 MiB in 57 packages
/docker-entrypoint.d/test-entrypoint.sh: line 2: apt-get: not found
/docker-entrypoint.d/test-entrypoint.sh: line 2: apt-get: not found
Configuration complete; ready for start up
PID   USER     TIME  COMMAND
    1 root      0:00 {docker-entrypoi} /usr/bin/dumb-init /bin/sh /usr/local/bin/docker-entrypoint.sh ps awfux
    7 atlantis  0:00 ps awfux

references

@RulerOf RulerOf requested review from a team as code owners May 15, 2024 19:59
@RulerOf RulerOf requested review from chenrui333, lukemassa and X-Guardian and removed request for a team May 15, 2024 19:59
@github-actions github-actions bot added the build Relating to how we build Atlantis label May 15, 2024
@RulerOf RulerOf changed the title Run /docker-entrypoint.d/ scripts as root feat: Run /docker-entrypoint.d/ scripts as root May 15, 2024
Previous behavior was to only use `gosu` if Atlantis was being run
@jippi
Copy link
Contributor

jippi commented May 15, 2024

I'm not a maintainer, but my 50c is that if you want to modify the Atlantis Docker image, modify it by building a custom image and installing the software needed through that - or tweak your securityContext/container UID/GID to run as root and gosu back to the atlantis user before returning and running the service.

Allowing the container to start as root by default would likely trigger a ton of security tools - my $DayJob Gatekeeper policy would flag this immediately and reject the deployment - making it a (potential) breaking change.

Could probably circumvent this by introducing a root version of the image with a new Docker tag as opt-in, but the complexity and overhead for that is not zero, and might not be worth the squeeze.

Don't get me wrong, I can absolutely see the value of having this capability, but its a pretty significant trade-off beyond the (rather straight forward) change. Perhaps documenting how to run the container as root and "off-ramp" the default path would serve as a middle ground?

🤔 perhaps leaving USER atlantis in the Dockerfile (retaining current behavior), and documenting how to override the init UID/GID to root will allow you to modify the image - but is treated as an off-ramp and something you need to opt-in to, rather than having to opt-out off

@@ -164,10 +165,13 @@ RUN apk add --no-cache \
bash~=5 \
openssh~=9 \
dumb-init~=1 \
gcompat~=1
gcompat~=1 \
su-exec~=0
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with Alpine, but is there a reason for using su-exec instead of gosu proper?

Copy link
Author

Choose a reason for hiding this comment

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

The gosu readme suggested su-exec specifically for alpine users since they do the same thing. See Install.md from gosu repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I wonder if its recommended purely from size POV or technical reasons - if its pure size, I would probably prefer the native gosu to keep the number of moving parts (and attack/bug vectors) low.

I know alpine users generally are obsessed with shaving bytes off images, but in this case the trade-off doesn't seem favourable in that regard to me.

If there are stability/technical reasons for it, thats of course a different story entirely :)

(again, not a maintainer, just an active user of Atlantis voicing an opinion)

@RulerOf
Copy link
Author

RulerOf commented May 16, 2024

Could probably circumvent this by introducing a root version of the image with a new Docker tag as opt-in, but the complexity and overhead for that is not zero, and might not be worth the squeeze.

Was thinking about this some more. If there's an appetite amongst the maintainers for a separate image tag, my needs would be well-suited by something like a common-tools tag that bundles oft-used third party packages (such as Python or jq) directly into the image.

We could survey the terraform provider module landscape and just determine what, say, the 500 most-popular modules use in their local-exec provisioners and suggest them for inclusion in such an image.

What do you think @jippi ?

@jamengual
Copy link
Contributor

Could probably circumvent this by introducing a root version of the image with a new Docker tag as opt-in, but the complexity and overhead for that is not zero, and might not be worth the squeeze.

Was thinking about this some more. If there's an appetite amongst the maintainers for a separate image tag, my needs would be well-suited by something like a common-tools tag that bundles oft-used third party packages (such as Python or jq) directly into the image.

We could survey the terraform provider module landscape and just determine what, say, the 500 most-popular modules use in their local-exec provisioners and suggest them for inclusion in such an image.

What do you think @jippi ?

This is a difficult one, and I will say I will be opposed since it will require more maintenance for us and be a big security issue for most.

I would like to know more about what you are trying to install that requires root and can't be compiled/run using an alternative user.

@jamengual
Copy link
Contributor

Another thing will be to add sudo to the image for this use case but that will need to be discussed.

@jamengual jamengual added needs discussion Large change that needs review from community/maintainers waiting-on-review Waiting for a review from a maintainer labels May 16, 2024
@jamengual
Copy link
Contributor

Another thing will be to add sudo to the image for this use case but that will need to be discussed.

sudo Is not a viable option.

I was checking to see if you can install Python on the user folder, and it is possible using the --user option, so I think exploring that path could show some results or otherwise create your own custom image.

@jippi
Copy link
Contributor

jippi commented May 17, 2024

Another thing will be to add sudo to the image for this use case but that will need to be discussed.

sudo Is not a viable option.

Totally agree; sudo + custom workflows + Repo Level atlantis.yaml in the baseline image could potentially be quite the challenge to manage effectively from a security point of view.

@jippi
Copy link
Contributor

jippi commented May 17, 2024

Could probably circumvent this by introducing a root version of the image with a new Docker tag as opt-in, but the complexity and overhead for that is not zero, and might not be worth the squeeze.

Was thinking about this some more. If there's an appetite amongst the maintainers for a separate image tag, my needs would be well-suited by something like a common-tools tag that bundles oft-used third party packages (such as Python or jq) directly into the image.
We could survey the terraform provider module landscape and just determine what, say, the 500 most-popular modules use in their local-exec provisioners and suggest them for inclusion in such an image.
What do you think @jippi ?

This is a difficult one, and I will say I will be opposed since it will require more maintenance for us and be a big security issue for most.

I would like to know more about what you are trying to install that requires root and can't be compiled/run using an alternative user.

It does sound like a pretty cool "sidecar" project to Atlantis for the broader community to build out; but doesn't align well with the conservative (and security-first) approach a tool like Atlantis should always take; and as mentioned, would be significant burden on an already very busy maintainer team :)

@@ -59,6 +59,7 @@ RUN apt-get update && \
unzip \
openssh-server \
dumb-init \
gosu \
Copy link
Member

Choose a reason for hiding this comment

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

We can avoid adding gosu by splitting up the directory run from the docker entry point.

@chenrui333
Copy link
Member

I dont think we should do such things (for whom might use this, they should build their own docker image). closing for now. Sorry about it.

@chenrui333 chenrui333 closed this May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Relating to how we build Atlantis needs discussion Large change that needs review from community/maintainers waiting-on-review Waiting for a review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scripts placed in /docker-entrypoint.d/ cannot perform useful work because root user access is unavailable
5 participants