Closed filoozom closed 4 years ago
@filoozom Thanks for the pull request. I think this looks like its off to a good start. I'll ask @jasonbukowski to also take a look.
For your questions:
For the question about what to write in the logText
I'd suggest we write an INFO
log line indicating that an ENV VAR based HMAC was provided. Don't echo the HMAC. (Although an argument could be made to echo the first few chars of that HMAC just so you could see if it matches what you'd expect as a Node operator, without revealing it in case of copy/paste of logs)
I think I would still suggest calling authKeysUpdate
to write the pair to RocksDB. I think that it would be reasonable to skip the backup though as all values are known.
Unless @jasonbukowski has any concerns I'd press forward with the doc changes in .env.sample
and docker-compose.yaml
in this repo. A separate pull request to apply those changes to chainpoint-node
repo would be fine as well.
Looks good to me so far.
I agree we should not echo the HMAC. Something like c392*******************************************************8063
could be helpful though instead, as @grempe pointed out.
Proposal
Add a
CHAINPOINT_NODE_HMAC_KEY
environment variable to make it easier to seed nodes, as everything can now be configured from a single place (at least if the node has been registered before).This is a rough WIP to make sure we're on the same page.
A few more steps to do:
CHAINPOINT_NODE_HMAC_KEY
documentation to.env.sample
anddocker-compose.yaml
CHAINPOINT_NODE_HMAC_KEY
to https://github.com/chainpoint/chainpoint-node if mergedA few questions:
authKeysUpdate
and/orbackupAuthKeysAsync
if the env var was provided? I guess I'd prefer to skip both of them, thoughts?logText
) for env var?Prettier
Not quite sure what's wrong with
prettier
yet as it changed some things automatically.