Page MenuHomePhabricator

[keyserver] change how we load config for the identity service client
ClosedPublic

Authored by varun on Mar 3 2023, 9:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 15, 11:39 PM
Unknown Object (File)
Feb 22 2024, 1:53 PM
Unknown Object (File)
Feb 22 2024, 1:05 PM
Unknown Object (File)
Feb 22 2024, 12:00 PM
Unknown Object (File)
Feb 22 2024, 8:17 AM
Unknown Object (File)
Feb 22 2024, 1:02 AM
Unknown Object (File)
Feb 21 2024, 4:02 AM
Unknown Object (File)
Feb 21 2024, 4:02 AM
Subscribers

Details

Summary

in production, we load secrets/facts from files into environment variables. here, we read the environment variable that corresponds to the identity_service_config file, getting back stringified json. then we try to parse the string directly to an IdentityServiceConfig struct. This is all done inside a lazy_static so that we can access the config easily from the rust-node-addon.

If the env var isn't set or there's an error reading it, we'll fall back on the default value defined in the Default trait implementation of IdentityServiceConfig.

If we get back a JSON string that can't be parsed into an IdentityServiceConfig struct, we will panic, because that means our JSON file is malformed.

I wanted to just use the importJSON function in utils, but that meant either 1. calling js from Rust, which I couldn't figure out how to do, or 2. passing the relevant config to all the functions exposed by the RustAPI, which seemed like a bad practice.

Test Plan

set the COMM_JSONCONFIG_secrets_identity_service_config env var with valid and then invalid json. then i unset the variable

with valid json, I was able to connect to a manually deployed test identity server.

with invalid json, the keyserver crashed, as expected

with no env var set, the keyserver connected to my local instance of the identity service

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun requested review of this revision.Mar 3 2023, 9:51 PM

map camelCase JSON to snake_case struct

Seems reasonable, defer to others on Rust

I think short term this is fine.

Long term I would like to see us not have "fallback" behavior in services if a "magic file" is present.

For example:

bash
# explict arguments
identity server --grpc-port 50051 --auth-token $COMM_IDENTITY_SERVICE_AUTH_TOKEN
# config file
identity server --config /var/run/private/secrets.json
# environemnt variables
COMM_IDENTITY_SERVICE_AUTH_TOKEN=... COMM_IDENTITY_GRPC_PROT=50050 identity server

Having an environment variable, to point to a configuration file, which then contains the actual values seems like two layers of abstraction, when one should be needed.

https://linear.app/comm/issue/ENG-2963 kind of already covers this, I'll just update the existing ticket.

This revision is now accepted and ready to land.Mar 4 2023, 4:12 PM

not sure why i reviewed knowing full well it was relating to the keyserver code, then responded about the server.