cartesi / rollups-node

Reference implementation of the Cartesi Rollups Node
Apache License 2.0
20 stars 62 forks source link

Publish a "distroless" rollups-node container image #404

Open endersonmaia opened 2 months ago

endersonmaia commented 2 months ago

๐Ÿ“š Context

The current rollups-node container image is based on debian:bookworm-slim and has a final size of ~90MB compressed.

We could reduce its size, and adopt a "distroless" approach, to reduce the image size and its attack surface.

An optional container image without shell support is essential since a shell can easily expose credentials used to access blockchain providers and private key credentials.

โœ”๏ธ Solution

We could use Ubuntu's chisel and create a smaller image, only with the files necessary to run rollups-node and its dependencies.

For security, we should have two images, for ex.:

๐Ÿ“ˆ Subtasks

marcelstanley commented 2 months ago

@endersonmaia, first of all, thanks for opening this issue!

This looks like a valuable improvement, however, it may trigger other demands, which we would need to evaluate, not to mention we'd need to give it the proper prioritization in our backlog.

So, let's work together to make sure this request is well defined and all relevant details and known impacts are mapped before going the proper prioritization flow.

tuler commented 2 months ago

Please consider this is a security concern.

Also consider most of the work was already done by @endersonmaia in PRs #407 and #408, but understandably requires effort from the node team to review it.

Do you have specific concerns to clarify the impact concerns?

endersonmaia commented 2 months ago

To add more context, we have an application running with rollups-node:1.3.0 that won't be upgraded, and that's the reason why I added the PRs to a previous release.

About the impact, I think we'd need to add documentation about how to use the cartesi/rollups-claimer image together with a cartesi/rollups-node and redis configuration.

And also some improvements on the e2e tests, if there are any in-place for the 1.[3,4].x releases.

It's not up to me decide which of those issues are blockers.

I expect that the node team can create those (and others) issues, and Iยดd be glad to help with more details and even getting some of those issues to tackle myself.

@tuler we could already use the image built by the CI, if we need to move forward before this gets accepted before we need to do the migrations from cloud providers.

marcelstanley commented 2 months ago

Sorry for the delay, guys.

Please consider this is a security concern.

Sure, @tuler, we're aware of that.

@endersonmaia captured the overall picture above.

The thing is we cannot afford generating those images without the proper changes to the e2e tests and releasing the resulting patches, which would require proper planning and work to the detriment of the development of Node v2.

Of course, the proposed changes would impact the e2e tests for V2 as well.

May I suggest we take this offline so we can work on a plan together?

endersonmaia commented 2 months ago

For the record, I was experimenting with tini as a PID 1 solution but decided not to add this to the default image. See related PRs.

The reasons are that, with docker, you can already use run --init, within fly.io, they provide their own /init because they create a VM with the container image contents, and for Kubernetes, you can workaround with Pod's process namespace and such.

So, ideally, for a more generic container image, we shouldn't need tini.

endersonmaia commented 2 months ago

I've updated the body of the issue with our new agreement on how to proceed.

I'm gonna work on a cartesi/rollups-node:1.3.0-distroless (and 1.4.0-distroless) release.

endersonmaia commented 1 month ago

As it was discussed on another channel, we decided to give up on this for the 1.x release, and assess this on 2.x releases.

I suggest we discuss the scope of this and write it down in the body of the issue.

We could keep this issue, or create a new one, it's up to you @cartesi/node-unit