Skip to content
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

Self contained front end using Webpack to build HTML and Webpack Dev Server to serve #678

Merged
merged 64 commits into from
Apr 24, 2024

Conversation

jonkafton
Copy link
Contributor

@jonkafton jonkafton commented Mar 26, 2024

What are the relevant tickets?

Load session from API and configure decoupled frontend for local development #616

This PR isolates the front end so the backend web service is no longer responsible for serving the base HTML or any static assets, with the intention that these are to be served separately from a storage service fronted by endpoints or CDN.

The frontend package root has moved from the project root and into ./frontends/ along with the ./static images folder.

Nginx has been configured to serve the built frontend files where there is a path match in ./frontends/mit-open/build, falling back to the index.html, located there. API and admin routes are passed to the Django web service.

This PR should not be merged until the task to deploy the static site is completed, CI and deployment config for decoupled front end #629, as it may prevent release. At minimum we'll need to establish that it is quickly deployable - aiming to minimize impact, the Nginx configuration should be deployable to Heroku to serve the built frontend if we choose to do so while working on #629 , although the nginx.conf changes have not been applied to the Nginx nginx.conf.erb config for Heroku.

Description (What does it do?)

  • Moves the frontend package root into ./frontends for a self contained front end, decluttering the project root.

  • Configures Webpack Dev Server for lightweight front end development (serve and watch the front end in isolation, hot module replacement for quick change feedback).

  • Configures Webpack to inject scripts and build the HTML index file (using HTMLWebpackPlugin)

  • Configures Webpack to copy images (moved from ./static/) from ./frontends/mit-open/public/images/ to the front end build (uses CopyWebpackPlugin).

  • Configures Webpack Dev Server to proxy API requests to target provided on environment. Overwrites the Origin header to the proxy target for Django admin's CSRF check.

  • Configures Nginx to serve on file matches in ./frontends/mit-open/build falling back to index.html for the single page app. ^/(api|login|logout|admin|static/admin|static/hijack)/ route to the web service on 8061. Are there any other routes that should be handled there?. The project root is no longer mounted to the nginx container in the Docker Compose run config, only the frontend build (and Nginx config).

  • Updates the manage.py collectstatic path to collect images from above (do we need this? intends to preserve dj-static file serving, though /static/ on the web service should never be hit for front end assets, though is still in use for admin panel assets).

  • Provides NPM scripts to run front end dev server in isolation against RC or Prod:

    • yarn watch to run the frontend dev server locally, proxying API routes to our locally running Docker Compose stack at localhost:8063. Serves on port 8080.
    • yarn watch:docker for running inside the Docker watch container in build/watch mode, proxying to nginx:8063.
    • yarn watch:rc for running in isolation in serve/watch mode, proxying API requests to mitopen-rc.odl.mit.edu.
    • yarn watch:prod for running in isolation in serve/watch mode, proxying API requests to mitopen.odl.mit.edu.
  • Repurposes the Docker Compose watch container to run Webpack Dev Server, using ./frontends as build context and not mount project root. Dockerfile-node moved into ./frontends. The watch container doesn't now serve on any port - it's job is to build the frontend on start so it is available to be served by Ngnix so as not to impact the application running locally with the single docker compose up. Also watches for changes. run-watch-dev.sh script removed and wait on watch logic removed from run-django-dev.sh.

  • Github Actions CI jobs updated for changes to project structure.

  • UI fetches the user session from the API at GET /api/v0/users/me/ so it is no longer reliant on window.SETTINGS. Unit tests updated to either mock the endpoint to expect the call to it, or to load the user into the TanStack Query cache where tests depend on first component render.

Housekeeping:

  • Upgrade yarn to 4.1.1 (in container build and across workspaces).
  • Adds private: true to all package.json files (yarn workspaces requirement).
  • Uses the lighter Alpine distribution as the base image for frontend Dockerfile.
  • Upgrade webpack-dev-server v4->v5.
  • Upgrade webpack-bundle-tracker v1->v3.
  • Refactor down the Webpack config, removing static file path logic not needed (removes build env vars MITOPEN_HOSTNAME and WEBPACK_PORT_MITOPEN).
  • Configures Webpack to clean the build folder each build (no stale build files).
  • Supported architectures added to .yarnrc.yml to build the @swc/core Linux binaries for the watch container (fixes "bindings not found" error).
  • Yarn lockfile and releases ignored from the check-added-large-files pre-commit check.

Not done:

How can this be tested?

Existing workflow:

  • There should be no changes to existing workflows for local development - docker compose up should work as before and serve the application at http://localhost:8063.
  • Login and logout should redirect to OIDC.
  • All admin routes should function correctly.
  • Changes to front end source are built by the watch container and updated on page refresh.

New frontend dev server:

Copying README notes over from Frontend Dev Server with Local Backend. This all needs to test correctly:

  • Frontend Dev Server with Local Backend. In this mode the front end is served with Wepback Dev Server, enabling Hot Module Replacement and faster feedback to changes. Dev Server proxies API requests through to a locally running backend stack. The front end is served at http://localhost:8080 and changes are hot reloaded into the page.
docker compose up nginx web db
yarn watch
  • Frontend Dev Server proxying to RC or Prod. When working on the front end in isolation or to test changes against APIs already running in RC and Prod, the frontend dev server is configured to run against our hosted API environments without running the backend stack locally. The front end is served at http://localhost:8080 and changes are hot reloaded into the page. Run the front end with one of:
yarn watch:rc
yarn watch:prod

Additional Context

RFC: Decouple the Frontend from the Backend Web Service #3646

Copy link

gitguardian bot commented Mar 28, 2024

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
9430286 Triggered Generic Password 0d6e7ad docker-compose.yml View secret
9430286 Triggered Generic Password f7dd808 docker-compose.yml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.04%. Comparing base (398ad13) to head (2b2b87c).
Report is 99 commits behind head on main.

❗ Current head 2b2b87c differs from pull request most recent head 36a70a4. Consider uploading reports for the commit 36a70a4 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #678       +/-   ##
===========================================
+ Coverage   72.58%   92.04%   +19.46%     
===========================================
  Files         281      144      -137     
  Lines       12581     6603     -5978     
  Branches     2184      971     -1213     
===========================================
- Hits         9132     6078     -3054     
+ Misses       3265      406     -2859     
+ Partials      184      119       -65     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Configures Ngnix for to serve static front end files in /frontends/mit-open/build

- Falls back to index.html for SPA routes

- Sets 1m expires header for index.html (we want a short cache on this as it will be overwritten during release)

- Removes the CORS * header (we shouldn't need this on file responses)

- Proxies known API routes to the uwsgi worker
@jonkafton
Copy link
Contributor Author

jonkafton commented Apr 22, 2024

That I do not think warrants a re-review. The line count was a red flag... +29,754 −1,320 . Could you remove the root-level yarn.lock?

Removed. I suspect it snuck its way back in during merge.

Merging

I tagged this "blocked" for now. Is it? What needs to be done before merging?

I've updated the Nginx config for Heroku to align with our local container config in this commit: 5aa641ca4.

We'll need to do a hands on merge and monitor the release to RC as I'm not sure whether we can test the Nginx config in advance. We could always deploy to a new Heroku environment first, but may not be worthwhile as we can quickly roll back RC while we try the release.

The other aspect hard to test before attempting the release is that Nginx is now expecting to find the built front end on Heroku in /app/frontends/mit-open/build to serve the static content. I've added a step to upload the the build as an artifact during the javascript-tests job (we may want a dedicated job, though we have it here and can avoid building twice). This should be fine as the release-candidate and production workflows trigger from the ci workflow, though there may be an issue sharing between workflows if they are treated separately and possibly the filepath it arrives on in Heroku. The existing Nginx has a root at /app, though this isn't a project directory. It may be a Heroku dyno default path - we'll need a trial run.

@jonkafton jonkafton removed the Blocked label Apr 24, 2024
@jonkafton jonkafton merged commit dafcf94 into main Apr 24, 2024
12 checks passed
jonkafton added a commit that referenced this pull request Apr 24, 2024
jonkafton added a commit that referenced this pull request Apr 25, 2024
…d errors (#825)

* Revert "Remove package manager config (#823)"

This reverts commit f03ed4b.

* Revert "Set engines to instruct Heroku to install yarn (#821)"

This reverts commit f44a4f2.

* Revert "Deployment fixes for static frontend on Heroku (#819)"

This reverts commit 07243ec.

* Revert "fixing compose mount (#818)"

This reverts commit 80292a4.

* Revert "Move hash.txt location to frontend build directory (#815)"

This reverts commit e0d9e01.

* Revert "Build front end to make available on Heroku (#813)"

This reverts commit fc06462.

* Revert "Self contained front end using Webpack to build HTML and Webpack Dev Server to serve (#678)"

This reverts commit dafcf94.
@odlbot odlbot mentioned this pull request Apr 26, 2024
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants