From 226bce4d78436c8df11442f80b5098bb88337b23 Mon Sep 17 00:00:00 2001 From: Sean Lin Date: Mon, 2 Mar 2020 09:41:52 -0800 Subject: [PATCH] Fix/another fix audit log not uploaded to s3 (#246) --- .../AuditLogsS3TimeBasedRollingPolicy.java | 1 - .../logger/service/S3LogUploaderService.java | 27 ++++++++++--------- .../AthenaAuditLoggerConfiguration.java | 20 +++++++++++--- .../service/S3LogUploaderServiceTest.java | 7 ++++- 4 files changed, 36 insertions(+), 19 deletions(-) diff --git a/cerberus-audit-logger-athena/src/main/java/ch/qos/logback/core/rolling/AuditLogsS3TimeBasedRollingPolicy.java b/cerberus-audit-logger-athena/src/main/java/ch/qos/logback/core/rolling/AuditLogsS3TimeBasedRollingPolicy.java index 6ca8dfe2b..42c97c976 100644 --- a/cerberus-audit-logger-athena/src/main/java/ch/qos/logback/core/rolling/AuditLogsS3TimeBasedRollingPolicy.java +++ b/cerberus-audit-logger-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/cerberus-audit-logger-athena/src/main/java/com/nike/cerberus/audit/logger/service/S3LogUploaderService.java b/cerberus-audit-logger-athena/src/main/java/com/nike/cerberus/audit/logger/service/S3LogUploaderService.java index 7618c0058..fad919816 100644 --- a/cerberus-audit-logger-athena/src/main/java/com/nike/cerberus/audit/logger/service/S3LogUploaderService.java +++ b/cerberus-audit-logger-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,6 +58,7 @@ public class S3LogUploaderService { private final String bucketRegion; private final boolean athenaLoggingEventListenerEnabled; private final AthenaService athenaService; + private Logger logger; @Autowired public S3LogUploaderService( @@ -65,11 +66,13 @@ public S3LogUploaderService( @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> 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 appender = (FiveMinuteRollingFileAppender) diff --git a/cerberus-audit-logger-athena/src/main/java/com/nike/cerberus/config/AthenaAuditLoggerConfiguration.java b/cerberus-audit-logger-athena/src/main/java/com/nike/cerberus/config/AthenaAuditLoggerConfiguration.java index 22827723c..7c6c7ebb2 100644 --- a/cerberus-audit-logger-athena/src/main/java/com/nike/cerberus/config/AthenaAuditLoggerConfiguration.java +++ b/cerberus-audit-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 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/cerberus-audit-logger-athena/src/test/java/com/nike/cerberus/audit/logger/service/S3LogUploaderServiceTest.java b/cerberus-audit-logger-athena/src/test/java/com/nike/cerberus/audit/logger/service/S3LogUploaderServiceTest.java index bcc9cb9a9..ea594dbf8 100644 --- a/cerberus-audit-logger-athena/src/test/java/com/nike/cerberus/audit/logger/service/S3LogUploaderServiceTest.java +++ b/cerberus-audit-logger-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