Refactor to support Glue protocol (separate PR)

[Randgalt]

Introduce RemoteUriFacade

Abstract RemoteS3Facade's remoteUri() into a new interface
that can be used/injected independently of S3 implementations.

Give Request logger its own config

Move the Request logging config into separate config class

Abstract some common bindings of

Abstract new static methods so portions of module can be used in other contexts

[Randgalt]

@mosiac1 I've added another commit with the change to AccessType we discussed. PTAL

[mosiac1]

Wouldn't we want Access and/or AccessType to be public scoped?

[mosiac1]

What do you think of this approach? It removes the need for 1 intermediary class

Pasting a diff, I don't have a good way of explaining it in a comment

Index: trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurityDynamicFeature.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurityDynamicFeature.java b/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurityDynamicFeature.java
--- a/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurityDynamicFeature.java	(revision 03c68f8ac6880af08e00422aaea199bc0addc3a6)
+++ b/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurityDynamicFeature.java	(date 1737999508310)
@@ -14,11 +14,10 @@
 package io.trino.aws.proxy.server.rest;
 
 import com.google.inject.Inject;
-import io.trino.aws.proxy.server.rest.ResourceSecurity.Access.PublicAccess;
-import io.trino.aws.proxy.server.rest.ResourceSecurity.Access.SigV4Access;
 import io.trino.aws.proxy.server.rest.ResourceSecurity.AccessType;
+import io.trino.aws.proxy.server.rest.ResourceSecurity.AccessType.PublicAccessType;
+import io.trino.aws.proxy.server.rest.ResourceSecurity.AccessType.SigV4AccessType;
 import io.trino.aws.proxy.spi.signing.SigningController;
-import io.trino.aws.proxy.spi.signing.SigningServiceType;
 import jakarta.ws.rs.container.DynamicFeature;
 import jakarta.ws.rs.container.ResourceInfo;
 import jakarta.ws.rs.core.FeatureContext;
@@ -46,10 +45,10 @@
     {
         if (resourceInfo.getResourceClass().getPackageName().startsWith("io.trino.aws")) {
             AccessType accessType = getAccessType(resourceInfo);
-            switch (accessType.access()) {
-                case PublicAccess _ -> {}
-                case SigV4Access(SigningServiceType signingServiceType) ->
-                        context.register(new SecurityFilter(signingController, signingServiceType, requestLoggerController));
+            switch (accessType) {
+                case PublicAccessType _ -> {}
+                case SigV4AccessType sigV4 ->
+                        context.register(new SecurityFilter(signingController, sigV4.getSigningServiceType(), requestLoggerController));
             }
         }
     }
Index: trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurity.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurity.java b/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurity.java
--- a/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurity.java	(revision 03c68f8ac6880af08e00422aaea199bc0addc3a6)
+++ b/trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/rest/ResourceSecurity.java	(date 1737999578942)
@@ -13,8 +13,8 @@
  */
 package io.trino.aws.proxy.server.rest;
 
-import io.trino.aws.proxy.server.rest.ResourceSecurity.Access.PublicAccess;
-import io.trino.aws.proxy.server.rest.ResourceSecurity.Access.SigV4Access;
+import io.trino.aws.proxy.server.rest.ResourceSecurity.AccessType.PublicAccessType;
+import io.trino.aws.proxy.server.rest.ResourceSecurity.AccessType.SigV4AccessType;
 import io.trino.aws.proxy.spi.signing.SigningServiceType;
 
 import java.lang.annotation.Retention;
@@ -23,69 +23,56 @@
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.ElementType.TYPE;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
-import static java.util.Objects.requireNonNull;
 
 @Retention(RUNTIME)
 @Target({TYPE, METHOD})
 public @interface ResourceSecurity
 {
-    sealed interface Access
-    {
-        record PublicAccess()
-                implements Access {}
-
-        record SigV4Access(SigningServiceType signingServiceType)
-                implements Access {}
-    }
-
-    abstract class AccessType
+    sealed interface AccessType
     {
-        private final Access access;
-
-        public Access access()
-        {
-            return access;
-        }
-
-        protected AccessType(Access access)
-        {
-            this.access = requireNonNull(access, "access is null");
-        }
-    }
-
-    class Public
-            extends AccessType
-    {
-        public Public()
-        {
-            super(new PublicAccess());
-        }
-    }
-
-    class S3
-            extends AccessType
-    {
-        public S3()
-        {
-            super(new SigV4Access(SigningServiceType.S3));
-        }
-    }
+        sealed interface PublicAccessType
+                extends AccessType
+                permits Public
+        {}
 
-    class Sts
-            extends AccessType
+        non-sealed interface SigV4AccessType
+                extends AccessType
+        {
+            SigningServiceType getSigningServiceType();
+        }
+    }
+
+    final class Public
+            implements PublicAccessType
+    { }
+
+    final class S3
+            implements SigV4AccessType
     {
-        public Sts()
+        @Override
+        public SigningServiceType getSigningServiceType()
         {
-            super(new SigV4Access(SigningServiceType.STS));
+            return SigningServiceType.S3;
         }
     }
 
-    class Logs
-            extends AccessType
+    final class Sts
+            implements SigV4AccessType
     {
-        public Logs()
+        @Override
+        public SigningServiceType getSigningServiceType()
         {
-            super(new SigV4Access(SigningServiceType.LOGS));
+            return SigningServiceType.STS;
+        }
+    }
+
+    final class Logs
+            implements SigV4AccessType
+    {
+        @Override
+        public SigningServiceType getSigningServiceType()
+        {
+            return SigningServiceType.LOGS;
         }
     }
 

[Randgalt]

Wouldn't we want Access and/or AccessType to be public scoped

It's public by default in annotations

[Randgalt]

What do you think of this approach

sure - we can do that

[mosiac1]

Could we use awsV4Signer to be consistent with the AWS names?

awsS34Signer is also quite confusing so awsS3V4Signer it slightly nicer

[Randgalt]

Changes made