Skip to content

Add a docker file for building frontend assets #4843

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docker/frontend.dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
FROM ubuntu:bionic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We put files like this in contrib/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should use ubuntu:18.04 here which is easier to read.


ENV LANG C.UTF-8

# install latest python and nodejs
RUN apt-get update && \
apt-get install -y \
nodejs \
npm \
gettext \
git \
sudo \
build-essential \
python3 \
python3-dev \
python3-pip \
libxml2-dev \
libxslt1-dev \
zlib1g-dev

RUN bash -c "pip3 install pip setuptools --upgrade --force-reinstall"
RUN bash -c "pip3 install virtualenv"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to use virtualenv?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python requirements are installed into a virtualenv, they are less predictable if installed into a system env - things are more likely to break with time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't use a virtual environment either.

RUN bash -c "npm install -g bower"

# RUN adduser --system --shell /bin/bash --home "/repo" rtd

# A volume used to share `pex`/`whl` files and fixtures with docker host
VOLUME /repo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has come up any time docker has been discussed, but mounting like this is bound to result in permission errors, especially for any folks on Linux, which is majority of core team.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the essence of the whole Docker file though: It builds the output assets directly in your local files. I'm on Linux and it works fine. Without the use of a volume, I can't complete this PR. So might as well close it then.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@benjaoming when running this docker image, don't the generated files by the docker image end up belonging to root user?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@humitos not existing files, but I can see that newly created files are owned by root. So it applies to /static/.

Reference: moby/moby#3124

I can see these options:

  • Not using volumes and instead listing all the files/dirs to copy out of the container
  • Using docker compose and whatever features it has (seems from the above reference that it can do something)
  • Changing the setup to be aware of the appropriate UID of the invoking instance and creating a user for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some methods of inheriting the UID of user invoking Docker into the container and thus getting around the permission issue.

I haven't come across anything better. I can implement this.


# do the time-consuming base install commands
CMD /bin/bash -c "/usr/local/bin/virtualenv -p python3 rtd \
&& source rtd/bin/activate \
&& cd /repo \
&& pip install -r requirements.txt \
&& npm install \
&& bower --allow-root install \
&& npm run vendor \
&& npm run build"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The command that we are running now is:

npm ci && bower update && npm run build

27 changes: 27 additions & 0 deletions docs/development/standards.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,36 @@ may change, so that assets are compiled before deployment, however as our front
end assets are in a state of flux, it's easier to keep absolute sources checked
in.


Getting Started
---------------

We describe two different ways of building front end assets: Either in a
containerized environment (Docker) or step-by-step in your local development
environment.

Containerized build (Docker)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Front end assets can be built in a Docker container. This reduces the steps
required and can be considered more secure, since you won't expose your local
development environment to packages from NPM.

You will need to install Docker on your system, then run the following commands
from the root of your repository:

.. code-block:: console

# Builds the Docker image
docker image build . -t "rtd-frontend" -f docker/frontend.dockerfile

# Runs the image from the repository root
docker run -v $PWD/:/repo "rtd-frontend"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be priority over local build still. We don't have a requirement for docker for contributors yet, so I'd like to avoid making this assumption.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the heading are off here, I'd rather we keep a level 2 heading for Getting Started, which also is likely the target for interdoc links, and instead have sub headings for Local bulid and Containerized build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NPM and security is my concern. We are forcing people to install an unsafe environment directly in the dev runtime.

Secondly it's the annoyance of having to set it up, the NPM cache etc. Docker is more disposable. I'd rather explain Docker to a Python developer than Node-js and NPM.

I can fix the heading stuff.



Local builds
~~~~~~~~~~~~

You will need a working version of Node and NPM to get started. We won't cover
that here, as it varies from platform to platform.

Expand Down