delete TOKEN_REFRESH_ENABLED judge whether refresh by credentials pro…

…vider
apache · Dec 9, 2024 · 314f965 · 314f965
1 parent 7498e7e
commit 314f965
Show file tree

Hide file tree

Showing 4 changed files with 5 additions and 43 deletions.
diff --git a/paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalog.java b/paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalog.java
@@ -54,7 +54,6 @@ public class RESTCatalog implements Catalog {
     // a lazy thread pool for token refresh
     private final AuthSession catalogAuth;
     private volatile ScheduledExecutorService refreshExecutor = null;
-    private boolean keepTokenRefreshed;
 
     private static final ObjectMapper objectMapper = RESTObjectMapper.create();
 
@@ -80,8 +79,7 @@ public RESTCatalog(Options options) {
         CredentialsProvider credentialsProvider =
                 CredentialsProviderFactory.createCredentialsProvider(
                         options, RESTCatalog.class.getClassLoader());
-        this.keepTokenRefreshed = options.get(RESTCatalogOptions.TOKEN_REFRESH_ENABLED);
-        if (keepTokenRefreshed) {
+        if (credentialsProvider.keepRefreshed()) {
             this.catalogAuth =
                     AuthSession.fromRefreshCredentialsProvider(
                             tokenRefreshExecutor(), this.baseHeader, credentialsProvider);
@@ -215,10 +213,6 @@ private Map<String, String> headers() {
     }
 
     private ScheduledExecutorService tokenRefreshExecutor() {
-        if (!keepTokenRefreshed) {
-            return null;
-        }
-
         if (refreshExecutor == null) {
             synchronized (this) {
                 if (refreshExecutor == null) {

diff --git a/paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalogOptions.java b/paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalogOptions.java
@@ -55,14 +55,9 @@ public class RESTCatalogOptions {
                     .durationType()
                     .defaultValue(Duration.ofHours(1))
                     .withDescription(
-                            "REST Catalog auth token expires in.The token generates system refresh frequency is t1," +
-                                    " the token expires time is t2, we need to guarantee that t2 > t1," +
-                                    " the token validity time is [t2 - t1, t2], and the expires time defined here needs to be less than (t2 - t1)");
-    public static final ConfigOption<Boolean> TOKEN_REFRESH_ENABLED =
-            ConfigOptions.key("token-refresh-enabled")
-                    .booleanType()
-                    .defaultValue(false)
-                    .withDescription("REST Catalog auth token refresh enable.");
+                            "REST Catalog auth token expires in.The token generates system refresh frequency is t1,"
+                                    + " the token expires time is t2, we need to guarantee that t2 > t1,"
+                                    + " the token validity time is [t2 - t1, t2], and the expires time defined here needs to be less than (t2 - t1)");
     public static final ConfigOption<String> TOKEN_PROVIDER_PATH =
             ConfigOptions.key("token.provider.path")
                     .stringType()

diff --git a/...re/src/main/java/org/apache/paimon/rest/auth/BearTokenFileCredentialsProviderFactory.java b/...re/src/main/java/org/apache/paimon/rest/auth/BearTokenFileCredentialsProviderFactory.java
@@ -19,7 +19,6 @@
 package org.apache.paimon.rest.auth;
 
 import org.apache.paimon.options.Options;
-import org.apache.paimon.rest.RESTCatalogOptions;
 
 import static org.apache.paimon.rest.RESTCatalogOptions.TOKEN_EXPIRATION_TIME;
 import static org.apache.paimon.rest.RESTCatalogOptions.TOKEN_PROVIDER_PATH;
@@ -38,12 +37,7 @@ public CredentialsProvider create(Options options) {
             throw new IllegalArgumentException(TOKEN_PROVIDER_PATH.key() + " is required");
         }
         String tokenFilePath = options.get(TOKEN_PROVIDER_PATH);
-        boolean keepTokenRefreshed = options.get(RESTCatalogOptions.TOKEN_REFRESH_ENABLED);
-        if (keepTokenRefreshed) {
-            if (!options.getOptional(TOKEN_EXPIRATION_TIME).isPresent()) {
-                throw new IllegalArgumentException(
-                        TOKEN_EXPIRATION_TIME.key() + " is required when token refresh enabled");
-            }
+        if (options.getOptional(TOKEN_EXPIRATION_TIME).isPresent()) {
             long tokenExpireInMills = options.get(TOKEN_EXPIRATION_TIME).toMillis();
             return new BearTokenFileCredentialsProvider(tokenFilePath, tokenExpireInMills);
 

diff --git a/paimon-core/src/test/java/org/apache/paimon/rest/auth/CredentialsProviderFactoryTest.java b/paimon-core/src/test/java/org/apache/paimon/rest/auth/CredentialsProviderFactoryTest.java
@@ -69,7 +69,6 @@ public void testCreateBearTokenFileCredentialsProviderSuccess() throws Exception
         String token = UUID.randomUUID().toString();
         FileUtils.writeStringToFile(tokenFile, token);
         options.set(RESTCatalogOptions.TOKEN_PROVIDER_PATH, tokenFile.getPath());
-        options.set(CREDENTIALS_PROVIDER, CredentialsProviderType.BEAR_TOKEN_FILE.name());
         BearTokenFileCredentialsProvider credentialsProvider =
                 (BearTokenFileCredentialsProvider)
                         CredentialsProviderFactory.createCredentialsProvider(
@@ -95,9 +94,7 @@ public void testCreateRefreshBearTokenFileCredentialsProviderSuccess() throws Ex
         File tokenFile = folder.newFile(fileName);
         String token = UUID.randomUUID().toString();
         FileUtils.writeStringToFile(tokenFile, token);
-        options.set(CREDENTIALS_PROVIDER, CredentialsProviderType.BEAR_TOKEN_FILE.name());
         options.set(RESTCatalogOptions.TOKEN_PROVIDER_PATH, tokenFile.getPath());
-        options.set(RESTCatalogOptions.TOKEN_REFRESH_ENABLED, true);
         options.set(RESTCatalogOptions.TOKEN_EXPIRATION_TIME, Duration.ofSeconds(10L));
         BearTokenFileCredentialsProvider credentialsProvider =
                 (BearTokenFileCredentialsProvider)
@@ -106,24 +103,6 @@ public void testCreateRefreshBearTokenFileCredentialsProviderSuccess() throws Ex
         assertEquals(token, credentialsProvider.token());
     }
 
-    @Test
-    public void testCreateRefreshBearTokenFileCredentialsProviderFail() throws Exception {
-        Options options = new Options();
-        String fileName = "token";
-        File tokenFile = folder.newFile(fileName);
-        String token = UUID.randomUUID().toString();
-        FileUtils.writeStringToFile(tokenFile, token);
-        options.set(CREDENTIALS_PROVIDER, CredentialsProviderType.BEAR_TOKEN_FILE.name());
-        options.set(RESTCatalogOptions.TOKEN_PROVIDER_PATH, tokenFile.getPath());
-        options.set(RESTCatalogOptions.TOKEN_REFRESH_ENABLED, true);
-        options.set(CREDENTIALS_PROVIDER, CredentialsProviderType.BEAR_TOKEN_FILE.name());
-        assertThrows(
-                IllegalArgumentException.class,
-                () ->
-                        CredentialsProviderFactory.createCredentialsProvider(
-                                options, this.getClass().getClassLoader()));
-    }
-
     @Test
     public void getCredentialsProviderTypeByConfWhenDefineTokenPath() {
         Options options = new Options();