Skip to content

Commit bb9a7d4

Browse files
authored
Allow plugins to copy folders into their config dir during installation (#19343)
Signed-off-by: Craig Perkins <[email protected]>
1 parent d9272b5 commit bb9a7d4

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
@@ -39,6 +39,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3939
- Create and attach interclusterTest and yamlRestTest code coverage reports to gradle check task([#19165](https://github.com/opensearch-project/OpenSearch/pull/19165))
4040
- Optimized date histogram aggregations by preventing unnecessary object allocations in date rounding utils ([19088](https://github.com/opensearch-project/OpenSearch/pull/19088))
4141
- Optimize source conversion in gRPC search hits using zero-copy BytesRef ([#19280](https://github.com/opensearch-project/OpenSearch/pull/19280))
42+
- Allow plugins to copy folders into their config dir during installation ([#19343](https://github.com/opensearch-project/OpenSearch/pull/19343))
4243
- Add failureaccess as runtime dependency to transport-grpc module ([#19339](https://github.com/opensearch-project/OpenSearch/pull/19339))
4344

4445
### Fixed

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

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

961961
try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpConfigDir)) {
962962
for (Path srcFile : stream) {
963-
if (Files.isDirectory(srcFile)) {
964-
throw new UserException(PLUGIN_MALFORMED, "Directories not allowed in config dir for plugin " + info.getName());
965-
}
966-
967963
Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile));
968964
if (Files.exists(destFile) == false) {
969-
Files.copy(srcFile, destFile);
970-
setFileAttributes(destFile, CONFIG_FILES_PERMS);
971-
if (destConfigDirAttributes != null) {
972-
setOwnerGroup(destFile, destConfigDirAttributes);
965+
if (Files.isDirectory(srcFile)) {
966+
copyWithPermissions(srcFile, destFile, CONFIG_DIR_PERMS, destConfigDirAttributes);
967+
copyDirectoryRecursively(srcFile, destFile, destConfigDirAttributes);
968+
} else {
969+
copyWithPermissions(srcFile, destFile, CONFIG_FILES_PERMS, destConfigDirAttributes);
973970
}
974971
}
975972
}
@@ -995,6 +992,42 @@ private static void setFileAttributes(final Path path, final Set<PosixFilePermis
995992
}
996993
}
997994

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

10001033
@Override

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

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

434434
try (DirectoryStream<Path> stream = Files.newDirectoryStream(configDir)) {
435435
for (Path file : stream) {
436-
assertFalse("not a dir", Files.isDirectory(file));
437-
438436
if (isPosix) {
439437
PosixFileAttributes attributes = Files.readAttributes(file, PosixFileAttributes.class);
440438
if (user != null) {
@@ -803,9 +801,14 @@ public void testConfigContainsDir() throws Exception {
803801
Files.createDirectories(dirInConfigDir);
804802
Files.createFile(dirInConfigDir.resolve("myconfig.yml"));
805803
String pluginZip = createPluginUrl("fake", pluginDir);
806-
UserException e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1()));
807-
assertTrue(e.getMessage(), e.getMessage().contains("Directories not allowed in config dir for plugin"));
808-
assertInstallCleaned(env.v2());
804+
installPlugin(pluginZip, env.v1());
805+
assertPlugin("fake", pluginDir, env.v2());
806+
807+
// Verify the directory and file were installed
808+
Path installedConfigDir = env.v2().configDir().resolve("fake").resolve("foo");
809+
assertTrue(Files.exists(installedConfigDir));
810+
assertTrue(Files.isDirectory(installedConfigDir));
811+
assertTrue(Files.exists(installedConfigDir.resolve("myconfig.yml")));
809812
}
810813

811814
public void testMissingDescriptor() throws Exception {

0 commit comments

Comments
 (0)