Skip to content

fix: use higher uid/gid in docker image #3645

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

olevski
Copy link
Member

@olevski olevski commented Apr 14, 2025

This makes it so that the ui images can run with arbitrary uid/gid.

Also the nginx image is already setup to run with a bunch of things being in /tmp. The image has this in its /etc/nginx.conf file:

$ cat /etc/nginx/nginx.conf

worker_processes  auto;

error_log  /var/log/nginx/error.log notice;
pid        /tmp/nginx.pid;


events {
    worker_connections  1024;
}


http {
    proxy_temp_path /tmp/proxy_temp;
    client_body_temp_path /tmp/client_temp;
    fastcgi_temp_path /tmp/fastcgi_temp;
    uwsgi_temp_path /tmp/uwsgi_temp;
    scgi_temp_path /tmp/scgi_temp;

This is in line with the docs about running a non-privileged image here: (see running as non root)
https://hub.docker.com/_/nginx

To test locally:

cd client
docker build -t test .
docker run -ti --rm -p 8080:8080 -u 100000:100000 test

/deploy renku=release-0.68.0

@RenkuBot
Copy link
Contributor

You can access the deployment of this PR at https://renku-ci-ui-3645.dev.renku.ch

Copy link
Member

@leafty leafty left a comment

Choose a reason for hiding this comment

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

General comment: before the nginx process starts, everything inside the html and config folders should be made read-only. There is no reason to let files be modified since they are all static at that point.

Comment on lines +30 to +31
COPY --from=builder --chown=$UID:$GID --chmod=a+rwx /app/build /tmp/nginx/html
COPY --from=builder --chown=$UID:$GID --chmod=a+rwx /app/storybook-static /tmp/nginx/html/storybook
Copy link
Member

Choose a reason for hiding this comment

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

I don't think a+w is acceptable, use rwxr-xr-x please.

COPY --from=builder --chown=$UID:$GID --chmod=a+rwx /app/build /tmp/nginx/html
COPY --from=builder --chown=$UID:$GID --chmod=a+rwx /app/storybook-static /tmp/nginx/html/storybook
COPY docker-entrypoint.sh /tmp/docker-entrypoint.sh
COPY scripts/generate_sitemap.sh /tmp/scripts/generate_sitemap.sh
Copy link
Member

Choose a reason for hiding this comment

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

Why not keep it in /app/scripts/generate_sitemap.sh?

touch /tmp/nginx/html/sitemap.xml && \
touch /tmp/nginx/html/privacy-statement.md && \
touch /tmp/nginx/html/terms-of-use.md && \
chmod a+rw -R /tmp/nginx/html
Copy link
Member

Choose a reason for hiding this comment

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

Same here, rw-r--r-- permissions.

@@ -16,7 +16,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

export NGINX_PATH=/usr/share/nginx/html
export NGINX_PATH=/tmp/nginx/html
Copy link
Member

Choose a reason for hiding this comment

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

Why not chown /usr/share/nginx/html?

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.

3 participants