seropian commented on code in PR #2989:
URL: https://github.com/apache/jackrabbit-oak/pull/2989#discussion_r3778660362


##########
oak-blob-cloud-azure/src/main/java/org/apache/jackrabbit/oak/blob/cloud/azure/blobstorage/v12/AzureBlobContainerProviderV12.java:
##########
@@ -0,0 +1,432 @@
+/*
+ * 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.jackrabbit.oak.blob.cloud.azure.blobstorage.v12;
+
+import com.azure.core.http.HttpClient;
+import com.azure.core.http.netty.NettyAsyncHttpClientBuilder;
+import com.azure.identity.ClientSecretCredential;
+import com.azure.identity.ClientSecretCredentialBuilder;
+import com.azure.storage.blob.BlobContainerClient;
+import com.azure.storage.blob.BlobServiceClient;
+import com.azure.storage.blob.BlobServiceClientBuilder;
+import com.azure.storage.blob.models.UserDelegationKey;
+import com.azure.storage.blob.sas.BlobSasPermission;
+import com.azure.storage.blob.sas.BlobServiceSasSignatureValues;
+import com.azure.storage.blob.specialized.BlockBlobClient;
+import com.azure.storage.common.policy.RequestRetryOptions;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.jackrabbit.oak.spi.blob.data.DataStoreException;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.net.URISyntaxException;
+import java.security.InvalidKeyException;
+import java.time.Duration;
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
+import java.util.Properties;
+import java.util.concurrent.atomic.AtomicReference;
+
+class AzureBlobContainerProviderV12 {
+    private static final Logger log = 
LoggerFactory.getLogger(AzureBlobContainerProviderV12.class);
+    private static final String DEFAULT_ENDPOINT_SUFFIX = "core.windows.net";
+    private final String azureConnectionString;
+    private final String accountName;
+    private final String containerName;
+    private final String blobEndpoint;
+    private final String sasToken;
+    private final String accountKey;
+    private final String tenantId;
+    private final String clientId;
+    private final String clientSecret;
+    // Retry policy fixed at activation time from the same properties as all 
other config.
+    private final RequestRetryOptions retryOptions;
+    // Shared HTTP client — one Netty event loop per provider instance, reused 
across all Azure SDK
+    // client builds. Proxy settings are fixed at activation time so one 
client suffices.
+    private final HttpClient httpClient;
+    // Cached credential — token cache is per-instance, recreating on every 
SAS call would
+    // force a new OAuth round-trip each time.
+    private final ClientSecretCredential clientSecretCredential;
+    // Cached service client for user-delegation SAS generation — avoids 
allocating a new Netty
+    // event loop and connection pool on every SAS call.
+    private final AtomicReference<BlobServiceClient> cachedBlobServiceClient = 
new AtomicReference<>();
+    // Cached container client for non-SP SAS signing — signing is local HMAC, 
so one client
+    // per activation is sufficient regardless of how many SAS calls are made.
+    private final AtomicReference<BlobContainerClient> 
cachedContainerForSigning = new AtomicReference<>();
+    // Cached user delegation key — Azure issues one key per round-trip; 
reusing it across all
+    // presigned URI generations in an upload/download avoids O(N) calls to 
the userdelegationkey
+    // endpoint (N = number of parts). Azure allows keys valid up to 7 days.
+    // Package-private for test injection.
+    final AtomicReference<CachedDelegationKey> cachedDelegationKey = new 
AtomicReference<>();
+
+    // Request keys for the full 7-day window so they cover any SAS expiry 
we'd generate.
+    // Also the hard upper bound Azure allows for a user delegation key's 
lifetime — package-private
+    // so callers can validate configured presigned-URI expiries against it 
(see AzureBlobStoreBackendV12).
+    static final Duration DELEGATION_KEY_LIFETIME = Duration.ofDays(7);
+    // Renew early enough to cover clock skew between this host and Azure.
+    private static final Duration DELEGATION_KEY_RENEWAL_BUFFER = 
Duration.ofSeconds(60);
+
+    private AzureBlobContainerProviderV12(Builder builder) {
+        this.azureConnectionString = builder.azureConnectionString;
+        this.accountName = builder.accountName;
+        this.containerName = builder.containerName;
+        this.blobEndpoint = builder.blobEndpoint;
+        this.sasToken = builder.sasToken;
+        this.accountKey = builder.accountKey;
+        this.tenantId = builder.tenantId;
+        this.clientId = builder.clientId;
+        this.clientSecret = builder.clientSecret;
+        this.clientSecretCredential = 
StringUtils.isNoneBlank(builder.clientId, builder.clientSecret, 
builder.tenantId)
+                ? new ClientSecretCredentialBuilder()
+                .clientId(builder.clientId)
+                .clientSecret(builder.clientSecret)
+                .tenantId(builder.tenantId)
+                .build()
+                : null;
+        this.retryOptions = builder.retryOptions;
+        this.httpClient = new NettyAsyncHttpClientBuilder()
+                .proxy(UtilsV12.computeProxyOptions(builder.proxyHost, 
builder.proxyPort))
+                .build();
+    }
+
+    /**
+     * Constructs the Azure Storage endpoint URL.
+     * If a custom blobEndpoint is configured, it will be used.
+     * Otherwise, constructs the default endpoint using the account name.
+     *
+     * @param accountName        the storage account name
+     * @param customBlobEndpoint optional custom blob endpoint (can be null or 
empty)
+     * @return the endpoint URL to use
+     */
+    @NotNull
+    private static String getEndpointUrl(String accountName, String 
customBlobEndpoint) {
+        if (StringUtils.isNotBlank(customBlobEndpoint)) {
+            if (!customBlobEndpoint.startsWith("http://";) && 
!customBlobEndpoint.startsWith("https://";)) {
+                return "https://"; + customBlobEndpoint;
+            }
+            if (customBlobEndpoint.startsWith("http://";)) {
+                log.warn("Custom blob endpoint uses cleartext HTTP — 
credentials and data will be transmitted unencrypted: {}", customBlobEndpoint);
+            }
+            return customBlobEndpoint;
+        }
+        // Default public endpoint
+        return String.format("https://%s.blob.%s";, accountName, 
DEFAULT_ENDPOINT_SUFFIX);
+    }
+
+    public String getContainerName() {
+        return containerName;
+    }
+
+    public String getAzureConnectionString() {
+        return azureConnectionString;
+    }
+
+    @NotNull
+    public BlobContainerClient getBlobContainer() 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 
UtilsV12.getBlobContainerFromConnectionString(getAzureConnectionString(), 
containerName, retryOptions, httpClient);
+        } else if (authenticateViaServicePrincipal()) {
+            log.debug("connecting to azure blob storage via service principal 
credentials");
+            // Reuse the cached BlobServiceClient — derives a container client 
from the same pipeline.
+            return 
getOrCreateBlobServiceClient().getBlobContainerClient(containerName);
+        } else if (StringUtils.isNotBlank(sasToken)) {
+            log.debug("connecting to azure blob storage via sas token");
+            final String connectionStringWithSasToken = 
UtilsV12.getConnectionStringForSas(sasToken, blobEndpoint, accountName);
+            return 
UtilsV12.getBlobContainerFromConnectionString(connectionStringWithSasToken, 
containerName, retryOptions, httpClient);
+        }
+        log.debug("connecting to azure blob storage via access key");
+        final String connectionStringWithAccountKey = 
UtilsV12.getConnectionString(accountName, accountKey, blobEndpoint);
+        return 
UtilsV12.getBlobContainerFromConnectionString(connectionStringWithAccountKey, 
containerName, retryOptions, httpClient);
+    }
+
+    @NotNull
+    public String generateSharedAccessSignature(RequestRetryOptions 
retryOptions,

Review Comment:
   The old single-class `AzureBlobContainerProviderV12` was refactored into an 
abstract base class with `ServicePrincipalProvider` and `SharedKeyProvider` 
subclasses as part of addressing jsedding's review. 
`generateSharedAccessSignature` is now abstract on the base class and called in 
`AzureBlobStoreBackendV12` at line 1065.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to