This is an automated email from the ASF dual-hosted git repository.
amoghj pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 6d5951149f REST, OAuth2: Remove deprecated RefreshingAuthManager
(#14229)
6d5951149f is described below
commit 6d5951149f5bd49d80141502ebb5b80c41cb29f7
Author: Alexandre Dutra <[email protected]>
AuthorDate: Thu Oct 2 06:39:22 2025 +0200
REST, OAuth2: Remove deprecated RefreshingAuthManager (#14229)
---
.palantir/revapi.yml | 11 +++
.../iceberg/aws/s3/signer/TestS3RestSigner.java | 6 +-
.../apache/iceberg/rest/auth/OAuth2Manager.java | 28 ++++---
.../iceberg/rest/auth/RefreshingAuthManager.java | 93 ----------------------
.../iceberg/rest/auth/TestOAuth2Manager.java | 80 -------------------
5 files changed, 31 insertions(+), 187 deletions(-)
diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml
index 971021d24b..4ba3d8250d 100644
--- a/.palantir/revapi.yml
+++ b/.palantir/revapi.yml
@@ -1370,6 +1370,17 @@ acceptedBreaks:
new: "class org.apache.iceberg.encryption.EncryptingFileIO"
justification: "New method for Manifest List reading"
org.apache.iceberg:iceberg-core:
+ - code: "java.class.noLongerInheritsFromClass"
+ old: "class org.apache.iceberg.rest.auth.OAuth2Manager"
+ new: "class org.apache.iceberg.rest.auth.OAuth2Manager"
+ justification: "Removing deprecations for 1.11.0"
+ - code: "java.class.nowImplementsInterface"
+ old: "class org.apache.iceberg.rest.auth.OAuth2Manager"
+ new: "class org.apache.iceberg.rest.auth.OAuth2Manager"
+ justification: "Removing deprecations for 1.11.0"
+ - code: "java.class.removed"
+ old: "class org.apache.iceberg.rest.auth.RefreshingAuthManager"
+ justification: "Removing deprecations for 1.11.0"
- code: "java.field.constantValueChanged"
old: "field org.apache.iceberg.rest.ResourcePaths.V1_TABLE_SCAN_PLAN"
new: "field org.apache.iceberg.rest.ResourcePaths.V1_TABLE_SCAN_PLAN"
diff --git
a/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java
b/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java
index 6dac75aa49..eb683e8392 100644
---
a/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java
+++
b/aws/src/integration/java/org/apache/iceberg/aws/s3/signer/TestS3RestSigner.java
@@ -36,6 +36,7 @@ import org.apache.iceberg.aws.s3.MinioUtil;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.rest.auth.OAuth2Properties;
+import org.apache.iceberg.util.ThreadPools;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.handler.gzip.GzipHandler;
import org.eclipse.jetty.servlet.ServletContextHandler;
@@ -123,8 +124,9 @@ public class TestS3RestSigner {
// there aren't other token refreshes being scheduled after every sign
request and after
// TestS3RestSigner completes all tests, there should be only this single
token in the queue
// that is scheduled for refresh
- assertThat(S3V4RestSignerClient.authManager)
- .extracting("refreshExecutor")
+ assertThat(ThreadPools.authRefreshPool())
+ // internal field in
java.util.concurrent.Executors.DelegatedScheduledExecutorService
+ .extracting("e")
.asInstanceOf(type(ScheduledThreadPoolExecutor.class))
.satisfies(
executor -> {
diff --git a/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Manager.java
b/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Manager.java
index 5f74db0e2e..1717b6215c 100644
--- a/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Manager.java
+++ b/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Manager.java
@@ -22,7 +22,9 @@ import java.time.Duration;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ScheduledExecutorService;
import java.util.function.Function;
+import javax.annotation.Nullable;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.catalog.SessionCatalog;
import org.apache.iceberg.catalog.TableIdentifier;
@@ -34,10 +36,11 @@ import org.apache.iceberg.rest.RESTUtil;
import org.apache.iceberg.rest.ResourcePaths;
import org.apache.iceberg.rest.responses.OAuthTokenResponse;
import org.apache.iceberg.util.PropertyUtil;
+import org.apache.iceberg.util.ThreadPools;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class OAuth2Manager extends RefreshingAuthManager {
+public class OAuth2Manager implements AuthManager {
private static final Logger LOG =
LoggerFactory.getLogger(OAuth2Manager.class);
@@ -62,9 +65,9 @@ public class OAuth2Manager extends RefreshingAuthManager {
private long startTimeMillis;
private OAuthTokenResponse authResponse;
private AuthSessionCache sessionCache;
+ private boolean keepRefreshed = true;
public OAuth2Manager(String managerName) {
- super(managerName + "-token-refresh");
this.name = managerName;
}
@@ -109,7 +112,7 @@ public class OAuth2Manager extends RefreshingAuthManager {
AuthConfig config = AuthConfig.fromProperties(properties);
Map<String, String> headers = OAuth2Util.authHeaders(config.token());
OAuth2Util.AuthSession session = new OAuth2Util.AuthSession(headers,
config);
- keepRefreshed(config.keepRefreshed());
+ keepRefreshed = config.keepRefreshed();
// authResponse comes from the init phase: this means we already fetched a
token
// so reuse it now and turn token refresh on.
if (authResponse != null) {
@@ -160,7 +163,7 @@ public class OAuth2Manager extends RefreshingAuthManager {
Map<String, String> headers = OAuth2Util.authHeaders(config.token());
OAuth2Util.AuthSession parent = new OAuth2Util.AuthSession(headers,
config);
- keepRefreshed(config.keepRefreshed());
+ keepRefreshed = config.keepRefreshed();
// Important: this method is invoked from standalone components; we must
not assume that
// the refresh client and session cache have been initialized, because
catalogSession()
@@ -188,14 +191,10 @@ public class OAuth2Manager extends RefreshingAuthManager {
@Override
public void close() {
- try {
- super.close();
- } finally {
- AuthSessionCache cache = sessionCache;
- this.sessionCache = null;
- if (cache != null) {
- cache.close();
- }
+ AuthSessionCache cache = sessionCache;
+ this.sessionCache = null;
+ if (cache != null) {
+ cache.close();
}
}
@@ -272,6 +271,11 @@ public class OAuth2Manager extends RefreshingAuthManager {
refreshClient, refreshExecutor(), response,
System.currentTimeMillis(), parent);
}
+ @Nullable
+ protected ScheduledExecutorService refreshExecutor() {
+ return keepRefreshed ? ThreadPools.authRefreshPool() : null;
+ }
+
private static void warnIfOAuthServerUriNotSet(Map<String, String>
properties) {
if (!properties.containsKey(OAuth2Properties.OAUTH2_SERVER_URI)) {
String credential = properties.get(OAuth2Properties.CREDENTIAL);
diff --git
a/core/src/main/java/org/apache/iceberg/rest/auth/RefreshingAuthManager.java
b/core/src/main/java/org/apache/iceberg/rest/auth/RefreshingAuthManager.java
deleted file mode 100644
index d51a8c3b24..0000000000
--- a/core/src/main/java/org/apache/iceberg/rest/auth/RefreshingAuthManager.java
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.iceberg.rest.auth;
-
-import java.util.List;
-import java.util.concurrent.Future;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
-import javax.annotation.Nullable;
-import org.apache.iceberg.util.ThreadPools;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/**
- * An {@link AuthManager} that provides machinery for refreshing
authentication data asynchronously,
- * using a background thread pool.
- *
- * @deprecated since 1.10.0, will be removed in 1.11.0; use {@link
ThreadPools#authRefreshPool()}.
- */
-@Deprecated
-public abstract class RefreshingAuthManager implements AuthManager {
-
- private static final Logger LOG =
LoggerFactory.getLogger(RefreshingAuthManager.class);
-
- private final String executorNamePrefix;
- private boolean keepRefreshed = true;
- private volatile ScheduledExecutorService refreshExecutor;
-
- protected RefreshingAuthManager(String executorNamePrefix) {
- this.executorNamePrefix = executorNamePrefix;
- }
-
- public void keepRefreshed(boolean keep) {
- this.keepRefreshed = keep;
- }
-
- @Override
- public void close() {
- ScheduledExecutorService service = refreshExecutor;
- this.refreshExecutor = null;
- if (service != null) {
- List<Runnable> tasks = service.shutdownNow();
- tasks.forEach(
- task -> {
- if (task instanceof Future) {
- ((Future<?>) task).cancel(true);
- }
- });
-
- try {
- if (!service.awaitTermination(1, TimeUnit.MINUTES)) {
- LOG.warn("Timed out waiting for refresh executor to terminate");
- }
- } catch (InterruptedException e) {
- LOG.warn("Interrupted while waiting for refresh executor to
terminate", e);
- Thread.currentThread().interrupt();
- }
- }
- }
-
- @Nullable
- protected ScheduledExecutorService refreshExecutor() {
- if (!keepRefreshed) {
- return null;
- }
-
- if (refreshExecutor == null) {
- synchronized (this) {
- if (refreshExecutor == null) {
- this.refreshExecutor =
ThreadPools.newScheduledPool(executorNamePrefix, 1);
- }
- }
- }
-
- return refreshExecutor;
- }
-}
diff --git
a/core/src/test/java/org/apache/iceberg/rest/auth/TestOAuth2Manager.java
b/core/src/test/java/org/apache/iceberg/rest/auth/TestOAuth2Manager.java
index 677d7768b0..c9a65a5bd7 100644
--- a/core/src/test/java/org/apache/iceberg/rest/auth/TestOAuth2Manager.java
+++ b/core/src/test/java/org/apache/iceberg/rest/auth/TestOAuth2Manager.java
@@ -63,10 +63,6 @@ class TestOAuth2Manager {
try (OAuth2Manager manager = new OAuth2Manager("test");
OAuth2Util.AuthSession session = manager.initSession(client,
properties)) {
assertThat(session.headers()).isEmpty();
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should not create refresh executor for init session")
- .isNull();
}
Mockito.verifyNoInteractions(client);
}
@@ -77,10 +73,6 @@ class TestOAuth2Manager {
try (OAuth2Manager manager = new OAuth2Manager("test");
OAuth2Util.AuthSession session = manager.initSession(client,
properties)) {
assertThat(session.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should not create refresh executor for init session")
- .isNull();
}
Mockito.verifyNoInteractions(client);
}
@@ -91,10 +83,6 @@ class TestOAuth2Manager {
try (OAuth2Manager manager = new OAuth2Manager("test");
OAuth2Util.AuthSession session = manager.initSession(client,
properties)) {
assertThat(session.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should not create refresh executor for init session")
- .isNull();
}
Mockito.verify(client)
.postForm(
@@ -118,10 +106,6 @@ class TestOAuth2Manager {
try (OAuth2Manager manager = new OAuth2Manager("test");
OAuth2Util.AuthSession catalogSession = manager.catalogSession(client,
properties)) {
assertThat(catalogSession.headers()).isEmpty();
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should not create refresh executor when no token and no
credentials provided")
- .isNull();
}
Mockito.verify(client).withAuthSession(any());
Mockito.verifyNoMoreInteractions(client);
@@ -133,10 +117,6 @@ class TestOAuth2Manager {
try (OAuth2Manager manager = new OAuth2Manager("test");
OAuth2Util.AuthSession catalogSession = manager.catalogSession(client,
properties)) {
assertThat(catalogSession.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when token provided")
- .isNotNull();
}
Mockito.verify(client).withAuthSession(any());
Mockito.verifyNoMoreInteractions(client);
@@ -149,10 +129,6 @@ class TestOAuth2Manager {
try (OAuth2Manager manager = new OAuth2Manager("test");
OAuth2Util.AuthSession catalogSession = manager.catalogSession(client,
properties)) {
assertThat(catalogSession.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should not create refresh executor when refresh disabled")
- .isNull();
}
Mockito.verify(client).withAuthSession(any());
Mockito.verifyNoMoreInteractions(client);
@@ -167,10 +143,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession ignored = manager.initSession(client,
properties);
OAuth2Util.AuthSession catalogSession = manager.catalogSession(client,
properties)) {
assertThat(catalogSession.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when credentials provided")
- .isNotNull();
}
Mockito.verify(client)
.postForm(
@@ -196,10 +168,6 @@ class TestOAuth2Manager {
try (OAuth2Manager manager = new OAuth2Manager("test");
OAuth2Util.AuthSession catalogSession = manager.catalogSession(client,
properties)) {
assertThat(catalogSession.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when credentials provided")
- .isNotNull();
}
Mockito.verify(client)
.postForm(
@@ -224,10 +192,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession contextualSession =
manager.contextualSession(context, catalogSession)) {
assertThat(contextualSession).isSameAs(catalogSession);
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should not create refresh executor when no context credentials
provided")
- .isNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -251,10 +215,6 @@ class TestOAuth2Manager {
assertThat(contextualSession).isNotSameAs(catalogSession);
assertThat(contextualSession.headers())
.containsOnly(entry("Authorization", "Bearer context-token"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when contextual session created")
- .isNotNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -277,10 +237,6 @@ class TestOAuth2Manager {
manager.contextualSession(context, catalogSession)) {
assertThat(contextualSession).isNotSameAs(catalogSession);
assertThat(contextualSession.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when contextual session created")
- .isNotNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -314,10 +270,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession contextualSession =
manager.contextualSession(context, catalogSession)) {
assertThat(contextualSession.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when contextual session created")
- .isNotNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -378,10 +330,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession tableSession =
manager.tableSession(table, properties, catalogSession)) {
assertThat(tableSession).isSameAs(catalogSession);
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should not create refresh executor when no table credentials
provided")
- .isNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -403,10 +351,6 @@ class TestOAuth2Manager {
manager.tableSession(table, tableProperties, catalogSession)) {
assertThat(tableSession).isNotSameAs(catalogSession);
assertThat(tableSession.headers()).containsOnly(entry("Authorization",
"Bearer table-token"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when table session created")
- .isNotNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -427,10 +371,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession tableSession =
manager.tableSession(table, tableProperties, catalogSession)) {
assertThat(tableSession.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when table session created")
- .isNotNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -493,10 +433,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession tableSession =
manager.tableSession(table, tableProperties, catalogSession)) {
assertThat(tableSession).isSameAs(catalogSession);
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should not create refresh executor when table credentials were
filtered out")
- .isNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -514,10 +450,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession tableSession =
(OAuth2Util.AuthSession) manager.tableSession(client, properties))
{
assertThat(tableSession.headers()).isEmpty();
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should not create refresh executor when no table credentials
provided")
- .isNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -535,10 +467,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession tableSession =
(OAuth2Util.AuthSession) manager.tableSession(client,
tableProperties)) {
assertThat(tableSession.headers()).containsOnly(entry("Authorization",
"Bearer table-token"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when table session created")
- .isNotNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -556,10 +484,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession tableSession =
(OAuth2Util.AuthSession) manager.tableSession(client,
tableProperties)) {
assertThat(tableSession.headers()).containsOnly(entry("Authorization",
"Bearer test"));
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should create refresh executor when table session created")
- .isNotNull();
assertThat(manager)
.extracting("sessionCache")
.asInstanceOf(type(AuthSessionCache.class))
@@ -611,10 +535,6 @@ class TestOAuth2Manager {
OAuth2Util.AuthSession tableSession =
manager.tableSession(table, tableProperties, contextualSession)) {
manager.close();
- assertThat(manager)
- .extracting("refreshExecutor")
- .as("should close refresh executor")
- .isNull();
assertThat(manager).extracting("sessionCache").as("should close session
cache").isNull();
// all cached sessions should be closed
Mockito.verify(contextualSession).close();
