Skip to content

Commit

Permalink
Use Azure upload method instead of our own implementation (elastic#26751
Browse files Browse the repository at this point in the history
)

* Use Azure upload method instead of our own implementation

We are not following the Azure documentation about uploading blobs to Azure storage. https://docs.microsoft.com/en-us/azure/storage/blobs/storage-java-how-to-use-blob-storage#upload-a-blob-into-a-container

Instead we are using our own implementation which might cause some troubles and rarely some blobs can be not immediately commited just after we close the stream. Using the standard implementation provided by Azure team should allow us to benefit from all the magic Azure SDK team already wrote.

And well... Let's just read the doc!

* Adapt integration tests to secure settings

That was a missing part in elastic#23405.

* Simplify all the integration tests and *extends ESBlobStoreRepositoryIntegTestCase tests

    * removes IT `testForbiddenContainerName()` as it is useless. The plugin does not create anymore the container but expects that the user has created it before registering the repository
   * merges 2 IT classes so all IT tests are ran from one single class
   * We don't remove/create anymore the container between each single test but only for the test suite

Backport of elastic#26751 in 6.x branch
  • Loading branch information
dadoonet committed Sep 28, 2017
1 parent 29e97b6 commit 7786883
Show file tree
Hide file tree
Showing 12 changed files with 171 additions and 592 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,10 @@
import org.elasticsearch.common.blobstore.BlobMetaData;
import org.elasticsearch.common.blobstore.BlobPath;
import org.elasticsearch.common.blobstore.support.AbstractBlobContainer;
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.repositories.RepositoryException;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.HttpURLConnection;
import java.net.URISyntaxException;
import java.nio.file.FileAlreadyExistsException;
Expand Down Expand Up @@ -96,24 +93,11 @@ public void writeBlob(String blobName, InputStream inputStream, long blobSize) t
if (blobExists(blobName)) {
throw new FileAlreadyExistsException("blob [" + blobName + "] already exists, cannot overwrite");
}
logger.trace("writeBlob({}, stream, {})", blobName, blobSize);
try (OutputStream stream = createOutput(blobName)) {
Streams.copy(inputStream, stream);
}
}

private OutputStream createOutput(String blobName) throws IOException {
logger.trace("writeBlob({}, stream, {})", buildKey(blobName), blobSize);
try {
return new AzureOutputStream(blobStore.getOutputStream(blobStore.container(), buildKey(blobName)));
} catch (StorageException e) {
if (e.getHttpStatusCode() == HttpURLConnection.HTTP_NOT_FOUND) {
throw new NoSuchFileException(e.getMessage());
}
throw new IOException(e);
} catch (URISyntaxException e) {
throw new IOException(e);
} catch (IllegalArgumentException e) {
throw new RepositoryException(repositoryName, e.getMessage());
blobStore.writeBlob(buildKey(blobName), inputStream, blobSize);
} catch (URISyntaxException|StorageException e) {
throw new IOException("Can not write blob " + blobName, e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URISyntaxException;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -125,11 +124,6 @@ public InputStream getInputStream(String container, String blob) throws URISynta
return this.client.getInputStream(this.clientName, this.locMode, container, blob);
}

public OutputStream getOutputStream(String container, String blob) throws URISyntaxException, StorageException
{
return this.client.getOutputStream(this.clientName, this.locMode, container, blob);
}

public Map<String,BlobMetaData> listBlobsByPrefix(String container, String keyPath, String prefix) throws URISyntaxException, StorageException
{
return this.client.listBlobsByPrefix(this.clientName, this.locMode, container, keyPath, prefix);
Expand All @@ -139,4 +133,8 @@ public void moveBlob(String container, String sourceBlob, String targetBlob) thr
{
this.client.moveBlob(this.clientName, this.locMode, container, sourceBlob, targetBlob);
}

public void writeBlob(String blobName, InputStream inputStream, long blobSize) throws URISyntaxException, StorageException {
this.client.writeBlob(this.clientName, this.locMode, container, blobName, inputStream, blobSize);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.microsoft.azure.storage.StorageException;
import org.elasticsearch.SpecialPermission;

import java.io.IOException;
import java.net.SocketPermission;
import java.net.URISyntaxException;
import java.security.AccessController;
Expand Down Expand Up @@ -66,7 +67,7 @@ public static void doPrivilegedVoidException(StorageRunnable action) throws Stor

@FunctionalInterface
public interface StorageRunnable {
void executeCouldThrow() throws StorageException, URISyntaxException;
void executeCouldThrow() throws StorageException, URISyntaxException, IOException;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.Proxy;
import java.net.URISyntaxException;
import java.util.Locale;
import java.util.Map;

/**
Expand Down Expand Up @@ -77,12 +74,12 @@ final class Storage {
InputStream getInputStream(String account, LocationMode mode, String container, String blob)
throws URISyntaxException, StorageException, IOException;

OutputStream getOutputStream(String account, LocationMode mode, String container, String blob)
throws URISyntaxException, StorageException;

Map<String,BlobMetaData> listBlobsByPrefix(String account, LocationMode mode, String container, String keyPath, String prefix)
throws URISyntaxException, StorageException;

void moveBlob(String account, LocationMode mode, String container, String sourceBlob, String targetBlob)
throws URISyntaxException, StorageException;

void writeBlob(String account, LocationMode mode, String container, String blobName, InputStream inputStream, long blobSize) throws
URISyntaxException, StorageException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.elasticsearch.repositories.RepositoryException;

import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.EnumSet;
Expand Down Expand Up @@ -296,14 +295,6 @@ public InputStream getInputStream(String account, LocationMode mode, String cont
return SocketAccess.doPrivilegedException(() -> blockBlobReference.openInputStream(null, null, generateOperationContext(account)));
}

@Override
public OutputStream getOutputStream(String account, LocationMode mode, String container, String blob) throws URISyntaxException, StorageException {
logger.trace("writing container [{}], blob [{}]", container, blob);
CloudBlobClient client = this.getSelectedClient(account, mode);
CloudBlockBlob blockBlobReference = client.getContainerReference(container).getBlockBlobReference(blob);
return SocketAccess.doPrivilegedException(() -> blockBlobReference.openOutputStream(null, null, generateOperationContext(account)));
}

@Override
public Map<String, BlobMetaData> listBlobsByPrefix(String account, LocationMode mode, String container, String keyPath, String prefix) throws URISyntaxException, StorageException {
// NOTE: this should be here: if (prefix == null) prefix = "";
Expand Down Expand Up @@ -351,4 +342,15 @@ public void moveBlob(String account, LocationMode mode, String container, String
logger.debug("moveBlob container [{}], sourceBlob [{}], targetBlob [{}] -> done", container, sourceBlob, targetBlob);
}
}

@Override
public void writeBlob(String account, LocationMode mode, String container, String blobName, InputStream inputStream, long blobSize)
throws URISyntaxException, StorageException {
logger.trace("writeBlob({}, stream, {})", blobName, blobSize);
CloudBlobClient client = this.getSelectedClient(account, mode);
CloudBlobContainer blobContainer = client.getContainerReference(container);
CloudBlockBlob blob = blobContainer.getBlockBlobReference(blobName);
SocketAccess.doPrivilegedVoidException(() -> blob.upload(inputStream, blobSize, null, null, generateOperationContext(account)));
logger.trace("writeBlob({}, stream, {}) - done", blobName, blobSize);
}
}

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -20,36 +20,27 @@
package org.elasticsearch.cloud.azure;

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;

import java.io.IOException;
import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.SecureSettings;

public class AzureTestUtils {
/**
* Read settings from file when running integration tests with ThirdParty annotation.
* elasticsearch.yml file path has to be set with -Dtests.config=/path/to/elasticsearch.yml.
* @return Settings from elasticsearch.yml integration test file (for 3rd party tests)
* Mock secure settings from sysprops when running integration tests with ThirdParty annotation.
* Start the tests with {@code -Dtests.azure.account=AzureStorageAccount and -Dtests.azure.key=AzureStorageKey}
* @return Mock Settings from sysprops
*/
public static Settings readSettingsFromFile() {
Settings.Builder settings = Settings.builder();
public static SecureSettings generateMockSecureSettings() {
MockSecureSettings secureSettings = new MockSecureSettings();

// if explicit, just load it and don't load from env
try {
if (Strings.hasText(System.getProperty("tests.config"))) {
try {
settings.loadFromPath(PathUtils.get((System.getProperty("tests.config"))));
} catch (IOException e) {
throw new IllegalArgumentException("could not load azure tests config", e);
}
} else {
throw new IllegalStateException("to run integration tests, you need to set -Dtests.thirdparty=true and " +
"-Dtests.config=/path/to/elasticsearch.yml");
}
} catch (SettingsException exception) {
throw new IllegalStateException("your test configuration file is incorrect: " + System.getProperty("tests.config"), exception);
if (Strings.isEmpty(System.getProperty("tests.azure.account")) ||
Strings.isEmpty(System.getProperty("tests.azure.key"))) {
throw new IllegalStateException("to run integration tests, you need to set -Dtests.thirdparty=true and " +
"-Dtests.azure.account=azure-account -Dtests.azure.key=azure-key");
}
return settings.build();

secureSettings.setString("azure.client.default.account", System.getProperty("tests.azure.account"));
secureSettings.setString("azure.client.default.key", System.getProperty("tests.azure.key"));

return secureSettings;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@
import org.elasticsearch.common.blobstore.support.PlainBlobMetaData;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.io.Streams;
import org.elasticsearch.common.settings.Settings;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URISyntaxException;
import java.nio.file.NoSuchFileException;
import java.util.Locale;
Expand Down Expand Up @@ -84,13 +84,6 @@ public InputStream getInputStream(String account, LocationMode mode, String cont
return new ByteArrayInputStream(blobs.get(blob).toByteArray());
}

@Override
public OutputStream getOutputStream(String account, LocationMode mode, String container, String blob) throws URISyntaxException, StorageException {
ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
blobs.put(blob, outputStream);
return outputStream;
}

@Override
public Map<String, BlobMetaData> listBlobsByPrefix(String account, LocationMode mode, String container, String keyPath, String prefix) {
MapBuilder<String, BlobMetaData> blobsBuilder = MapBuilder.newMapBuilder();
Expand Down Expand Up @@ -120,6 +113,17 @@ public void moveBlob(String account, LocationMode mode, String container, String
}
}

@Override
public void writeBlob(String account, LocationMode mode, String container, String blobName, InputStream inputStream, long blobSize)
throws URISyntaxException, StorageException {
try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) {
blobs.put(blobName, outputStream);
Streams.copy(inputStream, outputStream);
} catch (IOException e) {
throw new StorageException("MOCK", "Error while writing mock stream", e);
}
}

/**
* Test if the given String starts with the specified prefix,
* ignoring upper/lower case.
Expand Down
Loading

0 comments on commit 7786883

Please sign in to comment.