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

Merged
merged 17 commits into from
Jan 20, 2025
Merged

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

merged 17 commits into from
Jan 20, 2025

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: 736902d

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 2025-01-08 08:52 UTC

Copy link

codecov bot commented Dec 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.99%. Comparing base (7839bf0) to head (736902d).
Report is 24 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop   #34162       +/-   ##
============================================
+ Coverage    59.14%   74.99%   +15.85%     
============================================
  Files         2821      516     -2305     
  Lines        67981    22746    -45235     
  Branches     15148     5518     -9630     
============================================
- Hits         40208    17059    -23149     
+ Misses       24938     5023    -19915     
+ Partials      2835      664     -2171     
Flag Coverage Δ
e2e ?
e2e-api ?
unit 74.99% <ø> (ø)

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
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

@debdutdeb
Copy link
Member Author

@sampaiodiego while I can reduce the number of layers (ignore the last commit), after checking further seems rocketchat's built code with all deps increased in size. I.e..

debdut@pop-os:~/git/Rocket.Chat$ docker run --rm -ti -u0 --entrypoint du rocketchat-local -sh /app
2.1G    /app
debdut@pop-os:~/git/Rocket.Chat$ docker run --rm -ti -u0 --entrypoint du rocketchat/rocket.chat:7.1.0 -sh /app
1.8G    /app

This reverts commit 7ec4096.
@sampaiodiego sampaiodiego merged commit 28da43e into develop Jan 20, 2025
49 checks passed
@sampaiodiego sampaiodiego deleted the VLN-82 branch January 20, 2025 12:36
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