plugins: disable support for config dir

When removing and installing again the plugin all configuration files will be removed in `config/pluginname` dir.
This is bad as users may have set and added specific configuration files.

During an install, if we detect already existing files in `config/pluginname` directory, we simply copy the new file to the same dir but we append a `.new` at the end.

Related to  #5064.

(cherry picked from commit 5da028f)
(cherry picked from commit 4cb1f95)

src/main/java/org/elasticsearch/common/io/FileSystemUtils.java

@@ -25,6 +25,11 @@ import org.elasticsearch.common.logging.ESLogger;
 import java.io.File;
 import java.io.FileFilter;
 import java.io.IOException;
+import java.nio.file.*;
+import java.nio.file.attribute.BasicFileAttributes;
+
+import static java.nio.file.FileVisitResult.CONTINUE;
+import static java.nio.file.FileVisitResult.SKIP_SUBTREE;
 
 /**
  *
@@ -183,4 +188,121 @@ public class FileSystemUtils {
             // ignore
         }
     }
+
+    /**
+     * This utility copy a full directory content (excluded) under
+     * a new directory but without overwriting existing files.
+     *
+     * When a file already exists in destination dir, the source file is copied under
+     * destination directory but with a suffix appended if set or source file is ignored
+     * if suffix is not set (null).
+     * @param source Source directory (for example /tmp/es/src)
+     * @param destination Destination directory (destination directory /tmp/es/dst)
+     * @param suffix When not null, files are copied with a suffix appended to the original name (eg: ".new")
+     *               When null, files are ignored
+     */
+    public static void moveFilesWithoutOverwriting(File source, final File destination, final String suffix) throws IOException {
+
+        // Create destination dir
+        FileSystemUtils.mkdirs(destination);
+
+        final int configPathRootLevel = source.toPath().getNameCount();
+
+        // We walk through the file tree from
+        Files.walkFileTree(source.toPath(), new SimpleFileVisitor<Path>() {
+            private Path buildPath(Path path) {
+                return destination.toPath().resolve(path);
+            }
+
+            @Override
+            public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) throws IOException {
+                // We are now in dir. We need to remove root of config files to have a relative path
+
+                // If we are not walking in root dir, we might be able to copy its content
+                // if it does not already exist
+                if (configPathRootLevel != dir.getNameCount()) {
+                    Path subpath = dir.subpath(configPathRootLevel, dir.getNameCount());
+                    Path path = buildPath(subpath);
+                    if (!Files.exists(path)) {
+                        // We just move the structure to new dir
+                        if (!dir.toFile().renameTo(path.toFile())) {
+                            throw new IOException("Could not move [" + dir + "] to [" + path + "]");
+                        }
+
+                        // We just ignore sub files from here
+                        return FileVisitResult.SKIP_SUBTREE;
+                    }
+                }
+
+                return FileVisitResult.CONTINUE;
+            }
+
+            @Override
+            public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+                Path subpath = null;
+
+                if (configPathRootLevel != file.getNameCount()) {
+                    subpath = file.subpath(configPathRootLevel, file.getNameCount());
+                }
+                Path path = buildPath(subpath);
+
+                if (!Files.exists(path)) {
+                    // We just move the new file to new dir
+                    Files.move(file, path);
+                } else if (suffix != null) {
+                    // If it already exists we try to copy this new version appending suffix to its name
+                    path = Paths.get(path.toString().concat(suffix));
+                    // We just move the file to new dir but with a new name (appended with suffix)
+                    Files.move(file, path, StandardCopyOption.REPLACE_EXISTING);
+                }
+
+                return FileVisitResult.CONTINUE;
+            }
+        });
+    }
+
+    /**
+     * Copy recursively a dir to a new location
+     * @param source source dir
+     * @param destination destination dir
+     */
+    public static void copyDirectoryRecursively(File source, File destination) throws IOException {
+        Files.walkFileTree(source.toPath(),
+                new TreeCopier(source.toPath(), destination.toPath()));
+    }
+
+    static class TreeCopier extends SimpleFileVisitor<Path> {
+        private final Path source;
+        private final Path target;
+
+        TreeCopier(Path source, Path target) {
+            this.source = source;
+            this.target = target;
+        }
+
+        @Override
+        public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) {
+            Path newDir = target.resolve(source.relativize(dir));
+            try {
+                Files.copy(dir, newDir);
+            } catch (FileAlreadyExistsException x) {
+                // We ignore this
+            } catch (IOException x) {
+                return SKIP_SUBTREE;
+            }
+            return CONTINUE;
+        }
+
+        @Override
+        public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
+            Path newFile = target.resolve(source.relativize(file));
+            try {
+                Files.copy(file, newFile);
+            } catch (IOException x) {
+                // We ignore this
+            }
+            return CONTINUE;
+        }
+    }
+
 }

src/main/java/org/elasticsearch/plugins/PluginManager.java

@@ -53,6 +53,7 @@ import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
 
 import static org.elasticsearch.common.Strings.hasLength;
+import static org.elasticsearch.common.io.FileSystemUtils.moveFilesWithoutOverwriting;
 import static org.elasticsearch.common.settings.ImmutableSettings.Builder.EMPTY_SETTINGS;
 
 /**
@@ -256,13 +257,10 @@ public class PluginManager {
 
         File configFile = new File(extractLocation, "config");
         if (configFile.exists() && configFile.isDirectory()) {
-            File toLocation = pluginHandle.configDir(environment);
-            debug("Found config, moving to " + toLocation.getAbsolutePath());
-            FileSystemUtils.deleteRecursively(toLocation);
-            if (!configFile.renameTo(toLocation)) {
-                throw new IOException("Could not move ["+ configFile.getAbsolutePath() + "] to [" + configFile.getAbsolutePath() + "]");
-            }
-            debug("Installed " + name + " into " + toLocation.getAbsolutePath());
+            File configDestLocation = pluginHandle.configDir(environment);
+            debug("Found config, moving to " + configDestLocation.getAbsolutePath());
+            moveFilesWithoutOverwriting(configFile, configDestLocation, ".new");
+            debug("Installed " + name + " into " + configDestLocation.getAbsolutePath());
             potentialSitePlugin = false;
         }
 
@@ -320,15 +318,7 @@ public class PluginManager {
             }
             removed = true;
         }
-        File configLocation = pluginHandle.configDir(environment);
-        if (configLocation.exists()) {
-            debug("Removing: " + configLocation.getPath());
-            if (!FileSystemUtils.deleteRecursively(configLocation)) {
-                throw new IOException("Unable to remove " + pluginHandle.name + ". Check file permissions on " +
-                        configLocation.toString());
-            }
-            removed = true;
-        }
+
         if (removed) {
             log("Removed " + name);
         } else {

src/test/java/org/elasticsearch/common/io/FileSystemUtilsTests.java

@@ -0,0 +1,135 @@
+/*
+ * 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.common.io;
+
+import org.elasticsearch.test.ElasticsearchTestCase;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Properties;
+
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFileExists;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+
+/**
+ * Unit tests for {@link org.elasticsearch.common.io.FileSystemUtils}.
+ */
+public class FileSystemUtilsTests extends ElasticsearchTestCase {
+
+    File src;
+    File dst;
+
+    @Before
+    public void copySourceFilesToTarget() throws IOException {
+        File globalTempDir = globalTempDir();
+        src = new File(globalTempDir, "iocopyappend-src");
+        dst = new File(globalTempDir, "iocopyappend-dst");
+        FileSystemUtils.mkdirs(src);
+        FileSystemUtils.mkdirs(dst);
+
+        // We first copy sources test files from src/test/resources
+        // Because after when the test runs, src files are moved to their destination
+        Properties props = new Properties();
+        try (InputStream is = FileSystemUtilsTests.class.getResource("rootdir.properties").openStream()) {
+            props.load(is);
+        }
+
+        FileSystemUtils.copyDirectoryRecursively(new File(props.getProperty("copyappend.root.dir")), src);
+    }
+
+    @Test
+    public void testMoveOverExistingFileAndAppend() throws IOException {
+
+        FileSystemUtils.moveFilesWithoutOverwriting(new File(src, "v1"), dst, ".new");
+        assertFileContent(dst, "file1.txt", "version1\n");
+        assertFileContent(dst, "dir/file2.txt", "version1\n");
+
+        FileSystemUtils.moveFilesWithoutOverwriting(new File(src, "v2"), dst, ".new");
+        assertFileContent(dst, "file1.txt", "version1\n");
+        assertFileContent(dst, "dir/file2.txt", "version1\n");
+        assertFileContent(dst, "file1.txt.new", "version2\n");
+        assertFileContent(dst, "dir/file2.txt.new", "version2\n");
+        assertFileContent(dst, "file3.txt", "version1\n");
+        assertFileContent(dst, "dir/subdir/file4.txt", "version1\n");
+
+        FileSystemUtils.moveFilesWithoutOverwriting(new File(src, "v3"), dst, ".new");
+        assertFileContent(dst, "file1.txt", "version1\n");
+        assertFileContent(dst, "dir/file2.txt", "version1\n");
+        assertFileContent(dst, "file1.txt.new", "version3\n");
+        assertFileContent(dst, "dir/file2.txt.new", "version3\n");
+        assertFileContent(dst, "file3.txt", "version1\n");
+        assertFileContent(dst, "dir/subdir/file4.txt", "version1\n");
+        assertFileContent(dst, "file3.txt.new", "version2\n");
+        assertFileContent(dst, "dir/subdir/file4.txt.new", "version2\n");
+        assertFileContent(dst, "dir/subdir/file5.txt", "version1\n");
+    }
+
+    @Test
+    public void testMoveOverExistingFileAndIgnore() throws IOException {
+        File dest = globalTempDir();
+
+        FileSystemUtils.moveFilesWithoutOverwriting(new File(src, "v1"), dest, null);
+        assertFileContent(dest, "file1.txt", "version1\n");
+        assertFileContent(dest, "dir/file2.txt", "version1\n");
+
+        FileSystemUtils.moveFilesWithoutOverwriting(new File(src, "v2"), dest, null);
+        assertFileContent(dest, "file1.txt", "version1\n");
+        assertFileContent(dest, "dir/file2.txt", "version1\n");
+        assertFileContent(dest, "file1.txt.new", null);
+        assertFileContent(dest, "dir/file2.txt.new", null);
+        assertFileContent(dest, "file3.txt", "version1\n");
+        assertFileContent(dest, "dir/subdir/file4.txt", "version1\n");
+
+        FileSystemUtils.moveFilesWithoutOverwriting(new File(src, "v3"), dest, null);
+        assertFileContent(dest, "file1.txt", "version1\n");
+        assertFileContent(dest, "dir/file2.txt", "version1\n");
+        assertFileContent(dest, "file1.txt.new", null);
+        assertFileContent(dest, "dir/file2.txt.new", null);
+        assertFileContent(dest, "file3.txt", "version1\n");
+        assertFileContent(dest, "dir/subdir/file4.txt", "version1\n");
+        assertFileContent(dest, "file3.txt.new", null);
+        assertFileContent(dest, "dir/subdir/file4.txt.new", null);
+        assertFileContent(dest, "dir/subdir/file5.txt", "version1\n");
+    }
+
+
+    /**
+     * Check that a file contains a given String
+     * @param dir root dir for file
+     * @param filename relative path from root dir to file
+     * @param expected expected content (if null, we don't expect any file)
+     */
+    public static void assertFileContent(File dir, String filename, String expected) throws IOException {
+        Assert.assertThat(dir.exists(), is(true));
+        File file = dir.toPath().resolve(filename).toFile();
+        if (expected == null) {
+            Assert.assertThat("file [" + file + "] should not exist.", file.exists(), is(false));
+        } else {
+            assertFileExists(file);
+            String fileContent = com.google.common.io.Files.toString(file, UTF8);
+            Assert.assertThat(fileContent, containsString(expected));
+        }
+    }
+}

src/test/java/org/elasticsearch/common/io/rootdir.properties

@@ -0,0 +1 @@
+copyappend.root.dir=${basedir}/src/test/resources/org/elasticsearch/common/io/copyappend

src/test/java/org/elasticsearch/plugins/PluginManagerTests.java

@@ -48,11 +48,13 @@ import java.io.IOException;
 import java.net.URI;
 import java.util.concurrent.TimeUnit;
 
+import static org.elasticsearch.common.io.FileSystemUtilsTests.assertFileContent;
 import static org.elasticsearch.test.ElasticsearchIntegrationTest.Scope;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertDirectoryExists;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFileExists;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.*;
 
 @ClusterScope(scope = Scope.TEST, numDataNodes = 0, transportClientRatio = 0.0)
 public class PluginManagerTests extends ElasticsearchIntegrationTest {
@@ -113,11 +115,11 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
 
             File[] plugins = pluginManager.getListInstalledPlugins();
 
-            assertThat(plugins.length, is(1));
-            assertTrue(pluginBinDir.exists());
-            assertTrue(pluginConfigDir.exists());
+            assertThat(plugins, arrayWithSize(1));
+            assertDirectoryExists(pluginBinDir);
+            assertDirectoryExists(pluginConfigDir);
             File toolFile = new File(pluginBinDir, "tool");
-            assertThat(toolFile.exists(), is(true));
+            assertFileExists(toolFile);
             assertThat(toolFile.canExecute(), is(true));
         } finally {
             // we need to clean up the copied dirs
@@ -126,6 +128,88 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
         }
     }
 
+    /**
+     * Test for #7890
+     */
+    @Test
+    public void testLocalPluginInstallWithBinAndConfigInAlreadyExistingConfigDir_7890() throws Exception {
+        String pluginName = "plugin-test";
+        Tuple<Settings, Environment> initialSettings = InternalSettingsPreparer.prepareSettings(
+                ImmutableSettings.settingsBuilder().build(), false);
+        Environment env = initialSettings.v2();
+
+        File configDir = env.configFile();
+        if (!configDir.exists() && !FileSystemUtils.mkdirs(configDir)) {
+            throw new IOException("Could not create config directory [" + configDir.getAbsolutePath() + "]");
+        }
+        File pluginConfigDir = new File(configDir, pluginName);
+
+        try {
+            PluginManager pluginManager = pluginManager(getPluginUrlForResource("plugin_with_config_v1.zip"), initialSettings);
+            pluginManager.downloadAndExtract(pluginName);
+
+            File[] plugins = pluginManager.getListInstalledPlugins();
+            assertThat(plugins, arrayWithSize(1));
+
+            /*
+            First time, our plugin contains:
+            - config/test.txt (version1)
+             */
+            assertFileContent(pluginConfigDir, "test.txt", "version1\n");
+
+            // We now remove the plugin
+            pluginManager.removePlugin(pluginName);
+            // We should still have test.txt
+            assertFileContent(pluginConfigDir, "test.txt", "version1\n");
+
+            // Installing a new plugin version
+            /*
+            Second time, our plugin contains:
+            - config/test.txt (version2)
+            - config/dir/testdir.txt (version1)
+            - config/dir/subdir/testsubdir.txt (version1)
+             */
+            pluginManager = pluginManager(getPluginUrlForResource("plugin_with_config_v2.zip"), initialSettings);
+            pluginManager.downloadAndExtract(pluginName);
+
+            assertFileContent(pluginConfigDir, "test.txt", "version1\n");
+            assertFileContent(pluginConfigDir, "test.txt.new", "version2\n");
+            assertFileContent(pluginConfigDir, "dir/testdir.txt", "version1\n");
+            assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt", "version1\n");
+
+            // Removing
+            pluginManager.removePlugin(pluginName);
+            assertFileContent(pluginConfigDir, "test.txt", "version1\n");
+            assertFileContent(pluginConfigDir, "test.txt.new", "version2\n");
+            assertFileContent(pluginConfigDir, "dir/testdir.txt", "version1\n");
+            assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt", "version1\n");
+
+            // Installing a new plugin version
+            /*
+            Third time, our plugin contains:
+            - config/test.txt (version3)
+            - config/test2.txt (version1)
+            - config/dir/testdir.txt (version2)
+            - config/dir/testdir2.txt (version1)
+            - config/dir/subdir/testsubdir.txt (version2)
+             */
+            pluginManager = pluginManager(getPluginUrlForResource("plugin_with_config_v3.zip"), initialSettings);
+            pluginManager.downloadAndExtract(pluginName);
+
+            assertFileContent(pluginConfigDir, "test.txt", "version1\n");
+            assertFileContent(pluginConfigDir, "test2.txt", "version1\n");
+            assertFileContent(pluginConfigDir, "test.txt.new", "version3\n");
+            assertFileContent(pluginConfigDir, "dir/testdir.txt", "version1\n");
+            assertFileContent(pluginConfigDir, "dir/testdir.txt.new", "version2\n");
+            assertFileContent(pluginConfigDir, "dir/testdir2.txt", "version1\n");
+            assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt", "version1\n");
+            assertFileContent(pluginConfigDir, "dir/subdir/testsubdir.txt.new", "version2\n");
+        } finally {
+            // we need to clean up the copied dirs
+            FileSystemUtils.deleteRecursively(pluginConfigDir);
+        }
+    }
+
     // For #7152
     @Test
     public void testLocalPluginInstallWithBinOnly_7152() throws Exception {
@@ -143,7 +227,7 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
             pluginManager.downloadAndExtract(pluginName);
             File[] plugins = pluginManager.getListInstalledPlugins();
             assertThat(plugins.length, is(1));
-            assertTrue(pluginBinDir.exists());
+            assertDirectoryExists(pluginBinDir);
         } finally {
             // we need to clean up the copied dirs
             FileSystemUtils.deleteRecursively(pluginBinDir);
@@ -299,7 +383,7 @@ public class PluginManagerTests extends ElasticsearchIntegrationTest {
 
         // We want to check that Plugin Manager moves content to _site
         String pluginDir = PLUGIN_DIR.concat("/plugin-site/_site");
-        assertThat(FileSystemUtils.exists(new File(pluginDir)), is(true));
+        assertFileExists(new File(pluginDir));
     }
 
 

src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java

@@ -70,14 +70,17 @@ import org.hamcrest.Matcher;
 import org.hamcrest.Matchers;
 import org.junit.Assert;
 
+import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
+import java.nio.file.Path;
 import java.util.*;
 import java.util.concurrent.TimeUnit;
 
 import static com.google.common.base.Predicates.isNull;
 import static org.elasticsearch.test.ElasticsearchTestCase.*;
+import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
 
@@ -771,4 +774,32 @@ public class ElasticsearchAssertions {
             return pluginInfo.getVersion();
         }
     };
+
+    /**
+     * Check if a file exists
+     */
+    public static void assertFileExists(File file) {
+        assertThat("file/dir [" + file + "] should exist.", file.exists(), is(true));
+    }
+
+    /**
+     * Check if a file exists
+     */
+    public static void assertFileExists(Path file) {
+        assertFileExists(file.toFile());
+    }
+
+    /**
+     * Check if a directory exists
+     */
+    public static void assertDirectoryExists(File dir) {
+        assertFileExists(dir);
+    }
+
+    /**
+     * Check if a directory exists
+     */
+    public static void assertDirectoryExists(Path dir) {
+        assertFileExists(dir);
+    }
 }

src/test/resources/org/elasticsearch/common/io/copyappend/v1/dir/file2.txt

@@ -0,0 +1 @@
+version1

src/test/resources/org/elasticsearch/common/io/copyappend/v1/file1.txt

@@ -0,0 +1 @@
+version1

src/test/resources/org/elasticsearch/common/io/copyappend/v2/dir/file2.txt

@@ -0,0 +1 @@
+version2

src/test/resources/org/elasticsearch/common/io/copyappend/v2/dir/subdir/file4.txt

@@ -0,0 +1 @@
+version1

src/test/resources/org/elasticsearch/common/io/copyappend/v2/file1.txt

@@ -0,0 +1 @@
+version2

src/test/resources/org/elasticsearch/common/io/copyappend/v2/file3.txt

@@ -0,0 +1 @@
+version1

src/test/resources/org/elasticsearch/common/io/copyappend/v3/dir/file2.txt

@@ -0,0 +1 @@
+version3

src/test/resources/org/elasticsearch/common/io/copyappend/v3/dir/subdir/file4.txt

@@ -0,0 +1 @@
+version2

src/test/resources/org/elasticsearch/common/io/copyappend/v3/dir/subdir/file5.txt

@@ -0,0 +1 @@
+version1

src/test/resources/org/elasticsearch/common/io/copyappend/v3/file1.txt

@@ -0,0 +1 @@
+version3

src/test/resources/org/elasticsearch/common/io/copyappend/v3/file3.txt

@@ -0,0 +1 @@
+version2