Add Express-Gateway to the official images

[XVincentX]

Hello guys,

I'd love Express-Gateway, the project we're building, be part of the Docker Official Image program.

I'm quite sure we'll have a lot to work on, but this is a start.

Thanks a lot!

Checklist for Review

NOTE: This checklist is intended for the use of the Official Images maintainers both to track the status of your PR and to help inform you and others of where we're at. As such, please leave the "checking" of items to the repository maintainers. If there is a point below for which you would like to provide additional information or note completion, please do so by commenting on the PR. Thanks! (and thanks for staying patient with us ❤️)

• associated with or contacted upstream?
• does it fit into one of the common categories? ("service", "language stack", "base distribution")
• is it reasonably popular, or does it solve a particular use case well?
• does a documentation PR exist? (should be reviewed and merged at roughly the same time so that we don't have an empty image page on the Hub for very long)
  • Add express-gateway docs docs#1144
• dockerization review for best practices and cache gotchas/improvements (ala the official review guidelines)?
• 2+ dockerization review?
• existing official images have been considered as a base? (ie, if foobar needs Node.js, has FROM node:... instead of grabbing node via other means been considered?)
• if FROM scratch, tarballs only exist in a single commit within the associated history?
• passes current tests? any simple new tests that might be appropriate to add? (https://github.com/docker-library/official-images/tree/master/test)

[XVincentX]

@yosifkit Any chance you guys could have a look into this?

[XVincentX]

@tianon Sorry to ping you personally — I realized you have been merging some pull requests lately and I was wondering whether you could help me with this.

Is there anything else I should be doing here? Thanks!

[docker-library-bot]

Hello! ✨

Thanks for your interest in contributing to the official images program. 💭

As you may have noticed, we've usually got a pretty decently sized queue of new images (not to mention image updates and maintenance of images under @docker-library which are maintained by the core official images team). As such, it may be some time before we get to reviewing this image (image updates get priority both because users expect them and because reviewing new images is a more involved process than reviewing updates), so we apologize in advance! Please be patient with us (and avoid poking us about your image via other communication means -- rest assured, we've seen your PR and it's in the queue). ❤️

We do try to proactively add and update the "new image checklist" on each PR, so if you haven't looked at it yet, that's a good use of time while you wait. ☔

Thanks! 💖 💙 💚 ❤️

[tianon]

Thanks @docker-library-bot ❤️

Sorry for the delay!

I think my first concern here is around the location of the Dockerfile -- I don't think you really want to have the Dockerfile you submit to us to be tied into your main Express Gateway development repository, especially given that the Dockerfile as-is won't build properly via our tooling (there's no way to populate that ARG EG_VERSION value here), and we need some way to ensure the Dockerfile rebuilds properly (and always contains the same version when rebuilt). In addition, your development repository will likely have a lot more churn in it, so it's harder to separate commits touching things the Dockerfile uses from other normal development commits.

There's quite a bit of content in https://github.com/docker-library/official-images#review-guidelines that I think will be helpful (especially "repeatability"), but there are also little things that'll need updating like swapping CMD node -e "require('express-gateway')().run();" over to CMD ["node", "-e", "require('express-gateway')().run();"] (which ensures there's no extra /bin/sh process running).

[XVincentX]

@tianon

Hey, thanks a lot for the reply!

Good point about the separate repository. This was something I wanted to do since the beginning, but given we have some automation on the repository (we automatically build Docker Images and other stuff when we publish a new version) I wanted to make sure that

1. The image makes sense as an official one
2. You guys would review the thing.

…before split and change the whole automation system into two parts.

I'd be more than happy to make the change once we're all set with the review. Does that sound good to you?

but there are also little things that'll need updating like swapping CMD node -e "require('express-gateway')().run();" over to CMD ["node", "-e", "require('express-gateway')().run();"] (which ensures there's no extra /bin/sh process running).

Oh cool, I didn't know such thing. Why is that happening when writing the CMD as a string?

[XVincentX]

@tianon I've just merged a change that's changing the CMD statement. Is there anything else you want me to address before I start moving the Docker images in their own repository?

[tianon]

I'd be more than happy to make the change once we're all set with the review. Does that sound good to you?

Sure, fair enough -- will you still be maintaining the older versions, or will they be dropped? (It's up to you -- the guideline we usually recommend is that you should only maintain whatever current releases are actively maintained upstream unless you've got a good reason to deviate.)

---

Oh cool, I didn't know such thing. Why is that happening when writing the CMD as a string?

See https://docs.docker.com/engine/reference/builder/#cmd:

If you use the shell form of the CMD, then the <command> will execute in
/bin/sh -c:

FROM ubuntu
CMD echo "This is a test." | wc -

If you want to run your <command> without a shell then you must
express the command as a JSON array and give the full path to the executable.
This array form is the preferred format of CMD. Any additional parameters
must be individually expressed as strings in the array:

FROM ubuntu
CMD ["/usr/bin/wc","--help"]

---

Is there anything else you want me to address before I start moving the Docker images in their own repository?

You've got a couple other items being COPY'd into the image -- will those be copied over to the new repository? Are those not things that get included automatically when you install from Yarn? It would also be acceptable to fetch those via a tagged release (ala https://github.com/ExpressGateway/express-gateway/tree/v1.10.2/lib/config ...), but it is preferred for those to be fetched in some way that provides some sort of fetching verification (checksum, GPG signature, etc), if possible (which Yarn and friends usually provide, so it'd be really great if those bits came directly through Yarn, but understandable if they don't / that's not feasible / the defaults don't make sense in a Docker environment).

Just to be clear: is Express-Gateway something users would run as a standalone service? Or is it more similar to something like Django (and thus more likely to be listed in a project's requirements.txt and installed alongside the project code)? We used to have an image for Django and Celery, for example, and found that the maintenance burden of them far outweighed any possible benefit of the image (since one doesn't run Django directly, but rather uses it with an application listed in requirements.txt and downloaded/installed like any/all other dependencies), so the image was somewhere between hard to use and entirely useless. 😅

[XVincentX]

Is Express-Gateway something users would run as a standalone service?

Definitely. Express Gateway is an API Gateway proxying request and manipulating then. You download the image and it's up and running listening on a port.

Will those be copied over to the new repository? Are those not things that get included automatically when you install from Yarn?

I think these files are already included in the yarn package. That would just be matter of moving files around. I'll figure this out.

we usually recommend is that you should only maintain whatever current releases are actively maintained upstream

That makes sense. Perfect!

[XVincentX]

@tianon Are multi stage builds supported?

[TimWolla]

@tianon Are multi stage builds supported?

They are not. See #3383.

[XVincentX]

Thanks @TimWolla. I can live with that for now!

[XVincentX]

@tianon @TimWolla This should now be ready.

[XVincentX]

Actually I have got a followup question — is there any way to provide a redirect from the old image expressgateway/express-gateway to the new one that'll be published? Or is the "migration" something I would have to communicate around to all the users?

[XVincentX]

@tianon Any update on this?

[XVincentX]

@tianon @TimWolla @yosifkit

Guys — this has been here for a lot of time. Is there anything I can do to speed up the process?

[XVincentX]

Hey @tianon — thanks for stopping by again here

Did it turn out that these are not included already?

Yes! They've been included because of the changes you requested :)

Does it get modified at runtime?

Yes; we have an internal hot reload mechanism (https://www.express-gateway.io/docs/runtime/) that lets you modify the configuration (and the gateway would reload) AND an Admin API that'll change the Gateway's configuration which will be reflected on the files stored there also.

Is that intentional?

It is — the idea is that the Admin API should not be exposed on the external network by default. If you want to do that, you will need to proxy it using the gateway itself (so you can secure it) or make the change explicit. Does that make sense to you?

[XVincentX]

@tianon I pushed another commit pointing Express Gateway to 1.13.x and with the VOLUME position change you requested.

[tianon]

What does it do if the provided VOLUME is empty, like it will be if a user bind-mounts instead of making a new "Docker volume"? If the user lets Docker create a new volume, it'll get pre-filled with the content of the image at the volume location, which will be this default configuration you've provided, but in every other case it'll stay empty, which I imagine will probably make Express-Gateway fail to start?

[XVincentX]

Yes, it won't be able to start saying there are missing files @tianon

Any suggestions on how to handle this better?

[tianon]

If there's not a way to convince Express-Gateway itself to regenerate in the face of an empty directory (or for it to provide sane defaults that can then be modified and saved via the Admin API endpoints you referenced), then I guess the best/only option is probably something hacky like what the WordPress image does -- put the default configuration files elsewhere, during startup detect whether key files we expect exist, and if they don't, copy in the defaults.

It definitely seems weird that the API is allowed to mutate the configuration, but if that's how it's supposed to work, so be it!

[XVincentX]

@tianon Thanks for the response! I'm so glad we're interacting so quickly after all this time waiting!

If there's not a way to convince Express-Gateway itself to regenerate in the face of an empty directory (or for it to provide sane defaults that can then be modified and saved via the Admin API endpoints you referenced), then I guess the best/only option is probably something hacky like what the WordPress image does -- put the default configuration files elsewhere, during startup detect whether key files we expect exist, and if they don't, copy in the defaults.

I guess this could be done in a docker-entrypoint.sh file — I would copy the files somewhere, mount the volume — check the files existence and if not — copy the files in the volume to initialize it — is it correct?

Followup question on that — why is crashing not an option? Wouldn't be ok to say "yeah ok thanks for the volume but I need some files into?" Just out of curiosity.

It definitely seems weird that the API is allowed to mutate the configuration, but if that's how it's supposed to work, so be it!

I can understand the concern however we have a reason; I do not want to annoy you with the details :)

[XVincentX]

@tianon

I've added now a docker-entrypoint.sh with the required logic to check for file presence and put defaults in case there are not provided. Let me know if I've missed something else or not.

[XVincentX]

@tianon Ping

[XVincentX]

@tianon Ping

[tianon]

Ok, let's get back on this! 😅 ❤️ (Your patience is very much appreciated.)

The docker-entrypoint.sh was added, but does appear to have been removed by your version bumping bot in ExpressGateway/docker-express-gateway@b488683 (FYI).

The entrypoint (https://github.com/ExpressGateway/docker-express-gateway/blob/d836632e50e5a26a9fdb33fa384e253a0cd11fb2/alpine/docker-entrypoint.sh) doesn't currently meet our https://github.com/docker-library/official-images#consistency requirements -- here's what I'd recommend:

1. Adjust Dockerfile something like so:

   COPY docker-entrypoint.sh /usr/local/bin/
   ENTRYPOINT ["docker-entrypoint.sh"]
   CMD ["node", "-e", "require('express-gateway')().run();"]

2. Adjust the end of docker-entrypoint.sh something like so:

        ...
   fi
   
   exec "$@"

There's a similar approach in https://github.com/mongo-express/mongo-express-docker which might be useful as an example (although it uses a dummy mongo-express command to indicate that node app will be run so that the particulars of the exact node command that gets invoked don't have to be part of the Dockerfile's CMD instruction).

Also, you've currently got your default config content split between the files you COPY and the entrypoint.sh script (which will likely eventually get out of sync) -- it would probably be worthwhile to instead COPY them to somewhere outside the volume so that the script can simply cp them if they don't exist. What do you think?

To clarify what I meant by "included in the yarn package", I meant some of these:

$ find / -name gateway.config.yml -o -name system.config.yml
/usr/local/share/.config/yarn/global/node_modules/express-gateway/bin/generators/gateway/templates/getting-started/config/gateway.config.yml
/usr/local/share/.config/yarn/global/node_modules/express-gateway/bin/generators/gateway/templates/getting-started/config/system.config.yml
/usr/local/share/.config/yarn/global/node_modules/express-gateway/bin/generators/gateway/templates/basic/config/gateway.config.yml
/usr/local/share/.config/yarn/global/node_modules/express-gateway/bin/generators/gateway/templates/basic/config/system.config.yml
/usr/local/share/.config/yarn/global/node_modules/express-gateway/lib/config/gateway.config.yml
/usr/local/share/.config/yarn/global/node_modules/express-gateway/lib/config/system.config.yml
/usr/local/share/.cache/yarn/v2/npm-express-gateway-1.14.0-3948cb03ca51f0d629576af07fee2275eb4bcf8e/bin/generators/gateway/templates/getting-started/config/gateway.config.yml
/usr/local/share/.cache/yarn/v2/npm-express-gateway-1.14.0-3948cb03ca51f0d629576af07fee2275eb4bcf8e/bin/generators/gateway/templates/getting-started/config/system.config.yml
/usr/local/share/.cache/yarn/v2/npm-express-gateway-1.14.0-3948cb03ca51f0d629576af07fee2275eb4bcf8e/bin/generators/gateway/templates/basic/config/gateway.config.yml
/usr/local/share/.cache/yarn/v2/npm-express-gateway-1.14.0-3948cb03ca51f0d629576af07fee2275eb4bcf8e/bin/generators/gateway/templates/basic/config/system.config.yml
/usr/local/share/.cache/yarn/v2/npm-express-gateway-1.14.0-3948cb03ca51f0d629576af07fee2275eb4bcf8e/lib/config/gateway.config.yml
/usr/local/share/.cache/yarn/v2/npm-express-gateway-1.14.0-3948cb03ca51f0d629576af07fee2275eb4bcf8e/lib/config/system.config.yml

If any of these is suitable for the initial volume initialization, then you might be able to skip those COPY bits entirely and just cp these in the entrypoint instead, right? That gives you less duplication to maintain (so is likely worth considering).

(Those files in .cache are probably ripe for pruning to make the image a little smaller too -- maybe there's a way to ask Yarn not to cache them, or perhaps a yarn cache clean command of some kind to remove them properly?)

Other than that, I think this looks OK and we should look to get you across the finish line here shortly. 👍

[XVincentX]

@tianon

The docker-entrypoint.sh was added, but does appear to have been removed by your version bumping bot in ExpressGateway/docker-express-gateway@b488683 (FYI).

Correct. I've restored it and updated the template.

Adjust Dockerfile something like so

Done

Adjust the end of docker-entrypoint.sh something like so

Done

perhaps a yarn cache clean command…

That's a way. Or, in case the docker builds you guys do allow to set ´--shm-size´ option during the build phase, then I could use that as a cache directory so its content gets discarded automatically once the build is done.

Good point also about the gateway and system configuration file. I'll look into the alternatives to see if that makes sense or not.

[XVincentX]

@tianon Done! I should have addressed all your comments!

[tianon]

This is looking really good. 👍

I'm still confused about COPY ./config /var/lib/eg though -- the entrypoint will copy those from the Node module itself on startup, so should that instead be copying them from that same place in the Dockerfile instead of COPY as well (or rely on the entrypoint in all cases)?

(Reviewing docs PR now. 🤘)

[yosifkit]

There are also now config/models/* files that are dropped into the folder that becomes a volume and not addressed by the entrypoint.

Most of the time, relying on Docker's autocopy when using a docker volume is not great; in order to work for all users, you have to have a second copy of the files and replicate the behavior in the entrypoint for those doing a bind-mount. You could either cp them from the examples included with the npm install, or COPY them elsewhere, like /usr/local/etc/eg/, and cp from there to the volume on start-up. If there are quite a few files, perhaps something like rsync --ignore-existing would do what you need?

[XVincentX]

Yeah I forgot about the model files, I'll have a look into this later. Thanks for pointing it out @yosifkit

[XVincentX]

@yosifkit @tianon Updated the script to copy all the required files and removed the COPY statement.

[yosifkit]

I only have two minor comments that could be addressed in a later PR. There is no latest tag (aka default), so users will not be able to docker pull express-gateway. The output is noisy even when the command is echo or bash. (cc @tianon)

Diff:

diff --git a/_bashbrew-arches b/_bashbrew-arches
index e69de29..fe5f0f6 100644
--- a/_bashbrew-arches
+++ b/_bashbrew-arches
@@ -0,0 +1,5 @@
+express-gateway:1.14.0 @ amd64
+express-gateway:1.14.0 @ arm64v8
+express-gateway:1.14.0 @ i386
+express-gateway:1.14.0 @ ppc64le
+express-gateway:1.14.0 @ s390x
diff --git a/_bashbrew-list b/_bashbrew-list
index e69de29..41d2f39 100644
--- a/_bashbrew-list
+++ b/_bashbrew-list
@@ -0,0 +1,3 @@
+express-gateway:1.x
+express-gateway:1.14.x
+express-gateway:1.14.0
diff --git a/express-gateway_1.14.0/Dockerfile b/express-gateway_1.14.0/Dockerfile
new file mode 100644
index 0000000..8c1bf54
--- /dev/null
+++ b/express-gateway_1.14.0/Dockerfile
@@ -0,0 +1,21 @@
+FROM node:10-alpine
+
+LABEL maintainer Vincenzo Chianese, vincenzo@express-gateway.io
+
+ARG EG_VERSION=1.14.0
+
+RUN yarn global add express-gateway@$EG_VERSION && yarn cache clean
+
+ENV NODE_ENV production
+ENV NODE_PATH /usr/local/share/.config/yarn/global/node_modules/
+ENV EG_CONFIG_DIR /var/lib/eg
+
+ENV CHOKIDAR_USEPOLLING true
+
+VOLUME /var/lib/eg
+
+EXPOSE 8080 9876
+
+COPY docker-entrypoint.sh /usr/local/bin/
+ENTRYPOINT ["docker-entrypoint.sh"]
+CMD ["node", "-e", "require('express-gateway')().run();"]
diff --git a/express-gateway_1.14.0/docker-entrypoint.sh b/express-gateway_1.14.0/docker-entrypoint.sh
new file mode 100755
index 0000000..bc2f7b6
--- /dev/null
+++ b/express-gateway_1.14.0/docker-entrypoint.sh
@@ -0,0 +1,44 @@
+#!/bin/sh
+set -e
+
+if [ ! -e /var/lib/eg/gateway.config.yml ]; then
+      echo >&2 "gateway.config.yml file not found in $PWD - copying now..."
+
+      cp /usr/local/share/.config/yarn/global/node_modules/express-gateway/bin/generators/gateway/templates/basic/config/gateway.config.yml \
+         /var/lib/eg/gateway.config.yml
+fi
+
+if [ ! -e /var/lib/eg/models ]; then
+      mkdir /var/lib/eg/models
+fi
+
+if [ ! -e /var/lib/eg/system.config.yml ]; then
+      echo >&2 "system.config.yml file not found in $PWD - copying now..."
+
+      cp /usr/local/share/.config/yarn/global/node_modules/express-gateway/bin/generators/gateway/templates/basic/config/system.config.yml \
+         /var/lib/eg/system.config.yml
+fi
+
+if [ ! -e /var/lib/eg/users.json ]; then
+      echo >&2 "users.json file not found in $PWD - copying now..."
+
+      cp /usr/local/share/.config/yarn/global/node_modules/express-gateway/lib/config/models/users.json \
+         /var/lib/eg/models/users.json
+fi
+
+if [ ! -e /var/lib/eg/applications.json ]; then
+      echo >&2 "applications.json file not found in $PWD - copying now..."
+
+      cp /usr/local/share/.config/yarn/global/node_modules/express-gateway/lib/config/models/applications.json \
+         /var/lib/eg/models/applications.json
+fi
+
+if [ ! -e /var/lib/eg/credentials.json ]; then
+      echo >&2 "credentials.json file not found in $PWD - copying now..."
+
+      cp /usr/local/share/.config/yarn/global/node_modules/express-gateway/lib/config/models/credentials.json \
+         /var/lib/eg/models/credentials.json
+fi
+
+
+exec "$@"

Build test of #3980; 448b4b4; amd64 (express-gateway):

$ bashbrew build express-gateway:1.x
Building bashbrew/cache:f18bd66f26817536a90a575553e018d8eda1dd56e1d6d164cd42626d0e4c6ede (express-gateway:1.x)
Tagging express-gateway:1.x
Tagging express-gateway:1.14.x
Tagging express-gateway:1.14.0

$ test/run.sh express-gateway:1.x
testing express-gateway:1.x
	'utc' [1/4]...passed
	'cve-2014--shellshock' [2/4]...passed
	'no-hard-coded-passwords' [3/4]...passed
	'override-cmd' [4/4]...gateway.config.yml file not found in / - copying now...
system.config.yml file not found in / - copying now...
users.json file not found in / - copying now...
applications.json file not found in / - copying now...
credentials.json file not found in / - copying now...
passed

[XVincentX]

@tianon @yosifkit
I've added the latest tag. Shall I remove the credentials.json file not found in / - copying now... sentences here and there?

[yosifkit]

The output could be useful to users, so I would just skip them if the command isn't node or something.

# maybe like this?
if [ "$1" = 'node' ]; then 
# do copy stuff
fi
exec "$@"

[XVincentX]

Is it ok if I address this in a following PR?

[yosifkit]

Is it ok if I address this in a following PR?

Fine by me. It passes the tests we have 😄

[XVincentX]

This is so great! Finally it happened!