Page MenuHomePhabricator
Differential D9398

[nix] Create new url facts
ClosedPublic
Actions

Authored by michal on Oct 6 2023, 8:31 AM.
Tags
None
Referenced Files
F3362194: D9398.diff
Sun, Nov 24, 9:46 PM
Unknown Object (File)
Wed, Nov 20, 1:21 AM
Unknown Object (File)
Mon, Nov 11, 8:07 PM
Unknown Object (File)
Mon, Nov 11, 6:41 PM
Unknown Object (File)
Sun, Nov 10, 11:41 PM
Unknown Object (File)
Sun, Nov 10, 9:24 PM
Unknown Object (File)
Fri, Nov 8, 12:29 AM
Unknown Object (File)
Fri, Nov 8, 12:29 AM
View All Files
Subscribers
ashoat
tomek
will

Details

Reviewers
inka
kamil
atul
ashoat
Commits
rCOMMc9bae039ffac: [nix] Create new url facts
Summary

Part of ENG-5153
Depends on D9397

We would like to separate hosting web app on dev environment to localhost:3000/webapp/ and move the keyserver logic to localhost:3000/keyserver/ (so in the end nothing will be hosted on localhost:3000/) to simplify the dev environment and allow for web <-> keyserver separation. We can also use this opportunity to cleanup the naming. This diff automatically adds new files on nix start. They will be used in the later diff.

Production will need to be updated so that keyserver_url.json contains data from squadcal_url.json and webapp_url.json contains data from commapp_url.json

Test Plan

Run nix develop, check if files are generated

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

michal created this revision.Oct 6 2023, 8:31 AM
Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptOct 6 2023, 8:31 AM
michal edited the summary of this revision. (Show Details)Oct 6 2023, 8:34 AM
michal added inline comments.
scripts/templates/keyserver_url.json
6

squadcal_url had a "proxy": "apache" here, but we aren't using apache in dev env anymore and it broke the ip detection when you pointed the web app at the keyserver endpoints.

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 6 2023, 8:47 AM
Harbormaster failed remote builds in B23056: Diff 31746!
michal updated this revision to Diff 31931.Oct 11 2023, 7:03 AM

Rebase

Harbormaster completed remote builds in B23187: Diff 31931.Oct 11 2023, 7:21 AM
michal requested review of this revision.Oct 11 2023, 7:21 AM
michal added a child revision: D9451: [web/native/keyserver] Start using new url facts.Oct 11 2023, 8:56 AM
michal edited the summary of this revision. (Show Details)Oct 16 2023, 3:17 AM
kamil accepted this revision.Oct 16 2023, 3:29 AM

LGTM, two notes:

  1. This will probably require changes in our docs to reflect new URLs
  2. Might be good for @ashoat to take a look as this will require adding a .json file to the prod keyserver
This revision is now accepted and ready to land.Oct 16 2023, 3:29 AM
ashoat added a comment.Oct 18 2023, 4:28 PM

Overall I agree with the renaming – this makes a lot more sense.

Won't we need a dev environment migration if we're changing the name of these URL files? We could make it happen with a MariaDB migration (keyserver/src/database/migration-config.js)... kind of weird pattern but it's what we've used before.

I can make the change manually in production (keyserver/.env before Docker build).

ashoat added a comment.Oct 18 2023, 5:18 PM

I think we will also likely need to update docs to mention /webapp/ instead of /comm/ for loading the web app

michal updated this revision to Diff 32272.Oct 20 2023, 9:06 AM

Added a migration for the new files alongside the nix automatically creating them on nix develop. I don't think we need to remove the old files, they will be just ignored and if someone had some custom configuration in it we don't want to override it. The docs changes are in the next diff because this diff doesn't change the behaviour and only handles adding the new files.

Harbormaster completed remote builds in B23405: Diff 32272.Oct 20 2023, 9:24 AM
ashoat accepted this revision.Oct 22 2023, 3:45 PM

Thanks!! Reminder to make sure you mention to me after you land this, so I can make the appropriate changes to keyserver/.env on my keyserver in production before deploying the changes

michal updated this revision to Diff 32498.Oct 30 2023, 7:18 AM

Rebase migration

Harbormaster failed remote builds in B23568: Diff 32498!Oct 30 2023, 7:28 AM
Harbormaster completed remote builds in B23568: Diff 32498.Oct 30 2023, 12:37 PM
Closed by commit rCOMMc9bae039ffac: [nix] Create new url facts (authored by Michal Gniadek <michal.gniadek@swmansion.com>). · Explain WhyOct 31 2023, 2:55 AM
This revision was automatically updated to reflect the committed changes.
Michal Gniadek <michal.gniadek@swmansion.com> added a commit: rCOMMc9bae039ffac: [nix] Create new url facts.

Revision Contents
Changeset List

PathSize
scripts/
16 lines
templates/
keyserver_url.json
commapp_url.json
4 lines
webapp_url.json
commapp_url.json
4 lines

Diff 31746

View Options

scripts/create_url_facts.sh

Loading...
View Options

scripts/templates/commapp_url.json

Loading...
View Options

scripts/templates/keyserver_url.json

Loading...
View Options

scripts/templates/squadcal_url.json

Loading...
View Options

scripts/templates/webapp_url.json

Loading...