Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AzureStorageServiceTests and remove log traces #30924

Merged
merged 1 commit into from
May 29, 2018
Merged

Fix AzureStorageServiceTests and remove log traces #30924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 32 additions & 37 deletions ...ory-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.microsoft.azure.storage.blob.ListBlobItem;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.blobstore.BlobMetaData;
import org.elasticsearch.common.blobstore.support.PlainBlobMetaData;
import org.elasticsearch.common.collect.MapBuilder;
Expand All @@ -45,73 +46,67 @@
import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;

public class AzureStorageServiceImpl extends AbstractComponent implements AzureStorageService {

final Map<String, AzureStorageSettings> storageSettings;

final Map<String, CloudBlobClient> clients = new HashMap<>();
final Map<String, CloudBlobClient> clients;

public AzureStorageServiceImpl(Settings settings, Map<String, AzureStorageSettings> storageSettings) {
super(settings);

this.storageSettings = storageSettings;

if (storageSettings.isEmpty()) {
// If someone did not register any settings, they basically can't use the plugin
throw new IllegalArgumentException("If you want to use an azure repository, you need to define a client configuration.");
}

logger.debug("starting azure storage client instance");

// We register all regular azure clients
for (Map.Entry<String, AzureStorageSettings> azureStorageSettingsEntry : this.storageSettings.entrySet()) {
logger.debug("registering regular client for account [{}]", azureStorageSettingsEntry.getKey());
createClient(azureStorageSettingsEntry.getValue());
}
this.storageSettings = storageSettings;
this.clients = createClients(storageSettings);
}

void createClient(AzureStorageSettings azureStorageSettings) {
try {
logger.trace("creating new Azure storage client using account [{}], key [{}], endpoint suffix [{}]",
azureStorageSettings.getAccount(), azureStorageSettings.getKey(), azureStorageSettings.getEndpointSuffix());
private Map<String, CloudBlobClient> createClients(final Map<String, AzureStorageSettings> storageSettings) {
final Map<String, CloudBlobClient> clients = new HashMap<>();
for (Map.Entry<String, AzureStorageSettings> azureStorageEntry : storageSettings.entrySet()) {
final String clientName = azureStorageEntry.getKey();
final AzureStorageSettings clientSettings = azureStorageEntry.getValue();
try {
logger.trace("creating new Azure storage client with name [{}]", clientName);
String storageConnectionString =
"DefaultEndpointsProtocol=https;"
+ "AccountName=" + clientSettings.getAccount() + ";"
+ "AccountKey=" + clientSettings.getKey();

final String endpointSuffix = clientSettings.getEndpointSuffix();
if (Strings.hasLength(endpointSuffix)) {
storageConnectionString += ";EndpointSuffix=" + endpointSuffix;
}
// Retrieve storage account from connection-string.
CloudStorageAccount storageAccount = CloudStorageAccount.parse(storageConnectionString);

String storageConnectionString =
"DefaultEndpointsProtocol=https;"
+ "AccountName=" + azureStorageSettings.getAccount() + ";"
+ "AccountKey=" + azureStorageSettings.getKey();
// Create the blob client.
CloudBlobClient client = storageAccount.createCloudBlobClient();

String endpointSuffix = azureStorageSettings.getEndpointSuffix();
if (endpointSuffix != null && !endpointSuffix.isEmpty()) {
storageConnectionString += ";EndpointSuffix=" + endpointSuffix;
// Register the client
clients.put(clientSettings.getAccount(), client);
} catch (Exception e) {
logger.error(() -> new ParameterizedMessage("Can not create azure storage client [{}]", clientName), e);
}
// Retrieve storage account from connection-string.
CloudStorageAccount storageAccount = CloudStorageAccount.parse(storageConnectionString);

// Create the blob client.
CloudBlobClient client = storageAccount.createCloudBlobClient();

// Register the client
this.clients.put(azureStorageSettings.getAccount(), client);
} catch (Exception e) {
logger.error("can not create azure storage client: {}", e.getMessage());
}
return Collections.unmodifiableMap(clients);
}

CloudBlobClient getSelectedClient(String clientName, LocationMode mode) {
logger.trace("selecting a client named [{}], mode [{}]", clientName, mode.name());
AzureStorageSettings azureStorageSettings = this.storageSettings.get(clientName);
if (azureStorageSettings == null) {
throw new IllegalArgumentException("Can not find named azure client [" + clientName + "]. Check your settings.");
throw new IllegalArgumentException("Unable to find client with name [" + clientName + "]");
}

CloudBlobClient client = this.clients.get(azureStorageSettings.getAccount());

if (client == null) {
throw new IllegalArgumentException("Can not find an azure client named [" + azureStorageSettings.getAccount() + "]");
throw new IllegalArgumentException("No account defined for client with name [" + clientName + "]");
}

// NOTE: for now, just set the location mode in case it is different;
Expand Down
98 changes: 44 additions & 54 deletions ...ry-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.microsoft.azure.storage.RetryExponentialRetry;
import com.microsoft.azure.storage.blob.CloudBlobClient;
import com.microsoft.azure.storage.core.Base64;

import org.elasticsearch.common.settings.MockSecureSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.settings.SettingsException;
Expand All @@ -36,6 +35,7 @@
import java.net.URISyntaxException;
import java.net.UnknownHostException;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.Map;

import static org.elasticsearch.repositories.azure.AzureStorageServiceImpl.blobNameFromUri;
Expand All @@ -49,31 +49,14 @@

public class AzureStorageServiceTests extends ESTestCase {

private MockSecureSettings buildSecureSettings() {
MockSecureSettings secureSettings = new MockSecureSettings();
secureSettings.setString("azure.client.azure1.account", "myaccount1");
secureSettings.setString("azure.client.azure1.key", "mykey1");
secureSettings.setString("azure.client.azure2.account", "myaccount2");
secureSettings.setString("azure.client.azure2.key", "mykey2");
secureSettings.setString("azure.client.azure3.account", "myaccount3");
secureSettings.setString("azure.client.azure3.key", "mykey3");
return secureSettings;
}
private Settings buildSettings() {
Settings settings = Settings.builder()
.setSecureSettings(buildSecureSettings())
.build();
return settings;
}

public void testReadSecuredSettings() {
MockSecureSettings secureSettings = new MockSecureSettings();
secureSettings.setString("azure.client.azure1.account", "myaccount1");
secureSettings.setString("azure.client.azure1.key", "mykey1");
secureSettings.setString("azure.client.azure1.key", encodeKey("mykey1"));
secureSettings.setString("azure.client.azure2.account", "myaccount2");
secureSettings.setString("azure.client.azure2.key", "mykey2");
secureSettings.setString("azure.client.azure2.key", encodeKey("mykey2"));
secureSettings.setString("azure.client.azure3.account", "myaccount3");
secureSettings.setString("azure.client.azure3.key", "mykey3");
secureSettings.setString("azure.client.azure3.key", encodeKey("mykey3"));
Settings settings = Settings.builder().setSecureSettings(secureSettings)
.put("azure.client.azure3.endpoint_suffix", "my_endpoint_suffix").build();

Expand All @@ -88,9 +71,9 @@ public void testReadSecuredSettings() {
public void testCreateClientWithEndpointSuffix() {
MockSecureSettings secureSettings = new MockSecureSettings();
secureSettings.setString("azure.client.azure1.account", "myaccount1");
secureSettings.setString("azure.client.azure1.key", Base64.encode("mykey1".getBytes(StandardCharsets.UTF_8)));
secureSettings.setString("azure.client.azure1.key", encodeKey("mykey1"));
secureSettings.setString("azure.client.azure2.account", "myaccount2");
secureSettings.setString("azure.client.azure2.key", Base64.encode("mykey2".getBytes(StandardCharsets.UTF_8)));
secureSettings.setString("azure.client.azure2.key", encodeKey("mykey2"));
Settings settings = Settings.builder().setSecureSettings(secureSettings)
.put("azure.client.azure1.endpoint_suffix", "my_endpoint_suffix").build();
AzureStorageServiceImpl azureStorageService = new AzureStorageServiceImpl(settings, AzureStorageSettings.load(settings));
Expand All @@ -103,41 +86,41 @@ public void testCreateClientWithEndpointSuffix() {

public void testGetSelectedClientWithNoPrimaryAndSecondary() {
try {
new AzureStorageServiceMockForSettings(Settings.EMPTY);
new AzureStorageServiceImpl(Settings.EMPTY, Collections.emptyMap());
fail("we should have raised an IllegalArgumentException");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage(), is("If you want to use an azure repository, you need to define a client configuration."));
}
}

public void testGetSelectedClientNonExisting() {
AzureStorageServiceImpl azureStorageService = new AzureStorageServiceMockForSettings(buildSettings());
AzureStorageServiceImpl azureStorageService = createAzureService(buildSettings());
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
azureStorageService.getSelectedClient("azure4", LocationMode.PRIMARY_ONLY);
});
assertThat(e.getMessage(), is("Can not find named azure client [azure4]. Check your settings."));
assertThat(e.getMessage(), is("Unable to find client with name [azure4]"));
}

public void testGetSelectedClientDefaultTimeout() {
Settings timeoutSettings = Settings.builder()
.setSecureSettings(buildSecureSettings())
.put("azure.client.azure3.timeout", "30s")
.build();
AzureStorageServiceImpl azureStorageService = new AzureStorageServiceMockForSettings(timeoutSettings);
AzureStorageServiceImpl azureStorageService = createAzureService(timeoutSettings);
CloudBlobClient client1 = azureStorageService.getSelectedClient("azure1", LocationMode.PRIMARY_ONLY);
assertThat(client1.getDefaultRequestOptions().getTimeoutIntervalInMs(), nullValue());
CloudBlobClient client3 = azureStorageService.getSelectedClient("azure3", LocationMode.PRIMARY_ONLY);
assertThat(client3.getDefaultRequestOptions().getTimeoutIntervalInMs(), is(30 * 1000));
}

public void testGetSelectedClientNoTimeout() {
AzureStorageServiceImpl azureStorageService = new AzureStorageServiceMockForSettings(buildSettings());
AzureStorageServiceImpl azureStorageService = createAzureService(buildSettings());
CloudBlobClient client1 = azureStorageService.getSelectedClient("azure1", LocationMode.PRIMARY_ONLY);
assertThat(client1.getDefaultRequestOptions().getTimeoutIntervalInMs(), is(nullValue()));
}

public void testGetSelectedClientBackoffPolicy() {
AzureStorageServiceImpl azureStorageService = new AzureStorageServiceMockForSettings(buildSettings());
AzureStorageServiceImpl azureStorageService = createAzureService(buildSettings());
CloudBlobClient client1 = azureStorageService.getSelectedClient("azure1", LocationMode.PRIMARY_ONLY);
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), is(notNullValue()));
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), instanceOf(RetryExponentialRetry.class));
Expand All @@ -149,7 +132,7 @@ public void testGetSelectedClientBackoffPolicyNbRetries() {
.put("azure.client.azure1.max_retries", 7)
.build();

AzureStorageServiceImpl azureStorageService = new AzureStorageServiceMockForSettings(timeoutSettings);
AzureStorageServiceImpl azureStorageService = createAzureService(timeoutSettings);
CloudBlobClient client1 = azureStorageService.getSelectedClient("azure1", LocationMode.PRIMARY_ONLY);
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), is(notNullValue()));
assertThat(client1.getDefaultRequestOptions().getRetryPolicyFactory(), instanceOf(RetryExponentialRetry.class));
Expand All @@ -159,7 +142,7 @@ public void testNoProxy() {
Settings settings = Settings.builder()
.setSecureSettings(buildSecureSettings())
.build();
AzureStorageServiceMockForSettings mock = new AzureStorageServiceMockForSettings(settings);
AzureStorageServiceImpl mock = createAzureService(settings);
assertThat(mock.storageSettings.get("azure1").getProxy(), nullValue());
assertThat(mock.storageSettings.get("azure2").getProxy(), nullValue());
assertThat(mock.storageSettings.get("azure3").getProxy(), nullValue());
Expand All @@ -172,7 +155,7 @@ public void testProxyHttp() throws UnknownHostException {
.put("azure.client.azure1.proxy.port", 8080)
.put("azure.client.azure1.proxy.type", "http")
.build();
AzureStorageServiceMockForSettings mock = new AzureStorageServiceMockForSettings(settings);
AzureStorageServiceImpl mock = createAzureService(settings);
Proxy azure1Proxy = mock.storageSettings.get("azure1").getProxy();

assertThat(azure1Proxy, notNullValue());
Expand All @@ -192,7 +175,7 @@ public void testMultipleProxies() throws UnknownHostException {
.put("azure.client.azure2.proxy.port", 8081)
.put("azure.client.azure2.proxy.type", "http")
.build();
AzureStorageServiceMockForSettings mock = new AzureStorageServiceMockForSettings(settings);
AzureStorageServiceImpl mock = createAzureService(settings);
Proxy azure1Proxy = mock.storageSettings.get("azure1").getProxy();
assertThat(azure1Proxy, notNullValue());
assertThat(azure1Proxy.type(), is(Proxy.Type.HTTP));
Expand All @@ -211,7 +194,7 @@ public void testProxySocks() throws UnknownHostException {
.put("azure.client.azure1.proxy.port", 8080)
.put("azure.client.azure1.proxy.type", "socks")
.build();
AzureStorageServiceMockForSettings mock = new AzureStorageServiceMockForSettings(settings);
AzureStorageServiceImpl mock = createAzureService(settings);
Proxy azure1Proxy = mock.storageSettings.get("azure1").getProxy();
assertThat(azure1Proxy, notNullValue());
assertThat(azure1Proxy.type(), is(Proxy.Type.SOCKS));
Expand All @@ -227,7 +210,7 @@ public void testProxyNoHost() {
.put("azure.client.azure1.proxy.type", randomFrom("socks", "http"))
.build();

SettingsException e = expectThrows(SettingsException.class, () -> new AzureStorageServiceMockForSettings(settings));
SettingsException e = expectThrows(SettingsException.class, () -> createAzureService(settings));
assertEquals("Azure Proxy type has been set but proxy host or port is not defined.", e.getMessage());
}

Expand All @@ -238,7 +221,7 @@ public void testProxyNoPort() {
.put("azure.client.azure1.proxy.type", randomFrom("socks", "http"))
.build();

SettingsException e = expectThrows(SettingsException.class, () -> new AzureStorageServiceMockForSettings(settings));
SettingsException e = expectThrows(SettingsException.class, () -> createAzureService(settings));
assertEquals("Azure Proxy type has been set but proxy host or port is not defined.", e.getMessage());
}

Expand All @@ -249,7 +232,7 @@ public void testProxyNoType() {
.put("azure.client.azure1.proxy.port", 8080)
.build();

SettingsException e = expectThrows(SettingsException.class, () -> new AzureStorageServiceMockForSettings(settings));
SettingsException e = expectThrows(SettingsException.class, () -> createAzureService(settings));
assertEquals("Azure Proxy port or host have been set but proxy type is not defined.", e.getMessage());
}

Expand All @@ -261,26 +244,10 @@ public void testProxyWrongHost() {
.put("azure.client.azure1.proxy.port", 8080)
.build();

SettingsException e = expectThrows(SettingsException.class, () -> new AzureStorageServiceMockForSettings(settings));
SettingsException e = expectThrows(SettingsException.class, () -> createAzureService(settings));
assertEquals("Azure proxy host is unknown.", e.getMessage());
}

/**
* This internal class just overload createClient method which is called by AzureStorageServiceImpl.doStart()
*/
class AzureStorageServiceMockForSettings extends AzureStorageServiceImpl {
AzureStorageServiceMockForSettings(Settings settings) {
super(settings, AzureStorageSettings.load(settings));
}

// We fake the client here
@Override
void createClient(AzureStorageSettings azureStorageSettings) {
this.clients.put(azureStorageSettings.getAccount(),
new CloudBlobClient(URI.create("https://" + azureStorageSettings.getAccount())));
}
}

public void testBlobNameFromUri() throws URISyntaxException {
String name = blobNameFromUri(new URI("https://myservice.azure.net/container/path/to/myfile"));
assertThat(name, is("path/to/myfile"));
Expand All @@ -291,4 +258,27 @@ public void testBlobNameFromUri() throws URISyntaxException {
name = blobNameFromUri(new URI("https://127.0.0.1/container/path/to/myfile"));
assertThat(name, is("path/to/myfile"));
}

private static MockSecureSettings buildSecureSettings() {
MockSecureSettings secureSettings = new MockSecureSettings();
secureSettings.setString("azure.client.azure1.account", "myaccount1");
secureSettings.setString("azure.client.azure1.key", encodeKey("mykey1"));
secureSettings.setString("azure.client.azure2.account", "myaccount2");
secureSettings.setString("azure.client.azure2.key", encodeKey("mykey2"));
secureSettings.setString("azure.client.azure3.account", "myaccount3");
secureSettings.setString("azure.client.azure3.key", encodeKey("mykey3"));
return secureSettings;
}

private static Settings buildSettings() {
return Settings.builder().setSecureSettings(buildSecureSettings()).build();
}

private static AzureStorageServiceImpl createAzureService(final Settings settings) {
return new AzureStorageServiceImpl(settings, AzureStorageSettings.load(settings));
}

private static String encodeKey(final String value) {
return Base64.encode(value.getBytes(StandardCharsets.UTF_8));
}
}