This is an automated email from the ASF dual-hosted git repository.

mmerli pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pulsar.git


The following commit(s) were added to refs/heads/master by this push:
     new 56ebe36  ProxyForwardAuthDataTest shouldn't reuse pulsar client (#1230)
56ebe36 is described below

commit 56ebe368ec2eb8569218ca4b438ee53f2bf5145e
Author: Ivan Kelly <iv...@apache.org>
AuthorDate: Tue Feb 13 18:54:29 2018 +0100

    ProxyForwardAuthDataTest shouldn't reuse pulsar client (#1230)
    
    * ProxyForwardAuthDataTest shouldn't reuse pulsar client
    
    This test was flaking due to the pulsar client being reused. The test
    starts a proxy service, subscribes to a topic, stops the proxy, starts
    a new proxy and tries to subscribe again.
    
    If using the same client for both connections, the client will have an
    open connection to the first proxy, which, when the proxy is stopped,
    will be closed by netty asynchronously. This can race with the second
    subscription attempt, and cause it to fail with
    ConnectionClosedException. Specifically, if the subscription attempt
    runs before the netty callback runs, the subscription attempt will try
    to subscribe on a dead connection.
    
    The immediate fix for this test, to stop the flaking, is just to use a
    different client for each attempt.
    
    * Remove trailing whitespace
---
 .../proxy/server/ProxyForwardAuthDataTest.java     | 55 +++++++++-------------
 1 file changed, 23 insertions(+), 32 deletions(-)

diff --git 
a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyForwardAuthDataTest.java
 
b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyForwardAuthDataTest.java
index 2309ebb..d4b8e4d 100644
--- 
a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyForwardAuthDataTest.java
+++ 
b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyForwardAuthDataTest.java
@@ -25,7 +25,6 @@ import java.util.Set;
 
 import org.apache.bookkeeper.test.PortManager;
 import org.apache.pulsar.client.admin.PulsarAdmin;
-import org.apache.pulsar.client.api.Consumer;
 import org.apache.pulsar.client.api.ConsumerConfiguration;
 import org.apache.pulsar.client.api.ProducerConsumerBase;
 import org.apache.pulsar.client.api.PulsarClient;
@@ -48,7 +47,7 @@ public class ProxyForwardAuthDataTest extends 
ProducerConsumerBase {
     private static final Logger log = 
LoggerFactory.getLogger(ProxyForwardAuthDataTest.class);
     private int webServicePort;
     private int servicePort;
-    
+
     @BeforeMethod
     @Override
     protected void setup() throws Exception {
@@ -60,11 +59,11 @@ public class ProxyForwardAuthDataTest extends 
ProducerConsumerBase {
         
conf.setBrokerClientAuthenticationPlugin(BasicAuthentication.class.getName());
         conf.setBrokerClientAuthenticationParameters("authParam:broker");
         conf.setAuthenticateOriginalAuthData(true);
-        
+
         Set<String> superUserRoles = new HashSet<String>();
         superUserRoles.add("admin");
         conf.setSuperUserRoles(superUserRoles);
-        
+
         Set<String> providers = new HashSet<String>();
         providers.add(BasicAuthenticationProvider.class.getName());
         conf.setAuthenticationProviders(providers);
@@ -79,9 +78,9 @@ public class ProxyForwardAuthDataTest extends 
ProducerConsumerBase {
 
     @Override
     protected void cleanup() throws Exception {
-        super.internalCleanup();       
+        super.internalCleanup();
     }
-    
+
     @Test
     void testForwardAuthData() throws Exception {
         log.info("-- Starting {} test --", methodName);
@@ -95,15 +94,13 @@ public class ProxyForwardAuthDataTest extends 
ProducerConsumerBase {
         String subscriptionName = "my-subscriber-name";
         String clientAuthParams = "authParam:client";
         String proxyAuthParams = "authParam:proxy";
-        
+
         admin.properties().createProperty("my-property",
                 new PropertyAdmin(Lists.newArrayList("appid1", "appid2"), 
Sets.newHashSet("use")));
         admin.namespaces().createNamespace(namespaceName);
-        
         admin.namespaces().grantPermissionOnNamespace(namespaceName, "proxy", 
Sets.newHashSet(AuthAction.consume, AuthAction.produce));
         admin.namespaces().grantPermissionOnNamespace(namespaceName, "client", 
Sets.newHashSet(AuthAction.consume, AuthAction.produce));
 
-        
         // Step 2: Run Pulsar Proxy without forwarding authData - expect 
Exception
         ProxyConfiguration proxyConfig = new ProxyConfiguration();
         proxyConfig.setAuthenticationEnabled(true);
@@ -111,35 +108,29 @@ public class ProxyForwardAuthDataTest extends 
ProducerConsumerBase {
         proxyConfig.setServicePort(servicePort);
         proxyConfig.setWebServicePort(webServicePort);
         proxyConfig.setBrokerServiceURL("pulsar://localhost:" + BROKER_PORT);
-        
         
proxyConfig.setBrokerClientAuthenticationPlugin(BasicAuthentication.class.getName());
         proxyConfig.setBrokerClientAuthenticationParameters(proxyAuthParams);
 
         Set<String> providers = new HashSet<>();
         providers.add(BasicAuthenticationProvider.class.getName());
         proxyConfig.setAuthenticationProviders(providers);
-        ProxyService proxyService = new ProxyService(proxyConfig);
-
-        proxyService.start();
-        PulsarClient proxyClient = createPulsarClient(proxyServiceUrl, 
clientAuthParams);
-        Consumer consumer;
-        boolean exceptionOccured = false;
-        try {
-            consumer = proxyClient.subscribe(topicName, subscriptionName);
-        } catch(Exception ex) {
-            exceptionOccured  = true;
-        }         
-        Assert.assertTrue(exceptionOccured);
-        proxyService.close();
-        
+
+        try (ProxyService proxyService = new ProxyService(proxyConfig);
+             PulsarClient proxyClient = createPulsarClient(proxyServiceUrl, 
clientAuthParams)) {
+            proxyService.start();
+            proxyClient.subscribe(topicName, subscriptionName);
+            Assert.fail("Shouldn't be able to subscribe, auth required");
+        } catch (PulsarClientException.AuthorizationException e) {
+            // expected behaviour
+        }
+
         // Step 3: Create proxy with forwardAuthData enabled
         proxyConfig.setForwardAuthorizationCredentials(true);
-        proxyService = new ProxyService(proxyConfig);
-
-        proxyService.start();
-        consumer = proxyClient.subscribe(topicName, subscriptionName);   
-        Assert.assertTrue(exceptionOccured);
-        proxyService.close();
+        try (ProxyService proxyService = new ProxyService(proxyConfig);
+             PulsarClient proxyClient = createPulsarClient(proxyServiceUrl, 
clientAuthParams)) {
+            proxyService.start();
+            proxyClient.subscribe(topicName, subscriptionName).close();
+        }
     }
 
     private void createAdminClient() throws PulsarClientException {
@@ -147,9 +138,9 @@ public class ProxyForwardAuthDataTest extends 
ProducerConsumerBase {
         org.apache.pulsar.client.api.ClientConfiguration clientConf = new 
org.apache.pulsar.client.api.ClientConfiguration();
         clientConf.setAuthentication(BasicAuthentication.class.getName(), 
adminAuthParams);
 
-        admin = spy(new PulsarAdmin(brokerUrl, clientConf));        
+        admin = spy(new PulsarAdmin(brokerUrl, clientConf));
     }
-    
+
     private PulsarClient createPulsarClient(String proxyServiceUrl, String 
authParams) throws PulsarClientException {
         org.apache.pulsar.client.api.ClientConfiguration clientConf = new 
org.apache.pulsar.client.api.ClientConfiguration();
         clientConf.setAuthentication(BasicAuthentication.class.getName(), 
authParams);

-- 
To stop receiving notification emails like this one, please contact
mme...@apache.org.

Reply via email to