Skip to content

fix: Use node image instead of curl download#256

Closed
MCarlomagno wants to merge 6 commits intomainfrom
use-node-image
Closed

fix: Use node image instead of curl download#256
MCarlomagno wants to merge 6 commits intomainfrom
use-node-image

Conversation

@MCarlomagno
Copy link
Member

Summary

Installs nodejs through the chainguards node image instead of using curl download for installation

Testing Process

Run

cargo build
docker compose build
docker compose up -d

Go to docker terminal and run

pnpm --version

Checklist

  • Add a reference to related issues in the PR description.
  • Add unit tests if applicable.

Comment on lines 35 to 39

# Copy plugins folder and install dependencies
COPY ./plugins /app/plugins
RUN chown -R nonroot:nonroot /app/plugins
WORKDIR /app/plugins
RUN pnpm install --frozen-lockfile
Copy link
Contributor

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

Copy link
Contributor

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).

Copy link
Contributor

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 to node:22-slim for the final image. Since we won't need ts-node, typescript, or pnpm at runtime, this would reduce the final image size to ~51MB with a pinned version.

While the Wolfi base is only ~6.5MB, node:latest-dev is ~262MB, so ~51MB seems reasonable.

Copy link
Contributor

@LuisUrrutia LuisUrrutia Jun 4, 2025

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

Copy link
Contributor

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 to node:22-slim for the final image. Since we won't need ts-node, typescript, or pnpm at runtime, this would reduce the final image size to ~51MB with a pinned version.

While the Wolfi base is only ~6.5MB, node:latest-dev is ~262MB, so ~51MB seems reasonable.

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 latest tag), 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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 latest tag), so support efforts will be something to take into consideration.

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.

Copy link
Contributor

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

100% agree this is a problem, One of the reason why i thought sidecar is a better option for plugins

Copy link
Contributor

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

100% agree this is a problem, One of the reason why i thought sidecar is a better option for plugins

Sidecar of shared volume - in monitor we support python, bash and TS scripts and it works perfectly fine (shared volume).

Copy link
Contributor

@d-carmo d-carmo Jun 5, 2025

Choose a reason for hiding this comment

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

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 latest tag), so support efforts will be something to take into consideration.

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.

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.

Comment on lines +29 to 35
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
WORKDIR /app/plugins
RUN pnpm install --frozen-lockfile
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added USER nobody at the end, I think node stage does not include "nonroot" user

FROM --platform=${BUILDPLATFORM} cgr.dev/chainguard/glibc-dynamic:latest

# Node image
FROM cgr.dev/chainguard/node:latest-dev AS node
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use latest instead. Maybe both in dev and prod images.

Copy link
Contributor

Choose a reason for hiding this comment

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

The advantage of the -dev versions is that they come with some extra tools or debugging modes enabled, making it easier to troubleshoot. The downside is that, if run in production, the attack surface will be a bit bigger (and also the final image size).


# Node image
FROM cgr.dev/chainguard/node:latest-dev AS node

Copy link
Contributor

Choose a reason for hiding this comment

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

We could also add:

ENV NODE_ENV=production

RUN npm install -g pnpm ts-node typescript

# Copy plugins folder and install dependencies
COPY ./plugins /app/plugins
Copy link
Contributor

Choose a reason for hiding this comment

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

It should probably be: COPY --chown=nonroot:nonroot ./plugins /app/plugins so we are sure the user is correct.

Copy link
Member Author

Choose a reason for hiding this comment

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

Using --chown=nobody as nonroot doesnt exist at that stage

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked the passwd file and doesnt include any "nonroot" user

Screenshot 2025-06-04 at 10 27 32

d-carmo
d-carmo previously requested changes Jun 4, 2025
Copy link
Contributor

@d-carmo d-carmo left a comment

Choose a reason for hiding this comment

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

User is nonroot for chainguard images.

Comment on lines 17 to 18
# Setting up build directories
FROM --platform=${BUILDPLATFORM} cgr.dev/chainguard/glibc-dynamic:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this, looks like it's not being used

@MCarlomagno MCarlomagno requested review from a team as code owners June 5, 2025 00:34
Copy link
Contributor

@tirumerla tirumerla left a comment

Choose a reason for hiding this comment

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

@d-carmo we can merge this in and see how it goes for couple versions and make improvements parallely.

@d-carmo
Copy link
Contributor

d-carmo commented Jun 5, 2025

@d-carmo we can merge this in and see how it goes for couple versions and make improvements parallely.

There's nothing as permanent as a temporary change. I'll be opening a PR in a bit. It is almost done.

@son-oz can you also add your review here, please?

@d-carmo
Copy link
Contributor

d-carmo commented Jun 5, 2025

@d-carmo we can merge this in and see how it goes for couple versions and make improvements parallely.

There's nothing as permanent as a temporary change. I'll be opening a PR in a bit. It is almost done.

@son-oz can you also add your review here, please?

@MCarlomagno @tirumerla this is what I had in mind.

We can pin down the node version (I left the patch version out of the pinning - it will ensure it always installs the latest patch level) and makes it easier to add support for other stuff in the future (e.g: python, perl, java, bash, ...) with no need to add layers of images.

@d-carmo d-carmo dismissed their stale review June 5, 2025 14:14

No longer relevant

@MCarlomagno
Copy link
Member Author

Closing in favor of #266

@MCarlomagno MCarlomagno closed this Jun 6, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jun 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants