S3 credentials should be optional (#501)

artipie · Apr 26, 2023 · 72cf14c · 72cf14c
1 parent 5a2b260
commit 72cf14c
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 42 deletions.
diff --git a/asto-core/src/main/java/com/artipie/asto/factory/Config.java b/asto-core/src/main/java/com/artipie/asto/factory/Config.java
@@ -42,6 +42,13 @@ public interface Config {
      */
     Config config(String key);
 
+    /**
+     * Checks that there is a config data.
+     *
+     * @return True if no config data.
+     */
+    boolean isEmpty();
+
     /**
      * Strict storage config throws {@code NullPointerException} when value is not exist.
      *
@@ -86,6 +93,11 @@ public Config config(final String key) {
                 String.format("No config found for key %s", key)
             );
         }
+
+        @Override
+        public boolean isEmpty() {
+            return this.original == null || this.original.isEmpty();
+        }
     }
 
     /**
@@ -149,6 +161,11 @@ public Config config(final String key) {
             return new YamlStorageConfig(this.original.yamlMapping(key));
         }
 
+        @Override
+        public boolean isEmpty() {
+            return this.original == null || this.original.isEmpty();
+        }
+
         @Override
         public String toString() {
             return this.original.toString();

diff --git a/asto-s3/src/main/java/com/artipie/asto/s3/S3StorageFactory.java b/asto-s3/src/main/java/com/artipie/asto/s3/S3StorageFactory.java
@@ -45,40 +45,43 @@ private static S3AsyncClient s3Client(final Config cfg) {
         final S3AsyncClientBuilder builder = S3AsyncClient.builder();
         Optional.ofNullable(cfg.string("region")).ifPresent(val -> builder.region(Region.of(val)));
         endpoint(cfg).ifPresent(val -> builder.endpointOverride(URI.create(val)));
-        return builder
-            .credentialsProvider(
-                S3StorageFactory.credentials(
-                    new Config.StrictStorageConfig(cfg)
-                        .config("credentials")
-                )
-            )
-            .build();
+        setCredentialsProvider(builder, cfg);
+        return builder.build();
     }
 
     /**
-     * Creates {@link StaticCredentialsProvider} instance based on config.
+     * Sets a credentials provider into the passed builder.
      *
-     * @param cred Credentials config.
-     * @return Credentials provider.
+     * @param builder Builder.
+     * @param cfg S3 storage configuration.
      */
-    private static StaticCredentialsProvider credentials(final Config cred) {
-        final String type = cred.string("type");
-        if ("basic".equals(type)) {
-            return StaticCredentialsProvider.create(
-                AwsBasicCredentials.create(
-                    cred.string("accessKeyId"),
-                    cred.string("secretAccessKey")
-                )
-            );
-        } else {
-            throw new IllegalArgumentException(
-                String.format("Unsupported S3 credentials type: %s", type)
-            );
+    private static void setCredentialsProvider(
+        final S3AsyncClientBuilder builder,
+        final Config cfg
+    ) {
+        final Config credentials = cfg.config("credentials");
+        if (!credentials.isEmpty()) {
+            final String type = credentials.string("type");
+            if ("basic".equals(type)) {
+                builder.credentialsProvider(
+                    StaticCredentialsProvider.create(
+                        AwsBasicCredentials.create(
+                            credentials.string("accessKeyId"),
+                            credentials.string("secretAccessKey")
+                        )
+                    )
+                );
+            } else {
+                throw new IllegalArgumentException(
+                    String.format("Unsupported S3 credentials type: %s", type)
+                );
+            }
         }
     }
 
     /**
      * Obtain endpoint from storage config. The parameter is optional.
+     *
      * @param cfg Storage config
      * @return Endpoint value is present
      */

diff --git a/asto-s3/src/test/java/com/artipie/asto/S3StorageFactoryTest.java b/asto-s3/src/test/java/com/artipie/asto/S3StorageFactoryTest.java
@@ -26,7 +26,7 @@ public final class S3StorageFactoryTest {
      * @checkstyle MethodNameCheck (3 lines)
      */
     @Test
-    void shouldCreateS3Storage() {
+    void shouldCreateS3StorageConfigHasCredentials() {
         MatcherAssert.assertThat(
             new StoragesLoader()
                 .newObject(
@@ -50,4 +50,27 @@ void shouldCreateS3Storage() {
             new IsInstanceOf(S3Storage.class)
         );
     }
+
+    /**
+     * Test for S3 storage factory.
+     *
+     * @checkstyle MethodNameCheck (3 lines)
+     */
+    @Test
+    void shouldCreateS3StorageConfigDoesNotHaveCredentials() {
+        MatcherAssert.assertThat(
+            new StoragesLoader()
+                .newObject(
+                    "s3",
+                    new Config.YamlStorageConfig(
+                        Yaml.createYamlMappingBuilder()
+                            .add("region", "us-east-1")
+                            .add("bucket", "aaa")
+                            .add("endpoint", "http://localhost")
+                            .build()
+                    )
+                ),
+            new IsInstanceOf(S3Storage.class)
+        );
+    }
 }
diff --git a/asto-s3/src/test/java/com/artipie/asto/s3/S3HeadMetaTest.java b/asto-s3/src/test/java/com/artipie/asto/s3/S3HeadMetaTest.java
@@ -25,7 +25,7 @@ void readSize() {
                     .contentLength(len)
                     .eTag("empty")
                     .build()
-            ).read(Meta.OP_SIZE).get(),
+            ).read(Meta.OP_SIZE).orElseThrow(IllegalStateException::new),
             new IsEqual<>(len)
         );
     }
@@ -39,7 +39,7 @@ void readHash() {
                     .contentLength(0L)
                     .eTag(hash)
                     .build()
-            ).read(Meta.OP_MD5).get(),
+            ).read(Meta.OP_MD5).orElseThrow(IllegalStateException::new),
             new IsEqual<>(hash)
         );
     }

diff --git a/asto-s3/src/test/java/com/artipie/asto/s3/S3StorageTest.java b/asto-s3/src/test/java/com/artipie/asto/s3/S3StorageTest.java
@@ -10,15 +10,18 @@
 import com.amazonaws.services.s3.model.MultipartUpload;
 import com.amazonaws.services.s3.model.ObjectMetadata;
 import com.amazonaws.services.s3.model.S3Object;
+import com.amihaiemil.eoyaml.Yaml;
 import com.artipie.asto.Content;
 import com.artipie.asto.Key;
 import com.artipie.asto.Meta;
+import com.artipie.asto.Storage;
 import com.artipie.asto.blocking.BlockingStorage;
+import com.artipie.asto.factory.Config;
+import com.artipie.asto.factory.StoragesLoader;
 import com.google.common.io.ByteStreams;
 import io.reactivex.Flowable;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
-import java.net.URI;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
@@ -35,10 +38,6 @@
 import org.junit.jupiter.api.condition.DisabledOnOs;
 import org.junit.jupiter.api.condition.OS;
 import org.junit.jupiter.api.extension.RegisterExtension;
-import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
-import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
-import software.amazon.awssdk.regions.Region;
-import software.amazon.awssdk.services.s3.S3AsyncClient;
 
 /**
  * Tests for {@link S3Storage}.
@@ -258,15 +257,25 @@ private byte[] download(final AmazonS3 client, final String key) throws IOExcept
         }
     }
 
-    private S3Storage storage() {
-        final String endpoint = String.format("http://localhost:%d", MOCK.getHttpPort());
-        final S3AsyncClient client = S3AsyncClient.builder()
-            .region(Region.of("us-east-1"))
-            .credentialsProvider(
-                StaticCredentialsProvider.create(AwsBasicCredentials.create("foo", "bar"))
-            )
-            .endpointOverride(URI.create(endpoint))
-            .build();
-        return new S3Storage(client, this.bucket, endpoint);
+    private Storage storage() {
+        return new StoragesLoader()
+            .newObject(
+                "s3",
+                new Config.YamlStorageConfig(
+                    Yaml.createYamlMappingBuilder()
+                        .add("region", "us-east-1")
+                        .add("bucket", this.bucket)
+                        .add("endpoint", String.format("http://localhost:%d", MOCK.getHttpPort()))
+                        .add(
+                            "credentials",
+                            Yaml.createYamlMappingBuilder()
+                                .add("type", "basic")
+                                .add("accessKeyId", "foo")
+                                .add("secretAccessKey", "bar")
+                                .build()
+                        )
+                        .build()
+                )
+            );
     }
 }