Refactor Dockerfile to reduce image size

[drawks]

• resolves #1274
• refactors Dockerfile to optimize for size
  • converts to use a two stage build with only the minimum in the final stage
  • applies yarn-audit-fix during build stage setup to automatically upgrade to latest patch version of dependencies
  • converts final build step to use the --production flag to prevent development dependencies from being present in final image

In addition to reducing the overall size of the container (reduced from over 1gb to under 200MB) this change results in container with far fewer known vulnerable node module.

before   latest                 2ab391c36636   23 minutes ago       1.17GB
after      latest                 91697973ff7b   About a minute ago   182MB

[orta]

Yeah, this is good - I expect this might end up dropping TypeScript support for folks - any chance that dependency can be added manually to the build process. Then I'm good to merge

[airtonix]

Yeah, this is good - I expect this might end up dropping TypeScript support for folks - any chance that dependency can be added manually to the build process. Then I'm good to merge

Apart from having a Dockerfile that demonstrates how to build it into someone elses own image, I'm still not even convinced of the benefit of have a dangerjs docker image.

Most things will require people to bring in other packages anyway, the most obvious is Typescript. Which then illustrates where this becomes a slippery slope: how many packages will you start including in the docker image?

[orta]

I'm not here to debate whether it makes sense to make a Danger dockerfile, enough people have asked and eventually added it themselves. I don't think it makes sense, but that doesn't mean folks in esoteric build/CI systems have the same constraints as me.

Having this PR break someones docker setup because you can't run a dangerfile.ts isn't a slippery slope argument, for me that's the baseline with respect for danger to run in a project. Which I think is pretty reasonable as a constraint.

[ivankatliarchuk]

There is definitely a benefit. Upstream official docker image always nice to have. Everyone could beald an image FROM an official one.

probably worth to add .dockerignore too

node_modules/
.git/
env/
.github/
.vscode/

[drawks]

Yeah, this is good - I expect this might end up dropping TypeScript support for folks - any chance that dependency can be added manually to the build process. Then I'm good to merge

As I mentioned above i'm not really a node/js dev, if it is expected that Typescript is always needed alongside danger would it make sense to add it to packages.json so it is always pulled as a dependency?

edit: I've added a commit that does this. I think the PR as it stands addresses all the issues discussed in reviews up to this point.

[drawks]

Perhaps it can be added inside the dockerfile?

Gotcha, I can do that

[orta]

Cool, this seems reasonable to me 👍🏻

[glensc]

@drawks your git rebase totally messed up, some dependabot updates pulled in:

• https://github.com/danger/danger-js/pull/1275/commits

@orta why you have merged these? pull request doesn't even mention updating dependencies. should have been separate pr

[orta]

I read the lockfile changes, they were harmless and only affected this repo

[glensc]

I wonder where the dependabot changes came. from it's not enabled in https://github.com/danger/danger-js repo... at least no open pr's here.

[drawks]

That was my fault, I switched computers on my last update and accidentally rebased on my own fork which has dependabot enabled.

Apologies.