ierandra commented on code in PR #1861:
URL: https://github.com/apache/jackrabbit-oak/pull/1861#discussion_r1862378411


##########
oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/AzureBlobContainerProvider.java:
##########
@@ -171,176 +152,99 @@ public String getContainerName() {
         return containerName;
     }
 
+    public String getAzureConnectionString() {
+        return azureConnectionString;
+    }
+
     @NotNull
-    public CloudBlobContainer getBlobContainer() throws DataStoreException {
-        return this.getBlobContainer(null);
+    public BlobContainerClient getBlobContainer() throws DataStoreException {
+        return this.getBlobContainer(null, false, null);
     }
 
     @NotNull
-    public CloudBlobContainer getBlobContainer(@Nullable BlobRequestOptions 
blobRequestOptions) throws DataStoreException {
+    public BlobContainerClient getBlobContainer(@Nullable RequestRetryOptions 
retryOptions, boolean isProxyNeeded, Properties properties) 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);
+            return getBlobContainerFromConnectionString();
         } else if (authenticateViaServicePrincipal()) {
             log.debug("connecting to azure blob storage via service principal 
credentials");
-            return getBlobContainerFromServicePrincipals(blobRequestOptions);
+            return getBlobContainerFromServicePrincipals(accountName, 
clientId, clientSecret, tenantId, retryOptions);
         } 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);
+            return Utils.getBlobContainer(accountName, accountKey, 
connectionStringWithSasToken, containerName, retryOptions, isProxyNeeded, 
properties);
         }
         log.debug("connecting to azure blob storage via access key");
         final String connectionStringWithAccountKey = 
Utils.getConnectionString(accountName, accountKey, blobEndpoint);
-        return Utils.getBlobContainer(connectionStringWithAccountKey, 
containerName, blobRequestOptions);
-    }
-
-    @NotNull
-    private CloudBlobContainer getBlobContainerFromServicePrincipals(@Nullable 
BlobRequestOptions blobRequestOptions) throws DataStoreException {
-        StorageCredentialsToken storageCredentialsToken = 
getStorageCredentials();
-        try {
-            CloudStorageAccount cloud = new 
CloudStorageAccount(storageCredentialsToken, true, DEFAULT_ENDPOINT_SUFFIX, 
accountName);
-            CloudBlobClient cloudBlobClient = cloud.createCloudBlobClient();
-            if (blobRequestOptions != null) {
-                cloudBlobClient.setDefaultRequestOptions(blobRequestOptions);
-            }
-            return cloudBlobClient.getContainerReference(containerName);
-        } catch (URISyntaxException | StorageException e) {
-            throw new DataStoreException(e);
-        }
-    }
-
-    @NotNull
-    private StorageCredentialsToken getStorageCredentials() {
-        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);
-        }
-        return storageCredentialsToken;
+        return Utils.getBlobContainer(accountName, accountKey, 
connectionStringWithAccountKey, containerName, retryOptions, isProxyNeeded, 
properties);
     }
 
     @NotNull
-    public String generateSharedAccessSignature(BlobRequestOptions 
requestOptions,
+    public String generateSharedAccessSignature(RequestRetryOptions 
retryOptions,
                                                 String key,
-                                                
EnumSet<SharedAccessBlobPermissions> permissions,
+                                                BlobSasPermission 
blobSasPermissions,
                                                 int expirySeconds,
-                                                SharedAccessBlobHeaders 
optionalHeaders) throws DataStoreException, URISyntaxException, 
StorageException, InvalidKeyException {
-        SharedAccessBlobPolicy policy = new SharedAccessBlobPolicy();
-        Date expiry = Date.from(Instant.now().plusSeconds(expirySeconds));
-        policy.setSharedAccessExpiryTime(expiry);
-        policy.setPermissions(permissions);
+                                                boolean isProxyNeeded, 
Properties properties) throws DataStoreException, URISyntaxException, 
InvalidKeyException {
+
+        OffsetDateTime expiry = 
OffsetDateTime.now().plusSeconds(expirySeconds);
+        BlobServiceSasSignatureValues serviceSasSignatureValues = new 
BlobServiceSasSignatureValues(expiry, blobSasPermissions);
 
-        CloudBlockBlob blob = 
getBlobContainer(requestOptions).getBlockBlobReference(key);
+        BlockBlobClient blob = getBlobContainer(retryOptions, isProxyNeeded, 
properties).getBlobClient(key).getBlockBlobClient();
 
         if (authenticateViaServicePrincipal()) {
-            return generateUserDelegationKeySignedSas(blob, policy, 
optionalHeaders, expiry);
+            return generateUserDelegationKeySignedSas(blob, 
serviceSasSignatureValues, expiry);
         }
-        return generateSas(blob, policy, optionalHeaders);
+        return generateSas(blob, serviceSasSignatureValues);
     }
 
     @NotNull
-    private String generateUserDelegationKeySignedSas(CloudBlockBlob blob,
-                                                      SharedAccessBlobPolicy 
policy,
-                                                      SharedAccessBlobHeaders 
optionalHeaders,
-                                                      Date expiry) throws 
StorageException {
-        fillEmptyHeaders(optionalHeaders);
-        UserDelegationKey userDelegationKey = 
blob.getServiceClient().getUserDelegationKey(Date.from(Instant.now().minusSeconds(900)),
-                expiry);
-        return optionalHeaders == null ? 
blob.generateUserDelegationSharedAccessSignature(userDelegationKey, policy) :
-                
blob.generateUserDelegationSharedAccessSignature(userDelegationKey, policy, 
optionalHeaders, null, null);
+    public String generateUserDelegationKeySignedSas(BlockBlobClient 
blobClient,
+                                                     
BlobServiceSasSignatureValues serviceSasSignatureValues,
+                                                     OffsetDateTime 
expiryTime) {
+
+        BlobServiceClient blobServiceClient = new BlobServiceClientBuilder()
+                .endpoint(String.format(String.format("https://%s.%s";, 
accountName, DEFAULT_ENDPOINT_SUFFIX)))
+                .credential(getClientSecretCredential())
+                .buildClient();
+        OffsetDateTime startTime = OffsetDateTime.now(ZoneOffset.UTC);
+        UserDelegationKey userDelegationKey = 
blobServiceClient.getUserDelegationKey(startTime, expiryTime);
+        return blobClient.generateUserDelegationSas(serviceSasSignatureValues, 
userDelegationKey);
     }
 
-    /* set empty headers as blank string due to a bug in Azure SDK
-     * Azure SDK considers null headers as 'null' string which corrupts the 
string to sign and generates an invalid
-     * sas token
-     * */
-    private void fillEmptyHeaders(SharedAccessBlobHeaders 
sharedAccessBlobHeaders) {
-        final String EMPTY_STRING = "";
-        Optional.ofNullable(sharedAccessBlobHeaders)
-                .ifPresent(headers -> {
-                    if (StringUtils.isBlank(headers.getCacheControl())) {
-                        headers.setCacheControl(EMPTY_STRING);
-                    }
-                    if (StringUtils.isBlank(headers.getContentDisposition())) {
-                        headers.setContentDisposition(EMPTY_STRING);
-                    }
-                    if (StringUtils.isBlank(headers.getContentEncoding())) {
-                        headers.setContentEncoding(EMPTY_STRING);
-                    }
-                    if (StringUtils.isBlank(headers.getContentLanguage())) {
-                        headers.setContentLanguage(EMPTY_STRING);
-                    }
-                    if (StringUtils.isBlank(headers.getContentType())) {
-                        headers.setContentType(EMPTY_STRING);
-                    }
-                });
-    }
-
-    @NotNull
-    private String generateSas(CloudBlockBlob blob,
-                               SharedAccessBlobPolicy policy,
-                               SharedAccessBlobHeaders optionalHeaders) throws 
InvalidKeyException, StorageException {
-        return optionalHeaders == null ? 
blob.generateSharedAccessSignature(policy, null) :
-                blob.generateSharedAccessSignature(policy,
-                        optionalHeaders, null, null, null, true);
+    private BlobContainerClient getBlobContainerFromConnectionString() {
+        return new BlobContainerClientBuilder()
+                .connectionString(getAzureConnectionString())
+                .containerName(getContainerName())
+                .buildClient();
     }
 
     private boolean authenticateViaServicePrincipal() {
         return StringUtils.isBlank(azureConnectionString) &&
                 StringUtils.isNoneBlank(accountName, tenantId, clientId, 
clientSecret);
     }
 
-    private class TokenRefresher implements Runnable {
-        @Override
-        public void run() {
-            try {
-                log.debug("Checking for azure access token expiry at: {}", 
LocalDateTime.now());
-                OffsetDateTime tokenExpiryThreshold = 
OffsetDateTime.now().plusMinutes(5);
-                if (accessToken.getExpiresAt() != null && 
accessToken.getExpiresAt().isBefore(tokenExpiryThreshold)) {
-                    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;
-                    }
-                    // 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);
-            }
-        }
+    private ClientSecretCredential getClientSecretCredential() {
+        return new ClientSecretCredentialBuilder()
+                .clientId(clientId)
+                .clientSecret(clientSecret)
+                .tenantId(tenantId)
+                .build();
     }
 
-    @Override
-    public void close() {
-        new ExecutorCloser(executorService).close();
-        log.info("Refresh token executor service shutdown completed");
+    @NotNull
+    private BlobContainerClient getBlobContainerFromServicePrincipals(String 
accountName, String clientId, String clientSecret, String tenantId, 
RequestRetryOptions retryOptions) throws DataStoreException {

Review Comment:
   `clientId `, `clientSecret` and `tenantId` are never used in this method



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to