Skip to content
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

KNOX-2238 CM discovery - Add TLS support to Phoenix auto discovery #267

Merged
merged 2 commits into from Feb 20, 2020

Conversation

stoty
Copy link
Contributor

@stoty stoty commented Feb 19, 2020

What changes were proposed in this pull request?

Add TLS support for Phoenix Query Server for the CM service discovery module

The TLS support was omitted from the original discovery module, as PQS didn't have TLS support yet.
Now I have re-added the TLS support. The code is a little different from most similar modules, because the ssl_enabled property is not present in older CM releases, and we have to handle that as well.

How was this patch tested?

  • Wrote and run new unit test
  • Tested against live Cloudera Manager instance which had the the TLS for PQS support
  • Tested against live older Cloudera Manager instance which did not have the TLS for PQS support

In both cases I was able to connect to the auto discovered PQS service.

Copy link
Contributor

@risdenk risdenk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some of the logic around sslEnabledRaw can be simplified. At minimum should only need one getRoleConfigValue(roleConfig, SSL_ENABLED) call.

@risdenk risdenk merged commit 7d4e997 into apache:master Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants