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

Add: Alpine image as option for build #12548

Merged
merged 14 commits into from
Jan 21, 2022
Merged

Conversation

geekgonecrazy
Copy link
Member

@geekgonecrazy geekgonecrazy commented Nov 6, 2018

Background

Running existing image through Clair shows several CVE's mostly in glibc. I tried upgrading the packages but very few went away and more were added. Tried switching rocketchat/base to debian:stretch... but most of the glibc CVE's persisted.

So ditched Debian all together and put it on an alpine base.

Doing this does require a couple of hacks to get sharp running.

  1. For some reason when meteor build happens it is installing and including sharp in bundle/programs/server/npm/node_modules it I guess pre-compiled so is not compatible with the libc libraries included in the alpine image.
  2. Installing in bundle/programs/server/npm does not work with out git, not sure what's going on.. but I didn't want more dependencies.. so installed bundle/programs/server and then moved it into place.
  3. Installing 0.21.0 instead of our 0.20.3 because it includes pre-built for alpine preventing the need to install libvips from alpine leading to a lot of other issues.
  4. Doing exact same procedure with grpc. I think sharp actually depends on it, so included it in the hack for sharp section 😉. But the one bundled is not compatible.
  5. Running npm rebuild bcrypt --build-from-source with out this any time it ran bcrypt it would segment fault.

For what its worth.. all of these are similar issues compiling on different cpu architectures also. Many i've wrestled with to get our snap compiling on arm

We might should reconsider bundling sharp and grpc inside and instead having them be installed with npm install in the installation process.

Summary

All of this being said.. goes from a nice list of CVE's down to none reported with clair

Its published to docker hub for testing as rocketchat/rocket.chat:0.71.1-1 currently if anyone wants to test

Todo

  • Add user rocketchat back

@rodrigok
Copy link
Member

rodrigok commented Nov 6, 2018

I'm afraid to approve it since I didn't test, but the code seems good 😄

@geekgonecrazy
Copy link
Member Author

I just added to the description, but its published at this tag: rocketchat/rocket.chat:0.71.1-1 If you want to test the result

@geekgonecrazy geekgonecrazy changed the title [WIP]Switch docker image to Alpine base Add: Alpine image as option for build Feb 14, 2020
@geekgonecrazy
Copy link
Member Author

@LuluGO I know you had to update the alpine image recently can you drop your changes as a commit here?

…arp and grpc version according to packages.json
LuluGO
LuluGO previously approved these changes Feb 18, 2020
Copy link
Contributor

@LuluGO LuluGO left a comment

Choose a reason for hiding this comment

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

I tested latest version of the Dockerfile alpine with the enterprise (2.4.5) and community (2.4.9) bundles

@sampaiodiego
Copy link
Member

oh, this needs an update =)

@ggazzo ggazzo added this to the 3.2.0 milestone Apr 14, 2020
@sampaiodiego sampaiodiego modified the milestones: 3.2.0, 4.0.0 Apr 20, 2020
@geekgonecrazy
Copy link
Member Author

geekgonecrazy commented Dec 2, 2021

@sampaiodiego @rodrigok updated :)

For testing I do:

cd .docker
curl -L https://releases.rocket.chat/latest/download -o rc.tar
tar xvf rc.tar
docker build -f Dockerfile.alpine -t rc-alpine .

If we end up replacing main.. we might have to tweak a few things and then of course rename

@sampaiodiego
Copy link
Member

that's great @geekgonecrazy .. I'll give it a try

Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

I was able to get it running with latest rocket.chat with those few changes..

I would if we should publish the bundle/tarball without sharp, so instead of removing it before build the Docker image we would do it before publishing.. wdyt? this would be a breaking change though, so we'd target the next major.

we also could start publishing the alpine version under its own tag so we can start dog fooding, wdyt?

.docker/Dockerfile.alpine Outdated Show resolved Hide resolved
.docker/Dockerfile.alpine Outdated Show resolved Hide resolved
.docker/Dockerfile.alpine Outdated Show resolved Hide resolved
@sampaiodiego
Copy link
Member

better wait for #24075

Co-authored-by: Diego Sampaio <chinello@gmail.com>
Comment on lines +12 to +16
# Start hack for sharp...
&& rm -rf npm/node_modules/sharp \
&& npm install sharp@0.29.3 \
&& mv node_modules/sharp npm/node_modules/sharp \
# End hack for sharp
Copy link
Member

@debdutdeb debdutdeb Jan 11, 2022

Choose a reason for hiding this comment

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

Suggested change
# Start hack for sharp...
&& rm -rf npm/node_modules/sharp \
&& npm install sharp@0.29.3 \
&& mv node_modules/sharp npm/node_modules/sharp \
# End hack for sharp
# Start hack for sharp...
&& npm install --no-save sharp@$(node -pe 'require("./npm/node_modules/sharp/package.json").version') \
&& mv node_modules/sharp npm/node_modules/sharp \
# End hack for sharp

Copy link
Member

Choose a reason for hiding this comment

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

this won't be effective anymore though if sharp doesn't get bundled anymore as @sampaiodiego said -

I would if we should publish the bundle/tarball without sharp

Copy link
Member

Choose a reason for hiding this comment

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

yep.. I realised my phrase was misspelled 😂 what I meant was that really would like to not ship sharp with the tarball, so this hack would not be required.. but the on current scenario this hack is required :(

sampaiodiego
sampaiodiego previously approved these changes Jan 20, 2022
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

I've added the changes to GitHub workflow file.. it will now create alpine variants for releases and develop image builds.. =)

Co-authored-by: Debdut Chakraborty <debdut.chakraborty@rocket.chat>
@geekgonecrazy geekgonecrazy merged commit fbe10e7 into develop Jan 21, 2022
@geekgonecrazy geekgonecrazy deleted the docker-alpine-base branch January 21, 2022 18:07
gabriellsh added a commit that referenced this pull request Jan 25, 2022
…age-template-2

* 'develop' of github.com:RocketChat/Rocket.Chat: (81 commits)
  Language update from LingoHub 🤖 (#24268)
  Regression: Fix incompatibility of apps http requests (#24276)
  [IMPROVE] lib/Statistics improved and metrics collector (#24177)
  [FIX] Fixing the changing custom status behavior (#24218)
  Regression: Align Omni-Source icon sizes with designs (#24269)
  Regression: Fix Inactive Departments still visible on Livechat (#24267)
  [FIX] Solved Report Message Blank (#24262)
  [FIX] Errors on advanced sync prevent LDAP users from logging in (#23958)
  Chore: Convert model LoginServiceConfiguration to raw (#24187)
  [FIX] Make canned responses popup dependent on Canned_responses_enabled setting (#23804)
  [FIX] Wrong german translation for 2FA-Promt (#24126)
  Bump follow-redirects from 1.14.5 to 1.14.7 in /ee/server/services (#24182)
  Chore: Update pino and pino-pretty (#24242)
  [FIX] Avoid updating all rooms with visitor abandonment queries (#24252)
  Add: Alpine image as option for build (#12548)
  Fixed broken links in setup wizard (#24248)
  [FIX] Apps Contextual Bar not carrying title and room information   (#24241)
  Chore: Bump fuselage hooks (#24233)
  Regression: Remove extra call to `useOutsideClick` hook not following the function signature (#24243)
  [FIX] Change canned response model index to match other definition (#24235)
  ...
@sampaiodiego sampaiodiego mentioned this pull request Jan 29, 2022
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.

None yet

7 participants