Add Iceberg REST auth server configuration property

Add comment and source

Add test

Author: kiersten-stokes

presto-docs/src/main/sphinx/connector/iceberg.rst

@@ -210,6 +210,9 @@ Property Name                                        Description
                                                      Available values are ``NONE`` or ``OAUTH2`` (default: ``NONE``).
                                                      ``OAUTH2`` requires either a credential or token.
 
+``iceberg.rest.auth.oauth2.uri``                     OAUTH2 server endpoint URI.
+                                                     Example: ``https://localhost:9191``
+
 ``iceberg.rest.auth.oauth2.credential``              The credential to use for OAUTH2 authentication.
                                                      Example: ``key:secret``
 

presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestCatalogFactory.java

@@ -35,6 +35,7 @@
 import static java.util.Objects.requireNonNull;
 import static org.apache.iceberg.CatalogProperties.URI;
 import static org.apache.iceberg.rest.auth.OAuth2Properties.CREDENTIAL;
+import static org.apache.iceberg.rest.auth.OAuth2Properties.OAUTH2_SERVER_URI;
 import static org.apache.iceberg.rest.auth.OAuth2Properties.TOKEN;
 
 public class IcebergRestCatalogFactory
@@ -67,6 +68,11 @@ protected Map<String, String> getCatalogProperties(ConnectorSession session)
 
         catalogConfig.getAuthenticationType().ifPresent(type -> {
             if (type == OAUTH2) {
+                // The oauth2/tokens endpoint of the REST catalog spec has been deprecated and will
+                // be removed in Iceberg 2.0 (https://github.com/apache/iceberg/pull/10603)
+                // TODO auth server URI will eventually need to be made a required property
+                catalogConfig.getAuthenticationServerUri().ifPresent(authServerUri -> properties.put(OAUTH2_SERVER_URI, authServerUri));
+
                 if (!catalogConfig.credentialOrTokenExists()) {
                     throw new IllegalStateException("iceberg.rest.auth.oauth2 requires either a credential or a token");
                 }

presto-iceberg/src/main/java/com/facebook/presto/iceberg/rest/IcebergRestConfig.java

@@ -25,6 +25,7 @@ public class IcebergRestConfig
     private String serverUri;
     private SessionType sessionType;
     private AuthenticationType authenticationType;
+    private String authenticationServerUri;
     private String credential;
     private String token;
 
@@ -68,6 +69,19 @@ public IcebergRestConfig setAuthenticationType(AuthenticationType authentication
         return this;
     }
 
+    public Optional<String> getAuthenticationServerUri()
+    {
+        return Optional.ofNullable(authenticationServerUri);
+    }
+
+    @Config("iceberg.rest.auth.oauth2.uri")
+    @ConfigDescription("The URI to connect to the OAUTH2 server")
+    public IcebergRestConfig setAuthenticationServerUri(String authServerUri)
+    {
+        this.authenticationServerUri = authServerUri;
+        return this;
+    }
+
     public Optional<String> getCredential()
     {
         return Optional.ofNullable(credential);

presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergRestConfig.java

@@ -32,6 +32,7 @@ public void testDefaults()
         assertRecordedDefaults(ConfigAssertions.recordDefaults(IcebergRestConfig.class)
                 .setServerUri(null)
                 .setAuthenticationType(null)
+                .setAuthenticationServerUri(null)
                 .setCredential(null)
                 .setToken(null)
                 .setSessionType(null));
@@ -43,6 +44,7 @@ public void testExplicitPropertyMappings()
         Map<String, String> properties = ImmutableMap.<String, String>builder()
                 .put("iceberg.rest.uri", "http://localhost:xxx")
                 .put("iceberg.rest.auth.type", "OAUTH2")
+                .put("iceberg.rest.auth.oauth2.uri", "http://localhost:yyy")
                 .put("iceberg.rest.auth.oauth2.credential", "key:secret")
                 .put("iceberg.rest.auth.oauth2.token", "SXVLUXUhIExFQ0tFUiEK")
                 .put("iceberg.rest.session.type", "USER")
@@ -51,6 +53,7 @@ public void testExplicitPropertyMappings()
         IcebergRestConfig expected = new IcebergRestConfig()
                 .setServerUri("http://localhost:xxx")
                 .setAuthenticationType(OAUTH2)
+                .setAuthenticationServerUri("http://localhost:yyy")
                 .setCredential("key:secret")
                 .setToken("SXVLUXUhIExFQ0tFUiEK")
                 .setSessionType(USER);

presto-iceberg/src/test/java/com/facebook/presto/iceberg/rest/TestIcebergSmokeRest.java

@@ -29,6 +29,7 @@
 import com.facebook.presto.testing.QueryRunner;
 import com.google.common.collect.ImmutableMap;
 import org.apache.iceberg.Table;
+import org.apache.iceberg.rest.RESTCatalog;
 import org.assertj.core.util.Files;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
@@ -42,12 +43,15 @@
 import static com.facebook.presto.iceberg.FileFormat.PARQUET;
 import static com.facebook.presto.iceberg.IcebergQueryRunner.ICEBERG_CATALOG;
 import static com.facebook.presto.iceberg.IcebergUtil.getNativeIcebergTable;
+import static com.facebook.presto.iceberg.rest.AuthenticationType.OAUTH2;
 import static com.facebook.presto.iceberg.rest.IcebergRestTestUtil.getRestServer;
 import static com.facebook.presto.iceberg.rest.IcebergRestTestUtil.restConnectorProperties;
 import static com.google.common.io.MoreFiles.deleteRecursively;
 import static com.google.common.io.RecursiveDeleteOption.ALLOW_INSECURE;
 import static java.lang.String.format;
+import static org.apache.iceberg.rest.auth.OAuth2Properties.OAUTH2_SERVER_URI;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.testng.Assert.assertEquals;
 
 @Test
 public class TestIcebergSmokeRest
@@ -106,12 +110,11 @@ protected QueryRunner createQueryRunner()
                 Optional.of(warehouseLocation.toPath()));
     }
 
-    protected IcebergNativeCatalogFactory getCatalogFactory()
+    protected IcebergNativeCatalogFactory getCatalogFactory(IcebergRestConfig restConfig)
     {
         IcebergConfig icebergConfig = new IcebergConfig()
                 .setCatalogType(REST)
-                .setCatalogWarehouse(warehouseLocation.getAbsolutePath().toString());
-        IcebergRestConfig restConfig = new IcebergRestConfig().setServerUri(serverUri);
+                .setCatalogWarehouse(warehouseLocation.getAbsolutePath());
 
         return new IcebergRestCatalogFactory(
                 icebergConfig,
@@ -125,7 +128,8 @@ protected IcebergNativeCatalogFactory getCatalogFactory()
     @Override
     protected Table getIcebergTable(ConnectorSession session, String schema, String tableName)
     {
-        return getNativeIcebergTable(getCatalogFactory(),
+        IcebergRestConfig restConfig = new IcebergRestConfig().setServerUri(serverUri);
+        return getNativeIcebergTable(getCatalogFactory(restConfig),
                 session,
                 SchemaTableName.valueOf(schema + "." + tableName));
     }
@@ -192,4 +196,20 @@ public void testMetadataDeleteOnTableWithUnsupportedSpecsWhoseDataAllDeleted(Str
             super.testMetadataDeleteOnTableWithUnsupportedSpecsWhoseDataAllDeleted(version, mode);
         }
     }
+
+    @Test
+    public void testSetOauth2ServerUriPropertyI()
+    {
+        String authEndpoint = "http://localhost:8888";
+        IcebergRestConfig restConfig = new IcebergRestConfig()
+                .setServerUri(serverUri)
+                .setAuthenticationType(OAUTH2)
+                .setToken("SXVLUXUhIExFQ0tFUiEK")
+                .setAuthenticationServerUri(authEndpoint);
+
+        IcebergRestCatalogFactory catalogFactory = (IcebergRestCatalogFactory) getCatalogFactory(restConfig);
+        RESTCatalog catalog = (RESTCatalog) catalogFactory.getCatalog(getSession().toConnectorSession());
+
+        assertEquals(catalog.properties().get(OAUTH2_SERVER_URI), authEndpoint);
+    }
 }