Update Dockerfile to not depend on custom base image

[sampaiodiego]

Proposed changes

Depending on "base image" adds an additional step every time we need to update a dependency on our Docker image so I'm changing the base image to a node-slim.

I have also updated to Debian Buster (previously using Stretch), the current stable version.

Some changes described below might be considered breaking, so please let me know if I need to change anything.

Issue(s)

RocketChat/Docker.Official.Image#112 (comment)

How to test or reproduce

Screenshots

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• Improvement (non-breaking change which improves a current function)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Hotfix (a major bugfix that has to be merged asap)
• Documentation Update (if none of the other choices apply)

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works (if applicable)
• I have added necessary documentation (if applicable)
• Any dependent changes have been merged and published in downstream modules

Changelog

Further comments

Some important changes included in this change:

• removed curl - just like on our official image, it's still used during node installation process but was removed from the final image
• removed rocket.chat certificate and gpg (gnupg dirmr) - apparently not being used
• removed imagemagick dependency - it was used before to resize uploaded images, but we're currently using sharp that doesn't depend on imagemagick

[Sing-Li]

Awesome. Also improves transparency of implementation for open source community. (Even though source to base is available, it requires effort to locate and is mostly mysterious.)

[geekgonecrazy]

I don’t see any of the removed items as a breaking change so long as functionality of rc doesn’t some how depend on them.

The only downside I guess really is the one liner to prep stuff is kinda long. Wonder if putting in a single script that is added early on and cached would be acceptable?

[geekgonecrazy]

Can we put all prep of the image in one layer, and then add /app and finally the npm install and cleanup?

That way we maximize cache?

[sampaiodiego]

Can we put all prep of the image in one layer, and then add /app and finally the npm install and cleanup?

That way we maximize cache?

you mean move other things to same layer as the nodejs installation?

I've done this way to keep it easy to update, so one layer for node and another one for rocket.chat stuff (user creation and dependencies).

although maximising cache is good, we don't have docker cache on github actions 😞

[geekgonecrazy]

This is technically valid and we could probably merge.

Just doesn’t feel like a great dockerfile. But I might be spoiled to dockerizing go binaries ;)

A thought/question?

Do we want to keep maintaining nodejs inside?

If I remember right originally we started off with the rocketchat/base image not because of nodejs. But actually because we wanted to pack our gpg keys and nodejs both in so when we fetched from the release service we could verify.

but since we are bringing the bundle in from the ci. That original purpose is gone.

On the official image we deviated from a nodejs because it vanished on us. Do we have that worry here or could we depend on it? For the mongo all in one I think we have no choice. But for our main image?

[sampaiodiego]

On the official image we deviated from a nodejs because it vanished on us. Do we have that worry here or could we depend on it? For the mongo all in one I think we have no choice. But for our main image?

right, I guess we can be based on official node here as we don't have same issues we have on our official image, the idea was to have similar Dockerfiles for both versions, but I agree we don't need all this noise here..

I'll make the changes and update the PR description.. =)

[geekgonecrazy]

There we go! That doesn’t feel so massive