Skip to content

Commit

Permalink
OAK-9704 AzureBlobStoreBackend: empty string as null in boolean property
Browse files Browse the repository at this point in the history
When configured from OSGi using interpolation, an unset property can
default to an empty string. The empty string is interpreted as null, so
that the correct default is selected.
  • Loading branch information
jelmini committed Feb 23, 2022
1 parent bfa326d commit 0994fbb
Show file tree
Hide file tree
Showing 5 changed files with 367 additions and 15 deletions.
7 changes: 7 additions & 0 deletions oak-blob-cloud-azure/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,13 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.arakelian</groupId>
<artifactId>docker-junit-rule</artifactId>
<scope>test</scope>
</dependency>

</dependencies>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,13 @@ public void init() throws DataStoreException {

try {
Utils.setProxyIfNeeded(properties);
containerName = (String) properties.get(AzureConstants.AZURE_BLOB_CONTAINER_NAME);
createBlobContainer = PropertiesUtil.toBoolean(properties.getProperty(AzureConstants.AZURE_CREATE_CONTAINER), true);
containerName = properties.getProperty(AzureConstants.AZURE_BLOB_CONTAINER_NAME);
createBlobContainer = PropertiesUtil.toBoolean(
Strings.emptyToNull(properties.getProperty(AzureConstants.AZURE_CREATE_CONTAINER)), true);
connectionString = Utils.getConnectionStringFromProperties(properties);

concurrentRequestCount = PropertiesUtil.toInteger(
properties.get(AzureConstants.AZURE_BLOB_CONCURRENT_REQUESTS_PER_OPERATION),
properties.getProperty(AzureConstants.AZURE_BLOB_CONCURRENT_REQUESTS_PER_OPERATION),
DEFAULT_CONCURRENT_REQUEST_COUNT);
if (concurrentRequestCount < DEFAULT_CONCURRENT_REQUEST_COUNT) {
LOG.warn("Invalid setting [{}] for concurrentRequestsPerOperation (too low); resetting to {}",
Expand All @@ -184,12 +185,12 @@ public void init() throws DataStoreException {
}
LOG.info("Using concurrentRequestsPerOperation={}", concurrentRequestCount);

retryPolicy = Utils.getRetryPolicy((String)properties.get(AzureConstants.AZURE_BLOB_MAX_REQUEST_RETRY));
retryPolicy = Utils.getRetryPolicy(properties.getProperty(AzureConstants.AZURE_BLOB_MAX_REQUEST_RETRY));
if (properties.getProperty(AzureConstants.AZURE_BLOB_REQUEST_TIMEOUT) != null) {
requestTimeout = PropertiesUtil.toInteger(properties.getProperty(AzureConstants.AZURE_BLOB_REQUEST_TIMEOUT), RetryPolicy.DEFAULT_CLIENT_RETRY_COUNT);
}
presignedDownloadURIVerifyExists =
PropertiesUtil.toBoolean(properties.get(AzureConstants.PRESIGNED_HTTP_DOWNLOAD_URI_VERIFY_EXISTS), true);
presignedDownloadURIVerifyExists = PropertiesUtil.toBoolean(
Strings.emptyToNull(properties.getProperty(AzureConstants.PRESIGNED_HTTP_DOWNLOAD_URI_VERIFY_EXISTS)), true);

CloudBlobContainer azureContainer = getAzureContainer();

Expand Down Expand Up @@ -253,11 +254,11 @@ public InputStream read(DataIdentifier identifier) throws DataStoreException {
return is;
}
catch (StorageException e) {
LOG.info("Error reading blob. identifier=%s", key);
LOG.info("Error reading blob. identifier={}", key);
throw new DataStoreException(String.format("Cannot read blob. identifier=%s", key), e);
}
catch (URISyntaxException e) {
LOG.debug("Error reading blob. identifier=%s", key);
LOG.debug("Error reading blob. identifier={}", key);
throw new DataStoreException(String.format("Cannot read blob. identifier=%s", key), e);
} finally {
if (contextClassLoader != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ public static String getConnectionStringFromProperties(Properties properties) {

return getConnectionString(
accountName,
accountKey);
accountKey,
blobEndpoint);
}

private static String getConnectionStringForSas(String sasUri, String blobEndpoint, String accountName) {
Expand All @@ -152,12 +153,15 @@ private static String getConnectionStringForSas(String sasUri, String blobEndpoi
}
}

public static String getConnectionString(final String accountName, final String accountKey) {
return String.format(
"DefaultEndpointsProtocol=https;AccountName=%s;AccountKey=%s",
accountName,
accountKey
);
public static String getConnectionString(final String accountName, final String accountKey, String blobEndpoint) {
StringBuilder connString = new StringBuilder("DefaultEndpointsProtocol=https");
connString.append(";AccountName=").append(accountName);
connString.append(";AccountKey=").append(accountKey);

if (!Strings.isNullOrEmpty(blobEndpoint)) {
connString.append(";BlobEndpoint=").append(blobEndpoint);
}
return connString.toString();
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,239 @@
/*
* 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;

import com.google.common.collect.ImmutableSet;
import com.microsoft.azure.storage.StorageException;
import com.microsoft.azure.storage.blob.CloudBlobContainer;
import com.microsoft.azure.storage.blob.SharedAccessBlobPermissions;
import com.microsoft.azure.storage.blob.SharedAccessBlobPolicy;
import java.io.IOException;
import java.net.URISyntaxException;
import java.time.Duration;
import java.time.Instant;
import java.util.Date;
import java.util.EnumSet;
import java.util.Properties;
import java.util.Set;
import java.util.stream.StreamSupport;
import org.jetbrains.annotations.NotNull;
import org.junit.After;
import org.junit.ClassRule;
import org.junit.Test;

import static com.microsoft.azure.storage.blob.SharedAccessBlobPermissions.ADD;
import static com.microsoft.azure.storage.blob.SharedAccessBlobPermissions.CREATE;
import static com.microsoft.azure.storage.blob.SharedAccessBlobPermissions.LIST;
import static com.microsoft.azure.storage.blob.SharedAccessBlobPermissions.READ;
import static com.microsoft.azure.storage.blob.SharedAccessBlobPermissions.WRITE;
import static java.util.stream.Collectors.toSet;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

public class AzureBlobStoreBackendTest {
@ClassRule
public static AzuriteDockerRule azurite = new AzuriteDockerRule();

private static final String CONTAINER_NAME = "blobstore";
private static final EnumSet<SharedAccessBlobPermissions> READ_ONLY = EnumSet.of(READ, LIST);
private static final EnumSet<SharedAccessBlobPermissions> READ_WRITE = EnumSet.of(READ, LIST, CREATE, WRITE, ADD);
private static final ImmutableSet<String> BLOBS = ImmutableSet.of("blob1", "blob2");

private CloudBlobContainer container;

@After
public void tearDown() throws Exception {
if (container != null) {
container.deleteIfExists();
}
}

@Test
public void initWithSharedAccessSignature_readOnly() throws Exception {
CloudBlobContainer container = createBlobContainer();
String sasToken = container.generateSharedAccessSignature(policy(READ_ONLY), null);

AzureBlobStoreBackend azureBlobStoreBackend = new AzureBlobStoreBackend();
azureBlobStoreBackend.setProperties(getConfigurationWithSasToken(sasToken));

azureBlobStoreBackend.init();

assertWriteAccessNotGranted(azureBlobStoreBackend);
assertReadAccessGranted(azureBlobStoreBackend, BLOBS);
}

@Test
public void initWithSharedAccessSignature_readWrite() throws Exception {
CloudBlobContainer container = createBlobContainer();
String sasToken = container.generateSharedAccessSignature(policy(READ_WRITE), null);

AzureBlobStoreBackend azureBlobStoreBackend = new AzureBlobStoreBackend();
azureBlobStoreBackend.setProperties(getConfigurationWithSasToken(sasToken));

azureBlobStoreBackend.init();

assertWriteAccessGranted(azureBlobStoreBackend, "file");
assertReadAccessGranted(azureBlobStoreBackend, concat(BLOBS, "file"));
}

@Test
public void connectWithSharedAccessSignatureURL_expired() throws Exception {
CloudBlobContainer container = createBlobContainer();
SharedAccessBlobPolicy expiredPolicy = policy(READ_WRITE, yesterday());
String sasToken = container.generateSharedAccessSignature(expiredPolicy, null);

AzureBlobStoreBackend azureBlobStoreBackend = new AzureBlobStoreBackend();
azureBlobStoreBackend.setProperties(getConfigurationWithSasToken(sasToken));

azureBlobStoreBackend.init();

assertWriteAccessNotGranted(azureBlobStoreBackend);
assertReadAccessNotGranted(azureBlobStoreBackend);
}

@Test
public void initWithAccessKey() throws Exception {
AzureBlobStoreBackend azureBlobStoreBackend = new AzureBlobStoreBackend();
azureBlobStoreBackend.setProperties(getConfigurationWithAccessKey());

azureBlobStoreBackend.init();

assertWriteAccessGranted(azureBlobStoreBackend, "file");
assertReadAccessGranted(azureBlobStoreBackend, ImmutableSet.of("file"));
}

@Test
public void initWithConnectionURL() throws Exception {
AzureBlobStoreBackend azureBlobStoreBackend = new AzureBlobStoreBackend();
azureBlobStoreBackend.setProperties(getConfigurationWithConnectionString());

azureBlobStoreBackend.init();

assertWriteAccessGranted(azureBlobStoreBackend, "file");
assertReadAccessGranted(azureBlobStoreBackend, ImmutableSet.of("file"));
}

private CloudBlobContainer createBlobContainer() throws Exception {
container = azurite.getContainer("blobstore");
for (String blob : BLOBS) {
container.getBlockBlobReference(blob + ".txt").uploadText(blob);
}
return container;
}

private static Properties getConfigurationWithSasToken(String sasToken) {
Properties properties = getBasicConfiguration();
properties.setProperty(AzureConstants.AZURE_SAS, sasToken);
properties.setProperty(AzureConstants.AZURE_CREATE_CONTAINER, "false");
return properties;
}

private static Properties getConfigurationWithAccessKey() {
Properties properties = getBasicConfiguration();
properties.setProperty(AzureConstants.AZURE_STORAGE_ACCOUNT_KEY, AzuriteDockerRule.ACCOUNT_KEY);
return properties;
}

@NotNull
private static Properties getConfigurationWithConnectionString() {
Properties properties = getBasicConfiguration();
properties.setProperty(AzureConstants.AZURE_CONNECTION_STRING, getConnectionString());
return properties;
}

@NotNull
private static Properties getBasicConfiguration() {
Properties properties = new Properties();
properties.setProperty(AzureConstants.AZURE_BLOB_CONTAINER_NAME, CONTAINER_NAME);
properties.setProperty(AzureConstants.AZURE_STORAGE_ACCOUNT_NAME, AzuriteDockerRule.ACCOUNT_NAME);
properties.setProperty(AzureConstants.AZURE_BLOB_ENDPOINT, azurite.getBlobEndpoint());
properties.setProperty(AzureConstants.AZURE_CREATE_CONTAINER, "");
return properties;
}

@NotNull
private static SharedAccessBlobPolicy policy(EnumSet<SharedAccessBlobPermissions> permissions, Instant expirationTime) {
SharedAccessBlobPolicy sharedAccessBlobPolicy = new SharedAccessBlobPolicy();
sharedAccessBlobPolicy.setPermissions(permissions);
sharedAccessBlobPolicy.setSharedAccessExpiryTime(Date.from(expirationTime));
return sharedAccessBlobPolicy;
}

@NotNull
private static SharedAccessBlobPolicy policy(EnumSet<SharedAccessBlobPermissions> permissions) {
return policy(permissions, Instant.now().plus(Duration.ofDays(7)));
}

private static void assertReadAccessGranted(AzureBlobStoreBackend backend, Set<String> expectedBlobs) throws Exception {
CloudBlobContainer container = backend.getAzureContainer();
Set<String> actualBlobNames = StreamSupport.stream(container.listBlobs().spliterator(), false)
.map(blob -> blob.getUri().getPath())
.map(path -> path.substring(path.lastIndexOf('/') + 1))
.collect(toSet());
Set<String> expectedBlobNames = expectedBlobs.stream().map(name -> name + ".txt").collect(toSet());

assertEquals(expectedBlobNames, actualBlobNames);

Set<String> actualBlobContent = actualBlobNames.stream()
.map(name -> {
try {
return container.getBlockBlobReference(name).downloadText();
} catch (StorageException | IOException | URISyntaxException e) {
throw new RuntimeException("Error while reading blob " + name, e);
}
})
.collect(toSet());
assertEquals(expectedBlobs, actualBlobContent);
}

private static void assertWriteAccessGranted(AzureBlobStoreBackend backend, String blob) throws Exception {
backend.getAzureContainer()
.getBlockBlobReference(blob + ".txt").uploadText(blob);
}

private static void assertWriteAccessNotGranted(AzureBlobStoreBackend backend) {
try {
assertWriteAccessGranted(backend, "test.txt");
fail("Write access should not be granted, but writing to the storage succeeded.");
} catch (Exception e) {
// successful
}
}

private static void assertReadAccessNotGranted(AzureBlobStoreBackend backend) {
try {
assertReadAccessGranted(backend, BLOBS);
fail("Read access should not be granted, but reading from the storage succeeded.");
} catch (Exception e) {
// successful
}
}

private static Instant yesterday() {
return Instant.now().minus(Duration.ofDays(1));
}

private static ImmutableSet<String> concat(ImmutableSet<String> set, String element) {
return ImmutableSet.<String>builder().addAll(set).add(element).build();
}

private static String getConnectionString() {
return Utils.getConnectionString(AzuriteDockerRule.ACCOUNT_NAME, AzuriteDockerRule.ACCOUNT_KEY, azurite.getBlobEndpoint());
}
}
Loading

0 comments on commit 0994fbb

Please sign in to comment.