diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageServiceImpl.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageServiceImpl.java index f21dbdfd269f4..6f4f8cfea9609 100644 --- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageServiceImpl.java +++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureStorageServiceImpl.java @@ -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; @@ -45,6 +46,7 @@ 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; @@ -52,66 +54,59 @@ public class AzureStorageServiceImpl extends AbstractComponent implements AzureStorageService { final Map storageSettings; - - final Map clients = new HashMap<>(); + final Map clients; public AzureStorageServiceImpl(Settings settings, Map 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 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 createClients(final Map storageSettings) { + final Map clients = new HashMap<>(); + for (Map.Entry 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; diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java index 72cd015f14847..447826dbf833f 100644 --- a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java +++ b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureStorageServiceTests.java @@ -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; @@ -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; @@ -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(); @@ -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)); @@ -103,7 +86,7 @@ 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.")); @@ -111,11 +94,11 @@ public void testGetSelectedClientWithNoPrimaryAndSecondary() { } 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() { @@ -123,7 +106,7 @@ public void testGetSelectedClientDefaultTimeout() { .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); @@ -131,13 +114,13 @@ public void testGetSelectedClientDefaultTimeout() { } 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)); @@ -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)); @@ -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()); @@ -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()); @@ -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)); @@ -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)); @@ -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()); } @@ -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()); } @@ -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()); } @@ -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")); @@ -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)); + } }