-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add support for OpenShift compatible NGINX Docker images #188
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
Conversation
Initial implementation of Dockerfiles to produce OpenShift compatible NGINX Docker images
|
|
What's different about OpenShift which makes the existing images incompatible? |
|
OpenShift (or more specifically the wrappers that OpenShift creates around Docker images) does not allow containers that run as root which results in the NGINX images failing at startup. These Dockerfiles have the necessary modifications to run NGINX as non-root. |
Implement support for OpenShift Dockerfiles
|
|
||
| EXPOSE 8080 8443 | ||
|
|
||
| USER 998 |
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.
Where does the magic number 998 come from? Is it guaranteed to be the same always? Why not to use "nginx" just like we add in rpm post-scripts?
|
|
||
| ADD nginx.repo /etc/yum.repos.d/nginx.repo | ||
|
|
||
| RUN curl -sO http://nginx.org/keys/nginx_signing.key && \ |
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.
Insecure. Please use constructions similar to https://github.com/nginxinc/docker-nginx/blob/master/mainline/stretch/Dockerfile#L11
| sed -i -e '/user/!b' -e '/nginx/!b' -e '/nginx/d' /etc/nginx/nginx.conf && \ | ||
| sed -i -e '/listen/!b' -e '/80;/!b' -e 's/80;/8080;/' /etc/nginx/conf.d/default.conf && \ | ||
| # modify perms for non-root runtime | ||
| chown -R 998 /var/cache/nginx /etc/nginx && \ |
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.
No, that's bad - the files in /etc/nginx will be writable by nginx user, which means in case of vulnerability attacker could rewrite the configurations.
| sed -i -e '/listen/!b' -e '/80;/!b' -e 's/80;/8080;/' /etc/nginx/conf.d/default.conf && \ | ||
| # modify perms for non-root runtime | ||
| chown -R 998 /var/cache/nginx /etc/nginx && \ | ||
| chmod -R g=u /var/cache/nginx /etc/nginx |
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.
Why?
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.
See https://docs.openshift.org/latest/creating_images/guidelines.html#openshift-origin-specific-guidelines -> Support Arbitrary User IDs
| # Change pid file location & remove nginx user & change port to 8080 | ||
| sed -i 's/\/var\/run\/nginx.pid/\/var\/cache\/nginx\/nginx.pid/g' /etc/nginx/nginx.conf && \ | ||
| sed -i -e '/user/!b' -e '/nginx/!b' -e '/nginx/d' /etc/nginx/nginx.conf && \ | ||
| sed -i -e '/listen/!b' -e '/80;/!b' -e 's/80;/8080;/' /etc/nginx/conf.d/default.conf && \ |
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.
This change should be documented as it's a big behaviour change from usual images.
|
For the official-images we don't really want images to create extra tags that are only meant for a specific vendor/cloud/ci platform. We support being able to run an image as non-root and many service images support running as any arbitrary user, like postgres, but we resist changes that are meant only for a specific environment like docker-library/docker#12. If the goal is to make it run as non-root, I was able to get it to run with minimal changes. This can then be run as any user On a new enough kernel (4.11+) you can even skip the port 80 swapping and run it with docker run -it --rm --user 1000:1000 \
--sysctl net.ipv4.ip_unprivileged_port_start=0 \
--mount type=volume,destination=/var/cache/nginx,volume-opt=type=tmpfs,volume-opt=device=tmpfs \
--mount type=volume,destination=/var/run,volume-opt=type=tmpfs,volume-opt=device=tmpfs \
nginx |
The current kernel employed by OpenShift is 4.9 LTS. I tested using --sysctl just in case but no luck.
On the other hand, this works and lets you run NGINX as non-root. However, to get this image running within OpenShift I still need to push it to Dockerhub.
That makes sense. Would shifting the focus from OpenShift to a non-root Docker image be OK then? A quick Google search reveals some interest in running NGINX as non-root in various environments. |
From the official-images team we would have no objections to this. We just wanted to make sure that an environment specific image doesn't pop up. It is up to the Nginx maintainer on whether to support running the image as an arbitrary user. On a side note you could look at automated builds (https://docs.docker.com/docker-hub/builds/) and repository links (https://docs.docker.com/docker-hub/builds/#repository-links) to have an up-to-date image on the Docker Hub using my example Dockerfile. |
|
Cool. I'll create another PR with a different approach 😄 |
Initial implementation of Dockerfiles to produce OpenShift compatible NGINX Docker images