Fix/another fix audit log not uploaded to s3 (#246)

Nike-Inc · Mar 2, 2020 · 226bce4 · 226bce4
1 parent 9d47010
commit 226bce4
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 19 deletions.
diff --git a/...r-athena/src/main/java/ch/qos/logback/core/rolling/AuditLogsS3TimeBasedRollingPolicy.java b/...r-athena/src/main/java/ch/qos/logback/core/rolling/AuditLogsS3TimeBasedRollingPolicy.java
@@ -41,7 +41,6 @@ public AuditLogsS3TimeBasedRollingPolicy(
     this.bucketRegion = bucketRegion;
   }
 
-  @Autowired
   public void setS3LogUploaderService(S3LogUploaderService s3LogUploaderService) {
     this.s3LogUploaderService = s3LogUploaderService;
     if (logChunkFileS3Queue.size() > 0) {

diff --git a/...ger-athena/src/main/java/com/nike/cerberus/audit/logger/service/S3LogUploaderService.java b/...ger-athena/src/main/java/com/nike/cerberus/audit/logger/service/S3LogUploaderService.java
@@ -16,11 +16,13 @@
 
 package com.nike.cerberus.audit.logger.service;
 
+import ch.qos.logback.classic.Logger;
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.rolling.AuditLogsS3TimeBasedRollingPolicy;
 import ch.qos.logback.core.rolling.FiveMinuteRollingFileAppender;
 import com.amazonaws.services.s3.AmazonS3;
 import com.nike.cerberus.audit.logger.S3ClientFactory;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.File;
 import java.util.Optional;
 import java.util.concurrent.ExecutorService;
@@ -31,8 +33,6 @@
 import javax.annotation.PreDestroy;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.io.FileUtils;
-import org.apache.commons.io.FilenameUtils;
-import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
@@ -58,18 +58,21 @@ public class S3LogUploaderService {
   private final String bucketRegion;
   private final boolean athenaLoggingEventListenerEnabled;
   private final AthenaService athenaService;
+  private Logger logger;
 
   @Autowired
   public S3LogUploaderService(
       @Value("${cerberus.audit.athena.bucket}") String bucket,
       @Value("${cerberus.audit.athena.bucketRegion}") String bucketRegion,
       @Value("${cerberus.audit.athena.enabled:false}") boolean athenaLoggingEventListenerEnabled,
       AthenaService athenaService,
-      S3ClientFactory s3ClientFactory) {
+      S3ClientFactory s3ClientFactory,
+      ch.qos.logback.classic.Logger logger) {
     this.bucket = bucket;
     this.bucketRegion = bucketRegion;
     this.athenaLoggingEventListenerEnabled = athenaLoggingEventListenerEnabled;
     this.athenaService = athenaService;
+    this.logger = logger;
 
     amazonS3 = s3ClientFactory.getClient(bucketRegion);
 
@@ -123,28 +126,27 @@ public void ingestLog(String filename) {
    * @param filename The file to upload to s3
    * @param retryCount The retry count
    */
+  @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN")
   private void processLogFile(String filename, int retryCount) {
-    String filteredFilename = FilenameUtils.getName(filename);
-    log.info(
-        "process log file called with filename: {}, retry count: {}", filteredFilename, retryCount);
-    final File rolledLogFile = new File(filteredFilename);
+    log.info("process log file called with filename: {}, retry count: {}", filename, retryCount);
+    final File rolledLogFile = new File(filename);
     // poll for 30 seconds waiting for file to exist or bail
     int i = 0;
     do {
       sleep(1, TimeUnit.SECONDS);
       log.info(
           "Does '{}' exist: {}, length: {}, can read: {}, poll count: {}",
-          filteredFilename,
+          filename,
           rolledLogFile.exists(),
           rolledLogFile.length(),
           rolledLogFile.canRead(),
           i);
       i++;
-    } while (!rolledLogFile.exists() || i >= 30);
+    } while (!rolledLogFile.exists() && i <= 30);
 
     // if file does not exist or empty, do nothing
     if (!rolledLogFile.exists() || rolledLogFile.length() == 0) {
-      log.error("File '{}' does not exist or is empty returning", filteredFilename);
+      log.error("File '{}' does not exist or is empty returning", filename);
       return;
     }
 
@@ -165,7 +167,7 @@ private void processLogFile(String filename, int retryCount) {
           e);
       if (retryCount < 10) {
         sleep(1, TimeUnit.SECONDS);
-        processLogFile(filteredFilename, retryCount + 1);
+        processLogFile(filename, retryCount + 1);
       }
       throw e;
     }
@@ -194,8 +196,7 @@ public void executeServerShutdownHook() {
   private Optional<AuditLogsS3TimeBasedRollingPolicy<ILoggingEvent>> getRollingPolicy() {
 
     if (athenaLoggingEventListenerEnabled) {
-      ch.qos.logback.classic.Logger auditLogger =
-          (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(ATHENA_LOG_NAME);
+      ch.qos.logback.classic.Logger auditLogger = this.logger;
 
       FiveMinuteRollingFileAppender<ILoggingEvent> appender =
           (FiveMinuteRollingFileAppender<ILoggingEvent>)

diff --git a/...-logger-athena/src/main/java/com/nike/cerberus/config/AthenaAuditLoggerConfiguration.java b/...-logger-athena/src/main/java/com/nike/cerberus/config/AthenaAuditLoggerConfiguration.java
@@ -25,9 +25,12 @@
 import ch.qos.logback.core.util.FileSize;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
+import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.ComponentScan;
@@ -49,8 +52,17 @@ public class AthenaAuditLoggerConfiguration {
 
   @Autowired
   public AthenaAuditLoggerConfiguration(
+      @Value("${cerberus.audit.athena.log.path:#{null}}") String logPath,
       AuditLogsS3TimeBasedRollingPolicy<ILoggingEvent> auditLogsS3TimeBasedRollingPolicy) {
 
+    if (StringUtils.isBlank(logPath)) {
+      logPath = "";
+    } else if (!logPath.endsWith("/")) {
+      logPath += "/";
+      FilenameUtils.getPath(
+          logPath); // this shouldn't be necessary because the path is provided by Spring config,
+      // but extra safety
+    }
     this.auditLogsS3TimeBasedRollingPolicy = auditLogsS3TimeBasedRollingPolicy;
 
     LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory();
@@ -73,12 +85,12 @@ public AthenaAuditLoggerConfiguration(
         new FiveMinuteRollingFileAppender<>();
     fiveMinuteRollingFileAppender.setName(ATHENA_LOG_APPENDER_NAME);
     fiveMinuteRollingFileAppender.setContext(loggerContext);
-    fiveMinuteRollingFileAppender.setFile(hostname + "-audit.log");
+    fiveMinuteRollingFileAppender.setFile(logPath + hostname + "-audit.log");
     fiveMinuteRollingFileAppender.setEncoder(patternLayoutEncoder);
 
     this.auditLogsS3TimeBasedRollingPolicy.setContext(loggerContext);
     this.auditLogsS3TimeBasedRollingPolicy.setFileNamePattern(
-        hostname + "-audit.%d{yyyy-MM-dd-HH-mm, UTC}.log.gz");
+        logPath + hostname + "-audit.%d{yyyy-MM-dd_HH-mm, UTC}.log.gz");
     this.auditLogsS3TimeBasedRollingPolicy.setMaxHistory(100);
     this.auditLogsS3TimeBasedRollingPolicy.setParent(fiveMinuteRollingFileAppender);
     this.auditLogsS3TimeBasedRollingPolicy.setTotalSizeCap(FileSize.valueOf("10gb"));
@@ -97,7 +109,7 @@ public AthenaAuditLoggerConfiguration(
   }
 
   @Bean
-  public Logger getAthenaAuditLogger() {
-    return athenaAuditLogger;
+  public ch.qos.logback.classic.Logger getAthenaAuditLogger() {
+    return (ch.qos.logback.classic.Logger) athenaAuditLogger;
   }
 }
diff --git a/...athena/src/test/java/com/nike/cerberus/audit/logger/service/S3LogUploaderServiceTest.java b/...athena/src/test/java/com/nike/cerberus/audit/logger/service/S3LogUploaderServiceTest.java
@@ -19,6 +19,8 @@
 import static junit.framework.TestCase.assertEquals;
 import static org.mockito.MockitoAnnotations.initMocks;
 
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.LoggerContext;
 import com.nike.cerberus.audit.logger.S3ClientFactory;
 import org.junit.Before;
 import org.junit.Test;
@@ -32,11 +34,14 @@ public class S3LogUploaderServiceTest {
 
   private S3LogUploaderService s3LogUploader;
 
+  private Logger logger = new LoggerContext().getLogger("test-logger");
+
   @Before
   public void before() {
     initMocks(this);
     s3LogUploader =
-        new S3LogUploaderService("fake-bucket", "us-west-2", true, athenaService, s3ClientFactory);
+        new S3LogUploaderService(
+            "fake-bucket", "us-west-2", true, athenaService, s3ClientFactory, logger);
   }
 
   @Test