>From Michael Blow <[email protected]>: Michael Blow has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21190?usp=email )
Change subject: [NO ISSUE][*DB][STO] Clean up Azure cloud client configuration ...................................................................... [NO ISSUE][*DB][STO] Clean up Azure cloud client configuration Ext-ref: MB-71312 Change-Id: I83760d446ee5c0859332e7783cc458274494de3d (cherry picked from commit 11359d40f98e91b85eba3c7695738be489576806) Change-Id: Ied2e8017898c425318d139134cb145daaf65e964 Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21190 Reviewed-by: Michael Blow <[email protected]> Tested-by: Michael Blow <[email protected]> Reviewed-by: Hussain Towaileb <[email protected]> Integration-Tests: Jenkins <[email protected]> --- M asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageClientConfig.java M asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageCloudClient.java M asterixdb/asterix-cloud/src/test/java/org/apache/asterix/cloud/azure/LSMAzBlobStorageTest.java M asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/azure/blob/BlobUtils.java M hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/StringUtil.java 5 files changed, 67 insertions(+), 54 deletions(-) Approvals: Jenkins: Verified Hussain Towaileb: Looks good to me, approved Michael Blow: Looks good to me, but someone else must approve; Verified diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageClientConfig.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageClientConfig.java index daea8d2..38b0e0f 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageClientConfig.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageClientConfig.java @@ -18,6 +18,8 @@ */ package org.apache.asterix.cloud.clients.azure.blobstorage; +import static org.apache.hyracks.util.StringUtil.quoteNullableString; + import java.util.Map; import java.util.Objects; @@ -25,58 +27,57 @@ import org.apache.asterix.common.config.ICloudProperties; import org.apache.asterix.external.util.ExternalDataConstants; import org.apache.asterix.external.util.azure.AzureConstants; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import com.azure.core.util.Configuration; import com.azure.identity.DefaultAzureCredential; import com.azure.identity.DefaultAzureCredentialBuilder; import com.azure.storage.blob.models.AccessTier; public class AzBlobStorageClientConfig { + private static final Logger LOGGER = LogManager.getLogger(); // Ref: https://learn.microsoft.com/en-us/rest/api/storageservices/blob-batch?tabs=microsoft-entra-id static final int MAX_CONCURRENT_REQUESTS = 20; private static final AccessTier INTERNAL_STORAGE_ACCESS_TIER = AccessTier.HOT; - private final int writeBufferSize; - private final String region; private final String endpoint; + private final String container; private final String prefix; - - private final boolean anonymousAuth; - private final long profilerLogInterval; - private final String bucket; + private final AccessTier accessTier; + private final int writeBufferSize; private final long tokenAcquireTimeout; private final int writeMaxRequestsPerSeconds; private final int readMaxRequestsPerSeconds; + private final long profilerLogInterval; private final boolean storageDisableSSLVerify; private final int requestsMaxHttpConnections; private final int requestsMaxPendingHttpConnections; private final int requestsHttpConnectionAcquireTimeout; - private final AccessTier accessTier; private final int maxIdleSeconds; private final int maxLifetimeSeconds; + private final String clientId; - public AzBlobStorageClientConfig(String region, String endpoint, String prefix, boolean anonymousAuth, - long profilerLogInterval, String bucket, int writeBufferSize) { + public AzBlobStorageClientConfig(String endpoint, String prefix, long profilerLogInterval, String container, + int writeBufferSize) { // TODO(mblow): using the same values as our defaults for blob storage seems sus, refactor to configurable and // and use reasonable defaults - this(region, endpoint, prefix, anonymousAuth, profilerLogInterval, bucket, 1, 0, 0, writeBufferSize, false, - null, CloudProperties.MAX_HTTP_CONNECTIONS_DEFAULT, - CloudProperties.MAX_PENDING_HTTP_CONNECTIONS_DEFAULT, + this(endpoint, container, prefix, profilerLogInterval, 1, 0, 0, writeBufferSize, false, null, + CloudProperties.MAX_HTTP_CONNECTIONS_DEFAULT, CloudProperties.MAX_PENDING_HTTP_CONNECTIONS_DEFAULT, CloudProperties.HTTP_CONNECTION_ACQUIRE_TIMEOUT_DEFAULT, CloudProperties.HTTP_CONNECTION_MAX_IDLE_SECONDS_DEFAULT, - CloudProperties.HTTP_CONNECTION_MAX_LIFETIME_SECONDS_DEFAULT); + CloudProperties.HTTP_CONNECTION_MAX_LIFETIME_SECONDS_DEFAULT, null); } - public AzBlobStorageClientConfig(String region, String endpoint, String prefix, boolean anonymousAuth, - long profilerLogInterval, String bucket, long tokenAcquireTimeout, int writeMaxRequestsPerSeconds, - int readMaxRequestsPerSeconds, int writeBufferSize, boolean storageDisableSSLVerify, AccessTier accessTier, - int requestsMaxHttpConnections, int requestsMaxPendingHttpConnections, - int requestsHttpConnectionAcquireTimeout, int maxIdleSeconds, int maxLifetimeSeconds) { - this.region = Objects.requireNonNull(region, "region"); + public AzBlobStorageClientConfig(String endpoint, String container, String prefix, long profilerLogInterval, + long tokenAcquireTimeout, int writeMaxRequestsPerSeconds, int readMaxRequestsPerSeconds, + int writeBufferSize, boolean storageDisableSSLVerify, AccessTier accessTier, int requestsMaxHttpConnections, + int requestsMaxPendingHttpConnections, int requestsHttpConnectionAcquireTimeout, int maxIdleSeconds, + int maxLifetimeSeconds, String clientId) { this.endpoint = endpoint; this.prefix = Objects.requireNonNull(prefix, "prefix"); - this.anonymousAuth = anonymousAuth; this.profilerLogInterval = profilerLogInterval; - this.bucket = bucket; + this.container = container; this.tokenAcquireTimeout = tokenAcquireTimeout; this.writeMaxRequestsPerSeconds = writeMaxRequestsPerSeconds; this.readMaxRequestsPerSeconds = readMaxRequestsPerSeconds; @@ -89,19 +90,21 @@ this.accessTier = accessTier; this.maxIdleSeconds = maxIdleSeconds; this.maxLifetimeSeconds = maxLifetimeSeconds; + this.clientId = clientId; } public static AzBlobStorageClientConfig of(ICloudProperties cloudProperties) { - return new AzBlobStorageClientConfig(cloudProperties.getStorageRegion(), cloudProperties.getStorageEndpoint(), - cloudProperties.getStoragePrefix(), cloudProperties.isStorageAnonymousAuth(), - cloudProperties.getProfilerLogInterval(), cloudProperties.getStorageBucket(), + // TODO(mblow): the client id should be coming in by way fo the cloud properties + String clientId = System.getenv(Configuration.PROPERTY_AZURE_CLIENT_ID); + return new AzBlobStorageClientConfig(cloudProperties.getStorageEndpoint(), cloudProperties.getStorageBucket(), + cloudProperties.getStoragePrefix(), cloudProperties.getProfilerLogInterval(), cloudProperties.getTokenAcquireTimeout(), cloudProperties.getWriteMaxRequestsPerSecond(), cloudProperties.getReadMaxRequestsPerSecond(), cloudProperties.getWriteBufferSize(), cloudProperties.isStorageDisableSSLVerify(), INTERNAL_STORAGE_ACCESS_TIER, cloudProperties.getRequestsMaxHttpConnections(), cloudProperties.getRequestsMaxPendingHttpConnections(), cloudProperties.getRequestsHttpConnectionAcquireTimeout(), cloudProperties.getRequestsHttpConnectionMaxIdleSeconds(), - cloudProperties.getRequestsHttpConnectionMaxLifetimeSeconds()); + cloudProperties.getRequestsHttpConnectionMaxLifetimeSeconds(), clientId); } public static AzBlobStorageClientConfig of(Map<String, String> configuration, int writeBufferSize) { @@ -114,14 +117,8 @@ // Dummy values; String region = ""; String prefix = ""; - boolean anonymousAuth = false; - return new AzBlobStorageClientConfig(region, endPoint, prefix, anonymousAuth, profilerLogInterval, bucket, - writeBufferSize); - } - - public String getRegion() { - return region; + return new AzBlobStorageClientConfig(endPoint, prefix, profilerLogInterval, bucket, writeBufferSize); } public String getEndpoint() { @@ -132,24 +129,20 @@ return prefix; } - public String getBucket() { - return bucket; + public String getContainer() { + return container; } public long getProfilerLogInterval() { return profilerLogInterval; } - public boolean isAnonymousAuth() { - return anonymousAuth; - } - public boolean isStorageDisableSSLVerify() { return storageDisableSSLVerify; } public DefaultAzureCredential createCredentialsProvider() { - return new DefaultAzureCredentialBuilder().build(); + return new DefaultAzureCredentialBuilder().managedIdentityClientId(clientId).build(); } public long getTokenAcquireTimeout() { @@ -198,4 +191,23 @@ public int getMaxLifetimeSeconds() { return maxLifetimeSeconds; } + + public String getClientId() { + return clientId; + } + + @Override + public String toString() { + return "AzBlobStorageClientConfig{" + "endpoint=" + quoteNullableString(endpoint) + ", " + "container=" + + quoteNullableString(container) + ", " + "prefix=" + quoteNullableString(prefix) + ", " + "accessTier=" + + accessTier + ", " + "writeBufferSize=" + writeBufferSize + ", " + "tokenAcquireTimeout=" + + tokenAcquireTimeout + ", " + "writeMaxRequestsPerSeconds=" + writeMaxRequestsPerSeconds + ", " + + "readMaxRequestsPerSeconds=" + readMaxRequestsPerSeconds + ", " + "profilerLogInterval=" + + profilerLogInterval + ", " + "storageDisableSSLVerify=" + storageDisableSSLVerify + ", " + + "requestsMaxHttpConnections=" + requestsMaxHttpConnections + ", " + + "requestsMaxPendingHttpConnections=" + requestsMaxPendingHttpConnections + ", " + + "requestsHttpConnectionAcquireTimeout=" + requestsHttpConnectionAcquireTimeout + ", " + + "maxIdleSeconds=" + maxIdleSeconds + ", " + "maxLifetimeSeconds=" + maxLifetimeSeconds + ", " + + "clientId=" + quoteNullableString(clientId) + '}'; + } } diff --git a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageCloudClient.java b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageCloudClient.java index 49e911e..bb59992 100644 --- a/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageCloudClient.java +++ b/asterixdb/asterix-cloud/src/main/java/org/apache/asterix/cloud/clients/azure/blobstorage/AzBlobStorageCloudClient.java @@ -90,10 +90,6 @@ public class AzBlobStorageCloudClient implements ICloudClient { private static final String BUCKET_ROOT_PATH = ""; - public static final String AZURITE_ENDPOINT = "http://127.0.0.1:15055/devstoreaccount1/";; - private static final String AZURITE_ACCOUNT_NAME = "devstoreaccount1"; - private static final String AZURITE_ACCOUNT_KEY = - "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw=="; private final ICloudGuardian guardian; private final BlobContainerClient blobContainerClient; private final BlobContainerAsyncClient blobContainerAsyncClient; @@ -108,8 +104,8 @@ public AzBlobStorageCloudClient(AzBlobStorageClientConfig config, BlobServiceClient blobServiceClient, BlobServiceAsyncClient asyncBlobServiceClient, ICloudGuardian guardian) { - this.blobContainerClient = blobServiceClient.getBlobContainerClient(config.getBucket()); - this.blobContainerAsyncClient = asyncBlobServiceClient.getBlobContainerAsyncClient(config.getBucket()); + this.blobContainerClient = blobServiceClient.getBlobContainerClient(config.getContainer()); + this.blobContainerAsyncClient = asyncBlobServiceClient.getBlobContainerAsyncClient(config.getContainer()); this.config = config; this.guardian = guardian; this.accessTier = config.getAccessTier(); @@ -121,6 +117,7 @@ profiler = new RequestLimiterNoOpProfiler(limiter); } guardian.setCloudClient(this); + LOGGER.debug("Created Azure Cloud Client with config: {}", config); } @Override @@ -442,17 +439,12 @@ if (storageAccount != null && storageKey != null) { builder.credential(new StorageSharedKeyCredential(storageAccount, storageKey)); - } else if (config.isAnonymousAuth()) { - // TODO(mblow): this mapping anonymous auth -> Azurite default account (hack) should be removed ASAP - builder.credential(new StorageSharedKeyCredential(AZURITE_ACCOUNT_NAME, AZURITE_ACCOUNT_KEY)); } else { builder.credential(config.createCredentialsProvider()); } } private static String getEndpoint(AzBlobStorageClientConfig config) { - // TODO(mblow): this mapping anonymous auth -> Azurite default endpoint (hack) should be removed ASAP - return config.isAnonymousAuth() ? AZURITE_ENDPOINT + config.getBucket() - : config.getEndpoint() + "/" + config.getBucket(); + return config.getEndpoint() + "/" + config.getContainer(); } } diff --git a/asterixdb/asterix-cloud/src/test/java/org/apache/asterix/cloud/azure/LSMAzBlobStorageTest.java b/asterixdb/asterix-cloud/src/test/java/org/apache/asterix/cloud/azure/LSMAzBlobStorageTest.java index e876834..dc0e4f4 100644 --- a/asterixdb/asterix-cloud/src/test/java/org/apache/asterix/cloud/azure/LSMAzBlobStorageTest.java +++ b/asterixdb/asterix-cloud/src/test/java/org/apache/asterix/cloud/azure/LSMAzBlobStorageTest.java @@ -116,8 +116,8 @@ int writeBufferSize = StorageUtil.getIntSizeInBytes(5, StorageUtil.StorageUnit.MEGABYTE); URI blobStore = URI.create(blobServiceClient.getAccountUrl()); String endpoint = blobStore.getScheme() + "://" + blobStore.getAuthority() + "/devstoreaccount1"; - AzBlobStorageClientConfig config = new AzBlobStorageClientConfig(MOCK_SERVER_REGION, endpoint, "", false, 0, - PLAYGROUND_CONTAINER, 1, 0, 0, writeBufferSize, true, null, 1000, 10000, 120, 120, 0); + AzBlobStorageClientConfig config = new AzBlobStorageClientConfig(endpoint, PLAYGROUND_CONTAINER, "", 0, 1, 0, 0, + writeBufferSize, true, null, 1000, 10000, 120, 120, 0, null); CLOUD_CLIENT = new AzBlobStorageCloudClient(config, ICloudGuardian.NoOpCloudGuardian.INSTANCE); } diff --git a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/azure/blob/BlobUtils.java b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/azure/blob/BlobUtils.java index 8b6b99f..b4caa52 100644 --- a/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/azure/blob/BlobUtils.java +++ b/asterixdb/asterix-external-data/src/main/java/org/apache/asterix/external/util/azure/blob/BlobUtils.java @@ -58,6 +58,7 @@ import org.apache.hyracks.api.exceptions.IWarningCollector; import org.apache.hyracks.api.exceptions.SourceLocation; import org.apache.hyracks.api.exceptions.Warning; +import org.apache.hyracks.util.annotations.AiProvenance; import com.azure.core.credential.AzureSasCredential; import com.azure.core.http.netty.NettyAsyncHttpClientBuilder; @@ -75,7 +76,6 @@ import io.netty.handler.ssl.SslContext; import io.netty.handler.ssl.SslContextBuilder; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; -import org.apache.hyracks.util.annotations.AiProvenance; import reactor.netty.http.client.HttpClient; public class BlobUtils { diff --git a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/StringUtil.java b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/StringUtil.java index df33f6b..a7d131e 100644 --- a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/StringUtil.java +++ b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/StringUtil.java @@ -24,6 +24,7 @@ import org.apache.commons.collections4.map.LRUMap; import org.apache.commons.text.WordUtils; +import org.apache.hyracks.util.annotations.AiProvenance; public class StringUtil { private static final Map<String, String> CAMEL_CACHE = Collections.synchronizedMap(new LRUMap<>(1024)); @@ -51,6 +52,14 @@ return sb.toString(); } + /** + * Quotes a nullable string value with single quotes if non-null, otherwise returns null (no quotes). + */ + @AiProvenance(agent = AiProvenance.Agent.GPT_4_1, tool = AiProvenance.Tool.GITHUB_COPILOT, contributionKind = AiProvenance.ContributionKind.GENERATED) + public static String quoteNullableString(String value) { + return value == null ? "null" : "'" + value + "'"; + } + @FunctionalInterface public interface ICharAccessor<T> { char charAt(T input, int index); -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/21190?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: asterixdb Gerrit-Branch: phoenix Gerrit-Change-Id: Ied2e8017898c425318d139134cb145daaf65e964 Gerrit-Change-Number: 21190 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Blow <[email protected]> Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Ritik Raj <[email protected]> Gerrit-CC: Anon. E. Moose #1000171
