Add a docker file for building frontend assets

[benjaoming]

I built the assets in #4842 using this docker file.

Following several incidents of malware on NPM, I think it's better to describe this practice before the other practices for building assets? It's simpler, anyways.

I would also note that tracking built assets in the repo is insecure since minified or vendorized assets are hard/impossible to verify during review processes. So actually, you should reject the assets that I put in #4842 :)

[codecov]

Codecov Report

Merging #4843 into master will increase coverage by 2.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4843      +/-   ##
==========================================
+ Coverage   76.39%   78.55%   +2.15%     
==========================================
  Files         158      173      +15     
  Lines        9980    10667     +687     
  Branches     1261     1354      +93     
==========================================
+ Hits         7624     8379     +755     
+ Misses       2016     1939      -77     
- Partials      340      349       +9

Impacted Files	Coverage Δ
readthedocs/search/signals.py	63.41% <0%> (-36.59%)	⬇️
readthedocs/builds/admin.py	64.51% <0%> (-31.32%)	⬇️
readthedocs/integrations/utils.py	41.66% <0%> (-20.84%)	⬇️
readthedocs/projects/admin.py	71.91% <0%> (-19%)	⬇️
readthedocs/config/parser.py	85.71% <0%> (-14.29%)	⬇️
readthedocs/builds/querysets.py	71.28% <0%> (-8.07%)	⬇️
readthedocs/doc_builder/backends/sphinx.py	63.87% <0%> (-5.93%)	⬇️
readthedocs/builds/views.py	80% <0%> (-5.72%)	⬇️
readthedocs/core/signals.py	81.25% <0%> (-5.42%)	⬇️
readthedocs/oauth/utils.py	28.12% <0%> (-5.21%)	⬇️
... and 178 more

[benjaoming]

I've modified the section headers a bit to clarify the difference between the containerized build and the "local build" (any better label?).

[ericholscher]

Interesting approach. This could make it easier for folks to generate assets locally. I'll leave this up to @davidfischer or @agjohnson to approve, since they are more versed w/ the frontend stuff, but it makes sense to me.

[davidfischer]

If we are going to have a docker container, why not have the ability to run a complete RTD installation in docker? I don't want to necessarily create more work on @benjaoming but I want to start a conversation on whether that seems useful. Overall, I think it is.

[benjaoming]

It should be fairly easy to setup a Docker configuration for running the entire thing, too. Am not sure however, if there could be any rabbit holes hidden.. but once this seed is planted, perhaps someone who wanted or needed the whole development workflow dockerized could do that. I'm not an expert, so for instance I didn't use docker-compose.. I just went ahead with the simple stuff that I know of :)

[humitos]

why not have the ability to run a complete RTD installation in docker?

This is way complicated. There are a couple of people that already tried and it's hard.

I personally tried to replicate a similar architecture that we have in production with docker-compose and I gave up.

I'm not an expert on the JS world, but considering that we had different problems based on the config of each core developer when building the assets, I think it's a good approach to start using a simple docker image to build them and do not rely on the developer's setup.

[ericholscher]

Yea, I think having a docker env for building static files makes sense, since we've hit issues with it, and a lot of our devs will be familiar with Python more so than JS and the dependencies. It definitely would make my life a bit easier to not have to keep an updated JS env for generating assets.

[agjohnson]

Moving this PR to design decision for now. We start new feature with design discussions to avoid rejecting PRs. Also, throwing this in the refactor milestone, as this isn't quite aligned with our roadmap. You can read more on our triage process here:
https://docs.readthedocs.io/en/latest/roadmap.html#triaging-issues

Having said that, I'm probably, like, +0 on this. For release engineering, this could be really handy. I don't want to make this the blessed/prescribed method of asset generation though, as we don't yet require Docker for contributors. This is a doc change though.

However, the main hangup I have is that we have similar code on our commercial fork and I wouldn't prioritize engineering time towards reinventing that as well. Our roadmap is focused building product features, so spending more time on backend engineering is ultimately a distraction. Having differing release processes is confusing for ops.

why not have the ability to run a complete RTD installation in docker?
This is way complicated. There are a couple of people that already tried and it's hard.

On the surface, a Dockerfile/Docker compose setup sounds okay. However, the limitations of Docker make this hard and awkward to use for development, which means it won't get a lot of use by core and will rot without maintenance, or at very least, a distraction. So, I'm a strong -1 on this, and would rather contributors avoid spending time here.

So actually, you should reject the assets that I put in
I would also note that tracking built assets in the repo is insecure

We rebuild assets every release. Built assets are a requirement to running RTD, so instead of requiring that every dev be familiar with asset generation, we vendorize these files instead. I think this is the correct path for a code base that is not frequented or maintained by JS devs. This puts asset generation only on the very few folks that touch our javascript or image assets.

[agjohnson]

Just noting some feedback here as well, but we're moving this to design decision first. I'd hold off on spending time here until we have a chance to accept the feature. I'll leave changes requested while we're deciding.

[agjohnson]

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.

[agjohnson]

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.

[benjaoming]

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.

[agjohnson]

We put files like this in contrib/

[humitos]

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

[agjohnson]

Is there a reason to use virtualenv?

[benjaoming]

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.

[humitos]

I wouldn't use a virtual environment either.

[agjohnson]

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.

[benjaoming]

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.

[humitos]

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

[benjaoming]

@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

[benjaoming]

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.

[humitos]

@rtfd/core are there more thoughts on this?

I think the idea of using Docker for this is good, but considering what Anthony said regarding different deploy processed, etc, I'm more tempted to close this PR for now and revisit later when we have more time to spend on this process.

[agjohnson]

The design project we have coming up will be moving us in a direction to use webpack. Let's re-evaluate then if it would be helpful. The webpack config is a) significantly less complex and b) probably more stable than a solution that uses a bower assets and hacks to get everything to work together c) going to change our entire build process anyways.

This could still be valid at that point, and would hopefully be easier to implement across multiple repositories.

[benjaoming]

The Docker file seems most relevant as long as there are these odd hacks, could go in the dump together with Bower once the Webpack solution is implemented (or be adapted to work with it)?

• Implement a UID variable for the docker volume
• Move to contrib

[humitos]

We had have some problems while building the assets lately. I'd be 👍 on having this process standardized and built inside our new inv docker. namespace commands for docker-related tasks. I imagine something like

1. inv docker.build --assets: build the docker image for assets
2. inv docker.assets: build the assets using the docker image built in the previous step (runs using the same UID:GID that used to call inv command and uses them for the new files created)

[humitos]

I wouldn't use a virtual environment either.

[humitos]

The command that we are running now is:

npm ci && bower update && npm run build

[humitos]

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

[benjaoming]

Thanks for looking into all this. Am afraid that I don't have time/motivation to follow up on this, and it sounds like there are people in line with far more opinions than me about how to containerize JS stack hell :)

I wish you good luck with finding a solution to this, but I'd leave the initiative to someone else.

I can only hope that the focus is on solving the immediate problems of asset buildings firstly, they are likely wasting a lot of time for contributors. Then, move on to the optimizations as you start using it in everyday dev'ing.

[humitos]

Thanks @benjaminp for your contribution. I will use this PR to continue from here and adapt what you have done to fit our requirements. I'm sorry this got stale for so long.

[agjohnson]

@humitos This isn't a strong priority as we'll be switching build stacks soon. We shouldn't invest time here as we'll have to redo everything again anyways.