-
Notifications
You must be signed in to change notification settings - Fork 56
fix: Use node image instead of curl download #256
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
Changes from 4 commits
cb86a78
08c143c
b1dd827
7057798
5b8c8df
6964070
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,28 +17,21 @@ RUN --mount=type=cache,target=/usr/local/cargo/registry \ | |
| # Setting up build directories | ||
| FROM --platform=${BUILDPLATFORM} cgr.dev/chainguard/glibc-dynamic:latest | ||
|
||
|
|
||
| # Node image | ||
| FROM cgr.dev/chainguard/node:latest AS node | ||
| ENV NODE_ENV=production | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could also add: ENV NODE_ENV=production |
||
| WORKDIR /app | ||
| COPY --from=base --chown=nonroot:nonroot /usr/app/bin/openzeppelin-relayer /app/openzeppelin-relayer | ||
| COPY --from=base /usr/lib/libssl.so.3 /usr/lib/libssl.so.3 | ||
| COPY --from=base /usr/lib/libcrypto.so.3 /usr/lib/libcrypto.so.3 | ||
|
|
||
| # Install plugin dependencies | ||
| ARG TARGETARCH | ||
| ENV NODE_VERSION=v20.19.2 | ||
|
|
||
| # Install Node.js | ||
| USER root | ||
| RUN apk add --no-cache curl && \ | ||
| curl -fsSL https://nodejs.org/download/release/${NODE_VERSION}/node-${NODE_VERSION}-linux-${TARGETARCH}.tar.xz \ | ||
| | tar -xJ --strip-components=1 -C /usr/local | ||
| ENV PATH="/usr/local/bin:$PATH" | ||
|
|
||
| # Install pnpm and ts-node | ||
| USER root | ||
| RUN npm install -g pnpm ts-node typescript | ||
|
|
||
| # Copy plugins folder and install dependencies | ||
| COPY ./plugins /app/plugins | ||
| RUN chown -R nonroot:nonroot /app/plugins | ||
| COPY --chown=nobody ./plugins /app/plugins | ||
| WORKDIR /app/plugins | ||
| RUN pnpm install --frozen-lockfile | ||
|
Comment on lines
+29
to
34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added |
||
|
|
||
|
|
@@ -51,5 +44,7 @@ ENV METRICS_PORT=8081 | |
|
|
||
| EXPOSE ${APP_PORT}/tcp ${METRICS_PORT}/tcp | ||
|
|
||
| USER nobody | ||
|
|
||
| # starting up | ||
| ENTRYPOINT ["/app/openzeppelin-relayer"] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After installing switch back to nonroot user that way the app will run as nonroot instead of root
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the node image will bring an image with unnecessary stuff in it. After the tests I did (just after the alpha release of monitors), the smallest working image was using the wolfi image (check here for monitors example).
In the example linked, just like we pinned python's version, it should also be possible to pin node's version (it may be simpler to pin the major one, as it should always have the latest patched and verified one in the wolfi repo).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for using the Node image is to support plugins. As far as I know, it's based on Wolfi with only the minimal software required to run. I haven't done the POC, but installing Node manually would likely result in a larger image.
One improvement we can make is adopting a two-step build process. We could use
node:latest-dev(which includes pnpm) to build the plugin components into CJS output, and then switch tonode:22-slimfor the final image. Since we won't needts-node,typescript, orpnpmat runtime, this would reduce the final image size to ~51MB with a pinned version.While the Wolfi base is only ~6.5MB,
node:latest-devis ~262MB, so ~51MB seems reasonable.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a quick comparison:
Wolfie Base: 134MB (without node)
Node latest: 185MB (doesn’t include pnpm, ts-node, typescript)
Manual Installation: 204MB (doesn’t include pnpm, ts-node, typescript)
This has been done in macOS with targeting Linux amd64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is making the process more complex than required. Later on, if we want to also allow plugins in another language (e.g: python) we will need to add one more image to the build step. Let's also keep in mind that the OSS (non-subscription) set of chainguard images do not allow to specify some versions (e.g: node will always be their
latesttag), so support efforts will be something to take into consideration.With the case of the base wolfi-base image, we can pin the version down (as long as it exists in their repos - they usually have the stable and the latest to pick from). what we need to be careful is to remove the apk toolset as a last step on the image building process, reducing further the fingerprint and the attack surface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something I discussed with @MCarlomagno. Marcos mentioned we could just add it to docs "for every release we will have latest node release" which i'm ok with. Yes things will break but it's the same as using chainguard latest images...we either have to pay them for images for specific tags or just live with knowing things will break. My opinion we should just use images other than chainguard and take care of security ourselves that way we pin to specific tags and digests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100% agree this is a problem, One of the reason why i thought sidecar is a better option for plugins
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sidecar of shared volume - in monitor we support python, bash and TS scripts and it works perfectly fine (shared volume).
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's something to take up with @son-oz . All in all, it won't be a big deal using the wolfi image and pinning the node version (it was done for monitors). As wolfi is the base image for chainguard, it will always have a few different major versions of node, allowing more flexibility. Going with a generic image and trimming it down/patching everything ourselves will be more hassle than what's worth in the mid/long term.
I'll try to get some time on the side to present a proposal for the image.