[keyserver] configure separation of primary secondary logic
ClosedPublic
Actions

Authored by will on Jul 9 2024, 7:21 PM.

Details

Reviewers

ashoat
varun

Commits

rCOMM4163b6376fd6: [keyserver] configure separation of primary secondary logic

Summary

This introduces COMM_NODE_ROLE which specifies whether a node is a primary or secondary node.

Certain logic in keyserver/src/keyserver.js should only be performed by a primary node. This includes:

run the migration
establish tunnelbroker connection
print the QR Code
create authoritative keyserver config files

Both secondary and primary nodes should still perform:

have the master process create fork processes
route express traffic
verify the user is logged in

Depends on D12691

Test Plan

Confirmed through logs that secondary nodes didn't establish tunnelbroker connection.
Will test further if migration is primary only later in stack with migration script

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

will created this revision.Jul 9 2024, 7:21 PM

Herald added a subscriber: tomek. · View Herald TranscriptJul 9 2024, 7:21 PM

will edited the summary of this revision. (Show Details)Jul 9 2024, 7:22 PM

will added a parent revision: D12691: [terraform] include REDIS_URL as part of keyserver terraform config.

will added a child revision: D12709: [terraform] Ensure only one running ecs task for primary service.Jul 9 2024, 7:25 PM

Harbormaster completed remote builds in B30249: Diff 42188.Jul 9 2024, 7:39 PM

will requested review of this revision.Jul 9 2024, 7:39 PM

ashoat accepted this revision.Jul 10 2024, 10:23 AM

ashoat added inline comments.

docs/nix_keyserver_deployment.md
40–42 ↗	(On Diff #42188)	Wouldn't have thought to update this doc given it's focused on the Docker Compose workflow. But I guess it doesn't hurt Let's make these edits though
keyserver/src/keyserver.js
93–99 ↗	(On Diff #42188)	I think a ternary expression would be both more readable and more concise here: const isPrimaryNode = process.env.COMM_NODE_ROLE ? process.env.COMM_NODE_ROLE === 'primary' : true;
112 ↗	(On Diff #42188)	We'll probably end up reworking this logic at a later point... I think we should probably start with `verifyUserLoggedIn`, and only if it fails should we consider displaying the QR code in the terminal But `shouldDisplayQRCodeInTerminal` is hardcoded to `false` right now so it doesn't really matter We can probably rethink this once we update `shouldDisplayQRCodeInTerminal`. cc @varun