Skip to content

Commit 9ff0603

Browse files
committed
Core: Simplify AuthManager API
The current `AuthManager` interface exposes various methods for creating sessions: `initSession()`, `catalogSession()`, `contextualSession()`, `tableSession()`, etc. Naming its methods after the intended usage of the returned session was, in hindsight, a questionable design choice: in some cases, the right method to call may not be so obvious, and if more session "flavors" are need in the future, more methods would need to be added to the interface. This PR aims at fixing this by unifying all previous methods under one single method. The increased flexibility of the new method is due to its argument of type `AuthScope`, which can be freely implemented according to the caller's needs.
1 parent bf2f552 commit 9ff0603

File tree

15 files changed

+866
-256
lines changed

15 files changed

+866
-256
lines changed

aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthManager.java

+51-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import org.apache.iceberg.rest.RESTClient;
2626
import org.apache.iceberg.rest.RESTUtil;
2727
import org.apache.iceberg.rest.auth.AuthManager;
28+
import org.apache.iceberg.rest.auth.AuthScope;
29+
import org.apache.iceberg.rest.auth.AuthScopes;
2830
import org.apache.iceberg.rest.auth.AuthSession;
2931
import software.amazon.awssdk.auth.signer.Aws4Signer;
3032

@@ -36,22 +38,58 @@
3638
@SuppressWarnings("unused") // loaded by reflection
3739
public class RESTSigV4AuthManager implements AuthManager {
3840

39-
private final Aws4Signer signer = Aws4Signer.create();
41+
private final Aws4Signer signer;
4042
private final AuthManager delegate;
4143

4244
private Map<String, String> catalogProperties = Map.of();
4345

4446
public RESTSigV4AuthManager(String name, AuthManager delegate) {
4547
this.delegate = Preconditions.checkNotNull(delegate, "Invalid delegate: null");
48+
signer = Aws4Signer.create();
4649
}
4750

51+
private RESTSigV4AuthManager(AuthManager delegate, Aws4Signer signer) {
52+
this.delegate = Preconditions.checkNotNull(delegate, "Invalid delegate: null");
53+
this.signer = signer;
54+
}
55+
56+
@Override
57+
public AuthManager withClient(RESTClient client) {
58+
return new RESTSigV4AuthManager(delegate.withClient(client), signer);
59+
}
60+
61+
@Override
62+
public AuthSession authSession(AuthScope scope) {
63+
AwsProperties awsProperties;
64+
if (scope instanceof AuthScopes.Catalog) {
65+
this.catalogProperties = scope.properties();
66+
awsProperties = new AwsProperties(catalogProperties);
67+
} else {
68+
awsProperties = new AwsProperties(RESTUtil.merge(catalogProperties, scope.properties()));
69+
}
70+
71+
return new RESTSigV4AuthSession(signer, delegate.authSession(scope), awsProperties);
72+
}
73+
74+
/**
75+
* @deprecated since 1.9.0, will be removed in 1.10.0; use {@link #authSession(AuthScope)}
76+
* instead.
77+
*/
4878
@Override
79+
@Deprecated
80+
@SuppressWarnings("deprecation")
4981
public RESTSigV4AuthSession initSession(RESTClient initClient, Map<String, String> properties) {
5082
return new RESTSigV4AuthSession(
5183
signer, delegate.initSession(initClient, properties), new AwsProperties(properties));
5284
}
5385

86+
/**
87+
* @deprecated since 1.9.0, will be removed in 1.10.0; use {@link #authSession(AuthScope)}
88+
* instead.
89+
*/
5490
@Override
91+
@Deprecated
92+
@SuppressWarnings("deprecation")
5593
public RESTSigV4AuthSession catalogSession(
5694
RESTClient sharedClient, Map<String, String> properties) {
5795
this.catalogProperties = properties;
@@ -60,15 +98,27 @@ public RESTSigV4AuthSession catalogSession(
6098
signer, delegate.catalogSession(sharedClient, catalogProperties), awsProperties);
6199
}
62100

101+
/**
102+
* @deprecated since 1.9.0, will be removed in 1.10.0; use {@link #authSession(AuthScope)}
103+
* instead.
104+
*/
63105
@Override
106+
@Deprecated
107+
@SuppressWarnings("deprecation")
64108
public AuthSession contextualSession(SessionCatalog.SessionContext context, AuthSession parent) {
65109
AwsProperties contextProperties =
66110
new AwsProperties(RESTUtil.merge(catalogProperties, context.properties()));
67111
return new RESTSigV4AuthSession(
68112
signer, delegate.contextualSession(context, parent), contextProperties);
69113
}
70114

115+
/**
116+
* @deprecated since 1.9.0, will be removed in 1.10.0; use {@link #authSession(AuthScope)}
117+
* instead.
118+
*/
71119
@Override
120+
@Deprecated
121+
@SuppressWarnings("deprecation")
72122
public AuthSession tableSession(
73123
TableIdentifier table, Map<String, String> properties, AuthSession parent) {
74124
AwsProperties tableProperties =

aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.iceberg.rest.auth.AuthManager;
3333
import org.apache.iceberg.rest.auth.AuthManagers;
3434
import org.apache.iceberg.rest.auth.AuthSession;
35+
import org.apache.iceberg.rest.auth.ImmutableAuthScopes;
3536
import org.apache.iceberg.rest.credentials.Credential;
3637
import org.apache.iceberg.rest.responses.LoadCredentialsResponse;
3738
import software.amazon.awssdk.auth.credentials.AwsCredentials;
@@ -81,9 +82,11 @@ private RESTClient httpClient() {
8182
if (null == client) {
8283
synchronized (this) {
8384
if (null == client) {
84-
authManager = AuthManagers.loadAuthManager("s3-credentials-refresh", properties);
8585
HTTPClient httpClient = HTTPClient.builder(properties).uri(properties.get(URI)).build();
86-
authSession = authManager.catalogSession(httpClient, properties);
86+
authManager =
87+
AuthManagers.loadAuthManager("s3-credentials-refresh", properties)
88+
.withClient(httpClient);
89+
authSession = authManager.authSession(ImmutableAuthScopes.Standalone.of(properties));
8790
client = httpClient.withAuthSession(authSession);
8891
}
8992
}

aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.apache.iceberg.rest.auth.AuthManager;
4343
import org.apache.iceberg.rest.auth.AuthManagers;
4444
import org.apache.iceberg.rest.auth.AuthSession;
45+
import org.apache.iceberg.rest.auth.ImmutableAuthScopes;
4546
import org.apache.iceberg.rest.auth.OAuth2Properties;
4647
import org.apache.iceberg.rest.auth.OAuth2Util;
4748
import org.apache.iceberg.util.PropertyUtil;
@@ -156,7 +157,8 @@ AuthSession authSession() {
156157
if (null == authSession) {
157158
synchronized (S3V4RestSignerClient.class) {
158159
if (null == authSession) {
159-
authManager = AuthManagers.loadAuthManager("s3-signer", properties());
160+
authManager =
161+
AuthManagers.loadAuthManager("s3-signer", properties()).withClient(httpClient());
160162
ImmutableMap.Builder<String, String> properties =
161163
ImmutableMap.<String, String>builder()
162164
.putAll(properties())
@@ -173,7 +175,9 @@ AuthSession authSession() {
173175
properties.put(OAuth2Properties.CREDENTIAL, credential());
174176
}
175177

176-
authSession = authManager.tableSession(httpClient(), properties.buildKeepingLast());
178+
authSession =
179+
authManager.authSession(
180+
ImmutableAuthScopes.Standalone.of(properties.buildKeepingLast()));
177181
}
178182
}
179183
}

aws/src/test/java/org/apache/iceberg/aws/TestRESTSigV4AuthManager.java

+34-36
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,26 @@
2020

2121
import static org.assertj.core.api.Assertions.assertThat;
2222
import static org.assertj.core.api.Assertions.assertThatThrownBy;
23+
import static org.assertj.core.api.InstanceOfAssertFactories.type;
2324
import static org.mockito.ArgumentMatchers.any;
2425
import static org.mockito.Mockito.when;
2526

2627
import java.util.Map;
2728
import org.apache.iceberg.catalog.SessionCatalog;
2829
import org.apache.iceberg.catalog.TableIdentifier;
29-
import org.apache.iceberg.rest.HTTPClient;
30-
import org.apache.iceberg.rest.RESTClient;
3130
import org.apache.iceberg.rest.auth.AuthManager;
3231
import org.apache.iceberg.rest.auth.AuthManagers;
3332
import org.apache.iceberg.rest.auth.AuthProperties;
3433
import org.apache.iceberg.rest.auth.AuthSession;
34+
import org.apache.iceberg.rest.auth.ImmutableAuthScopes;
3535
import org.apache.iceberg.rest.auth.NoopAuthManager;
3636
import org.apache.iceberg.rest.auth.OAuth2Manager;
3737
import org.apache.iceberg.rest.auth.OAuth2Util;
3838
import org.junit.jupiter.api.Test;
3939
import org.mockito.Mockito;
40+
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
41+
import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider;
42+
import software.amazon.awssdk.regions.Region;
4043

4144
class TestRESTSigV4AuthManager {
4245

@@ -106,64 +109,59 @@ void createInvalidCustomDelegate() {
106109
@Test
107110
void initSession() {
108111
AuthManager delegate = Mockito.mock(AuthManager.class);
109-
when(delegate.initSession(any(), any())).thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
110-
RESTClient client = Mockito.mock(RESTClient.class);
112+
when(delegate.authSession(any())).thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
111113
AuthManager manager = new RESTSigV4AuthManager("test", delegate);
112-
AuthSession authSession = manager.initSession(client, awsProperties);
113-
assertThat(authSession)
114-
.isInstanceOf(RESTSigV4AuthSession.class)
115-
.extracting("delegate")
116-
.isInstanceOf(OAuth2Util.AuthSession.class);
114+
AuthSession authSession = manager.authSession(ImmutableAuthScopes.Initial.of(awsProperties));
115+
assertAuthSession(authSession);
117116
}
118117

119118
@Test
120-
void catalogSession() {
119+
void authSession() {
121120
AuthManager delegate = Mockito.mock(AuthManager.class);
122-
when(delegate.catalogSession(any(), any()))
123-
.thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
124-
RESTClient client = Mockito.mock(RESTClient.class);
121+
when(delegate.authSession(any())).thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
125122
AuthManager manager = new RESTSigV4AuthManager("test", delegate);
126-
AuthSession authSession = manager.catalogSession(client, awsProperties);
127-
assertThat(authSession)
128-
.isInstanceOf(RESTSigV4AuthSession.class)
129-
.extracting("delegate")
130-
.isInstanceOf(OAuth2Util.AuthSession.class);
123+
AuthSession authSession = manager.authSession(ImmutableAuthScopes.Catalog.of(awsProperties));
124+
assertAuthSession(authSession);
131125
}
132126

133127
@Test
134128
void contextualSession() {
135129
AuthManager delegate = Mockito.mock(AuthManager.class);
136-
when(delegate.catalogSession(any(), any()))
137-
.thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
138-
when(delegate.contextualSession(any(), any()))
139-
.thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
130+
when(delegate.authSession(any())).thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
140131
AuthManager manager = new RESTSigV4AuthManager("test", delegate);
141-
manager.catalogSession(Mockito.mock(HTTPClient.class), awsProperties);
132+
AuthSession catalogSession = manager.authSession(ImmutableAuthScopes.Catalog.of(awsProperties));
142133
AuthSession authSession =
143-
manager.contextualSession(
144-
Mockito.mock(SessionCatalog.SessionContext.class), Mockito.mock(AuthSession.class));
145-
assertThat(authSession)
146-
.isInstanceOf(RESTSigV4AuthSession.class)
147-
.extracting("delegate")
148-
.isInstanceOf(OAuth2Util.AuthSession.class);
134+
manager.authSession(
135+
ImmutableAuthScopes.Contextual.of(
136+
Mockito.mock(SessionCatalog.SessionContext.class), catalogSession));
137+
assertAuthSession(authSession);
149138
}
150139

151140
@Test
152141
void tableSession() {
153142
AuthManager delegate = Mockito.mock(AuthManager.class);
154-
when(delegate.catalogSession(any(), any()))
155-
.thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
156-
when(delegate.tableSession(any(), any(), any()))
157-
.thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
143+
when(delegate.authSession(any())).thenReturn(Mockito.mock(OAuth2Util.AuthSession.class));
158144
AuthManager manager = new RESTSigV4AuthManager("test", delegate);
159-
manager.catalogSession(Mockito.mock(HTTPClient.class), awsProperties);
145+
AuthSession catalogSession = manager.authSession(ImmutableAuthScopes.Catalog.of(awsProperties));
160146
AuthSession authSession =
161-
manager.tableSession(
162-
Mockito.mock(TableIdentifier.class), Map.of(), Mockito.mock(AuthSession.class));
147+
manager.authSession(
148+
ImmutableAuthScopes.Table.of(
149+
Mockito.mock(TableIdentifier.class), Map.of(), catalogSession));
150+
assertAuthSession(authSession);
151+
}
152+
153+
private static void assertAuthSession(AuthSession authSession) {
163154
assertThat(authSession)
164155
.isInstanceOf(RESTSigV4AuthSession.class)
165156
.extracting("delegate")
166157
.isInstanceOf(OAuth2Util.AuthSession.class);
158+
assertThat(authSession).extracting("signer").isNotNull();
159+
assertThat(authSession).extracting("signingRegion").isEqualTo(Region.of("us-west-2"));
160+
assertThat(authSession)
161+
.extracting("credentialsProvider")
162+
.asInstanceOf(type(StaticCredentialsProvider.class))
163+
.extracting(StaticCredentialsProvider::resolveCredentials)
164+
.isEqualTo(AwsBasicCredentials.create("id", "secret"));
167165
}
168166

169167
@Test

aws/src/test/java/org/apache/iceberg/aws/TestRESTSigV4Signer.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.iceberg.rest.auth.AuthManagers;
3333
import org.apache.iceberg.rest.auth.AuthProperties;
3434
import org.apache.iceberg.rest.auth.AuthSession;
35+
import org.apache.iceberg.rest.auth.ImmutableAuthScopes;
3536
import org.apache.iceberg.rest.auth.OAuth2Util;
3637
import org.apache.iceberg.rest.responses.ConfigResponse;
3738
import org.apache.iceberg.rest.responses.OAuthTokenResponse;
@@ -78,8 +79,9 @@ public static void beforeClass() {
7879
.build()
7980
.withAuthSession(AuthSession.EMPTY);
8081

81-
authManager = AuthManagers.loadAuthManager("test", properties);
82-
AuthSession authSession = authManager.catalogSession(httpClient, properties);
82+
authManager = AuthManagers.loadAuthManager("test", properties).withClient(httpClient);
83+
AuthSession authSession =
84+
authManager.authSession(ImmutableAuthScopes.Standalone.of(properties));
8385

8486
client = httpClient.withAuthSession(authSession);
8587
}

0 commit comments

Comments
 (0)