This is an automated email from the ASF dual-hosted git repository.
more pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new a7eed67f8 KNOX-2730 - Fix missing Correlation id in audit logs (#558)
a7eed67f8 is described below
commit a7eed67f81799f2bba24d7f22aeb49a893d6a526
Author: Sandeep Moré <[email protected]>
AuthorDate: Fri Apr 15 05:15:37 2022 -0400
KNOX-2730 - Fix missing Correlation id in audit logs (#558)
---
.../org/apache/knox/gateway/GatewayFilter.java | 29 ++++++++++++++++------
.../org/apache/knox/gateway/GatewayServlet.java | 28 ++++-----------------
.../org/apache/knox/gateway/AuditLoggingTest.java | 13 +++++++---
.../org/apache/knox/gateway/GatewayFilterTest.java | 6 +++--
4 files changed, 40 insertions(+), 36 deletions(-)
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 73ed23f08..9b8a9480f 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
@@ -95,8 +95,14 @@ public class GatewayFilter implements Filter {
@Override
public void doFilter( ServletRequest servletRequest, ServletResponse
servletResponse, FilterChain filterChain ) throws IOException, ServletException
{
doFilter( servletRequest, servletResponse );
- if( filterChain != null ) {
- filterChain.doFilter( servletRequest, servletResponse );
+ try {
+ if (filterChain != null) {
+ filterChain.doFilter(servletRequest, servletResponse);
+ }
+ } finally {
+ auditLog(servletRequest, servletResponse);
+ // Make sure to destroy the correlationContext to prevent threading
issues
+ CorrelationServiceFactory.getCorrelationService().detachContext();
}
}
@@ -187,16 +193,10 @@ 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.
@@ -254,6 +254,19 @@ public class GatewayFilter implements Filter {
}
}
+ private void auditLog(ServletRequest servletRequest, ServletResponse
servletResponse) {
+ final int status = ((HttpServletResponse) servletResponse).getStatus();
+ final String requestUri, actionOutcome;
+ if (HttpServletResponse.SC_SERVICE_UNAVAILABLE == status) {
+ requestUri = ServletRequestUtils.getContextPathWithQuery(servletRequest);
+ actionOutcome = ActionOutcome.UNAVAILABLE;
+ } else {
+ requestUri = (String)
servletRequest.getAttribute(AbstractGatewayFilter.SOURCE_REQUEST_CONTEXT_URL_ATTRIBUTE_NAME);
+ actionOutcome = ActionOutcome.SUCCESS;
+ }
+ auditor.audit(Action.ACCESS, requestUri, ResourceType.URI, actionOutcome,
RES.responseStatus(status));
+ }
+
private class Chain implements FilterChain {
private List<Holder> chainList;
private String resourceRole;
diff --git
a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java
b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java
index 785bc3973..3621bb66b 100644
--- a/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java
+++ b/gateway-server/src/main/java/org/apache/knox/gateway/GatewayServlet.java
@@ -17,23 +17,17 @@
*/
package org.apache.knox.gateway;
-import org.apache.knox.gateway.audit.api.Action;
-import org.apache.knox.gateway.audit.api.ActionOutcome;
import org.apache.knox.gateway.audit.api.AuditService;
import org.apache.knox.gateway.audit.api.AuditServiceFactory;
-import org.apache.knox.gateway.audit.api.Auditor;
-import org.apache.knox.gateway.audit.api.ResourceType;
-import org.apache.knox.gateway.audit.log4j.audit.AuditConstants;
+import org.apache.knox.gateway.audit.api.CorrelationServiceFactory;
import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.descriptor.GatewayDescriptor;
import org.apache.knox.gateway.descriptor.GatewayDescriptorFactory;
-import org.apache.knox.gateway.filter.AbstractGatewayFilter;
import org.apache.knox.gateway.i18n.messages.MessagesFactory;
import org.apache.knox.gateway.i18n.resources.ResourcesFactory;
import org.apache.knox.gateway.services.ServiceType;
import org.apache.knox.gateway.services.GatewayServices;
import org.apache.knox.gateway.services.metrics.MetricsService;
-import org.apache.knox.gateway.util.ServletRequestUtils;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
@@ -60,8 +54,6 @@ public class GatewayServlet implements Servlet, Filter {
private static final GatewayMessages LOG = MessagesFactory.get(
GatewayMessages.class );
private static final AuditService auditService =
AuditServiceFactory.getAuditService();
- private static final Auditor auditor =
AuditServiceFactory.getAuditService().getAuditor(AuditConstants.DEFAULT_AUDITOR_NAME,
AuditConstants.KNOX_SERVICE_NAME,
- AuditConstants.KNOX_COMPONENT_NAME);
private FilterConfigAdapter filterConfig;
private GatewayFilter filter;
@@ -140,9 +132,10 @@ public class GatewayServlet implements Servlet, Filter {
} else {
((HttpServletResponse)servletResponse).setStatus(
HttpServletResponse.SC_SERVICE_UNAVAILABLE );
}
- auditLog(servletRequest, servletResponse);
} finally {
auditService.detachContext();
+ // Make sure to destroy the correlationContext to prevent threading
issues
+ CorrelationServiceFactory.getCorrelationService().detachContext();
}
}
@@ -168,24 +161,13 @@ public class GatewayServlet implements Servlet, Filter {
} else {
((HttpServletResponse)servletResponse).setStatus(
HttpServletResponse.SC_SERVICE_UNAVAILABLE );
}
- auditLog(servletRequest, servletResponse);
} finally {
auditService.detachContext();
+ // Make sure to destroy the correlationContext to prevent threading
issues
+ CorrelationServiceFactory.getCorrelationService().detachContext();
}
}
- private void auditLog(ServletRequest servletRequest, ServletResponse
servletResponse) {
- final int status = ((HttpServletResponse) servletResponse).getStatus();
- final String requestUri, actionOutcome;
- if (HttpServletResponse.SC_SERVICE_UNAVAILABLE == status) {
- requestUri = ServletRequestUtils.getContextPathWithQuery(servletRequest);
- actionOutcome = ActionOutcome.UNAVAILABLE;
- } else {
- requestUri = (String)
servletRequest.getAttribute(AbstractGatewayFilter.SOURCE_REQUEST_CONTEXT_URL_ATTRIBUTE_NAME);
- actionOutcome = ActionOutcome.SUCCESS;
- }
- auditor.audit(Action.ACCESS, requestUri, ResourceType.URI, actionOutcome,
res.responseStatus(status));
- }
@Override
public String getServletInfo() {
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 3741f296d..2a5d27ce8 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
@@ -30,6 +30,7 @@ import
org.apache.knox.gateway.audit.log4j.audit.Log4jAuditContext;
import org.apache.knox.gateway.audit.log4j.correlation.Log4jCorrelationContext;
import org.apache.knox.gateway.config.GatewayConfig;
import org.apache.knox.gateway.dispatch.DefaultDispatch;
+import org.apache.knox.gateway.filter.AbstractGatewayFilter;
import org.apache.knox.test.log.CollectAppender;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -110,6 +111,9 @@ public class AuditLoggingTest {
EasyMock.expect( request.getServletContext() ).andReturn( context
).anyTimes();
EasyMock.expect( context.getAttribute(
GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(gatewayConfig).anyTimes();
+ EasyMock.expect( request.getAttribute(
+ AbstractGatewayFilter.SOURCE_REQUEST_CONTEXT_URL_ATTRIBUTE_NAME))
+ .andReturn( CONTEXT_PATH+PATH ).anyTimes();
EasyMock.expect(gatewayConfig.getHeaderNameForRemoteAddress()).andReturn(
"Custom-Forwarded-For").anyTimes();
EasyMock.replay( request );
@@ -148,12 +152,14 @@ public class AuditLoggingTest {
executor.awaitTermination(5, TimeUnit.SECONDS);
assertThat(executor.isTerminated(), is(true));
- assertThat( CollectAppender.queue.size(), is( numberTotalRequests ) );
+ assertThat( CollectAppender.queue.size(), is( numberTotalRequests * 2) );
// Use a set to make sure to dedupe any requestIds to get only unique ones
Set<String> requestIds = new HashSet<>();
for (LogEvent accessEvent : CollectAppender.queue) {
- verifyAuditEvent( accessEvent, CONTEXT_PATH + PATH, ResourceType.URI,
Action.ACCESS, ActionOutcome.UNAVAILABLE, null, "Request method: GET" );
+ verifyAuditEvent( accessEvent, CONTEXT_PATH + PATH, ResourceType.URI,
+ Action.ACCESS, accessEvent.getContextData().getValue("outcome"),
+ null, accessEvent.getMessage().getFormattedMessage() );
CorrelationContext cc = Log4jCorrelationContext.of(accessEvent);
// There are some events that do not have a CorrelationContext
associated (ie: deploy)
@@ -210,7 +216,8 @@ public class AuditLoggingTest {
gateway.doFilter( request, response, chain );
gateway.destroy();
- assertThat( CollectAppender.queue.size(), is( 1 ) );
+ // Now there are two audit log messages
+ assertThat( CollectAppender.queue.size(), is( 2 ) );
Iterator<LogEvent> iterator = CollectAppender.queue.iterator();
LogEvent accessEvent = iterator.next();
verifyAuditEvent( accessEvent, CONTEXT_PATH + PATH, ResourceType.URI,
diff --git
a/gateway-server/src/test/java/org/apache/knox/gateway/GatewayFilterTest.java
b/gateway-server/src/test/java/org/apache/knox/gateway/GatewayFilterTest.java
index a349819a9..32cbb0625 100644
---
a/gateway-server/src/test/java/org/apache/knox/gateway/GatewayFilterTest.java
+++
b/gateway-server/src/test/java/org/apache/knox/gateway/GatewayFilterTest.java
@@ -229,6 +229,8 @@ public class GatewayFilterTest {
EasyMock.replay(response,request,requestNoID,context,gatewayConfig);
+ FilterChain chain = EasyMock.createNiceMock( FilterChain.class );
+ EasyMock.replay( chain );
TestCorrelationFilter filter = new TestCorrelationFilter();
@@ -236,12 +238,12 @@ public class GatewayFilterTest {
GatewayFilter gateway = new GatewayFilter();
gateway.addFilter( "test-path/**", "test-filter", filter, null,
"test-role" );
gateway.init( config );
- gateway.doFilter( request, response );
+ gateway.doFilter( request, response, chain );
assertThat(filter.request_id, is( TEST_REQ_ID ) );
assertThat(filter.correlation_id, is( TEST_REQ_ID ) );
/* test the case where request id for request coming to knox is absent */
- gateway.doFilter( requestNoID, response );
+ gateway.doFilter( requestNoID, response, chain );
assertThat(filter.request_id, nullValue() );
assertThat(filter.correlation_id, notNullValue() );
assertThat(filter.correlation_id, not( TEST_REQ_ID ) );
