UCX-3589 SSL certificate setup for UcxUcc#24
UCX-3589 SSL certificate setup for UcxUcc#24josephkabraham wants to merge 13 commits intoinfinityoneframework:emetrotelfrom
Conversation
smpallen99
left a comment
smpallen99
left a comment
There was a problem hiding this comment.
Some changes to address.
lib/ucx_ucc/application.ex
Outdated
| def start(type, args) do | ||
| import Supervisor.Spec | ||
|
|
||
| UcxUcc.CertManager.set_endpoint_certs! :ucx_ucc, UcxUccWeb.Endpoint |
There was a problem hiding this comment.
I'd like to move this stuff to the UcxAdapter plugin. You will need to use the plugin's Application callbacks to run this, similar to how you did the Licensing since the modules makes a lot of assumptions that are very specific to our production product.
There was a problem hiding this comment.
Moved this startup function call to ucx_adapter/application.ex
lib/ucx_ucc/ucx_ucc_cert_manager.ex
Outdated
| @@ -0,0 +1,96 @@ | |||
| # Copyright (C) E-MetroTel, 2018 - All Rights Reserved | |||
There was a problem hiding this comment.
Move this to plugins/ucx_adapter/lib/ucx_adapter/cert_manager.ex with the UcxAdapter.CertManager module name.
There was a problem hiding this comment.
Moved the source to ucx_adapter plugin.
lib/ucx_ucc/ucx_ucc_cert_manager.ex
Outdated
|
|
||
| get_cert_file # uses the default #{@ssl_conf_file} file | ||
|
|
||
| get_cert_file(:mscs) # gets the config filename from the config |
There was a problem hiding this comment.
this should be :ucx_ucc, not :mscs
There was a problem hiding this comment.
Updated comment.
lib/ucx_ucc/ucx_ucc_cert_manager.ex
Outdated
| File.stream!(ssl_conf_file, [], :line) | ||
| |> Enum.reduce([], fn(line, acc) -> | ||
| line = Regex.replace ~r/#.*/, line, "" | ||
| cond do |
There was a problem hiding this comment.
Few things in this block.
find_keyalready trims the string. So no need to do it a second time.- Code guidelines
cond do
value = find_key(@ssl_cert, line) ->
acc
|> Keyword.put(:certfile, value)
|> Keyword.put_new(:cacertfile, value)
value = find_key(@ssl_cert_key, line) ->
Keyword.put(acc, :keyfile, value)
value = find_key(@ssl_ca_cert, line) ->
Keyword.put(acc, :cacertfile, value))
true -> acc
end
There was a problem hiding this comment.
Updated code.
lib/ucx_ucc/ucx_ucc_cert_manager.ex
Outdated
| endpoint = Application.get_env(app, endpoint_mod) | ||
| https = get_cert_info(app) | ||
| |> Enum.reduce(endpoint[:https] || [], fn({k,v}, https) -> | ||
| if(Keyword.get(https, k) != nil) do |
There was a problem hiding this comment.
should never compare against nil. Use is_nil/1. Also no parens for an if statement.
if is_nil Keyword.get(https, k), do: Keyword.put(https, k, v), else: https
Actually, there is already an API for this. Keyword.put_new/3. This could be as simple as
|> Enum.reduce(endpoint[:https] || [], fn {k, v}, https -> Keyword.put_new(https, k, v) end)
also note that the args of a fn should not be in parens.
There was a problem hiding this comment.
Updated code.
2 participants
Implemented as per WebRTC client and tested with the updated self signed certificate.