Run containers as a non-root user #2053
Conversation
There was a problem hiding this comment.
Overall looks great to me! Thanks for picking this up. Will approve once you've addressed the questions/concerned listed 😄
@iskhakov do you mind giving this a second set of 👀 ?
Co-authored-by: Joey Orlando <joseph.t.orlando@gmail.com>
engine/Dockerfile
Outdated
|
|
||
| # Create a group and user to run an app | ||
| ENV APP_USER=appuser | ||
| RUN groupadd --system --gid 1000 ${APP_USER} && \ |
There was a problem hiding this comment.
can we keep gid=2000 to make sure it is backwards-compatible?
There was a problem hiding this comment.
Yes, we can change to gid=2000.
I think it's a common practice to set the same uid and gid for a custom user. I suppose that nothing would break, as process that starts application have the same user permissions. Something can be broken, if another process with some other uid and in the same group wants to use files with group permissions. But I suppose it's not applicable to dockerized application.
I'll change to gid=2000 if you prefer that.
There was a problem hiding this comment.
Hi @iskhakov @joeyorlando ! Change to gid=2000 or not ?
There was a problem hiding this comment.
I agree w/ @iskhakov , lets keep it to 2000 just to be safe. After that, I think we're safe to merge 😄
| RUN chown -R 1000:2000 /tmp/prometheus_django_metrics | ||
| ENV prometheus_multiproc_dir "/tmp/prometheus_django_metrics" | ||
| # Change to a non-root user (number is required by Kubernetes runAsNonRoot check) | ||
| USER 1000 |
There was a problem hiding this comment.
@alexintech actually one thought came to mind. If we move USER 1000 higher up to line 13, I think we may not need any of the chown commands?
There was a problem hiding this comment.
If we move USER 1000 higher up to line 13
That will break RUN apt-get install -y sqlite3 default-mysql-client postgresql-client here in dev target as it requires root.
There was a problem hiding this comment.
that makes sense 👍
I opened a PR to move USER to the end of the dev target so that it is present if a different build target is specified to docker build
What this PR does
Create a custom non-root user and use it to start an app. So uwsgi does not require to use
setUidandsetGidsystem calls.It handles errors while starting in Kubernetes with
runAsNonRoot: truecheck.Which issue(s) this PR fixes
closes #445
Checklist
pr:no public docsPR label added if not required)CHANGELOG.mdupdated (orpr:no changelogPR label added if not required)