knox-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From m...@apache.org
Subject [4/5] knox git commit: KNOX-1091 - Knox Audit Logging - duplicate correlation ids (Kevin Risden via Sandeep More)
Date Thu, 22 Mar 2018 17:52:38 GMT
KNOX-1091 - Knox Audit Logging - duplicate correlation ids (Kevin Risden via Sandeep More)


Project: http://git-wip-us.apache.org/repos/asf/knox/repo
Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/02f1b8b5
Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/02f1b8b5
Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/02f1b8b5

Branch: refs/heads/master
Commit: 02f1b8b5ad4ccf9936bb650c7228e401acee09cd
Parents: ee55e06
Author: Sandeep More <more@apache.org>
Authored: Wed Mar 21 13:12:24 2018 -0400
Committer: Sandeep More <more@apache.org>
Committed: Wed Mar 21 13:12:24 2018 -0400

----------------------------------------------------------------------
 .../org/apache/knox/gateway/GatewayFilter.java  |  21 +-
 .../knox/gateway/filter/CorrelationHandler.java |   7 +-
 .../apache/knox/gateway/AuditLoggingTest.java   |  63 ++++--
 .../apache/knox/test/log/CollectAppender.java   |   5 +-
 .../knox/gateway/GatewayCorrelationIdTest.java  | 205 +++++++++++++++++++
 .../src/test/resources/log4j.properties         |   3 +
 6 files changed, 274 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/knox/blob/02f1b8b5/gateway-server/src/main/java/org/apache/knox/gateway/GatewayFilter.java
----------------------------------------------------------------------
diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayFilter.java b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayFilter.java
index 25d4f75..b88691e 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayFilter.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayFilter.java
@@ -169,19 +169,7 @@ public class GatewayFilter implements Filter {
       servletRequest.setAttribute( AbstractGatewayFilter.TARGET_SERVICE_ROLE, chain.getResourceRole()
);
       try {
         chain.doFilter( servletRequest, servletResponse );
-      } catch( IOException e ) {
-        LOG.failedToExecuteFilter( e );
-        auditor.audit( Action.ACCESS, contextWithPathAndQuery, ResourceType.URI, ActionOutcome.FAILURE
);
-        throw e;
-      } catch( ServletException e ) {
-        LOG.failedToExecuteFilter( e );
-        auditor.audit( Action.ACCESS, contextWithPathAndQuery, ResourceType.URI, ActionOutcome.FAILURE
);
-        throw e;
-      } catch( RuntimeException e ) {
-        LOG.failedToExecuteFilter( e );
-        auditor.audit( Action.ACCESS, contextWithPathAndQuery, ResourceType.URI, ActionOutcome.FAILURE
);
-        throw e;
-      } catch( ThreadDeath e ) {
+      } catch( IOException | RuntimeException | ThreadDeath | ServletException e ) {
         LOG.failedToExecuteFilter( e );
         auditor.audit( Action.ACCESS, contextWithPathAndQuery, ResourceType.URI, ActionOutcome.FAILURE
);
         throw e;
@@ -189,11 +177,18 @@ public class GatewayFilter implements Filter {
         LOG.failedToExecuteFilter( e );
         auditor.audit( Action.ACCESS, contextWithPathAndQuery, ResourceType.URI, ActionOutcome.FAILURE
);
         throw new ServletException( e );
+      } finally {
+        // Make sure to destroy the correlationContext to prevent threading issues
+        CorrelationServiceFactory.getCorrelationService().detachContext();
       }
     } else {
       LOG.failedToMatchPath( requestPath );
       httpResponse.setStatus( HttpServletResponse.SC_NOT_FOUND );
+
+      // Make sure to destroy the correlationContext to prevent threading issues
+      CorrelationServiceFactory.getCorrelationService().detachContext();
     }
+    
     //KAM[ Don't do this or the Jetty default servlet will overwrite any response setup by
the filter.
     // filterChain.doFilter( servletRequest, servletResponse );
     //]

http://git-wip-us.apache.org/repos/asf/knox/blob/02f1b8b5/gateway-server/src/main/java/org/apache/knox/gateway/filter/CorrelationHandler.java
----------------------------------------------------------------------
diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/filter/CorrelationHandler.java
b/gateway-server/src/main/java/org/apache/knox/gateway/filter/CorrelationHandler.java
index 90933c3..c6aadc9 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/filter/CorrelationHandler.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/filter/CorrelationHandler.java
@@ -18,6 +18,7 @@
 package org.apache.knox.gateway.filter;
 
 import org.apache.knox.gateway.audit.api.CorrelationContext;
+import org.apache.knox.gateway.audit.api.CorrelationService;
 import org.apache.knox.gateway.audit.api.CorrelationServiceFactory;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.handler.HandlerWrapper;
@@ -33,13 +34,15 @@ public class CorrelationHandler extends HandlerWrapper {
   @Override
   public void handle( String target, Request baseRequest, HttpServletRequest request, HttpServletResponse
response )
       throws IOException, ServletException {
-    CorrelationContext correlationContext = CorrelationServiceFactory.getCorrelationService().createContext();
+    CorrelationService correlationService = CorrelationServiceFactory.getCorrelationService();
+    CorrelationContext correlationContext = correlationService.createContext();
     correlationContext.setRequestId( UUID.randomUUID().toString() );
     try {
       super.handle( target, baseRequest, request, response );
     } finally {
+      // Ensure that the correlationContext is destroyed between requests
       correlationContext.destroy();
+      correlationService.detachContext();
     }
   }
-
 }

http://git-wip-us.apache.org/repos/asf/knox/blob/02f1b8b5/gateway-server/src/test/java/org/apache/knox/gateway/AuditLoggingTest.java
----------------------------------------------------------------------
diff --git a/gateway-server/src/test/java/org/apache/knox/gateway/AuditLoggingTest.java b/gateway-server/src/test/java/org/apache/knox/gateway/AuditLoggingTest.java
index 03ee0d7..22bc308 100644
--- a/gateway-server/src/test/java/org/apache/knox/gateway/AuditLoggingTest.java
+++ b/gateway-server/src/test/java/org/apache/knox/gateway/AuditLoggingTest.java
@@ -26,9 +26,11 @@ import static org.junit.Assert.fail;
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
+import java.util.*;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
 
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
@@ -58,8 +60,12 @@ import org.easymock.EasyMock;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class AuditLoggingTest {
+  private static Logger LOG = LoggerFactory.getLogger( AuditLoggingTest.class );
+
   private static final String METHOD = "GET";
   private static final String PATH = "path";
   private static final String CONTEXT_PATH = "contextPath/";
@@ -86,7 +92,7 @@ public class AuditLoggingTest {
    * action=access request_type=uri outcome=unavailable
    * action=access request_type=uri outcome=success message=Response status: 404
    */
-  public void testNoFiltersAudit() throws ServletException, IOException {
+  public void testNoFiltersAudit() throws Exception {
     FilterConfig config = EasyMock.createNiceMock( FilterConfig.class );
     EasyMock.replay( config );
 
@@ -113,15 +119,48 @@ public class AuditLoggingTest {
     FilterChain chain = EasyMock.createNiceMock( FilterChain.class );
     EasyMock.replay( chain );
 
-    GatewayFilter gateway = new GatewayFilter();
-    gateway.init( config );
-    gateway.doFilter( request, response, chain );
-    gateway.destroy();
+    Random rnd = new Random();
+
+    // Make number of total requests between 1-100
+    int numberTotalRequests = rnd.nextInt(99) + 1;
+    Set<Callable<Void>> callables = new HashSet<>(numberTotalRequests);
+    for (int i = 0; i < numberTotalRequests; i++) {
+      callables.add(() -> {
+        GatewayFilter gateway = new GatewayFilter();
+        gateway.init( config );
+        gateway.doFilter( request, response, chain );
+        gateway.destroy();
+        return null;
+      });
+    }
 
-    assertThat( CollectAppender.queue.size(), is( 1 ) );
-    Iterator<LoggingEvent> iterator = CollectAppender.queue.iterator();
-    LoggingEvent accessEvent = iterator.next();
-    verifyAuditEvent( accessEvent, CONTEXT_PATH + PATH, ResourceType.URI, Action.ACCESS,
ActionOutcome.UNAVAILABLE, null, "Request method: GET" );
+    // Make number of concurrent requests between 1-10
+    int numberConcurrentRequests = rnd.nextInt( 9) + 1;
+
+    LOG.info("Executing %d total requests with %d concurrently", numberTotalRequests, numberConcurrentRequests);
+
+    ExecutorService executor = Executors.newFixedThreadPool(numberConcurrentRequests);
+    executor.invokeAll(callables);
+    executor.shutdown();
+    executor.awaitTermination(5, TimeUnit.SECONDS);
+    assertThat(executor.isTerminated(), is(true));
+
+    assertThat( CollectAppender.queue.size(), is( numberTotalRequests ) );
+
+    // Use a set to make sure to dedupe any requestIds to get only unique ones
+    Set<String> requestIds = new HashSet<>();
+    for (LoggingEvent accessEvent : CollectAppender.queue) {
+      verifyAuditEvent( accessEvent, CONTEXT_PATH + PATH, ResourceType.URI, Action.ACCESS,
ActionOutcome.UNAVAILABLE, null, "Request method: GET" );
+
+      CorrelationContext cc = (CorrelationContext)accessEvent.getMDC( Log4jCorrelationService.MDC_CORRELATION_CONTEXT_KEY
);
+      // There are some events that do not have a CorrelationContext associated (ie: deploy)
+      if(cc != null) {
+        requestIds.add(cc.getRequestId());
+      }
+    }
+
+    // There should be a unique correlation id for each request
+    assertThat(requestIds.size(), is(numberTotalRequests));
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/knox/blob/02f1b8b5/gateway-test-utils/src/main/java/org/apache/knox/test/log/CollectAppender.java
----------------------------------------------------------------------
diff --git a/gateway-test-utils/src/main/java/org/apache/knox/test/log/CollectAppender.java
b/gateway-test-utils/src/main/java/org/apache/knox/test/log/CollectAppender.java
index 3ab0c93..ff46e3e 100644
--- a/gateway-test-utils/src/main/java/org/apache/knox/test/log/CollectAppender.java
+++ b/gateway-test-utils/src/main/java/org/apache/knox/test/log/CollectAppender.java
@@ -29,9 +29,8 @@ public class CollectAppender extends AppenderSkeleton {
     super();
   }
 
-  public static BlockingQueue<LoggingEvent> queue = new LinkedBlockingQueue<LoggingEvent>();
-  public static boolean closed = false;
-
+  public static final BlockingQueue<LoggingEvent> queue = new LinkedBlockingQueue<>();
+  
   @Override
   protected void append( LoggingEvent event ) {
     event.getProperties();

http://git-wip-us.apache.org/repos/asf/knox/blob/02f1b8b5/gateway-test/src/test/java/org/apache/knox/gateway/GatewayCorrelationIdTest.java
----------------------------------------------------------------------
diff --git a/gateway-test/src/test/java/org/apache/knox/gateway/GatewayCorrelationIdTest.java
b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayCorrelationIdTest.java
new file mode 100644
index 0000000..b1b86fd
--- /dev/null
+++ b/gateway-test/src/test/java/org/apache/knox/gateway/GatewayCorrelationIdTest.java
@@ -0,0 +1,205 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway;
+
+import com.mycila.xmltool.XMLDoc;
+import com.mycila.xmltool.XMLTag;
+import org.apache.http.HttpStatus;
+import org.apache.knox.gateway.audit.api.CorrelationContext;
+import org.apache.knox.gateway.audit.log4j.correlation.Log4jCorrelationService;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.DefaultGatewayServices;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.test.TestUtils;
+import org.apache.knox.test.log.CollectAppender;
+import org.apache.log4j.spi.LoggingEvent;
+import org.hamcrest.MatcherAssert;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.net.URL;
+import java.util.*;
+import java.util.concurrent.*;
+
+import static io.restassured.RestAssured.given;
+import static org.apache.knox.test.TestUtils.LOG_ENTER;
+import static org.apache.knox.test.TestUtils.LOG_EXIT;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class GatewayCorrelationIdTest {
+
+  private static Logger LOG = LoggerFactory.getLogger( GatewayCorrelationIdTest.class );
+
+  public static GatewayConfig config;
+  public static GatewayServer gateway;
+  public static String gatewayUrl;
+  public static String clusterUrl;
+  private static GatewayTestDriver driver = new GatewayTestDriver();
+
+  @BeforeClass
+  public static void setupSuite() throws Exception {
+    LOG_ENTER();
+    URL resource = GatewayCorrelationIdTest.class.getClassLoader().getResource("users-dynamic.ldif");
+    assert resource != null;
+    driver.setupLdap( 0, new File(resource.toURI()) );
+    setupGateway();
+    CollectAppender.queue.clear();
+    LOG_EXIT();
+  }
+
+  @AfterClass
+  public static void cleanupSuite() throws Exception {
+    LOG_ENTER();
+    gateway.stop();
+    driver.cleanup();
+    CollectAppender.queue.clear();
+    LOG_EXIT();
+  }
+
+  public static void setupGateway() throws Exception {
+    File targetDir = new File( System.getProperty( "user.dir" ), "target" );
+    File gatewayDir = new File( targetDir, "gateway-home-" + UUID.randomUUID() );
+    gatewayDir.mkdirs();
+
+    GatewayTestConfig testConfig = new GatewayTestConfig();
+    config = testConfig;
+    testConfig.setGatewayHomeDir( gatewayDir.getAbsolutePath() );
+
+    File topoDir = new File( testConfig.getGatewayTopologyDir() );
+    topoDir.mkdirs();
+
+    File deployDir = new File( testConfig.getGatewayDeploymentDir() );
+    deployDir.mkdirs();
+
+    File descriptor = new File( topoDir, "test-cluster.xml" );
+    FileOutputStream stream = new FileOutputStream( descriptor );
+    createTopology().toStream( stream );
+    stream.close();
+
+    DefaultGatewayServices srvcs = new DefaultGatewayServices();
+    Map<String,String> options = new HashMap<>();
+    options.put( "persist-master", "false" );
+    options.put( "master", "password" );
+    try {
+      srvcs.init( testConfig, options );
+    } catch ( ServiceLifecycleException e ) {
+      e.printStackTrace(); // I18N not required.
+    }
+
+    gateway = GatewayServer.startGateway( testConfig, srvcs );
+    MatcherAssert.assertThat( "Failed to start gateway.", gateway, notNullValue() );
+
+    LOG.info( "Gateway port = " + gateway.getAddresses()[ 0 ].getPort() );
+
+    gatewayUrl = "http://localhost:" + gateway.getAddresses()[0].getPort() + "/" + config.getGatewayPath();
+    clusterUrl = gatewayUrl + "/test-cluster";
+  }
+
+  private static XMLTag createTopology() {
+    return XMLDoc.newDocument( true )
+        .addRoot( "topology" )
+        .addTag( "gateway" )
+        .addTag( "provider" )
+        .addTag( "role" ).addText( "authentication" )
+        .addTag( "name" ).addText( "ShiroProvider" )
+        .addTag( "enabled" ).addText( "true" )
+        .addTag( "param" )
+        .addTag( "name" ).addText( "main.ldapRealm" )
+        .addTag( "value" ).addText( "org.apache.knox.gateway.shirorealm.KnoxLdapRealm" ).gotoParent()
+        .addTag( "param" )
+        .addTag( "name" ).addText( "main.ldapRealm.userDnTemplate" )
+        .addTag( "value" ).addText( "uid={0},ou=people,dc=hadoop,dc=apache,dc=org" ).gotoParent()
+        .addTag( "param" )
+        .addTag( "name" ).addText( "main.ldapRealm.contextFactory.url" )
+        .addTag( "value" ).addText( driver.getLdapUrl() ).gotoParent()
+        .addTag( "param" )
+        .addTag( "name" ).addText( "main.ldapRealm.contextFactory.authenticationMechanism"
)
+        .addTag( "value" ).addText( "simple" ).gotoParent()
+        .addTag( "param" )
+        .addTag( "name" ).addText( "urls./**" )
+        .addTag( "value" ).addText( "authcBasic" ).gotoParent().gotoParent()
+        .addTag( "provider" )
+        .addTag( "role" ).addText( "identity-assertion" )
+        .addTag( "enabled" ).addText( "true" )
+        .addTag( "name" ).addText( "Default" ).gotoParent()
+        .addTag( "provider" )
+        .gotoRoot()
+        .addTag( "service" )
+        .addTag( "role" ).addText( "test-service-role" )
+        .gotoRoot();
+  }
+
+  @Test( timeout = TestUtils.MEDIUM_TIMEOUT )
+  public void testTestService() throws Exception {
+    LOG_ENTER();
+    String username = "guest";
+    String password = "guest-password";
+    String serviceUrl = clusterUrl + "/test-service-path/test-service-resource";
+
+    Random rnd = new Random();
+
+    // Make number of total requests between 1-100
+    int numberTotalRequests = rnd.nextInt(99) + 1;
+    Set<Callable<Void>> callables = new HashSet<>(numberTotalRequests);
+    for (int i = 0; i < numberTotalRequests; i++) {
+      callables.add(() -> {
+        given()
+            .auth().preemptive().basic( username, password )
+            .then()
+            .statusCode( HttpStatus.SC_OK )
+            .contentType( "text/plain" )
+            .body( is( "test-service-response" ) )
+            .when().get( serviceUrl );
+        return null;
+      });
+    }
+
+    // Make number of concurrent requests between 1-10
+    int numberConcurrentRequests = rnd.nextInt( 9) + 1;
+
+    LOG.info("Executing %d total requests with %d concurrently", numberTotalRequests, numberConcurrentRequests);
+
+    ExecutorService executor = Executors.newFixedThreadPool(numberConcurrentRequests);
+    executor.invokeAll(callables);
+    executor.shutdown();
+    executor.awaitTermination(5, TimeUnit.SECONDS);
+    assertThat(executor.isTerminated(), is(true));
+
+    // Use a set to make sure to dedupe any requestIds to get only unique ones
+    Set<String> requestIds = new HashSet<>();
+    for (LoggingEvent accessEvent : CollectAppender.queue) {
+      CorrelationContext cc = (CorrelationContext)accessEvent.getMDC( Log4jCorrelationService.MDC_CORRELATION_CONTEXT_KEY
);
+      // There are some events that do not have a CorrelationContext associated (ie: deploy)
+      if(cc != null) {
+        requestIds.add(cc.getRequestId());
+      }
+    }
+
+    // There should be a unique correlation id for each request
+    assertThat(requestIds.size(), is(numberTotalRequests));
+    
+    LOG_EXIT();
+  }
+}

http://git-wip-us.apache.org/repos/asf/knox/blob/02f1b8b5/gateway-test/src/test/resources/log4j.properties
----------------------------------------------------------------------
diff --git a/gateway-test/src/test/resources/log4j.properties b/gateway-test/src/test/resources/log4j.properties
index f3ee344..201b537 100644
--- a/gateway-test/src/test/resources/log4j.properties
+++ b/gateway-test/src/test/resources/log4j.properties
@@ -23,6 +23,9 @@ log4j.appender.stdout=org.apache.log4j.ConsoleAppender
 log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
 log4j.appender.stdout.layout.ConversionPattern=%5p [%c] %m%n
 
+log4j.logger.audit = INFO, collectappender
+log4j.appender.collectappender = org.apache.knox.test.log.CollectAppender
+
 #log4j.logger.org.apache.knox.gateway=DEBUG
 #log4j.logger.org.apache.knox.test=DEBUG
 #log4j.logger.org.apache.knox.gateway.http=TRACE


Mime
View raw message