Added logic to allow {dot} files on startup (#1437)

* Added logic to allow {dot} files on startup

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

* Ensures that only plugin directories are returned by findPluginDirs()

Signed-off-by: Ryan Bogan <rbogan@amazon.com>

* Prevents . files from being returned as plugins

Signed-off-by: Ryan Bogan <rbogan@amazon.com>
This commit is contained in:
Ryan Bogan 2021-11-04 10:01:55 -04:00 committed by GitHub
parent f54cc382d5
commit 286121f0dc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 66 deletions

View File

@ -215,33 +215,6 @@ public class SpawnerNoBootstrapTests extends LuceneTestCase {
equalTo("module [test_plugin] does not have permission to fork native controller"));
}
public void testSpawnerHandlingOfDesktopServicesStoreFiles() throws IOException {
final Path esHome = createTempDir().resolve("home");
final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), esHome.toString()).build();
final Environment environment = TestEnvironment.newEnvironment(settings);
Files.createDirectories(environment.modulesFile());
Files.createDirectories(environment.pluginsFile());
final Path desktopServicesStore = environment.modulesFile().resolve(".DS_Store");
Files.createFile(desktopServicesStore);
final Spawner spawner = new Spawner();
if (Constants.MAC_OS_X) {
// if the spawner were not skipping the Desktop Services Store files on macOS this would explode
spawner.spawnNativeControllers(environment, false);
} else {
// we do not ignore these files on non-macOS systems
final FileSystemException e = expectThrows(FileSystemException.class, () -> spawner.spawnNativeControllers(environment, false));
if (Constants.WINDOWS) {
assertThat(e, instanceOf(NoSuchFileException.class));
} else {
assertThat(e, hasToString(containsString("Not a directory")));
}
}
}
private void createControllerProgram(final Path outputFile) throws IOException {
final Path outputDir = outputFile.getParent();
Files.createDirectories(outputDir);

View File

@ -50,7 +50,6 @@ import org.opensearch.common.Strings;
import org.opensearch.common.collect.Tuple;
import org.opensearch.common.component.LifecycleComponent;
import org.opensearch.common.inject.Module;
import org.opensearch.common.io.FileSystemUtils;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Setting.Property;
import org.opensearch.common.settings.Settings;
@ -365,7 +364,10 @@ public class PluginsService implements ReportingService<PluginsAndModules> {
if (Files.exists(rootPath)) {
try (DirectoryStream<Path> stream = Files.newDirectoryStream(rootPath)) {
for (Path plugin : stream) {
if (FileSystemUtils.isDesktopServicesStore(plugin) || plugin.getFileName().toString().startsWith(".removing-")) {
if (plugin.getFileName().toString().startsWith(".") && !Files.isDirectory(plugin)) {
logger.warn(
"Non-plugin file located in the plugins folder with the following name: [" + plugin.getFileName() + "]"
);
continue;
}
if (seen.add(plugin.getFileName().toString()) == false) {

View File

@ -32,7 +32,10 @@
package org.opensearch.plugins;
import org.opensearch.common.logging.Loggers;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.apache.lucene.util.Constants;
import org.apache.lucene.util.LuceneTestCase;
import org.opensearch.LegacyESVersion;
@ -44,6 +47,7 @@ import org.opensearch.common.settings.Settings;
import org.opensearch.env.Environment;
import org.opensearch.env.TestEnvironment;
import org.opensearch.index.IndexModule;
import org.opensearch.test.MockLogAppender;
import org.opensearch.test.OpenSearchTestCase;
import org.hamcrest.Matchers;
@ -51,9 +55,7 @@ import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.InvocationTargetException;
import java.net.URL;
import java.nio.file.FileSystemException;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collection;
@ -154,50 +156,17 @@ public class PluginsServiceTests extends OpenSearchTestCase {
assertEquals(FilterablePlugin.class, scriptPlugins.get(0).getClass());
}
public void testHiddenFiles() throws IOException {
public void testHiddenDirectories() throws IOException {
final Path home = createTempDir();
final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), home).build();
final Path hidden = home.resolve("plugins").resolve(".hidden");
Files.createDirectories(hidden);
@SuppressWarnings("unchecked")
final IllegalStateException e = expectThrows(IllegalStateException.class, () -> newPluginsService(settings));
final String expected = "Could not load plugin descriptor for plugin directory [.hidden]";
assertThat(e, hasToString(containsString(expected)));
}
public void testDesktopServicesStoreFiles() throws IOException {
final Path home = createTempDir();
final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), home).build();
final Path plugins = home.resolve("plugins");
Files.createDirectories(plugins);
final Path desktopServicesStore = plugins.resolve(".DS_Store");
Files.createFile(desktopServicesStore);
if (Constants.MAC_OS_X) {
@SuppressWarnings("unchecked")
final PluginsService pluginsService = newPluginsService(settings);
assertNotNull(pluginsService);
} else {
final IllegalStateException e = expectThrows(IllegalStateException.class, () -> newPluginsService(settings));
assertThat(e.getMessage(), containsString("Could not load plugin descriptor for plugin directory [.DS_Store]"));
assertNotNull(e.getCause());
assertThat(e.getCause(), instanceOf(FileSystemException.class));
if (Constants.WINDOWS) {
assertThat(e.getCause(), instanceOf(NoSuchFileException.class));
} else {
// force a "Not a directory" exception to be thrown so that we can extract the locale-dependent message
final String expected;
try (InputStream ignored = Files.newInputStream(desktopServicesStore.resolve("not-a-directory"))) {
throw new AssertionError();
} catch (final FileSystemException inner) {
// locale-dependent translation of "Not a directory"
expected = inner.getReason();
}
assertThat(e.getCause(), hasToString(containsString(expected)));
}
}
}
public void testStartupWithRemovingMarker() throws IOException {
final Path home = createTempDir();
final Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), home).build();
@ -765,10 +734,25 @@ public class PluginsServiceTests extends OpenSearchTestCase {
assertThat(e.getMessage(), containsString("my_plugin requires Java"));
}
public void testFindPluginDirs() throws IOException {
public void testFindPluginDirs() throws Exception {
final Path plugins = createTempDir();
final MockLogAppender mockLogAppender = new MockLogAppender();
mockLogAppender.start();
mockLogAppender.addExpectation(
new MockLogAppender.SeenEventExpectation(
"[.test] warning",
"org.opensearch.plugins.PluginsService",
Level.WARN,
"Non-plugin file located in the plugins folder with the following name: [.DS_Store]"
)
);
final Logger testLogger = LogManager.getLogger(PluginsService.class);
Loggers.addAppender(testLogger, mockLogAppender);
final Path fake = plugins.resolve("fake");
Path testFile = plugins.resolve(".DS_Store");
Files.createFile(testFile);
PluginTestUtil.writePluginProperties(
fake,
@ -791,6 +775,7 @@ public class PluginsServiceTests extends OpenSearchTestCase {
}
assertThat(PluginsService.findPluginDirs(plugins), containsInAnyOrder(fake));
mockLogAppender.assertAllExpectationsMatched();
}
public void testExistingMandatoryClasspathPlugin() {