From f7d8b3321f9cdbfc98955d677e5ca98060d6137b Mon Sep 17 00:00:00 2001 From: Ashish Aggarwal Date: Sat, 17 Aug 2019 09:25:50 -0700 Subject: [PATCH] refactored logger that leaks credentials (#14) * refactored logger that leaks credentials * tweaking minimum code coverage --- .../blobs/dispatcher/s3/S3Dispatcher.java | 30 ++++++++++++------- .../dispatcher/s3/UploadProgressListener.java | 9 ++++-- pom.xml | 2 +- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/haystack-blobs/blobs-agent-dispatchers/src/main/java/com/expedia/www/haystack/agent/blobs/dispatcher/s3/S3Dispatcher.java b/haystack-blobs/blobs-agent-dispatchers/src/main/java/com/expedia/www/haystack/agent/blobs/dispatcher/s3/S3Dispatcher.java index 5f20932..ee39438 100644 --- a/haystack-blobs/blobs-agent-dispatchers/src/main/java/com/expedia/www/haystack/agent/blobs/dispatcher/s3/S3Dispatcher.java +++ b/haystack-blobs/blobs-agent-dispatchers/src/main/java/com/expedia/www/haystack/agent/blobs/dispatcher/s3/S3Dispatcher.java @@ -94,7 +94,11 @@ public class S3Dispatcher implements BlobDispatcher, AutoCloseable { private Timer dispatchTimer; @VisibleForTesting - S3Dispatcher(TransferManager transferManager, String bucketName, Boolean shouldWaitForUpload, int maxOutstandingRequests, CompressDecompressService compressDecompressService) { + S3Dispatcher(final TransferManager transferManager, + final String bucketName, + final Boolean shouldWaitForUpload, + final int maxOutstandingRequests, + final CompressDecompressService compressDecompressService) { this.transferManager = transferManager; this.bucketName = bucketName; this.shouldWaitForUpload = shouldWaitForUpload; @@ -213,7 +217,7 @@ public void initialize(final Config s3Config) { dispatchFailureMeter = SharedMetricRegistry.newMeter("s3.dispatch.failure"); dispatchTimer = SharedMetricRegistry.newTimer("s3.dispatch.timer"); - LOGGER.info("Successfully initialized the S3 dispatcher with config={}", s3Config); + LOGGER.info("Successfully initialized the S3 dispatcher..."); } private static TransferManager createTransferManager(final Config config) { @@ -231,15 +235,20 @@ private static TransferManager createTransferManager(final Config config) { } final AmazonS3 s3 = getS3Client(config, clientConfiguration); - - return TransferManagerBuilder.standard().withS3Client(s3) - .withMultipartUploadThreshold(MULTIPART_UPLOAD_THRESHOLD).build(); + return TransferManagerBuilder.standard() + .withS3Client(s3) + .withMultipartUploadThreshold(MULTIPART_UPLOAD_THRESHOLD) + .build(); } private static AmazonS3 getS3Client(Config config, ClientConfiguration clientConfiguration) { final String region = config.getString(REGION_PROPERTY); - final boolean pathStyleAccessEnabled = config.hasPath(AWS_PATH_STYLE_ACCESS_ENABLED) ? config.getBoolean(AWS_PATH_STYLE_ACCESS_ENABLED) : false; - final boolean disableChunkedEncoding = config.hasPath(AWS_DISABLE_CHUNKED_ENCODING) ? config.getBoolean(AWS_DISABLE_CHUNKED_ENCODING) : false; + + final boolean pathStyleAccessEnabled = config.hasPath(AWS_PATH_STYLE_ACCESS_ENABLED) && + config.getBoolean(AWS_PATH_STYLE_ACCESS_ENABLED); + + final boolean disableChunkedEncoding = config.hasPath(AWS_DISABLE_CHUNKED_ENCODING) && + config.getBoolean(AWS_DISABLE_CHUNKED_ENCODING); AmazonS3ClientBuilder s3ClientBuilder = AmazonS3ClientBuilder.standard() .withCredentials(buildCredentialProvider(config)) @@ -247,7 +256,8 @@ private static AmazonS3 getS3Client(Config config, ClientConfiguration clientCon .withPathStyleAccessEnabled(pathStyleAccessEnabled); if (config.hasPath(AWS_SERVICE_ENDPOINT)) { - s3ClientBuilder.withEndpointConfiguration(new AwsClientBuilder.EndpointConfiguration(config.getString(AWS_SERVICE_ENDPOINT), region)); + s3ClientBuilder.withEndpointConfiguration(new + AwsClientBuilder.EndpointConfiguration(config.getString(AWS_SERVICE_ENDPOINT), region)); } else { s3ClientBuilder.withRegion(region); } @@ -272,7 +282,8 @@ static AWSCredentialsProvider buildCredentialProvider(final Config config) { } CompressDecompressService.CompressionType findCompressionType(Config config) { - final String compressionType = config.hasPath(COMPRESSION_TYPE_CONFIG_KEY) ? config.getString(COMPRESSION_TYPE_CONFIG_KEY).toUpperCase() : "NONE"; + final String compressionType = config.hasPath(COMPRESSION_TYPE_CONFIG_KEY) ? + config.getString(COMPRESSION_TYPE_CONFIG_KEY).toUpperCase() : "NONE"; return CompressDecompressService.CompressionType.valueOf(compressionType); } @@ -289,5 +300,4 @@ public void close() { transferManager.shutdownNow(); } } - } diff --git a/haystack-blobs/blobs-agent-dispatchers/src/main/java/com/expedia/www/haystack/agent/blobs/dispatcher/s3/UploadProgressListener.java b/haystack-blobs/blobs-agent-dispatchers/src/main/java/com/expedia/www/haystack/agent/blobs/dispatcher/s3/UploadProgressListener.java index 8f276ba..96c5880 100644 --- a/haystack-blobs/blobs-agent-dispatchers/src/main/java/com/expedia/www/haystack/agent/blobs/dispatcher/s3/UploadProgressListener.java +++ b/haystack-blobs/blobs-agent-dispatchers/src/main/java/com/expedia/www/haystack/agent/blobs/dispatcher/s3/UploadProgressListener.java @@ -13,7 +13,10 @@ class UploadProgressListener implements ProgressListener { private final Meter dispatchFailureMeter; private final Timer.Context dispatchTimer; - UploadProgressListener(Logger logger, String objectKey, Meter dispatchFailureMeter, Timer.Context dispatchTimer) { + UploadProgressListener(final Logger logger, + final String objectKey, + final Meter dispatchFailureMeter, + final Timer.Context dispatchTimer) { Validate.notNull(logger); Validate.notEmpty(objectKey); Validate.notNull(dispatchFailureMeter); @@ -26,7 +29,7 @@ class UploadProgressListener implements ProgressListener { } @Override - public void progressChanged(ProgressEvent progressEvent) { + public void progressChanged(final ProgressEvent progressEvent) { final String msg = String.format("Progress event=%s file=%s transferred=%d", progressEvent.getEventType(), key, progressEvent.getBytesTransferred()); switch (progressEvent.getEventType()) { @@ -42,9 +45,11 @@ public void progressChanged(ProgressEvent progressEvent) { break; case TRANSFER_PART_FAILED_EVENT: logger.error(msg); + close(); break; default: logger.info(msg); + close(); break; } } diff --git a/pom.xml b/pom.xml index 8f13d74..9be738e 100644 --- a/pom.xml +++ b/pom.xml @@ -313,7 +313,7 @@ LINE COVEREDRATIO - 0.69 + 0.67