Page MenuHomePhabricator

[keyserver] Create json files with authoritative keyserver id
ClosedPublic

Authored by inka on Mar 8 2024, 2:45 AM.
Tags
None
Referenced Files
F3294867: D11282.diff
Sat, Nov 16, 8:23 PM
Unknown Object (File)
Wed, Nov 13, 4:35 AM
Unknown Object (File)
Wed, Nov 13, 4:35 AM
Unknown Object (File)
Wed, Nov 13, 4:35 AM
Unknown Object (File)
Wed, Nov 13, 4:35 AM
Unknown Object (File)
Wed, Nov 13, 4:35 AM
Unknown Object (File)
Wed, Nov 13, 4:35 AM
Unknown Object (File)
Oct 12 2024, 3:33 AM
Subscribers

Details

Summary

issue: ENG-7136
These files are used to configure the authoritative keyservers data on the clients. They are created by the keyserver, because to know the id the keyserver has to first log in.

One effect of this, is that the keyserver will have to be run before web and native the first time. I should mention that in the doc. I made a note to do that in ENG-7173

Test Plan

Tested that the files are correctly created when the keyserver is run

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

inka requested review of this revision.Mar 8 2024, 3:14 AM
keyserver/src/user/create-configs.js
9 ↗(On Diff #37945)

Should this only happen in dev mode?

33 ↗(On Diff #37945)

Should we be worried about using the sync versions of these calls instead of the async ones?

42 ↗(On Diff #37945)

Does this make sense in the Docker environment? Shouldn't we be using env vars there?

46 ↗(On Diff #37945)

Is this line longer than 80 chars? Can it be avoided?

keyserver/src/user/create-configs.js
42 ↗(On Diff #37945)

I will make this code be run only in dev mode, so it won't be run in Docker, since Docker keyserver is always prod.

46 ↗(On Diff #37945)

For some reason eslint/prettier doesn't allow

const keyserverAuthoritativeKeyserverFile = 
    `${keyserverFactsFolder}/${authoritativeKeyserverFile}`;

but also doesn't really tell why

image.png (238×1 px, 95 KB)

keyserver/src/keyserver.js
111 ↗(On Diff #37967)

I think it would be good to make it more clear that this only affects dev here. It would also be good to make it clear what config is being generated.

Two potential suggestions:

  1. Rename to createAuthoritativeKeyserverConfigFilesOnDev
  2. Rename to createAuthoritativeKeyserverConfigFiles and lift the process.env.NODE_ENV !== 'development' check to here

Make the code more readable

This revision is now accepted and ready to land.Mar 12 2024, 9:36 AM