This is an automated email from the ASF dual-hosted git repository.
amitj pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 868e308a1d Issues/oak 10781 (#1518)
868e308a1d is described below
commit 868e308a1d7832f086d397d528b2b786bd6e46cf
Author: Tushar <145645280+t-r...@users.noreply.github.com>
AuthorDate: Tue Jun 18 14:45:58 2024 +0530
Issues/oak 10781 (#1518)
OAK-10781: Access Token refresh in oak-blob-cloud-azure
* only one access token will exist per class instance, and there will only
be one token refresh exectuor that will check and refresh the access token.
* close executor
* reduce token refresh delay to 1 minute
---
oak-blob-cloud-azure/pom.xml | 7 +++
.../blobstorage/AzureBlobContainerProvider.java | 73 +++++++++++++---------
.../azure/blobstorage/AzureBlobStoreBackend.java | 1 +
.../azure/blobstorage/AzureDataStoreUtils.java | 10 +--
.../blob/cloud/azure/blobstorage/TestAzureDS.java | 10 +++
5 files changed, 66 insertions(+), 35 deletions(-)
diff --git a/oak-blob-cloud-azure/pom.xml b/oak-blob-cloud-azure/pom.xml
index c058e10419..328091dfce 100644
--- a/oak-blob-cloud-azure/pom.xml
+++ b/oak-blob-cloud-azure/pom.xml
@@ -351,6 +351,13 @@
<artifactId>testcontainers</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.jackrabbit</groupId>
+ <artifactId>oak-commons</artifactId>
+ <version>${project.version}</version>
+ <classifier>tests</classifier>
+ <scope>test</scope>
+ </dependency>
</dependencies>
diff --git
a/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobContainerProvider.java
b/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobContainerProvider.java
index 2f6d6b4f47..01522248b0 100644
---
a/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobContainerProvider.java
+++
b/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobContainerProvider.java
@@ -42,7 +42,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.Closeable;
-import java.io.IOException;
import java.net.URISyntaxException;
import java.security.InvalidKeyException;
import java.time.Instant;
@@ -51,6 +50,7 @@ import java.time.OffsetDateTime;
import java.time.format.DateTimeFormatter;
import java.util.Date;
import java.util.EnumSet;
+import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
import java.util.concurrent.Executors;
@@ -70,9 +70,12 @@ public class AzureBlobContainerProvider implements Closeable
{
private final String tenantId;
private final String clientId;
private final String clientSecret;
+ private ClientSecretCredential clientSecretCredential;
+ private AccessToken accessToken;
+ private StorageCredentialsToken storageCredentialsToken;
private static final long TOKEN_REFRESHER_INITIAL_DELAY = 45L;
private static final long TOKEN_REFRESHER_DELAY = 1L;
- private static final ScheduledExecutorService executorService =
Executors.newSingleThreadScheduledExecutor();
+ private final ScheduledExecutorService executorService =
Executors.newSingleThreadScheduledExecutor();
private AzureBlobContainerProvider(Builder builder) {
this.azureConnectionString = builder.azureConnectionString;
@@ -177,13 +180,17 @@ public class AzureBlobContainerProvider implements
Closeable {
public CloudBlobContainer getBlobContainer(@Nullable BlobRequestOptions
blobRequestOptions) throws DataStoreException {
// connection string will be given preference over service principals
/ sas / account key
if (StringUtils.isNotBlank(azureConnectionString)) {
+ log.debug("connecting to azure blob storage via
azureConnectionString");
return Utils.getBlobContainer(azureConnectionString,
containerName, blobRequestOptions);
} else if (authenticateViaServicePrincipal()) {
+ log.debug("connecting to azure blob storage via service principal
credentials");
return getBlobContainerFromServicePrincipals(blobRequestOptions);
} else if (StringUtils.isNotBlank(sasToken)) {
+ log.debug("connecting to azure blob storage via sas token");
final String connectionStringWithSasToken =
Utils.getConnectionStringForSas(sasToken, blobEndpoint, accountName);
return Utils.getBlobContainer(connectionStringWithSasToken,
containerName, blobRequestOptions);
}
+ log.debug("connecting to azure blob storage via access key");
final String connectionStringWithAccountKey =
Utils.getConnectionString(accountName, accountKey, blobEndpoint);
return Utils.getBlobContainer(connectionStringWithAccountKey,
containerName, blobRequestOptions);
}
@@ -205,19 +212,33 @@ public class AzureBlobContainerProvider implements
Closeable {
@NotNull
private StorageCredentialsToken getStorageCredentials() {
- ClientSecretCredential clientSecretCredential = new
ClientSecretCredentialBuilder()
- .clientId(clientId)
- .clientSecret(clientSecret)
- .tenantId(tenantId)
- .build();
- AccessToken accessToken = clientSecretCredential.getTokenSync(new
TokenRequestContext().addScopes(AZURE_DEFAULT_SCOPE));
- if (accessToken == null ||
StringUtils.isBlank(accessToken.getToken())) {
- log.error("Access token is null or empty");
- throw new IllegalArgumentException("Could not connect to azure
storage, access token is null or empty");
+ boolean isAccessTokenGenerated = false;
+ /* generate access token, the same token will be used for subsequent
access
+ * generated token is valid for 1 hour only and will be refreshed in
background
+ * */
+ if (accessToken == null) {
+ clientSecretCredential = new ClientSecretCredentialBuilder()
+ .clientId(clientId)
+ .clientSecret(clientSecret)
+ .tenantId(tenantId)
+ .build();
+ accessToken = clientSecretCredential.getTokenSync(new
TokenRequestContext().addScopes(AZURE_DEFAULT_SCOPE));
+ if (accessToken == null ||
StringUtils.isBlank(accessToken.getToken())) {
+ log.error("Access token is null or empty");
+ throw new IllegalArgumentException("Could not connect to azure
storage, access token is null or empty");
+ }
+ storageCredentialsToken = new StorageCredentialsToken(accountName,
accessToken.getToken());
+ isAccessTokenGenerated = true;
+ }
+
+ Objects.requireNonNull(storageCredentialsToken, "storage credentials
token cannot be null");
+
+ // start refresh token executor only when the access token is first
generated
+ if (isAccessTokenGenerated) {
+ log.info("starting refresh token task at: {}",
OffsetDateTime.now());
+ TokenRefresher tokenRefresher = new TokenRefresher();
+ executorService.scheduleWithFixedDelay(tokenRefresher,
TOKEN_REFRESHER_INITIAL_DELAY, TOKEN_REFRESHER_DELAY, TimeUnit.MINUTES);
}
- StorageCredentialsToken storageCredentialsToken = new
StorageCredentialsToken(accountName, accessToken.getToken());
- TokenRefresher tokenRefresher = new
TokenRefresher(clientSecretCredential, accessToken, storageCredentialsToken);
- executorService.scheduleWithFixedDelay(tokenRefresher,
TOKEN_REFRESHER_INITIAL_DELAY, TOKEN_REFRESHER_DELAY, TimeUnit.MINUTES);
return storageCredentialsToken;
}
@@ -292,20 +313,7 @@ public class AzureBlobContainerProvider implements
Closeable {
StringUtils.isNoneBlank(accountName, tenantId, clientId,
clientSecret);
}
- private static class TokenRefresher implements Runnable {
-
- private final ClientSecretCredential clientSecretCredential;
- private AccessToken accessToken;
- private final StorageCredentialsToken storageCredentialsToken;
-
- public TokenRefresher(ClientSecretCredential clientSecretCredential,
- AccessToken accessToken,
- StorageCredentialsToken storageCredentialsToken)
{
- this.clientSecretCredential = clientSecretCredential;
- this.accessToken = accessToken;
- this.storageCredentialsToken = storageCredentialsToken;
- }
-
+ private class TokenRefresher implements Runnable {
@Override
public void run() {
try {
@@ -315,12 +323,14 @@ public class AzureBlobContainerProvider implements
Closeable {
log.info("Access token is about to expire (5 minutes or
less) at: {}. New access token will be generated",
accessToken.getExpiresAt().format(DateTimeFormatter.ISO_LOCAL_DATE_TIME));
AccessToken newToken =
clientSecretCredential.getTokenSync(new
TokenRequestContext().addScopes(AZURE_DEFAULT_SCOPE));
+ log.info("New azure access token generated at: {}",
LocalDateTime.now());
if (newToken == null ||
StringUtils.isBlank(newToken.getToken())) {
log.error("New access token is null or empty");
return;
}
- this.accessToken = newToken;
-
this.storageCredentialsToken.updateToken(this.accessToken.getToken());
+ // update access token with newly generated token
+ accessToken = newToken;
+
storageCredentialsToken.updateToken(accessToken.getToken());
}
} catch (Exception e) {
log.error("Error while acquiring new access token: ", e);
@@ -329,7 +339,8 @@ public class AzureBlobContainerProvider implements
Closeable {
}
@Override
- public void close() throws IOException {
+ public void close() {
new ExecutorCloser(executorService).close();
+ log.info("Refresh token executor service shutdown completed");
}
}
diff --git
a/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
b/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
index b7338594a3..0eba902e7f 100644
---
a/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
+++
b/oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobStoreBackend.java
@@ -516,6 +516,7 @@ public class AzureBlobStoreBackend extends
AbstractSharedBackend {
@Override
public void close() throws DataStoreException {
+ azureBlobContainerProvider.close();
LOG.info("AzureBlobBackend closed.");
}
diff --git
a/oak-blob-cloud-azure/src/test/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureDataStoreUtils.java
b/oak-blob-cloud-azure/src/test/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureDataStoreUtils.java
index 8ef8df038e..e5a2a8e620 100644
---
a/oak-blob-cloud-azure/src/test/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureDataStoreUtils.java
+++
b/oak-blob-cloud-azure/src/test/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureDataStoreUtils.java
@@ -182,10 +182,12 @@ public class AzureDataStoreUtils extends DataStoreUtils {
Properties props = getAzureConfig();
props.setProperty(AzureConstants.AZURE_BLOB_CONTAINER_NAME,
containerName);
- CloudBlobContainer container =
AzureBlobContainerProvider.Builder.builder(containerName).initializeWithProperties(props)
- .build().getBlobContainer();
- boolean result = container.deleteIfExists();
- log.info("Container deleted. containerName={} existed={}",
containerName, result);
+ try (AzureBlobContainerProvider azureBlobContainerProvider =
AzureBlobContainerProvider.Builder.builder(containerName).initializeWithProperties(props)
+ .build()) {
+ CloudBlobContainer container =
azureBlobContainerProvider.getBlobContainer();
+ boolean result = container.deleteIfExists();
+ log.info("Container deleted. containerName={} existed={}",
containerName, result);
+ }
}
protected static HttpsURLConnection getHttpsConnection(long length, URI
uri) throws IOException {
diff --git
a/oak-blob-cloud-azure/src/test/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/TestAzureDS.java
b/oak-blob-cloud-azure/src/test/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/TestAzureDS.java
index f809a9e010..14eca1b94a 100644
---
a/oak-blob-cloud-azure/src/test/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/TestAzureDS.java
+++
b/oak-blob-cloud-azure/src/test/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/TestAzureDS.java
@@ -21,12 +21,15 @@ package
org.apache.jackrabbit.oak.blob.cloud.azure.blobstorage;
import static org.junit.Assume.assumeTrue;
import org.apache.jackrabbit.core.data.DataStore;
+import org.apache.jackrabbit.oak.commons.junit.LogCustomizer;
import org.apache.jackrabbit.oak.plugins.blob.datastore.AbstractDataStoreTest;
import org.junit.After;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
import java.util.Properties;
@@ -65,7 +68,14 @@ public class TestAzureDS extends AbstractDataStoreTest {
@After
public void tearDown() {
try {
+ LogCustomizer customizer =
LogCustomizer.forLogger(AzureBlobContainerProvider.class.getName())
+ .filter(Level.INFO)
+ .create();
+ customizer.starting();
super.tearDown();
+ Assert.assertEquals(1, customizer.getLogs().size());
+ Assert.assertEquals("Refresh token executor service shutdown completed",
customizer.getLogs().get(0));
+ customizer.finished();
AzureDataStoreUtils.deleteContainer(container);
} catch (Exception ignore) {
