honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
David Pilato 2016-12-08 18:54:00 +01:00
parent 18a3d6b4f3
commit 8b0df47381
2 changed files with 115 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
plugins/repository-azure/src/main/java/org/elasticsearch/repositories/azure/AzureRepository.java
View File

@ -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);

104
plugins/repository-azure/src/test/java/org/elasticsearch/repositories/azure/AzureRepositorySettingsTests.java Normal file
View File

@ -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));
}
}