Do not create missing directories in readonly repo (#41249)

Today we erroneously look for a node setting called `readonly` when deciding
whether or not to create a missing directory in a filesystem repository. This
change fixes this by using the repository setting instead.

Closes #41009
Relates #26909

server/src/main/java/org/elasticsearch/common/blobstore/fs/FsBlobStore.java

@@ -40,10 +40,10 @@ public class FsBlobStore implements BlobStore {
 
     private final boolean readOnly;
 
-    public FsBlobStore(Settings settings, Path path) throws IOException {
+    public FsBlobStore(Settings settings, Path path, boolean readonly) throws IOException {
         this.path = path;
-        this.readOnly = settings.getAsBoolean("readonly", false);
+        this.readOnly = readonly;
-        if (!this.readOnly) {
+        if (this.readOnly == false) {
             Files.createDirectories(path);
         }
         this.bufferSizeInBytes = (int) settings.getAsBytesSize("repositories.fs.buffer_size",
@@ -74,6 +74,11 @@ public class FsBlobStore implements BlobStore {
 
     @Override
     public void delete(BlobPath path) throws IOException {
+        assert readOnly == false : "should not delete anything from a readonly repository: " + path;
+        //noinspection ConstantConditions in case assertions are disabled
+        if (readOnly) {
+            throw new ElasticsearchException("unexpectedly deleting [" + path + "] from a readonly repository");
+        }
         IOUtils.rm(buildPath(path));
     }
 
@@ -84,7 +89,7 @@ public class FsBlobStore implements BlobStore {
 
     private synchronized Path buildAndCreate(BlobPath path) throws IOException {
         Path f = buildPath(path);
-        if (!readOnly) {
+        if (readOnly == false) {
             Files.createDirectories(f);
         }
         return f;

server/src/main/java/org/elasticsearch/repositories/fs/FsRepository.java

@@ -115,7 +115,7 @@ public class FsRepository extends BlobStoreRepository {
     protected BlobStore createBlobStore() throws Exception {
         final String location = REPOSITORIES_LOCATION_SETTING.get(metadata.settings());
         final Path locationFile = environment.resolveRepoFile(location);
-        return new FsBlobStore(environment.settings(), locationFile);
+        return new FsBlobStore(environment.settings(), locationFile, isReadOnly());
     }
 
     @Override

server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreContainerTests.java

@@ -37,6 +37,6 @@ public class FsBlobStoreContainerTests extends ESBlobStoreContainerTestCase {
         } else {
             settings = Settings.EMPTY;
         }
-        return new FsBlobStore(settings, createTempDir());
+        return new FsBlobStore(settings, createTempDir(), false);
     }
 }

server/src/test/java/org/elasticsearch/common/blobstore/fs/FsBlobStoreTests.java

@@ -42,15 +42,14 @@ public class FsBlobStoreTests extends ESBlobStoreTestCase {
         } else {
             settings = Settings.EMPTY;
         }
-        return new FsBlobStore(settings, createTempDir());
+        return new FsBlobStore(settings, createTempDir(), false);
     }
 
     public void testReadOnly() throws Exception {
-        Settings settings = Settings.builder().put("readonly", true).build();
         Path tempDir = createTempDir();
         Path path = tempDir.resolve("bar");
 
-        try (FsBlobStore store = new FsBlobStore(settings, path)) {
+        try (FsBlobStore store = new FsBlobStore(Settings.EMPTY, path, true)) {
             assertFalse(Files.exists(path));
             BlobPath blobPath = BlobPath.cleanPath().add("foo");
             store.blobContainer(blobPath);
@@ -61,8 +60,7 @@ public class FsBlobStoreTests extends ESBlobStoreTestCase {
             assertFalse(Files.exists(storePath));
         }
 
-        settings = randomBoolean() ? Settings.EMPTY : Settings.builder().put("readonly", false).build();
+        try (FsBlobStore store = new FsBlobStore(Settings.EMPTY, path, false)) {
-        try (FsBlobStore store = new FsBlobStore(settings, path)) {
             assertTrue(Files.exists(path));
             BlobPath blobPath = BlobPath.cleanPath().add("foo");
             BlobContainer container = store.blobContainer(blobPath);

server/src/test/java/org/elasticsearch/repositories/fs/FsBlobStoreRepositoryIT.java

@@ -18,12 +18,22 @@
  */
 package org.elasticsearch.repositories.fs;
 
+import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeUnit;
+import org.elasticsearch.core.internal.io.IOUtils;
 import org.elasticsearch.repositories.Repository;
 import org.elasticsearch.repositories.blobstore.ESBlobStoreRepositoryIntegTestCase;
 
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Stream;
+
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
 import static org.hamcrest.Matchers.instanceOf;
 
 public class FsBlobStoreRepositoryIT extends ESBlobStoreRepositoryIntegTestCase {
@@ -41,4 +51,47 @@ public class FsBlobStoreRepositoryIT extends ESBlobStoreRepositoryIntegTestCase
     protected void afterCreationCheck(Repository repository) {
         assertThat(repository, instanceOf(FsRepository.class));
     }
+
+    public void testMissingDirectoriesNotCreatedInReadonlyRepository() throws IOException, ExecutionException, InterruptedException {
+        final String repoName = randomAsciiName();
+        final Path repoPath = randomRepoPath();
+
+        logger.info("--> creating repository {} at {}", repoName, repoPath);
+
+        assertAcked(client().admin().cluster().preparePutRepository(repoName).setType("fs").setSettings(Settings.builder()
+            .put("location", repoPath)
+            .put("compress", randomBoolean())
+            .put("chunk_size", randomIntBetween(100, 1000), ByteSizeUnit.BYTES)));
+
+        String indexName = randomAsciiName();
+        int docCount = iterations(10, 1000);
+        logger.info("-->  create random index {} with {} records", indexName, docCount);
+        addRandomDocuments(indexName, docCount);
+        assertHitCount(client().prepareSearch(indexName).setSize(0).get(), docCount);
+
+        final String snapshotName = randomAsciiName();
+        logger.info("-->  create snapshot {}:{}", repoName, snapshotName);
+        assertSuccessfulSnapshot(client().admin().cluster().prepareCreateSnapshot(repoName, snapshotName)
+            .setWaitForCompletion(true).setIndices(indexName));
+
+        assertAcked(client().admin().indices().prepareDelete(indexName));
+        assertAcked(client().admin().cluster().prepareDeleteRepository(repoName));
+
+        final Path deletedPath;
+        try (Stream<Path> contents = Files.list(repoPath.resolve("indices"))) {
+            //noinspection OptionalGetWithoutIsPresent because we know there's a subdirectory
+            deletedPath = contents.filter(Files::isDirectory).findAny().get();
+            IOUtils.rm(deletedPath);
+        }
+        assertFalse(Files.exists(deletedPath));
+
+        assertAcked(client().admin().cluster().preparePutRepository(repoName).setType("fs").setSettings(Settings.builder()
+            .put("location", repoPath).put("readonly", true)));
+
+        final ElasticsearchException exception = expectThrows(ElasticsearchException.class, () ->
+            client().admin().cluster().prepareRestoreSnapshot(repoName, snapshotName).setWaitForCompletion(randomBoolean()).get());
+        assertThat(exception.getRootCause(), instanceOf(NoSuchFileException.class));
+
+        assertFalse("deleted path is not recreated in readonly repository", Files.exists(deletedPath));
+    }
 }

server/src/test/java/org/elasticsearch/snapshots/BlobStoreFormatIT.java

@@ -238,8 +238,7 @@ public class BlobStoreFormatIT extends AbstractSnapshotIntegTestCase {
     }
 
     protected BlobStore createTestBlobStore() throws IOException {
-        Settings settings = Settings.builder().build();
+        return new FsBlobStore(Settings.EMPTY, randomRepoPath(), false);
-        return new FsBlobStore(settings, randomRepoPath());
     }
 
     protected void randomCorruption(BlobContainer blobContainer, String blobName) throws IOException {