summaryrefslogtreecommitdiff
path: root/plugins/repository-azure
diff options
context:
space:
mode:
authorDavid Pilato <david@pilato.fr>2016-12-08 18:54:00 +0100
committerDavid Pilato <david@pilato.fr>2016-12-08 18:54:00 +0100
commit8b0df473814dc158eda8eee5fe974e019ff2c802 (patch)
tree3c244ba758a0fe0fbc1da4b4c2323fa491767768 /plugins/repository-azure
parent18a3d6b4f3f893d8b229791df36e13cd37c77050 (diff)
readonly on azure repository must be taken into account
While I was fixing a documentation issue (#22007), I looked at the code and discovered that we actually never read what the user entered as a `readonly` parameter when he creates an azure repository. So if someone sends: ``` PUT _snapshot/my_backup4 { "type": "azure", "settings": { "account": "my_account2", "location_mode": "primary_only", "readonly": true } } ``` The repository is not actually defined as `readonly`. It's caused by the fact we are always overwriting `readonly`setting based on `location_mode`. If a user sets it to `primary_only`, `readonly` is forced to `false`. If a user sets it to `primary_then_secondary`, `readonly` is forced to `false`. If a user sets it to `secondary_only`, `readonly` is forced to `false`. Note that with this change, a user can force a `secondary_only` repository to `readonly: false` which will lead him to an error later on when we check the repository as per definition in Azure, a secondary repository is not writable. Another option could have been to detect this mismatch and throw an exception in that case. Note sure it is worth writing more code though. Closes #22053.
Diffstat (limited to 'plugins/repository-azure')
-rw-r--r--plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java15
-rw-r--r--plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java104
2 files changed, 115 insertions, 4 deletions
diff --git a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java
index 1cf75780ce..2ae5fbe849 100644
--- a/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java
+++ b/plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java
@@ -100,11 +100,18 @@ public class AzureRepository extends BlobStoreRepository {
this.compress = getValue(metadata.settings(), settings, Repository.COMPRESS_SETTING, Storage.COMPRESS_SETTING);
String modeStr = getValue(metadata.settings(), settings, Repository.LOCATION_MODE_SETTING, Storage.LOCATION_MODE_SETTING);
- if (Strings.hasLength(modeStr)) {
- LocationMode locationMode = LocationMode.valueOf(modeStr.toUpperCase(Locale.ROOT));
- readonly = locationMode == LocationMode.SECONDARY_ONLY;
+ Boolean forcedReadonly = metadata.settings().getAsBoolean("readonly", null);
+ // If the user explicitly did not define a readonly value, we set it by ourselves depending on the location mode setting.
+ // For secondary_only setting, the repository should be read only
+ if (forcedReadonly == null) {
+ if (Strings.hasLength(modeStr)) {
+ LocationMode locationMode = LocationMode.valueOf(modeStr.toUpperCase(Locale.ROOT));
+ this.readonly = locationMode == LocationMode.SECONDARY_ONLY;
+ } else {
+ this.readonly = false;
+ }
} else {
- readonly = false;
+ readonly = forcedReadonly;
}
String basePath = getValue(metadata.settings(), settings, Repository.BASE_PATH_SETTING, Storage.BASE_PATH_SETTING);
diff --git a/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java
new file mode 100644
index 0000000000..68b3783b03
--- /dev/null
+++ b/plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java
@@ -0,0 +1,104 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.elasticsearch.repositories.azure;
+
+import com.microsoft.azure.storage.LocationMode;
+import com.microsoft.azure.storage.StorageException;
+import com.microsoft.azure.storage.blob.CloudBlobClient;
+import org.elasticsearch.cloud.azure.storage.AzureStorageService;
+import org.elasticsearch.cloud.azure.storage.AzureStorageServiceImpl;
+import org.elasticsearch.cloud.azure.storage.AzureStorageSettings;
+import org.elasticsearch.cluster.metadata.RepositoryMetaData;
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.env.NodeEnvironment;
+import org.elasticsearch.test.ESTestCase;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import static org.elasticsearch.cloud.azure.storage.AzureStorageServiceImpl.blobNameFromUri;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+
+public class AzureRepositorySettingsTests extends ESTestCase {
+
+ private AzureRepository azureRepository(Settings settings) throws StorageException, IOException, URISyntaxException {
+ Settings internalSettings = Settings.builder()
+ .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toAbsolutePath())
+ .putArray(Environment.PATH_DATA_SETTING.getKey(), tmpPaths())
+ .put(settings)
+ .build();
+ return new AzureRepository(new RepositoryMetaData("foo", "azure", internalSettings), new Environment(internalSettings), null);
+ }
+
+
+ public void testReadonlyDefault() throws StorageException, IOException, URISyntaxException {
+ assertThat(azureRepository(Settings.EMPTY).isReadOnly(), is(false));
+ }
+
+ public void testReadonlyDefaultAndReadonlyOn() throws StorageException, IOException, URISyntaxException {
+ assertThat(azureRepository(Settings.builder()
+ .put("readonly", true)
+ .build()).isReadOnly(), is(true));
+ }
+
+ public void testReadonlyWithPrimaryOnly() throws StorageException, IOException, URISyntaxException {
+ assertThat(azureRepository(Settings.builder()
+ .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.PRIMARY_ONLY.name())
+ .build()).isReadOnly(), is(false));
+ }
+
+ public void testReadonlyWithPrimaryOnlyAndReadonlyOn() throws StorageException, IOException, URISyntaxException {
+ assertThat(azureRepository(Settings.builder()
+ .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.PRIMARY_ONLY.name())
+ .put("readonly", true)
+ .build()).isReadOnly(), is(true));
+ }
+
+ public void testReadonlyWithSecondaryOnlyAndReadonlyOn() throws StorageException, IOException, URISyntaxException {
+ assertThat(azureRepository(Settings.builder()
+ .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.SECONDARY_ONLY.name())
+ .put("readonly", true)
+ .build()).isReadOnly(), is(true));
+ }
+
+ public void testReadonlyWithSecondaryOnlyAndReadonlyOff() throws StorageException, IOException, URISyntaxException {
+ assertThat(azureRepository(Settings.builder()
+ .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.SECONDARY_ONLY.name())
+ .put("readonly", false)
+ .build()).isReadOnly(), is(false));
+ }
+
+ public void testReadonlyWithPrimaryAndSecondaryOnlyAndReadonlyOn() throws StorageException, IOException, URISyntaxException {
+ assertThat(azureRepository(Settings.builder()
+ .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.PRIMARY_THEN_SECONDARY.name())
+ .put("readonly", true)
+ .build()).isReadOnly(), is(true));
+ }
+
+ public void testReadonlyWithPrimaryAndSecondaryOnlyAndReadonlyOff() throws StorageException, IOException, URISyntaxException {
+ assertThat(azureRepository(Settings.builder()
+ .put(AzureRepository.Repository.LOCATION_MODE_SETTING.getKey(), LocationMode.PRIMARY_THEN_SECONDARY.name())
+ .put("readonly", false)
+ .build()).isReadOnly(), is(false));
+ }
+}