From 976448e3395609276446e2b8dbeddb046823aa2e Mon Sep 17 00:00:00 2001 From: David Pilato Date: Wed, 30 Jul 2014 19:06:52 +0200 Subject: [PATCH] `_` is a forbidden character for container (repository) According to [Containers naming guide](http://msdn.microsoft.com/en-us/library/dd135715.aspx): > A container name must be a valid DNS name, conforming to the following naming rules: > > * Container names must start with a letter or number, and can contain only letters, numbers, and the dash (-) character. > * Every dash (-) character must be immediately preceded and followed by a letter or number; consecutive dashes are not permitted in container names. > * All letters in a container name must be lowercase. > * Container names must be from 3 through 63 characters long. We need to fix the documentation and control that before calling Azure API. The validation will come with issue #27. Closes #21. (cherry picked from commit 6531165) --- README.md | 14 +++++- .../azure/AzureSnapshotRestoreITest.java | 44 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index cc2410bbbbffc..975ed8c1ee981 100644 --- a/README.md +++ b/README.md @@ -395,9 +395,21 @@ client.admin().cluster().preparePutRepository("my_backup3") ).get(); ``` +Repository validation rules +--------------------------- + +According to the [containers naming guide](http://msdn.microsoft.com/en-us/library/dd135715.aspx), a container name must +be a valid DNS name, conforming to the following naming rules: + +* Container names must start with a letter or number, and can contain only letters, numbers, and the dash (-) character. +* Every dash (-) character must be immediately preceded and followed by a letter or number; consecutive dashes are not +permitted in container names. +* All letters in a container name must be lowercase. +* Container names must be from 3 through 63 characters long. + Testing -------- +======= Integrations tests in this plugin require working Azure configuration and therefore disabled by default. To enable tests prepare a config file elasticsearch.yml with the following content: diff --git a/src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreITest.java b/src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreITest.java index 007794b58848c..fe3dae5a2bb3e 100644 --- a/src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreITest.java +++ b/src/test/java/org/elasticsearch/repositories/azure/AzureSnapshotRestoreITest.java @@ -22,6 +22,7 @@ import com.microsoft.windowsazure.services.core.ServiceException; import com.microsoft.windowsazure.services.core.storage.StorageException; +import org.apache.lucene.util.LuceneTestCase; import org.elasticsearch.action.admin.cluster.repositories.put.PutRepositoryResponse; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.elasticsearch.action.admin.cluster.snapshots.restore.RestoreSnapshotResponse; @@ -32,6 +33,7 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.repositories.RepositoryException; import org.elasticsearch.repositories.RepositoryMissingException; import org.elasticsearch.snapshots.SnapshotState; import org.elasticsearch.test.ElasticsearchIntegrationTest; @@ -44,6 +46,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; /** * This test needs Azure to run and -Dtests.azure=true to be set @@ -224,6 +227,47 @@ public void testMultipleRepositories() { assertThat(clusterState.getMetaData().hasIndex("test-idx-2"), equalTo(true)); } + /** + * For issue #21: https://github.com/elasticsearch/elasticsearch-cloud-azure/issues/21 + */ + @Test @LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elasticsearch/elasticsearch-cloud-azure/issues/21") + public void testForbiddenContainerName() { + checkContainerName("", false); + checkContainerName("es", false); + checkContainerName("-elasticsearch", false); + checkContainerName("elasticsearch--integration", false); + checkContainerName("elasticsearch_integration", false); + checkContainerName("ElAsTicsearch_integration", false); + checkContainerName("123456789-123456789-123456789-123456789-123456789-123456789-1234", false); + checkContainerName("123456789-123456789-123456789-123456789-123456789-123456789-123", true); + checkContainerName("elasticsearch-integration", true); + checkContainerName("elasticsearch-integration-007", true); + } + + /** + * Create repository with wrong or correct container name + * @param container Container name we want to create + * @param correct Is this container name correct + */ + private void checkContainerName(String container, boolean correct) { + logger.info("--> creating azure repository with container name [{}]", container); + try { + PutRepositoryResponse putRepositoryResponse = client().admin().cluster().preparePutRepository("test-repo") + .setType("azure").setSettings(ImmutableSettings.settingsBuilder() + .put(AzureStorageService.Fields.CONTAINER, container) + .put(AzureStorageService.Fields.BASE_PATH, basePath) + .put(AzureStorageService.Fields.CHUNK_SIZE, randomIntBetween(1000, 10000)) + ).get(); + assertThat(putRepositoryResponse.isAcknowledged(), is(correct)); + client().admin().cluster().prepareDeleteRepository("test-repo").get(); + } catch (RepositoryException e) { + if (correct) { + // We did not expect any exception here :( + throw e; + } + } + } + /** * Deletes repositories, supports wildcard notation. */