Skip to content

Commit cbbac30

Browse files
authored
[Backport 2.19] Allow plugins to copy folders into their config dir during installation (#19343) (#19418)
* Allow plugins to copy folders into their config dir during installation (#19343) Signed-off-by: Craig Perkins <[email protected]> (cherry picked from commit bb9a7d4) * Fix compilation issue Signed-off-by: Craig Perkins <[email protected]> --------- Signed-off-by: Craig Perkins <[email protected]>
1 parent 1cec4d1 commit cbbac30

File tree

3 files changed

+50
-13
lines changed

3 files changed

+50
-13
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3535

3636
- Change single shard assignment log message from warn to debug ([#18186](https://github.com/opensearch-project/OpenSearch/pull/18186))
3737
- Replace centos:8 with almalinux:8 since centos docker images are deprecated ([#19154](https://github.com/opensearch-project/OpenSearch/pull/19154))
38+
- Allow plugins to copy folders into their config dir during installation ([#19343](https://github.com/opensearch-project/OpenSearch/pull/19343))
3839

3940
[Unreleased 2.19.x]: https://github.com/opensearch-project/OpenSearch/compare/fd9a9d90df25bea1af2c6a85039692e815b894f5...2.19

distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -966,16 +966,13 @@ private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDi
966966

967967
try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpConfigDir)) {
968968
for (Path srcFile : stream) {
969-
if (Files.isDirectory(srcFile)) {
970-
throw new UserException(PLUGIN_MALFORMED, "Directories not allowed in config dir for plugin " + info.getName());
971-
}
972-
973969
Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile));
974970
if (Files.exists(destFile) == false) {
975-
Files.copy(srcFile, destFile);
976-
setFileAttributes(destFile, CONFIG_FILES_PERMS);
977-
if (destConfigDirAttributes != null) {
978-
setOwnerGroup(destFile, destConfigDirAttributes);
971+
if (Files.isDirectory(srcFile)) {
972+
copyWithPermissions(srcFile, destFile, CONFIG_DIR_PERMS, destConfigDirAttributes);
973+
copyDirectoryRecursively(srcFile, destFile, destConfigDirAttributes);
974+
} else {
975+
copyWithPermissions(srcFile, destFile, CONFIG_FILES_PERMS, destConfigDirAttributes);
979976
}
980977
}
981978
}
@@ -1001,6 +998,42 @@ private static void setFileAttributes(final Path path, final Set<PosixFilePermis
1001998
}
1002999
}
10031000

1001+
/**
1002+
* Copies a file and sets permissions and ownership
1003+
*/
1004+
private static void copyWithPermissions(
1005+
Path srcFile,
1006+
Path destFile,
1007+
Set<PosixFilePermission> permissions,
1008+
PosixFileAttributes attributes
1009+
) throws IOException {
1010+
Files.copy(srcFile, destFile);
1011+
setFileAttributes(destFile, permissions);
1012+
if (attributes != null) {
1013+
setOwnerGroup(destFile, attributes);
1014+
}
1015+
}
1016+
1017+
/**
1018+
* Recursively copies directory contents from source to destination.
1019+
*/
1020+
private static void copyDirectoryRecursively(Path srcDir, Path destDir, PosixFileAttributes destConfigDirAttributes)
1021+
throws IOException {
1022+
try (DirectoryStream<Path> stream = Files.newDirectoryStream(srcDir)) {
1023+
for (Path srcFile : stream) {
1024+
Path destFile = destDir.resolve(srcDir.relativize(srcFile));
1025+
if (Files.exists(destFile) == false) {
1026+
if (Files.isDirectory(srcFile)) {
1027+
copyWithPermissions(srcFile, destFile, CONFIG_DIR_PERMS, destConfigDirAttributes);
1028+
copyDirectoryRecursively(srcFile, destFile, destConfigDirAttributes);
1029+
} else {
1030+
copyWithPermissions(srcFile, destFile, CONFIG_FILES_PERMS, destConfigDirAttributes);
1031+
}
1032+
}
1033+
}
1034+
}
1035+
}
1036+
10041037
private final List<Path> pathsToDeleteOnShutdown = new ArrayList<>();
10051038

10061039
@Override

distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,6 @@ void assertConfigAndBin(String name, Path original, Environment env) throws IOEx
423423

424424
try (DirectoryStream<Path> stream = Files.newDirectoryStream(configDir)) {
425425
for (Path file : stream) {
426-
assertFalse("not a dir", Files.isDirectory(file));
427-
428426
if (isPosix) {
429427
PosixFileAttributes attributes = Files.readAttributes(file, PosixFileAttributes.class);
430428
if (user != null) {
@@ -793,9 +791,14 @@ public void testConfigContainsDir() throws Exception {
793791
Files.createDirectories(dirInConfigDir);
794792
Files.createFile(dirInConfigDir.resolve("myconfig.yml"));
795793
String pluginZip = createPluginUrl("fake", pluginDir);
796-
UserException e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1()));
797-
assertTrue(e.getMessage(), e.getMessage().contains("Directories not allowed in config dir for plugin"));
798-
assertInstallCleaned(env.v2());
794+
installPlugin(pluginZip, env.v1());
795+
assertPlugin("fake", pluginDir, env.v2());
796+
797+
// Verify the directory and file were installed
798+
Path installedConfigDir = env.v2().configFile().resolve("fake").resolve("foo");
799+
assertTrue(Files.exists(installedConfigDir));
800+
assertTrue(Files.isDirectory(installedConfigDir));
801+
assertTrue(Files.exists(installedConfigDir.resolve("myconfig.yml")));
799802
}
800803

801804
public void testMissingDescriptor() throws Exception {

0 commit comments

Comments
 (0)