-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(database): Add ssl support #777
base: main
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build c395699918b813e65764d4f4c2f40f755362a414-PR-777Details
💛 - Coveralls |
4f17a38
to
13150b2
Compare
Adds ssl/tls support for the edgehog <-> database communication. Closes edgehog-device-manager#419. Signed-off-by: Luca Zaninotto <[email protected]>
13150b2
to
c395699
Compare
certfile = System.get_env("DATABASE_SSL_CACERTFILE") | ||
|
||
case {certfile, use_os_certs} do | ||
{nil, false} -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should raise regardless in this condition.
While it is a good idea to handle the situation, given that we can inform the user about the correct environment variables to set, the user may also just want to use :verify_none
, in which case it does not need to set them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I should have made this a private function, but this gets called only from line 172, so only if the user explicitly set DATABASE_SSL_VERIFY=true
, in that case the configuration requires either a valid certificate or to use the os certificates
Adds ssl/tls support for the edgehog <-> database communication.
Closes #419.