Be able to use tls when connecting to Hawkular #14054
Conversation
|
@miq-bot add-label providers/hawkular |
| end | ||
|
|
||
| def connect(_options = {}) | ||
| @client ||= self.class.raw_connect(hostname, | ||
| port, | ||
| authentication_userid('default'), | ||
| authentication_password('default')) | ||
| authentication_password('default'), | ||
| security_protocol, |
There was a problem hiding this comment.
Consider explicit default_endpoint.security_protocol.
I think EMS delegating port, security_protocol etc. to default endpoint was put in place mostly for backward compatibility with code written before Endpoint table was extracted, but spelling it out is better and more readable.
[That's why when I added ssl_cert_store to Endpoint, I didn't delegate it from EMS; if you disagree with this reasoning, you may want to delegate that too.]
There was a problem hiding this comment.
I didn't know about that. I was actually wondering about it.
I'll update the code. Thanks.
| @@ -55,24 +55,42 @@ def validate_authentication_status | |||
| {:available => true, :message => nil} | |||
| end | |||
|
|
|||
| def self.verify_ssl_mode(security_protocol) | |||
| case security_protocol | |||
| when nil, '', 'non-ssl', 'ssl-without-validation' | |||
There was a problem hiding this comment.
nil, '' are for treating existing providers as non-ssl, right? Are you planning to migrate existing providers to have non-ssl in DB?
And for 'non-ssl' this value doesn't matter, only using HTTP does, right?
It seems then the handling of nil, '', 'non-ssl' better belongs in entrypoint() when deciding HTTP/HTTPS.
There was a problem hiding this comment.
nil, '' are for treating existing providers as non-ssl, right? Are you planning to migrate existing providers to have non-ssl in DB?
I don't have plans for that. Do you think I should?
And for 'non-ssl' this value doesn't matter, only using HTTP does, right?
right
| end | ||
|
|
||
| def self.entrypoint(host, port, security_protocol) | ||
| return URI::HTTP.build(:host => host, :port => port.to_i).to_s if security_protocol == 'non-ssl' |
There was a problem hiding this comment.
I think this should also use HTTP when security_protocol is missing (nil, '')? See also comment in verify_ssl_mode().
I suspect this is why some of your tests failed on missing https:// VCR where they previously worked with http://.
josejulio
force-pushed
the
hawkular/add_ssl_support
branch
from
90cbe35 to
3469919
Compare
josejulio
force-pushed
the
hawkular/add_ssl_support
branch
from
3469919 to
9b9a0ab
Compare
|
Checked commit josejulio@9b9a0ab with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
@cben followed your suggestions, Thank you. |
|
@cben this looks good to me, if you give me a green check mark in the "Reviewers" section, I'll merge this. |
Support for specifying a secure or non secure connection with or without verifying the identity.
UI PR ManageIQ/manageiq-ui-classic#460