Tagging @leeevans to see if he can take a look at this to see how it might work with Broadsea.

@blootsvoets I'm supportive of adding a Dockerfile in this repo for use in deploying a standalone front end Atlas container. I expect you may also want to create a separate future pull request to add a Dockerfile in the WebAPI repo to deploy the backend as a separate container. Please work with @anthonysena to refresh this pull request with the latest Atlas repo code changes.

@leeevans thank you, I've rebased the code to the latest master to speed up the review process. Could you take another look please? I've made an accompanying PR for WebAPI: OHDSI/WebAPI#1384.

@blootsvoets - these changes look good from my side. Can you note any instructions here for deployment in docker using these assets? Maybe this is straightforward for experienced Docker users but I'd like to make sure that we note it for the user guide on the wiki.

Sure, can I suggest the following content of a wiki page:
https://gist.github.com/blootsvoets/7859a291436d852929ba3a32e5d18d8a

If needed, this wiki text could also be added to the OHDSI/WebAPI repo.

@blootsvoets thanks for making the requested changes. It would be helpful if you could add a docker-compose.yml file that just runs Atlas in it's own separate Docker container. I think we can then approve this pull request.

No problem, I've added a functional docker-compose in the docker/ folder and adapted the gist to describe this setup. I'm copying in @MaximMoinat in to further help this PR along. Note that it does not contain code for loading a vocabulary or a source.

@blootsvoets thanks for all of the quick updates to this PR. From my side this looks good but I'll look for @leeevans to give the approval here. I do need to think further about the PR you've contributed to WebAPI. I have yet to dig into the details you provided but hope to get some time for that soon.

Hi @blootsvoets I believe your intention with this PR was to build and launch an ATLAS frontend only docker container? Therefore the docker-compose file to add here would just show how that ATLAS UI only docker container would be launched.

FYI. There is already an existing OHDSI Docker based solution for launching Atlas/WebAPI which covers Windows/Mac/Linux OS and postgres/sqlserver/oracle DBMS: https://github.com/OHDSI/Broadsea. If you are interested in working on rearchitecting that solution I'd be happy to schedule a call to discuss your ideas along with @anthonysena

It’s just that I don’t see a lot of value in a docker-compose file for only the frontend: The browser will block requests due to CORS on some queries by default. By using a reverse proxy (traefik) it avoids all potential CORS issues. If the example only contains atlas, it may be more straightforward to just show the docker run command.

In the end, it would be nice if the docker image was uploaded using automatic build on docker hub. That way a repository like Broadsea can just reference existing docker images. I’ll be on leave the coming period, but @MaximMoinat could discuss it in more detail?

In any case, it looks like the wiki pages for setting up Atlas and WebAPI have some duplication, would it be helpful to have a single page explaining installation instructions for both?

I've updated the branch so it can cleanly merged (as well as the WebAPI counterpart OHDSI/WebAPI#1384). @leeevans could you please reconsider this PR?

This is still a great idea in general @blootsvoets. I would suggest some changes before this PR it is accepted, however:

1. The definitions for compositions of the Atlas and WebAPI services (Docker Compose for now, perhaps k8s later) should be left to the existing, separate repo, Broadsea as @leeevans suggested.
2. Accordingly, there should be no docker-compose YAML file in this repo (or in the WebAPI repo).
3. The documentation here should just address building and running the Atlas image (perhaps with a env file, optionally.)
4. Only placeholder references to WebAPI should be kept in the DockerFile.

@t-abdul-basser thank you for the detailed descriptions. I've removed the docker-compose files. On how to use it:

docker build -t ohdsi/atlas:<tag> .
docker run \
  -e WEBAPI_URL=<my external webapi URL> \
  -e ATLAS_HOSTNAME=<my external hostname> \
  -p "8080:80" \
  ohdsi/atlas:<tag>

It can then be accessed at http://localhost:8080/atlas/. To avoid CORS blocking requests to Atlas, run this behind a reverse proxy that hosts both Atlas and WebAPI behind the same hostname:port.

I can update the wiki with this description if this PR gets merged. I hope this addresses points 2-4. To address point 1, I can make a PR with Broadsea to add a docker-compose file.

Really looking forward to this. Something that might be addressed (or I could add a PR for this at a later point):

1. we should be able to avoid running the container as root by default by using the unprivileged nginx image and listening on port 8080 instead and running as a (numeric) non-root USER
2. The OCI Annotation Labels are a good way to trace back the source of an image

Using the unprivileged image is a good idea, I can change that. Indeed, when automated builds are set up, it would be good to add multiple OCI labels.

1. we should be able to avoid running the container as root by default by using the unprivileged nginx image and listening on port 8080 instead and running as a (numeric) non-root USER

Implemented in e009f03. I needed to change the startup and configuration scripts a bit to avoid permission issues.

I've also expanded gzip functionality so the webserver can statically serve Gzip-compressed files instead of needing to compress them during runtime.

This is a very high-quality dockerization. I look forward to seeing it merged.

One great aspect is that it is purely additive -- so users who are not using docker will essentially be unaffected by this PR. The users who ARE using docker will get a container image that closely follows or exceeds established best practices and widely-respected guidelines. To help evaluate this PR, I'll list the recommendations and how this container compares:

Use of Docker's official best practices

Evaluating this Dockerfile against the officially-published Dockerfile best practices:

✅ Create ephemeral containers

This Dockerfile has an entrypoint (inherited from nginxinc/nginx-unprivileged:1.19-alpine) that starts the main app very quickly after configuring it from the environment. It does not attempt to persist any state across container runs.

✅ Understand build context

This Dockerfile uses a multi-stage build, the first of which builds the node app and the second runs it. Therefore the first stage needs everything node needs to build the web app and the Dockerfile sits next to the package.json, right where it should be in this situation.

✅ Pipe Dockerfile through stdin

Perfectly possible of course, but not an applicable guideline

✅ Exclude with .dockerignore

This PR has a .dockerignore which is currently five lines including the critical node_modules and .git entries which reduce the build context size, prevent some potential security issues, and help ensure that the same container image gets built by different devs and CI/CD hosts

✅ Use multi-stage builds

This Dockerfile is multi-stage with node:12 first and nginxinc/nginx-unprivileged:1.19-alpine. This reduces the size of the resulting image keeping storage costs down for devs, CI/CD hosts, registries, and end users. Using a small hand-tuned final base image mitigates potential security issues, makes the container image easier to reason about, and keeps things fast.

✅ Don’t install unnecessary packages

This Dockerfile installs no additional OS-level deps in either stage (no apt nor apt-get in stage one, no apk in stage two).

App-level deps are installed installed in stage one, but they are pruned and only the production app code survives to the second stage.

✅ Decouple applications

The result of this Dockerfile is a single-purpose nginx container image serving a compiled JS web app. We're not running other things. Here are the docker top results for a container started by this image:

PID                 USER                TIME                COMMAND
4922                101                 0:00                nginx: master process nginx -g daemon off;
4980                101                 0:00                nginx: worker process
4981                101                 0:00                nginx: worker process

... just nginx and a couple preforked worker processes (as designed).

✅ Minimize the number of layers

This Dockerfile's RUN, COPY, and ADD instructions are optimized for docker's layer caching which shortens the development cycle and results in an extra layer or two BUT this is mitigated by the use of multiple stages.

I suspect that we can reduce the number of copies in the second stage by introducing complexity, which is not worth it IMHO.

✅ Sort multi-line arguments

Not applicable here. In this case there isn't a huge list of OS-level deps to install like many other Dockerfiles need. We're getting those from upstream base images.

There is a find command in the asset compression step that could be separated into multiple lines, but several of those arguments are order dependent.

If you wanted to be pedantic, you could make it:

RUN find . \
  -type f \
  "(" \
    -name "*.css" \
    -o -name "*.html" \
    -o -name "*.js" ! -name "config-local.js" \
    -o -name "*.json" \
    -o -name "*.svg" \
    -o -name "*.xml" \
  ")" \
  -print0 \
  | xargs -0 -n 1 gzip -k

... but I'm not sure if that's technically correct with find's operator precedence.

✅ Leverage build cache

Big time saver used well here. The Dockefile even documents that intention:

# First install dependencies. This part will be cached as long as
# the package(-lock).json files remain identical.
COPY package*.json /code/
RUN npm install

✅ FROM instruction best practices

the recommendation: use current official images and We recommend the Alpine image

this Dockerfile is based on node:12 (an official image) and nginxinc/nginx-unprivileged:1.19-alpine (both official and alpine-based)

⚠️ LABEL instruction best practices

the recommendation:

can add labels to your image to help organize images by project, record licensing information, to aid in automation, or for other reasons

this Dockerfile does not currently contain any labels and there has been a (stalling?) effort by the OCI to establish some standard labels, which @chgl mentioned previously. Certainly the maintainer label has been considered standard by many for quite a while.

@blootsvoets implies above that he intends for a future CI/CD system to add labels via build arguments. I think we probably want a mix of both here.

BUT I firmly believe this should not hold up the PR.

✅ RUN instruction best practices

the recommendation:

Split long or complex RUN statements on multiple lines separated with backslashes to make your Dockerfile more readable, understandable, and maintainable.

this is mostly intended for long lines that contain large lists of os-level package installs, which this Dockerfile doesn't have. There is one long line, the find command mentioned above. I think it is reasonable to keep it as-is.

✅ CMD instruction best practices

This Dockerfile inherits its properly-specified CMD from the base image:

CMD ["nginx", "-g", "daemon off;"]

✅ EXPOSE instruction best practices

the recommendation:

you should use the common, traditional port for your application. For example, an image containing the Apache web server would use EXPOSE 80, while an image containing MongoDB would use EXPOSE 27017 and so on.

This Dockerfile inherits one EXPOSE instruction from the base image:

EXPOSE 8080

This Dockerfile does not use port 80 because the entrypoint (and consequently the nginx app) is running as a non-privileged user within the container's UID namespace and therefore it can't bind to ports below 1024. I'm not sure why upstream choose 8080 instead of 8000, but there's no good reason to override that here.

✅ ENV instruction best practices

the recommendation:

The ENV instruction is also useful for providing required environment variables specific to services you wish to containerize, such as Postgres’s PGDATA.

Lastly, ENV can also be used to set commonly used version numbers so that version bumps are easier to maintain, as seen in the following example

[...]
Similar to having constant variables in a program (as opposed to hard-coding values), this approach lets you change a single ENV instruction to auto-magically bump the version of the software in your container.

This Dockerfile uses and documents 3 environment variables which are all observed by the entrypoint hook shell script docker/30-atlas-env-subst.sh:

• ATLAS_HOME - This appears to let you specify a different location for the atlas app, which could be useful during development in concert with bind mounts. I suspect to do that you might also need to specify additional nginx config to modify the document root, but I'm not sure.
• WEBAPI_URL - The entrypoint will update config-local.js with this value, so you can direct web clients to arbitrary webapi endpoints using only the environment.
• ATLAS_HOSTNAME - Setting this variable will cause the entrypoint to update the nginx server_name directive.

✅ ADD or COPY instruction best practices

the recommendation:

COPY is preferred.

[...]

[optimize position of COPY steps for fewer cache invalidations]

this Dockerfile uses no ADD instructions and as previously-mentioned the COPY instructions are organized in a cache-friendly way. Also multi-stage is used.

✅ ENTRYPOINT instruction best practices

This Dockerfile inherits its properly-specified ENTRYPOINT from the base image:

ENTRYPOINT ["/docker-entrypoint.sh"]

✅ VOLUME instruction best practices

the recommendation:

The VOLUME instruction should be used to expose any database storage area, configuration storage, or files/folders created by your docker container. You are strongly encouraged to use VOLUME for any mutable and/or user-serviceable parts of your image

Neither this Dockerfile nor its base image use a VOLUME instruction because there is no persistent state.

✅ USER instruction best practices

the recommendation:

If a service can run without privileges, use USER to change to a non-root user.

This Dockerfile's base image uses USER 101 which corresponds with this entry in the container's passwd file:

nginx:x:101:101:nginx:/var/cache/nginx:/sbin/nologin

✅ WORKDIR instruction best practices

the recommendation:

For clarity and reliability, you should always use absolute paths for your WORKDIR

This Dockerfile's first stage uses a WORKDIR so that the npm build happens in the right place. The final stage has no WORKDIR so it defaults to empty or /. This is fine.

✅ ONBUILD instruction best practices

the recommendation:

An ONBUILD command executes after the current Dockerfile build completes. ONBUILD executes in any child image derived FROM the current image. Think of the ONBUILD command as an instruction the parent Dockerfile gives to the child Dockerfile.

This Dockerfile does not use an ONBUILD instruction. This is fine.

Other notes

• docker/30-atlas-env-subst.sh passes shellcheck. I have a few minor nits (chiefly that the base image gives us gnu envsubst and I think we should use it instead of sed -- but the use of sed is more portable)

@glvnst Thank you! I'll definitely bookmark your best practices ;)

envsubst

I just noticed that the nginx image already includes a way to use envsubst (see https://github.com/nginxinc/docker-nginx/blob/master/mainline/alpine/20-envsubst-on-templates.sh), so 30-atlas-env-subst.sh might actually be unnecessary. But I agree this shouldn't block the PR.

@blootsvoets - nice speaking with you, @leeevans and Maxim earlier. I just wanted to note that I'm fully supportive of merging this in once @leeevans provides review/approval. Thanks!