build: Mutli-stage build and slim image for websocket container

[Yann-J]

SUMMARY

This PR improved the Docker build for the websocket image, in particular by:

• Using a lighter base image (node:16-alpine)
• Using a multi-stage build to avoid dragging devDependencies in the PRD image
• Not running as root (will prevent from exposing port < 1024)
• Publish image to docker hub with the same tags and labels as the main image

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Brought down the image size from 380Mb to 42Mb - see the builds published on DockerHub from my fork:

TESTING INSTRUCTIONS

cd superset-websocket
docker build . -t superset-websocket
# start the redis broker
docker run -it -e PORT=8080 -e REDIS_HOST=redis -e REDIS_PORT=6379 -e REDIS_PORT=6379 -e REDIS_SSL=false -e JWT_TOKEN=xxxabczzzxxxabczzzxxxabczzzxxxabczzzxxxabczzzxxxabczzz superset-websocket

ADDITIONAL INFORMATION

• Has associated issue: includes and supersedes ci(websocket): publish websocket docker image to dockerhub #21379
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[Yann-J]

Hello @craig-rueda no sure if you're also coordinating the docker changes, but I think this one is ready for review (and should be the last piece of my recent round of Helm updates) and should be really safe.

Note that I've integrated the changes from #21379 as well to club the reviews.

The new slim image has been running in production for us for about a week now.

[benjaminrene]

Hello @Yann-J. Yes it was my two first contributions to this project. Same thing for my PR on #21379. Thanks for your contributions.

[benjaminrene]

Not sure this is required since you run npm ci and npm run build from build container. I think you should copy node_modules directory as well to the runtime container instead of running npm install

[Yann-J]

It is because one of the objectives is to avoid having the devDependencies in the production image to keep it slim

[craig-rueda]

Yea, not sure about this one either. npm install can make the builds non-deterministic as it can do patch version bumps

[Yann-J]

Hmm, it shouldn't because we're also committing package-lock.json (although I see it's outdated and not in sync with package.json? some people tend to think it shouldn't be committed, but that's a misconception) - but more importantly this has little to do with whether the build is single or multi-stage - versions will be resolved at build time in both cases.

Note that I didn't invent this... multi-stage builds with a prd-only npm install in the final image are a well established pattern for nodejs apps.

[craig-rueda]

npm ci leverages the contents of the .lock file to make a "clean" install (https://docs.npmjs.com/cli/v8/commands/npm-ci#description)

Agree that multi-stage builds are good :). I just think that we should be doing an npm ci there, or just copy node_modules from the base

[Yann-J]

Oh, I think I get it, you mean just use npm ci rather than npm install, but still ignoring devDependencies (which in fact seems to be the default when NODE_ENV=production)?

Yeah, that seems to make sense, as long as we agree on not dragging the devDependencies (which in the build image, would be installed by npm ci since the build image doesn't have NODE_ENV=production - which is a good thing since the build image obviously needs those). Those don't just make the image fatter, they also increase the security footprint / exposure of the image...

I've just pushed the change, and also re-synced up the lock file.