You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2020/02/19 09:55:50 UTC
[GitHub] [knox] stoty opened a new pull request #267: KNOX-2238 CM discovery
- Add TLS support to Phoenix auto discovery
stoty opened a new pull request #267: KNOX-2238 CM discovery - Add TLS support to Phoenix auto discovery
URL: https://github.com/apache/knox/pull/267
## 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #267:
KNOX-2238 CM discovery - Add TLS support to Phoenix auto discovery
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #267: KNOX-2238 CM discovery - Add TLS support to Phoenix auto discovery
URL: https://github.com/apache/knox/pull/267#discussion_r381293198
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/model/phoenix/PhoenixServiceModelGenerator.java
##########
@@ -58,14 +59,24 @@ public ServiceModel generateService(ApiService service,
ApiServiceConfig serviceConfig,
ApiRole role,
ApiConfigList roleConfig) {
+
String hostname = role.getHostRef().getHostname();
- // Phoenix Query Server does not support https
+ String sslEnabledRaw = getRoleConfigValue(roleConfig, SSL_ENABLED);
String scheme = "http";
+
+ if (sslEnabledRaw != null) {
+ if (Boolean.parseBoolean(getRoleConfigValue(roleConfig, SSL_ENABLED))) {
Review comment:
If you are going to keep `sslEnabledRaw` - then no need to `null` check here and should use `sslEnabledRaw` inside the `parseBoolean`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk merged pull request #267: KNOX-2238 CM discovery -
Add TLS support to Phoenix auto discovery
Posted by GitBox <gi...@apache.org>.
risdenk merged pull request #267: KNOX-2238 CM discovery - Add TLS support to Phoenix auto discovery
URL: https://github.com/apache/knox/pull/267
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [knox] risdenk commented on a change in pull request #267:
KNOX-2238 CM discovery - Add TLS support to Phoenix auto discovery
Posted by GitBox <gi...@apache.org>.
risdenk commented on a change in pull request #267: KNOX-2238 CM discovery - Add TLS support to Phoenix auto discovery
URL: https://github.com/apache/knox/pull/267#discussion_r381292926
##########
File path: gateway-discovery-cm/src/main/java/org/apache/knox/gateway/topology/discovery/cm/model/phoenix/PhoenixServiceModelGenerator.java
##########
@@ -58,14 +59,24 @@ public ServiceModel generateService(ApiService service,
ApiServiceConfig serviceConfig,
ApiRole role,
ApiConfigList roleConfig) {
+
String hostname = role.getHostRef().getHostname();
- // Phoenix Query Server does not support https
+ String sslEnabledRaw = getRoleConfigValue(roleConfig, SSL_ENABLED);
Review comment:
This can be simplified. No need to store `sslEnabledRaw`. just do something like:
```
if (Boolean.parseBoolean(getRoleConfigValue(roleConfig, SSL_ENABLED))) {
scheme = "https";
}
```
`Boolean.parseBoolean` will handle null correctly and return false.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services