Page MenuHomePhabricator

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

Authored by varun on Fri, Mar 3, 9:35 PM.
Referenced Files
Unknown Object (File)
Sat, Mar 18, 3:36 AM
Unknown Object (File)
Fri, Mar 17, 9:51 PM
Unknown Object (File)
Fri, Mar 17, 5:58 PM
Unknown Object (File)
Thu, Mar 16, 2:23 AM
Unknown Object (File)
Sun, Mar 12, 2:08 PM
Unknown Object (File)
Sun, Mar 12, 2:07 PM
Unknown Object (File)
Sun, Mar 12, 2:07 PM
Unknown Object (File)
Sun, Mar 12, 10:45 AM



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

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

varun requested review of this revision.Fri, Mar 3, 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:

# 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

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. kind of already covers this, I'll just update the existing ticket.

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

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