Skip to content

Commit f538ea6

Browse files
committed
Use interface-based approach instead of Reflection for Credentials Object Creation
1 parent a0db54b commit f538ea6

File tree

9 files changed

+105
-61
lines changed

9 files changed

+105
-61
lines changed

athena-datalakegen2/src/main/java/com/amazonaws/athena/connectors/datalakegen2/DataLakeGen2MetadataHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ protected CredentialsProvider getCredentialProvider()
293293
return CredentialsProviderFactory.createCredentialProvider(
294294
getDatabaseConnectionConfig().getSecret(),
295295
getCachableSecretsManager(),
296-
DataLakeGen2OAuthCredentialsProvider.class
296+
new DataLakeGen2OAuthCredentialsProvider()
297297
);
298298
}
299299
}

athena-datalakegen2/src/main/java/com/amazonaws/athena/connectors/datalakegen2/DataLakeGen2OAuthCredentialsProvider.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121

2222
import com.amazonaws.athena.connector.credentials.CredentialsConstants;
2323
import com.amazonaws.athena.connector.credentials.OAuthCredentialsProvider;
24-
import com.amazonaws.athena.connector.lambda.security.CachableSecretsManager;
2524
import com.google.common.annotations.VisibleForTesting;
2625

2726
import java.net.URI;
@@ -38,15 +37,15 @@ public class DataLakeGen2OAuthCredentialsProvider extends OAuthCredentialsProvid
3837
private static final String SCOPE = "https://sql.azuresynapse.net/.default";
3938
private static final String TENANT_ID = "tenant_id";
4039

41-
public DataLakeGen2OAuthCredentialsProvider(String secretName, Map<String, String> secretMap, CachableSecretsManager secretsManager)
40+
public DataLakeGen2OAuthCredentialsProvider()
4241
{
43-
super(secretName, secretMap, secretsManager);
42+
super();
4443
}
4544

4645
@VisibleForTesting
47-
public DataLakeGen2OAuthCredentialsProvider(String secretName, Map<String, String> secretMap, CachableSecretsManager secretsManager, HttpClient httpClient)
46+
protected DataLakeGen2OAuthCredentialsProvider(HttpClient httpClient)
4847
{
49-
super(secretName, secretMap, secretsManager, httpClient);
48+
super(httpClient);
5049
}
5150

5251
@Override

athena-datalakegen2/src/main/java/com/amazonaws/athena/connectors/datalakegen2/DataLakeGen2RecordHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ protected CredentialsProvider getCredentialProvider()
8585
return CredentialsProviderFactory.createCredentialProvider(
8686
getDatabaseConnectionConfig().getSecret(),
8787
getCachableSecretsManager(),
88-
DataLakeGen2OAuthCredentialsProvider.class
88+
new DataLakeGen2OAuthCredentialsProvider()
8989
);
9090
}
9191
}

athena-datalakegen2/src/test/java/com/amazonaws/athena/connectors/datalakegen2/DataLakeGen2OAuthCredentialsProviderTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ public void setup()
7070
{
7171
MockitoAnnotations.openMocks(this);
7272
CachableSecretsManager cachableSecretsManager = new CachableSecretsManager(secretsManagerClient);
73-
credentialsProvider = new DataLakeGen2OAuthCredentialsProvider(SECRET_NAME, new HashMap<>(), cachableSecretsManager, httpClient);
73+
credentialsProvider = new DataLakeGen2OAuthCredentialsProvider(httpClient);
74+
credentialsProvider.initialize(SECRET_NAME, new HashMap<>(), cachableSecretsManager);
7475
objectMapper = new ObjectMapper();
7576
}
7677

athena-federation-sdk/src/main/java/com/amazonaws/athena/connector/credentials/CredentialsProviderFactory.java

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import software.amazon.awssdk.services.glue.model.FederationSourceErrorCode;
2828

2929
import java.io.IOException;
30-
import java.lang.reflect.Constructor;
3130
import java.util.Map;
3231

3332
/**
@@ -50,40 +49,37 @@ private CredentialsProviderFactory()
5049
*
5150
* @param secretName The name of the secret in AWS Secrets Manager
5251
* @param secretsManager The secrets manager instance
53-
* @param oAuthProviderClass The class of the OAuth provider to instantiate (must extend OAuthCredentialsProvider)
52+
* @param provider The InitializableCredentialsProvider instance to check and initialize
5453
* @return A new CredentialsProvider instance based on the secret configuration
5554
* @throws AthenaConnectorException if there are errors deserializing the secret or creating the provider
5655
*/
57-
public static <T extends OAuthCredentialsProvider> CredentialsProvider createCredentialProvider(
56+
public static CredentialsProvider createCredentialProvider(
5857
String secretName,
5958
CachableSecretsManager secretsManager,
60-
Class<T> oAuthProviderClass)
59+
InitializableCredentialsProvider provider)
6160
{
6261
if (StringUtils.isNotBlank(secretName)) {
6362
try {
6463
String secretString = secretsManager.getSecret(secretName);
6564
Map<String, String> secretMap = OBJECT_MAPPER.readValue(secretString, Map.class);
6665

67-
// Create an instance of the OAuth provider
68-
T provider;
69-
try {
70-
Constructor<T> constructor = oAuthProviderClass.getConstructor(
71-
String.class, Map.class, CachableSecretsManager.class);
72-
provider = constructor.newInstance(secretName, secretMap, secretsManager);
66+
if (provider instanceof OAuthCredentialsProvider) {
67+
OAuthCredentialsProvider oauthProvider = (OAuthCredentialsProvider) provider;
68+
try {
69+
// Check if OAuth is configured
70+
if (oauthProvider.isOAuthConfigured(secretMap)) {
71+
oauthProvider.initialize(secretName, secretMap, secretsManager);
72+
return oauthProvider;
73+
}
74+
}
75+
catch (RuntimeException e) {
76+
throw new AthenaConnectorException("Failed to create OAuth provider: " + e.getMessage(),
77+
ErrorDetails.builder()
78+
.errorCode(FederationSourceErrorCode.INTERNAL_SERVICE_EXCEPTION.toString())
79+
.errorMessage(e.getMessage())
80+
.build());
81+
}
7382
}
74-
catch (ReflectiveOperationException e) {
75-
throw new AthenaConnectorException("Failed to create OAuth provider: " + e.getMessage(),
76-
ErrorDetails.builder()
77-
.errorCode(FederationSourceErrorCode.INTERNAL_SERVICE_EXCEPTION.toString())
78-
.errorMessage(e.getMessage())
79-
.build());
80-
}
81-
82-
// Check if OAuth is configured
83-
if (provider.isOAuthConfigured(secretMap)) {
84-
return provider;
85-
}
86-
8783
// Fall back to default credentials if OAuth is not configured
8884
return new DefaultCredentialsProvider(secretString);
8985
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*-
2+
* #%L
3+
* athena-federation-sdk
4+
* %%
5+
* Copyright (C) 2019 - 2025 Amazon Web Services
6+
* %%
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
* #L%
19+
*/
20+
package com.amazonaws.athena.connector.credentials;
21+
22+
import com.amazonaws.athena.connector.lambda.security.CachableSecretsManager;
23+
24+
import java.util.Map;
25+
26+
/**
27+
* Extended contract for credentials providers that require initialization with configuration from AWS Secrets Manager.
28+
* Implementations must be initialized before use and may support various authentication methods including OAuth,
29+
* username/password, or other configurable authentication schemes.
30+
*/
31+
public interface InitializableCredentialsProvider extends CredentialsProvider
32+
{
33+
/**
34+
* Initializes this credential provider with the given configuration.
35+
* Must be called exactly once before any calls to getCredential().
36+
*
37+
* @param secretName The name of the secret in AWS Secrets Manager
38+
* @param secretMap The secret configuration containing authentication parameters
39+
* @param secretsManager The secrets manager instance for retrieving and updating secrets
40+
*/
41+
void initialize(String secretName, Map<String, String> secretMap, CachableSecretsManager secretsManager);
42+
}

athena-federation-sdk/src/main/java/com/amazonaws/athena/connector/credentials/OAuthCredentialsProvider.java

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import com.fasterxml.jackson.core.JsonProcessingException;
2525
import com.fasterxml.jackson.databind.JsonNode;
2626
import com.fasterxml.jackson.databind.ObjectMapper;
27+
import com.google.common.annotations.VisibleForTesting;
2728
import org.slf4j.Logger;
2829
import org.slf4j.LoggerFactory;
2930
import software.amazon.awssdk.services.glue.model.ErrorDetails;
@@ -39,33 +40,42 @@
3940

4041
import static com.amazonaws.athena.connector.credentials.CredentialsConstants.ACCESS_TOKEN;
4142
import static com.amazonaws.athena.connector.credentials.CredentialsConstants.EXPIRES_IN;
43+
import static com.amazonaws.athena.connector.credentials.CredentialsConstants.FETCHED_AT;
4244

4345
/**
4446
* Base class for OAuth credential providers.
4547
* Handles OAuth token lifecycle management.
4648
*/
47-
public abstract class OAuthCredentialsProvider implements CredentialsProvider
49+
public abstract class OAuthCredentialsProvider implements InitializableCredentialsProvider
4850
{
4951
private static final Logger LOGGER = LoggerFactory.getLogger(OAuthCredentialsProvider.class);
5052
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
5153

52-
private final String secretName;
53-
private final CachableSecretsManager secretsManager;
54-
private final Map<String, String> secretMap;
54+
private String secretName;
55+
private CachableSecretsManager secretsManager;
56+
private Map<String, String> secretMap;
57+
private HttpClient httpClient;
5558

56-
private final HttpClient httpClient;
59+
protected OAuthCredentialsProvider()
60+
{
61+
// Initialized later via initialize() method
62+
}
5763

58-
protected OAuthCredentialsProvider(String secretName, Map<String, String> secretMap, CachableSecretsManager secretsManager)
64+
@VisibleForTesting
65+
protected OAuthCredentialsProvider(HttpClient httpClient)
5966
{
60-
this(secretName, secretMap, secretsManager, HttpClient.newBuilder().build());
67+
this.httpClient = httpClient;
6168
}
6269

63-
protected OAuthCredentialsProvider(String secretName, Map<String, String> secretMap, CachableSecretsManager secretsManager, HttpClient httpClient)
70+
@Override
71+
public void initialize(String secretName, Map<String, String> secretMap, CachableSecretsManager secretsManager)
6472
{
6573
this.secretName = secretName;
66-
this.secretsManager = secretsManager;
6774
this.secretMap = secretMap;
68-
this.httpClient = httpClient;
75+
this.secretsManager = secretsManager;
76+
this.httpClient = (this.httpClient != null)
77+
? this.httpClient
78+
: HttpClient.newBuilder().build();
6979
}
7080

7181
@Override
@@ -121,7 +131,7 @@ private String getOAuthAccessToken(Map<String, String> secretMap) throws IOExcep
121131
String cached = secretMap.get(ACCESS_TOKEN);
122132
if (cached != null) {
123133
long expiresIn = Long.parseLong(secretMap.getOrDefault(EXPIRES_IN, "0"));
124-
long fetchedAt = Long.parseLong(secretMap.getOrDefault(CredentialsConstants.FETCHED_AT, "0"));
134+
long fetchedAt = Long.parseLong(secretMap.getOrDefault(FETCHED_AT, "0"));
125135
long now = System.currentTimeMillis() / 1000;
126136

127137
if ((now - fetchedAt) < expiresIn - 60) {
@@ -186,7 +196,7 @@ private String getOAuthAccessToken(Map<String, String> secretMap) throws IOExcep
186196

187197
secretMap.put(ACCESS_TOKEN, accessToken);
188198
secretMap.put(EXPIRES_IN, String.valueOf(expiresIn));
189-
secretMap.put(CredentialsConstants.FETCHED_AT, String.valueOf(fetchedAt));
199+
secretMap.put(FETCHED_AT, String.valueOf(fetchedAt));
190200

191201
try {
192202
String secretString = OBJECT_MAPPER.writeValueAsString(secretMap);

athena-federation-sdk/src/test/java/com/amazonaws/athena/connector/credentials/CredentialsProviderFactoryTest.java

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,6 @@ public void setup()
7070
// Test OAuth provider class for testing
7171
public static class TestOAuthProvider extends OAuthCredentialsProvider
7272
{
73-
public TestOAuthProvider(String secretName, Map<String, String> secretMap, CachableSecretsManager secretsManager)
74-
{
75-
super(secretName, secretMap, secretsManager);
76-
}
77-
7873
@Override
7974
protected boolean isOAuthConfigured(Map<String, String> secretMap)
8075
{
@@ -103,7 +98,7 @@ public void testCreateCredentialProvider_whenOAuthConfigured() throws JsonProces
10398
CredentialsProvider provider = CredentialsProviderFactory.createCredentialProvider(
10499
SECRET_NAME,
105100
cachableSecretsManager,
106-
TestOAuthProvider.class
101+
new TestOAuthProvider()
107102
);
108103
assertTrue(provider instanceof TestOAuthProvider);
109104
}
@@ -123,7 +118,7 @@ public void testCreateCredentialProvider_whenUsernamePasswordConfigured() throws
123118
CredentialsProvider provider = CredentialsProviderFactory.createCredentialProvider(
124119
SECRET_NAME,
125120
cachableSecretsManager,
126-
TestOAuthProvider.class
121+
new TestOAuthProvider()
127122
);
128123
assertTrue(provider instanceof DefaultCredentialsProvider);
129124
}
@@ -134,7 +129,7 @@ public void testCreateCredentialProvider_whenNoSecret()
134129
CredentialsProvider provider = CredentialsProviderFactory.createCredentialProvider(
135130
"",
136131
cachableSecretsManager,
137-
TestOAuthProvider.class
132+
new TestOAuthProvider()
138133
);
139134
assertNull(provider);
140135
}
@@ -151,7 +146,7 @@ public void testCreateCredentialProvider_whenInvalidJson_throwsException()
151146
CredentialsProviderFactory.createCredentialProvider(
152147
SECRET_NAME,
153148
cachableSecretsManager,
154-
TestOAuthProvider.class
149+
new TestOAuthProvider()
155150
);
156151
fail("Expected AthenaConnectorException");
157152
}
@@ -168,11 +163,6 @@ public void testCreateCredentialProvider_whenInvalidOAuthClass_throwsException()
168163
// Create a class that doesn't properly implement the required methods
169164
class InvalidOAuthProvider extends OAuthCredentialsProvider
170165
{
171-
public InvalidOAuthProvider(String secretName, Map<String, String> secretMap, CachableSecretsManager secretsManager)
172-
{
173-
super(secretName, secretMap, secretsManager);
174-
}
175-
176166
@Override
177167
protected boolean isOAuthConfigured(Map<String, String> secretMap)
178168
{
@@ -198,7 +188,7 @@ protected HttpRequest buildTokenRequest(Map<String, String> secretMap)
198188
CredentialsProviderFactory.createCredentialProvider(
199189
SECRET_NAME,
200190
cachableSecretsManager,
201-
InvalidOAuthProvider.class
191+
new InvalidOAuthProvider()
202192
);
203193
fail("Expected AthenaConnectorException");
204194
}

athena-federation-sdk/src/test/java/com/amazonaws/athena/connector/credentials/OAuthCredentialsProviderTest.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ public class OAuthCredentialsProviderTest
7575
public void setUp()
7676
{
7777
secretMap = new HashMap<>();
78-
credentialsProvider = new TestOAuthCredentialsProvider(TEST_SECRET_NAME, mockSecretsClient, mockHttpClient, secretMap);
78+
credentialsProvider = new TestOAuthCredentialsProvider(mockHttpClient);
79+
credentialsProvider.initialize(TEST_SECRET_NAME, secretMap, new CachableSecretsManager(mockSecretsClient));
7980
}
8081

8182
@Test
@@ -377,9 +378,14 @@ private String createTokenResponse()
377378

378379
private static class TestOAuthCredentialsProvider extends OAuthCredentialsProvider
379380
{
380-
public TestOAuthCredentialsProvider(String secretName, SecretsManagerClient secretsClient, HttpClient httpClient, Map<String, String> secretMap)
381+
public TestOAuthCredentialsProvider()
381382
{
382-
super(secretName, secretMap, new CachableSecretsManager(secretsClient), httpClient);
383+
super();
384+
}
385+
386+
public TestOAuthCredentialsProvider(HttpClient httpClient)
387+
{
388+
super(httpClient);
383389
}
384390

385391
@Override

0 commit comments

Comments
 (0)