[GitHub] mgodave commented on a change in pull request #1505: Proxy admin web requests

2018-04-06 Thread GitBox
mgodave commented on a change in pull request #1505: Proxy admin web requests
URL: https://github.com/apache/incubator-pulsar/pull/1505#discussion_r179798442
 
 

 ##
 File path: pulsar-proxy/pom.xml
 ##
 @@ -70,6 +70,11 @@
   jetty-servlets
 
 
+
+  org.eclipse.jetty
+  jetty-proxy
 
 Review comment:
   I spent a day or two following another approach until I had enough and went 
searching for a better answer, this seemed to fit the bill perfectly. 


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mgodave commented on a change in pull request #1505: Proxy admin web requests

2018-04-06 Thread GitBox
mgodave commented on a change in pull request #1505: Proxy admin web requests
URL: https://github.com/apache/incubator-pulsar/pull/1505#discussion_r179798302
 
 

 ##
 File path: 
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/AdminProxyHandler.java
 ##
 @@ -0,0 +1,83 @@
+package org.apache.pulsar.proxy.server;
+
+import java.io.IOException;
+import java.security.cert.X509Certificate;
+import javax.net.ssl.SSLContext;
+import org.apache.pulsar.client.api.Authentication;
+import org.apache.pulsar.client.api.AuthenticationDataProvider;
+import org.apache.pulsar.client.api.AuthenticationFactory;
+import org.apache.pulsar.client.api.PulsarClientException;
+import org.apache.pulsar.common.util.SecurityUtility;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.proxy.ProxyServlet;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class AdminProxyHandler extends ProxyServlet.Transparent {
+private static final Logger LOG = 
LoggerFactory.getLogger(AdminProxyHandler.class);
+
+private final ProxyConfiguration config;
+
+public AdminProxyHandler(ProxyConfiguration config) {
+this.config = config;
+}
+
+@Override
+protected HttpClient newHttpClient() {
+try {
+Authentication auth = AuthenticationFactory.create(
+config.getBrokerClientAuthenticationPlugin(),
+config.getBrokerClientAuthenticationParameters()
+);
+
+if (auth != null) {
+auth.start();
+}
+
+boolean useTls = 
config.getBrokerServiceURL().startsWith("https://;);
+
+if (useTls) {
+try {
+X509Certificate trustCertificates[] = SecurityUtility
+
.loadCertificatesFromPemFile(config.getTlsTrustCertsFilePath());
+
+SSLContext sslCtx;
+AuthenticationDataProvider authData = auth.getAuthData();
+if (authData.hasDataForTls()) {
+sslCtx = SecurityUtility.createSslContext(
+config.isTlsAllowInsecureConnection(),
+trustCertificates,
+authData.getTlsCertificates(),
+authData.getTlsPrivateKey()
+);
+} else {
+sslCtx = SecurityUtility.createSslContext(
+config.isTlsAllowInsecureConnection(),
+trustCertificates
+);
+}
+
+SslContextFactory contextFactory = new SslContextFactory();
+contextFactory.setSslContext(sslCtx);
+return new HttpClient(contextFactory);
+} catch (Exception e) {
+try {
+if (auth != null) {
+auth.close();
+}
+} catch (IOException ioe) {
+LOG.error("Failed to close the authentication 
service", ioe);
+}
+throw new 
PulsarClientException.InvalidConfigurationException(e.getMessage());
+}
+}
+} catch (PulsarClientException e) {
+e.printStackTrace();
 
 Review comment:
   I think I just left this becuase I wasn't sure what to do at the time. It 
certainly shouldn't stay like this, let me take a look.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mgodave commented on a change in pull request #1505: Proxy admin web requests

2018-04-06 Thread GitBox
mgodave commented on a change in pull request #1505: Proxy admin web requests
URL: https://github.com/apache/incubator-pulsar/pull/1505#discussion_r179798123
 
 

 ##
 File path: pulsar-proxy/pom.xml
 ##
 @@ -70,6 +70,11 @@
   jetty-servlets
 
 
+
+  org.eclipse.jetty
+  jetty-proxy
 
 Review comment:
   It should, I've explicitly added the option but I will test this.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] mgodave commented on a change in pull request #1505: Proxy admin web requests

2018-04-06 Thread GitBox
mgodave commented on a change in pull request #1505: Proxy admin web requests
URL: https://github.com/apache/incubator-pulsar/pull/1505#discussion_r179798017
 
 

 ##
 File path: 
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyServiceStarter.java
 ##
 @@ -121,6 +121,7 @@ public void run() {
 server.addServlet("/metrics", new ServletHolder(MetricsServlet.class));
 server.addRestResources("/", VipStatus.class.getPackage().getName(),
 VipStatus.ATTRIBUTE_STATUS_FILE_PATH, 
config.getStatusFilePath());
+server.addServlet("/admin", new ServletHolder(new 
AdminProxyHandler(config)));
 
 Review comment:
   done


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services