rules_docker icon indicating copy to clipboard operation
rules_docker copied to clipboard

Add node_modules linker to `nodejs_image`

Open codersasha opened this issue 3 years ago • 0 comments

PR Checklist

Please check if your PR fulfills the following requirements:

  • [ ] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • [X] Bugfix
  • [ ] Feature
  • [ ] Code style update (formatting, local variables)
  • [ ] Refactoring (no functional changes, no api changes)
  • [ ] Build related changes
  • [ ] CI related changes
  • [ ] Documentation content changes
  • [ ] Other... Please describe:

What is the current behavior?

Currently, nodejs images can't run on readonly filesystems. This is a blocking issue for using Bazel nodejs images in production workloads.

This is because nodejs binary rules run a "linker" script on startup (see the readme for more details - https://github.com/bazelbuild/rules_nodejs/tree/stable/internal/linker).

To skip this step, --nobazel_run_linker can be passed to templated_args in nodejs_image, e.g.:

load("@io_bazel_rules_docker//nodejs:image.bzl", "nodejs_image")

nodejs_image(
    name = "img",
    templated_args = [
        "--nobazel_run_linker",
    ],
)

However, this causes another issue - local dependencies (e.g. require("@myrepo/foo")) can't be resolved, since they aren't symlinked inside the node_modules folder as part of this linker step.

Issue Number: #2117

What is the new behavior?

To fix this, we effectively turn the .js linker script into a .bzl script, reading the manifest file contents and linking them. This allows a nodejs_image to be created that will run on a readonly filesystem. With this fix, the following image is readonly-fs compatible:

load("@io_bazel_rules_docker//nodejs:image.bzl", "nodejs_image")

nodejs_image(
    name = "img",
    data = [":js_lib"],
    entry_point = ":index.js",
    # These two parameters are needed until bazelbuild/rules_docker#2078 is fixed.
    include_node_repo_args = False,
    node_repository_name = "nodejs_linux_amd64",
    templated_args = [
        # This is needed to skip the linker, which creates the node_modules directory.
        "--nobazel_run_linker",
        # This is needed to skip writing over the node binary.
        "--nobazel_node_patches",
        # We need this so Bazel uses the node_modules resolver as though the linker ran (which it did, in
        # a previous step).
        "--nobazel_patch_module_resolver",
    ],
)

This creates a node_modules folder in the resulting image, in the same place its created when the binary runs - but instead, its created as part of the build process, rather than relying on the launcher.sh script to create it at runtime.

This will have no impact on images that run without the --nobazel_run_linker flag, since they will overwrite these files anyway. It's very likely that images currently being built with --nobazel_run_linker contain this bug but either:

a. Don't know they have this issue, since they don't import any local dependencies; or b. Have used their own custom hacks (such as a custom base image) to work around this issue.

So, either way, it's better for the files to exist there :) We could even update the nodejs flags to default to --nobazel_run_linker and --nobazel_patch_module_resolver, since the linker "step" of launcher.sh has now already run. Although, there's no harm in running them twice.

See the issue for more details.

Does this PR introduce a breaking change?

  • [ ] Yes
  • [X] No

Other information

Enabling nodejs images to run with readonly filesystems is crucial in allowing them to be used in production workloads. In most security scanning systems for kubernetes, the first critical risk issue raised is that apps are run with writeable filesystems. Without this fix, these built containers can't be deployed to these systems, rendering the nodejs built images effectively useless for large-scale systems.

codersasha avatar Jul 17 '22 06:07 codersasha