Commit graph

1481 commits

Author SHA1 Message Date
2e41f7bb0b Send Vary: Authorization cache header
I don't think this is actually relevant in-app right now, but I figured sending it is More Correct, and is likely to prevent future bugs if anything (and prevent future question about why we're _not_ sending it).

I also removed the `maxAge: 0` on `currentUser`, now that I've updated Fastly to no longer default to 5-minute caching when no cache time is specified. I can see why that's a reasonable default for Fastly, but we've been pretty careful about specifying Cache-Control headers when relevant, so the extra caching is mostly incorrect.
2021-11-23 13:00:56 -08:00
d4b115e805 Merge branch 'ansible' of github.com:matchu/impress-2020 into ansible 2021-11-23 12:42:59 -08:00
96d6d42120 Use persisted queries in dev, bc Next.js support
We had previously configured the client to not bother to try a GET request for GraphQL queries, and just jump straight to POST instead, because the `vercel dev` server for create-react-app reloaded the backend code for every request anyway, which doubled the dev response time.

The Next.js server is more efficient than this, and keeps some memory, so GET requests work similarly in dev as on prod now! (i.e. it fails the first time, but then succeeds on the second)

In this change, we remove the code to skip `createPersistedQueryLink` in development, and instead always call it. We simplify the code accordingly, too.
2021-11-23 12:35:20 -08:00
b941dce9fa Private cache headers in item search
If the user is searching for things they own or want, make sure we don't CDN cache it!

For many queries, this is taken care of in practice, because the search result includes `currentUserOwnsThis` and `currentUserWantsThis`. But I noticed in testing that, if the search result has no items, so those fields aren't actually part of the _response_, then the private header doesn't get set. So this mainly makes sure we don't accidentally cache an empty result from a user who didn't have anything they owned/wanted yet!
2021-11-16 13:09:45 -08:00
b73e2e1123 Send cache-control header for max-age=0, private
Some queries, like on `/your-outfits`, had the cache hint `max-age=0, private` set. In this case, our cache code sent no cache header, on the assumption that no header would result in no caching.

This was true on Vercel, but isn't true on our new Fastly setup! (Which makes sense, Vercel was a bit more aggressive here I think.)

This was causing an arbitrary user's data to be cached by Fastly as the result for `/your-outfits`. (We found this bug before launching the Fastly cache though, don't worry! No actual user data leaked!)

Now, as of this change, the `/your-outfits` query correctly sends a header of `Cache-Control: max-age=0, private`. This directs Fastly not to cache the result.

To fix this, we made a change to our HTTP header code, which is forked from Apollo's stuff.
2021-11-16 12:34:11 -08:00
cadf7487af Mark currentUser GQL as non-cacheable
Comments explain most of this! Vercel changed around the Cache-Control headers a bit to always essentially apply max-age:0 when scope:PRIVATE was true.

I'm noticing this isn't *fully* working yet though, because we're not getting a `Cache-Control: private` header, we're just getting no header at all. Fastly might aggressively choose to cache it anyway with etag stuff! I bet that's the fault of our caching middleware plugin thing, so I'll check on that!
2021-11-16 12:12:51 -08:00
002af474f8 Don't HTTP cache currentUserOwnsThis/wants
Hmm, I see, Vercel chews on Cache-Control headers a bit more than I'm used to, so anything marked `scope: PRIVATE` would not be cached at all.

But on a more standard server, this was coming out as privately cacheable, but for an actual amount of time (1 hour in the homepage case), because of the `maxAge` on other fields. That meant the device browser cache would hold onto the result, and not always reflect Own/Want changes upon page reload.

In this change, we set `maxAge: 0`, because we want this field to be very responsive. I also left `scope: PRIVATE`, even though I think it doesn't really matter if we're saying the field isn't cacheable anyway, because I want to set the precendent that `currentUser` fields need it, to avoid a potential gotcha if someone creates a cacheable `currentUser` field in the future. (That's important to be careful with though, because is it even okay for logouts to not clear it? TODO: Can we clear the private HTTP cache somehow? I guess we would need to include the current user ID in the URL?)
2021-11-16 12:04:16 -08:00
a67509aaec Merge branch 'main' into ansible 2021-11-16 11:23:01 -08:00
bf8fd81305 Post a message linking to Metaverse revocation 2021-11-15 17:42:02 -08:00
5039371a1d Simplify the page pool
Yeah ok, let's just run one browser instance and one pool.

I feel like I observed that, when I killed chromium in prod, pm2 noticed the abrupt loss of a child process and restarted the whole app process? which is rad? so maybe let's just trying relying on that and see how it goes
2021-11-13 02:27:24 -08:00
0c2939dfe4 Use Puppeteer instead of Playwright
We used Playwright in the first place to try to work around a Vercel deploy issue, and I'm not sure it really ended up mattering lol :p

But yeah, I'm putting the new Puppeteer code through the same prod stress test, and it just doesn't seem to be getting into the same broken state that Playwright was. I'm guessing it's just that Puppeteer has more investment in edge-case handling? (There's also the fact that we're no longer running things as root, which could have been a fucky problem, too?)
2021-11-13 02:16:58 -08:00
587aa09efc Oops, fix bug in pm2 setup
Oh, I made a typo that caused pm2 to be running our processes as `root` instead of `matchu`! Let's very fix that!! 😳

I noticed this because I'm trying Puppeteer, and it got upset about running in sandboxed mode as root, and I'm like "as root??"

So yeah, good, fixed, lol 😬
2021-11-13 02:12:05 -08:00
20fff261ef Switch to a small page pool
Hmm I am really not managing to keep the processes under control… maybe I'll try Puppeteer and see if it's just a bit more reliable??
2021-11-13 01:45:27 -08:00
18bc3df6f4 Use browser pooling for /api/assetImage
I tried running a pressure test against assetImage on prod with the open-source tool `wrk`:

```
wrk -t12 -c20 -d20s --timeout 20s 'https://impress-2020-box.openneo.net/api/assetImage?libraryUrl=https%3A%2F%2Fimages.neopets.com%2Fcp%2Fitems%2Fdata%2F000%2F000%2F522%2F522756_2bde0443ae%2F522756.js&size=600'
```

I found that, unsurprisingly, we run a lot of concurrent requests, which fill up memory with a lot of Chromium instances!

In this change, we declare a small pool of 2 browser contexts, to allow a bit of concurrency but still very strictly limit how many browser instances can actually get created. We might tune this number depending on the actual performance characteristics!
2021-11-12 23:35:30 -08:00
3ec0ae7557 Use localhost in /api/assetImage
Just another VERCEL_URL removal!
2021-11-12 22:08:06 -08:00
0a81f07849 Remove Waka values
The motivation is that I want VERCEL_URL and local net requests outta here :p and we were doing some cutesiness with leveraging the CDN cache to back the GQL fields. No more of that, folks! lol
2021-11-12 22:06:50 -08:00
991defffa1 /api/outfitImage makes direct GQL queries
Previously we were using HTTP queries to keep individual function bundle sizes small, but that doesn't matter in a server where all the code is shared!

The immediate motivation is that I want /api/outfitImage requesting against the same server, not impress-2020.openneo.net. For other stuff I'm probably gonna fix this by replacing VERCEL_URL with something else, but here I figured this was a change worth making anyway.
2021-11-12 21:53:22 -08:00
afd23fb4dd Bump version of graphql
This was actually kinda accidental, I thought I could uninstall it but then realized I couldn't. Anyway, it's updated now!
2021-11-12 21:52:14 -08:00
eaadfd09ef Delete outfitPageSSR
Oh right, we implemented this with Next.js SSR in `/pages/outfits/[id].js`, so we don't need this anymore!
2021-11-12 21:41:17 -08:00
9753cbe173 /api/assetImage fixes in production
Now that we're not on Vercel's AWS Lambda deployment, we can switch to something a bit more standard!

I also tweaked up our version of Playwright, because, hey, why not?

Getting the package list was a bit tricky, but we got there! Left a comment to explain where it's from.
2021-11-12 21:39:35 -08:00
5470a49651 Use utf8 in API error messages
I noticed this when Playwright was trying to draw cute ASCII art and it wasn't showing up right! Not a big deal, but it's a bit more correct to do this, so let's do it!
2021-11-12 21:17:20 -08:00
07d54b9a9e Add SSH keys to deploy-setup
This helps me set up new devices, while still keeping passworded SSH access locked down!
2021-11-12 20:14:44 -08:00
30d582a9c4 Re-add support for nice outfit image URLs
We removed this earlier in 7205455ccb, but now it's time to re-add it!
2021-11-12 19:53:34 -08:00
d37d958a36 Enable automatic updates & reboots on deploy box
Oh right, like the SSH stuff, I did this the first time I set up, but didn't add it to the script! I like having things in the script :3 (I also had forgotten to check on the time zone last time, nice to have it with some rigor!)
2021-11-04 19:17:35 -07:00
8f28f87bee Close most ports on the deploy box by default
I noticed that incoming port 3000 connections were being allowed, oops! Not a huge deal, but I don't want to allow connections without HTTPS, and I don't want surprise surface area even if I'm not currently aware of attacks on it. Close it out!
2021-11-04 18:57:00 -07:00
9310a250d6 Fix some bugs running deploy-setup from scratch
As an exercise, I've wiped the box clean, and I'm reinstalling from the scripts! :3

I added the SSH hardening rules to the playbook instead of doing them by hand this time.

I made a mistake with creating `/srv/impress-2020`, right, you need to *say* what it should be created *as* for the creation step to work!

I also guess my recent pm2 changes made it not actually be willing to start the app anymore, because `/srv/impress-2020/current` doesn't exist or have `node_modules` yet. I'm doing a cute thing where I create a placeholder app during setup, so there's always something to run, without introducing the complexities of a real deploy to the setup process.

And right, of course, we need to install nginx before running certbot! But we need to add certbot config *after* running certbot!

And then just some misc cleanups for consistency and correctness!
2021-11-03 23:11:50 -07:00
1e3e8391b4 Oops, fix YAML + bash mistake
Okay so you know how in 3f07933f7a I switched the newline stuff here?

Yeah, right, I forgot, newlines are significant in bash :p I forgot this because I've never used them inside a `bash -c` invocation, but like, of course

Now, I'm still using `|` for clarity and reduced dependence on magic, but getting my lines right :p
2021-11-03 17:34:07 -07:00
32ae1accce Oops, fix testing for node_modules
I somehow had it in my head that `realpath` would crash if the file didn't exist, but that's super not true! It returns the tentative path for if you _were_ gonna create this!
2021-11-03 17:28:10 -07:00
171558a64f Move assetImage and outfitImage back into Nextjs
This should let us actually start working with them locally and in new prod!
2021-11-03 17:07:25 -07:00
e8ed459afd Remove the web group permission stuff from deploy
I'm not doing this thoroughly enough for it to matter (e.g. the deployed rsynced versions aren't having the group permissions set).

I think doing this *right* (to be extensible to additional users) is too much complexity to be worth it, and doing it halfway is more confusing than helpful.

I did this because I was anticipating multi-users permissions to be a bit of an issue for like, granting the web server permission to access the source code. But it turns out, since we're running with pm2, it's all working just fine!
2021-11-03 16:59:23 -07:00
bd8ccf19d7 Remove monit from our deployment
Okay well, we added monit to solve a problem that I coincidentally solved within an hour of getting monit working lol!

This also enables us to remove the pm2 pid file, which we were only using to allow monit to track the pm2 app.
2021-11-03 16:48:38 -07:00
d17263139e Fix pm2 monitoring
Okay huh, while digging a bit into another issue, I found what was wrong with our config and pm2's built-in monitoring! You can't use `yarn start`, because the wrapper script breaks its ability to look inside and see what's happening.

I also removed the compiler flag thing from the `start` script in `package.json`, because I think it's redundant? There's no compilation to be done in a live server.

I think I might remove monit after this? It's nice extra resilience in a sense, but it feels like extra complexity when it's doing the job `pm2` is supposed to do. (And tbh I've almost never heard of nginx crashing, and if it does it's probably a scenario worth investigating by hand.)
2021-11-03 16:46:35 -07:00
3f07933f7a Fix minor YAML mistake
Oops, I didn't understand the different multiline string formats in YAML! I was using one that chomped through newlines, and converted them to normal spaces.

I think that didn't matter in this context anyway? because indentation is an exception to this behavior. What a weird behavior!

Anyway, uhh, yeah, I'll use the simpler multiline string format now 😅 for consistency and clarity!
2021-11-03 16:33:47 -07:00
792da067e3 Add monit watching for nginx and pm2
When I woke up this morning, the app had crashed because the mysql connection was closed!

I'm not sure, why that caused a _crash_? Or why pm2 didn't pick up on it, and said the process was still online? (Maybe the process was running, but the server had stopped?) Those could be good to investigate?…

…but better than diving too far into the details, is to just address the high-level problem: if the app goes down for unexpected reasons, I want it back up!! lol

In this change, we add `monit`, a solid system for monitoring processes (including checking for behavior, like responding to net requests), and configure it to watch the app process and the nginx process.

To test, you can run `pm2 stop impress-2020`, or `systemctl stop nginx`, to see that Monit brings them back up within seconds!

This does add some potential surprise if you're _trying_ to take the processes down. The easiest way is to send the stop command through monit, like `monit stop nginx`. This will disable monitoring until you start it again through monit, I think? (You can also disable/enable monitoring as a direct command, regardless of app state.)
2021-11-03 16:32:14 -07:00
2f874653bf Update pm2 tasks to update the config correctly
Previously, if you changed the pm2 ecosystem file content, it wouldn't actually be reflected in the running services. Now it will be!
2021-11-03 15:43:37 -07:00
7131bc0ea9 Set up certbot during setup playbook
You can see how, instead of the default experience where certbot edits your config for you, I've referenced the certificates in the config in the first place, and set up certbot to just generate them!

Also, I learned about certbot non-interactive mode! At first I wrote this with the Ansible `expect` module lol :p
2021-11-03 01:00:28 -07:00
9a4b905639 Set up basic nginx in front of impress-2020
It loads kinda! auth0 is crashing us because it refuses to run over http:// but hey! That's pretty cool!
2021-11-03 00:07:30 -07:00
4f372af132 Optimization: Reuse node_modules when possible
`yarn install` is slow, I'd like to skip it! Vercel and other hosts do a similar cheat here.
2021-11-02 23:56:50 -07:00
27b9f50afb Oops, Next.js builds are in .next/, not in build/
I was copying over an old create-react-app build folder, and the app was crashing remotely because there was no build!

Anyway woo it looks like the app is at least running now, `curl localhost:3000` shows results! >:3
2021-11-02 23:02:22 -07:00
9d41e80942 Use pm2 to run the deployed app 2021-11-02 22:49:45 -07:00
056c03fb42 Create a current version symlink
This is mostly just for convenience!
2021-11-02 21:53:20 -07:00
49cc2224b6 Refactor out a remote_versions_root variable 2021-11-02 21:29:14 -07:00
c13114b0cc Fix old version cleanup to sort by modified date
I was sorting by version name, which _was_ reliable until I changed my version name format, and then a deploy cleaned up the version it had just deployed because it no longer sorted to the end!

Rather than be dependent on a stable version name format, and set myself up for more surprises, I'm going to instead use the folder's modified time. While there could be surprising ways for folder timestamps to be updated, I expect it to be _more_ reliable overall.
2021-11-02 21:16:46 -07:00
bddcf91c02 Add more details to deploy README 2021-11-02 21:14:54 -07:00
a3859b36bf Safer deploy version names
Huh, I was previously using ISO timestamps, but it turns out Yarn doesn't handle binary paths very reliably when your folder names contain colons 😳 so `canvas` was failing with `node-gyp not found`

I've documented it here in this self-answered StackOverflow question, so hopefully the next future person searching will find my answer! https://stackoverflow.com/q/69819597/107415
2021-11-02 21:14:47 -07:00
fb5b0fe611 Add task to clean up older deployed versions 2021-11-02 19:02:36 -07:00
edd983c97a Refactor deploy to build locally, not remotely 2021-11-02 18:47:13 -07:00
d17c4cea8b Add yarn build to deploy playbook
also secret copying, which was required for the cached-data step
2021-11-02 16:45:07 -07:00
dde8cee1e3 Add deploy playbook: pulls git and installs deps 2021-11-02 16:36:39 -07:00
537aeb4118 Ansible tasks to set up web user, install Node 2021-11-02 16:01:30 -07:00