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

chore: use non-root user for alpine images #34162

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open

chore: use non-root user for alpine images #34162

wants to merge 15 commits into from

Conversation

debdutdeb
Copy link
Member

@debdutdeb debdutdeb commented Dec 11, 2024

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

VLN-82

Copy link
Contributor

dionisio-bot bot commented Dec 11, 2024

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented Dec 11, 2024

⚠️ No Changeset found

Latest commit: 7ec4096

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Dec 11, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-34162/
on branch gh-pages at 2024-12-27 14:32 UTC

Copy link

codecov bot commented Dec 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.85%. Comparing base (1184b83) to head (7ec4096).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop   #34162       +/-   ##
============================================
+ Coverage    59.37%   75.85%   +16.47%     
============================================
  Files         2819      514     -2305     
  Lines        67691    22353    -45338     
  Branches     15036     5426     -9610     
============================================
- Hits         40192    16955    -23237     
+ Misses       24682     4744    -19938     
+ Partials      2817      654     -2163     
Flag Coverage Δ
e2e ?
e2e-api ?
unit 75.85% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@debdutdeb debdutdeb changed the title chore: use non-root nobody user for alpine images chore: use non-root user for alpine images Dec 23, 2024
Comment on lines 7 to 18
# `nogroup` group is historically reserved for NFS.
# We don't use any NFS related tools in this image.
# For the same reason of NFS using the gid, we can also use it as long as there are no conflicts in terms of running processes with the same egid (which is 1 in our case).
# While 65533 raw gid could be used, renaming nogroup to rocketchat here for maximum compatibility with older debian image.
# More info on nobody/nogroup - https://wiki.ubuntu.com/nobody
# Debian wiki - https://wiki.debian.org/SystemGroups
# """
# daemon: Some unprivileged daemons that need to write to files on disk run as daemon.daemon (e.g., portmap, atd, probably others).
# Daemons that don't need to own any files can run as nobody.nogroup instead,
# and more complex or security conscious daemons run as dedicated users.
# The daemon user is also handy for locally installed daemons.
# """
Copy link
Member Author

Choose a reason for hiding this comment

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

may seem like an over-explanation, open to removing.

groupmod -n rocketchat nogroup && \
useradd -u 65533 -r -g rocketchat rocketchat && \
apk del deps && \
chown -R rocketchat:rocketchat /app
Copy link
Member Author

Choose a reason for hiding this comment

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

we could add --chown to all COPYs, however, it's not really that many files, one could argue that with COPY --chown it will build faster, but that number hasn't been noticeable. this way, we dont have to do many USER switching shenanigans.

@RocketChat RocketChat locked and limited conversation to collaborators Dec 23, 2024
@debdutdeb debdutdeb marked this pull request as ready for review December 23, 2024 12:28
@RocketChat RocketChat unlocked this conversation Dec 23, 2024
@sampaiodiego
Copy link
Member

because you moved the apk del to another layer the image size increased in 200MB, crossing the 2GB mark.. not sure if we want to do something about this though. for reference:

$ docker images                                 
REPOSITORY                                           TAG                  IMAGE ID       CREATED         SIZE
ghcr.io/rocketchat/rocket.chat                       pr-34836.alpine      dd8d46039547   19 hours ago    1.81GB
ghcr.io/rocketchat/rocket.chat                       pr-34162.alpine      248d81b785c5   33 hours ago    2.07GB
ghcr.io/rocketchat/rocket.chat                       pr-34192.alpine      95935ad7c658   9 days ago      1.82GB

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.

2 participants