HADOOP-18820. checkstyle and minor code cleanups

biggest change is that S3AInternals exports getBucketMetadata(),

this was used in a test, and its output indirectly available on getXattrs(/).

Change-Id: Iaf45ccbdd778be474e6ece07275ba179c215cf83

Author: steveloughran

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java

@@ -1444,6 +1444,17 @@ public HeadObjectResponse getObjectMetadata(Path path) throws IOException {
               "getObjectMetadata"));
     }
 
+    /**
+     * S3AInternals method.
+     * {@inheritDoc}.
+     */
+    @Override
+    @AuditEntryPoint
+    @Retries.RetryTranslated
+    public HeadBucketResponse getBucketMetadata() throws IOException {
+      return S3AFileSystem.this.getBucketMetadata();
+    }
+
     /**
      * Get a shared copy of the AWS credentials, with its reference
      * counter updated.
@@ -2821,7 +2832,8 @@ protected HeadObjectResponse getObjectMetadata(String key,
    * @throws UnknownStoreException the bucket is absent
    * @throws IOException  any other problem talking to S3
    */
-  @Retries.RetryRaw
+  @AuditEntryPoint
+  @Retries.RetryTranslated
   protected HeadBucketResponse getBucketMetadata() throws IOException {
     final HeadBucketResponse response = trackDurationAndSpan(STORE_EXISTS_PROBE, bucket, null,
         () -> invoker.retry("getBucketMetadata()", bucket, true, () -> {

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AInternals.java

@@ -22,6 +22,7 @@
 import java.nio.file.AccessDeniedException;
 
 import software.amazon.awssdk.services.s3.S3Client;
+import software.amazon.awssdk.services.s3.model.HeadBucketResponse;
 import software.amazon.awssdk.services.s3.model.HeadObjectResponse;
 
 import org.apache.hadoop.classification.InterfaceAudience;
@@ -96,4 +97,14 @@ public interface S3AInternals {
    * @return a reference to shared credentials.
    */
   AWSCredentialProviderList shareCredentials(String purpose);
+
+  /**
+   * Request bucket metadata.
+   * @return the metadata
+   * @throws UnknownStoreException the bucket is absent
+   * @throws IOException  any other problem talking to S3
+   */
+  @AuditEntryPoint
+  @Retries.RetryTranslated
+  HeadBucketResponse getBucketMetadata() throws IOException;
 }

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/SimpleAWSCredentialsProvider.java

@@ -61,7 +61,7 @@ public class SimpleAWSCredentialsProvider implements AwsCredentialsProvider {
    */
   public SimpleAWSCredentialsProvider(final URI uri, final Configuration conf)
       throws IOException {
-      this(getAWSAccessKeys(uri, conf));
+    this(getAWSAccessKeys(uri, conf));
   }
 
   /**
@@ -73,8 +73,8 @@ public SimpleAWSCredentialsProvider(final URI uri, final Configuration conf)
   @VisibleForTesting
   SimpleAWSCredentialsProvider(final S3xLoginHelper.Login login)
       throws IOException {
-      this.accessKey = login.getUser();
-      this.secretKey = login.getPassword();
+    this.accessKey = login.getUser();
+    this.secretKey = login.getPassword();
   }
 
   @Override

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/api/RequestFactory.java

@@ -41,13 +41,15 @@
 import software.amazon.awssdk.services.s3.model.StorageClass;
 import software.amazon.awssdk.services.s3.model.UploadPartRequest;
 
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
 import org.apache.hadoop.fs.PathIOException;
 import org.apache.hadoop.fs.s3a.S3AEncryptionMethods;
 import org.apache.hadoop.fs.s3a.auth.delegation.EncryptionSecrets;
 import org.apache.hadoop.fs.s3a.impl.PutObjectOptions;
 
 /**
- * Factory for S3 objects.
+ * Factory for S3 request objects.
  *
  * This is where the owner FS's {@code prepareRequest()}
  * callback is invoked to mark up a request for this span.
@@ -61,6 +63,8 @@
  * as there are no guarantees how they are processed.
  * That is: no guarantees of retry or translation.
  */
+@InterfaceStability.Unstable
+@InterfaceAudience.LimitedPrivate("testing/diagnostics")
 public interface RequestFactory {
 
   /**

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AAWSCredentialsProvider.java

@@ -80,7 +80,7 @@ static class BadCredentialsProviderConstructor
       implements AwsCredentialsProvider {
 
     @SuppressWarnings("unused")
-    public BadCredentialsProviderConstructor(String fsUri, Configuration conf) {
+    BadCredentialsProviderConstructor(String fsUri, Configuration conf) {
     }
 
     @Override
@@ -125,7 +125,7 @@ private void createFailingFS(Configuration conf) throws IOException {
   static class BadCredentialsProvider implements AwsCredentialsProvider {
 
     @SuppressWarnings("unused")
-    public BadCredentialsProvider(Configuration conf) {
+    BadCredentialsProvider(Configuration conf) {
     }
 
     @Override

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3ABucketExistence.java

@@ -131,7 +131,7 @@ private Configuration createConfigurationWithProbe(final int probe) {
         ENDPOINT,
         AWS_REGION);
     conf.setInt(S3A_BUCKET_PROBE, probe);
-    conf.set(AWS_REGION, "eu-west-1");
+    conf.set(AWS_REGION, EU_WEST_1);
     return conf;
   }
 
@@ -210,7 +210,7 @@ public void testAccessPointRequired() throws Exception {
    */
   private Configuration createArnConfiguration() {
     Configuration configuration = createConfigurationWithProbe(2);
-    configuration.set(AWS_REGION, "eu-west-1");
+    configuration.set(AWS_REGION, EU_WEST_1);
     return configuration;
   }
 

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java

@@ -19,8 +19,8 @@
 package org.apache.hadoop.fs.s3a;
 
 import java.io.File;
-import java.io.IOException;
 import java.net.URI;
+import java.nio.file.AccessDeniedException;
 import java.security.PrivilegedExceptionAction;
 
 import org.assertj.core.api.Assertions;
@@ -38,7 +38,6 @@
 import software.amazon.awssdk.services.s3.S3Client;
 import software.amazon.awssdk.services.s3.S3Configuration;
 import software.amazon.awssdk.services.s3.model.HeadBucketRequest;
-import software.amazon.awssdk.services.s3.model.S3Exception;
 import software.amazon.awssdk.services.sts.StsClient;
 import software.amazon.awssdk.services.sts.model.StsException;
 
@@ -60,6 +59,7 @@
 
 import static java.util.Objects.requireNonNull;
 import static org.apache.hadoop.fs.s3a.Constants.*;
+import static org.apache.hadoop.fs.s3a.S3ATestConstants.EU_WEST_1;
 import static org.apache.hadoop.fs.s3a.S3AUtils.*;
 import static org.apache.hadoop.fs.s3a.S3ATestUtils.*;
 import static org.apache.hadoop.test.LambdaTestUtils.intercept;
@@ -134,7 +134,6 @@ public void testEndpoint() throws Exception {
     } else {
       conf.set(Constants.ENDPOINT, endpoint);
       fs = S3ATestUtils.createTestFileSystem(conf);
-      S3Client s3 = getS3Client("test endpoint");
       String endPointRegion = "";
       // Differentiate handling of "s3-" and "s3." based endpoint identifiers
       String[] endpointParts = StringUtils.split(endpoint, '.');
@@ -173,7 +172,7 @@ protected void useFailFastConfiguration() {
   }
 
   /**
-   * Expect a filesystem to not be created from a configuration
+   * Expect a filesystem to not be created from a configuration.
    * @return the exception intercepted
    * @throws Exception any other exception
    */
@@ -540,36 +539,34 @@ public void testConfOptionPropagationToFS() throws Exception {
   }
 
   @Test(timeout = 10_000L)
-  public void testS3SpecificSignerOverride() throws IOException {
+  public void testS3SpecificSignerOverride() throws Exception {
     Configuration config = new Configuration();
+    removeBaseAndBucketOverrides(config,
+        CUSTOM_SIGNERS, SIGNING_ALGORITHM_S3, SIGNING_ALGORITHM_STS, AWS_REGION);
 
     config.set(CUSTOM_SIGNERS,
-        "CustomS3Signer:" + CustomS3Signer.class.getName() + ",CustomSTSSigner:"
-            + CustomSTSSigner.class.getName());
+        "CustomS3Signer:" + CustomS3Signer.class.getName()
+            + ",CustomSTSSigner:" + CustomSTSSigner.class.getName());
 
     config.set(SIGNING_ALGORITHM_S3, "CustomS3Signer");
     config.set(SIGNING_ALGORITHM_STS, "CustomSTSSigner");
 
-    config.set(AWS_REGION, "eu-west-1");
+    config.set(AWS_REGION, EU_WEST_1);
     fs = S3ATestUtils.createTestFileSystem(config);
 
     S3Client s3Client = getS3Client("testS3SpecificSignerOverride");
 
+    final String bucket = fs.getBucket();
     StsClient stsClient =
-        STSClientFactory.builder(config, fs.getBucket(), new AnonymousAWSCredentialsProvider(), "",
+        STSClientFactory.builder(config, bucket, new AnonymousAWSCredentialsProvider(), "",
             "").build();
 
-    try {
-      stsClient.getSessionToken();
-    } catch (StsException exception) {
-      // Expected 403, as credentials are not provided.
-    }
+    intercept(StsException.class, "", () ->
+        stsClient.getSessionToken());
 
-    try {
-      s3Client.headBucket(HeadBucketRequest.builder().bucket(fs.getBucket()).build());
-    } catch (S3Exception exception) {
-      // Expected 403, as credentials are not provided.
-    }
+    intercept(AccessDeniedException.class, "", () ->
+        Invoker.once("head", bucket, () ->
+            s3Client.headBucket(HeadBucketRequest.builder().bucket(bucket).build())));
 
     Assertions.assertThat(CustomS3Signer.isS3SignerCalled())
         .describedAs("Custom S3 signer not called").isTrue();

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEndpointRegion.java

@@ -25,7 +25,6 @@
 import java.util.List;
 
 import org.assertj.core.api.Assertions;
-import org.junit.Assert;
 import org.junit.Test;
 import software.amazon.awssdk.awscore.AwsExecutionAttribute;
 import software.amazon.awssdk.awscore.exception.AwsServiceException;
@@ -34,14 +33,12 @@
 import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
 import software.amazon.awssdk.services.s3.S3Client;
 import software.amazon.awssdk.services.s3.model.HeadBucketRequest;
-import software.amazon.awssdk.services.s3.model.S3Exception;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.s3a.statistics.impl.EmptyS3AStatisticsContext;
 
 import static org.apache.hadoop.fs.s3a.Constants.AWS_REGION;
 import static org.apache.hadoop.fs.s3a.Statistic.STORE_REGION_PROBE;
-import static org.apache.hadoop.fs.s3a.impl.InternalConstants.SC_301_MOVED_PERMANENTLY;
 import static org.apache.hadoop.test.LambdaTestUtils.intercept;
 
 /**
@@ -68,13 +65,7 @@ public void testWithoutRegionConfig() throws IOException {
     S3AFileSystem fs = new S3AFileSystem();
     fs.initialize(getFileSystem().getUri(), conf);
 
-    try  {
-      fs.getBucketMetadata();
-    } catch (S3Exception exception) {
-      if (exception.statusCode() == SC_301_MOVED_PERMANENTLY) {
-        Assert.fail(exception.toString());
-      }
-    }
+    fs.getS3AInternals().getBucketMetadata();
 
     Assertions.assertThat(fs.getInstrumentation().getCounterValue(STORE_REGION_PROBE))
         .describedAs("Region is not configured, region probe should have been made").isEqualTo(1);

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AMiscOperations.java

@@ -412,7 +412,7 @@ private static <T> T verifyNoTrailingSlash(String role, T o) {
   private GetBucketEncryptionResponse getDefaultEncryption() throws IOException {
     S3AFileSystem fs = getFileSystem();
     S3Client s3 = getS3AInternals().getAmazonS3V2ClientForTesting("check default encryption");
-    try {
+    try (AuditSpan s = span()){
       return Invoker.once("getBucketEncryption()",
           fs.getBucket(),
           () -> s3.getBucketEncryption(GetBucketEncryptionRequest.builder()

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestConstants.java

@@ -251,4 +251,9 @@ public interface S3ATestConstants {
    * Value: {@value}.
    */
   String PROJECT_BUILD_DIRECTORY_PROPERTY = "project.build.directory";
+
+  /**
+   * AWS ireland region.
+   */
+  String EU_WEST_1 = "eu-west-1";
 }