This adds a simple / functional nodejs_image rule. #254
Conversation
|
@alexeagle FYI Right now this will create an image with 3 additional layers:
|
There was a problem hiding this comment.
/cc @gregmagolan we should try this out when we get to protractor testing
|
|
||
| **It is notable that unlike the other image rules, `nodejs_image` is not | ||
| currently using the `gcr.io/distroless/nodejs` image for a handful of reasons.** | ||
| This is a switch we plan to make, when we can manage it. We are currently |
There was a problem hiding this comment.
I need to remove bash dependencies for a lighter windows install anyway, so we should follow up on this
There was a problem hiding this comment.
Is it possible to make the entrypoint itself Node.js?
If we can reduce the entrypoint logic to the point where I can just invoke Node directly (possibly bypassing a script that does that same) I'm happy to do that (I do that with Java).
|
|
||
| # Install your declared Node.js dependencies | ||
| npm_install( | ||
| name = "npm_deps", |
There was a problem hiding this comment.
is this the canonical name users should always pick?
There was a problem hiding this comment.
Not necessarily, I'm just not very creative I guess :)
README.md
Outdated
|
|
||
| # Download base images, etc. | ||
| load( | ||
| "//nodejs:image.bzl", |
There was a problem hiding this comment.
looks wrong, should have an external workspace reference if this is for a users WORKSPACE`?
| # other repositories. | ||
| def _external_dir(ctx): | ||
| # For @foo//bar/baz:blah this would translate to | ||
| # /app/bar/baz/blah.runfiles/foo/external |
There was a problem hiding this comment.
foo/external -> external/foo right?
by the way did you see bazelbuild/bazel#1262
There was a problem hiding this comment.
Depends if @foo is ourself or not, which I'll admit wasn't super clear to me when I first lifted this from one of these other functions (_reference_dir IIRC). From @foo a @bar reference would be: ...runfiles/foo/external/bar == ...runfiles/bar.
nodejs/image.bzl
Outdated
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| """A rule for creating a Rust container image. |
|
|
||
| # !!!! THIS IS A GENERATED FILE TO NOT EDIT IT BY HAND !!!! | ||
| # | ||
| # To regenerate this file, run ./update_deps.sh from the root of the |
| implementation = _dep_layer_impl, | ||
| ) | ||
|
|
||
| load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary") |
There was a problem hiding this comment.
note, that is a macro (mostly to attach the ibazel tag for fast dev roundtrip)
so it's possible you want to reach into //internal:node.bzl instead
| # Put the Node binary into its own layer. | ||
| "@nodejs//:bin/node", | ||
| # node_modules can get large, it should be in its own layer. | ||
| node_modules, |
There was a problem hiding this comment.
if you're concerned about size, you could try filtering out stuff.
This isn't the right spot to do it - but where the node_modules filegroup was declared, it's nice to drop things like test data that npm authors lazily published along with their package
There was a problem hiding this comment.
Yes, I think we want this in rules_nodejs :)
There was a problem hiding this comment.
I filter /test/* now, hopefully that helps and doesn't break things
mattmoor
force-pushed
the
node_image
branch
2 times, most recently
from
1a1debf to
aa7a0b5
Compare
|
Twiddling for CI |
mattmoor
changed the title
|
@dlorenc this should be RFAL (assuming CI is happy). |
For now these images are based on gcr.io/google-appengine/base instead of gcr.io/distroless/nodejs for two reasons: 1. The nodejs_binary runfiles includes the node binary, so we need not use a Node.JS base runtime. 1. The generated nodejs_binary entrypoint is currently a bash script, so if/until we can move this logic into our own entrypoint we need a more substantial base runtime.
For now these images are based on gcr.io/google-appengine/base instead of gcr.io/distroless/nodejs for two reasons: