refactor processPluginSearchPaths$ and add test for inaccessible mani…

…fest
elastic · Jun 23, 2020 · 5c00548 · 5c00548
1 parent f011c93
commit 5c00548
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 34 deletions.
diff --git a/src/core/server/plugins/discovery/plugins_discovery.test.ts b/src/core/server/plugins/discovery/plugins_discovery.test.ts
@@ -43,6 +43,12 @@ const Plugins = {
     'kibana.json': JSON.stringify({ id: 'plugin', version: '1' }),
   }),
   missingManifest: () => ({}),
+  inaccessibleManifest: () => ({
+    'kibana.json': mockFs.file({
+      mode: 0, // 0000,
+      content: JSON.stringify({ id: 'plugin', version: '1' }),
+    }),
+  }),
   valid: (id: string) => ({
     'kibana.json': JSON.stringify({
       id,
@@ -234,6 +240,43 @@ describe('plugins discovery system', () => {
     );
   });
 
+  it('return an error when the manifest file is not accessible', async () => {
+    const { plugin$, error$ } = discover(new PluginsConfig(pluginConfig, env), coreContext);
+
+    mockFs(
+      {
+        [KIBANA_ROOT]: {
+          src: {
+            plugins: {
+              plugin_a: {
+                ...Plugins.inaccessibleManifest(),
+                nested_plugin: Plugins.valid('nestedPlugin'),
+              },
+            },
+          },
+        },
+      },
+      { createCwd: false }
+    );
+
+    const plugins = await plugin$.pipe(toArray()).toPromise();
+    expect(plugins).toHaveLength(0);
+
+    const errors = await error$
+      .pipe(
+        map((error) => error.toString()),
+        toArray()
+      )
+      .toPromise();
+
+    const errorPath = manifestPath('plugin_a');
+    expect(errors).toEqual(
+      expect.arrayContaining([
+        `Error: EACCES, permission denied '${errorPath}' (missing-manifest, ${errorPath})`,
+      ])
+    );
+  });
+
   it('discovers plugins in nested directories', async () => {
     const { plugin$, error$ } = discover(new PluginsConfig(pluginConfig, env), coreContext);
 

diff --git a/src/core/server/plugins/discovery/plugins_discovery.ts b/src/core/server/plugins/discovery/plugins_discovery.ts
@@ -98,41 +98,14 @@ function processPluginSearchPaths$(
   ): Observable<string | PluginDiscoveryError> {
     return from([ent]).pipe(
       mergeMap((entry) => {
-        return fsStat$(resolve(entry.dir, 'kibana.json')).pipe(
-          mergeMap((stats) => {
-            // `kibana.json` exists in given directory, we got a plugin
-            if (stats.isFile()) {
-              return [entry.dir];
-            }
+        return findManifestInFolder(entry.dir, () => {
+          if (entry.depth > maxScanDepth) {
             return [];
-          }),
-          catchError((manifestStatError) => {
-            // did not find manifest. recursively process sub directories until we reach max depth.
-            if (manifestStatError.code === 'ENOENT') {
-              if (entry.depth <= maxScanDepth) {
-                return fsReadDir$(entry.dir).pipe(
-                  mergeMap((subDirs: string[]) =>
-                    subDirs.map((subDir) => resolve(entry.dir, subDir))
-                  ),
-                  mergeMap((subDir) =>
-                    fsStat$(subDir).pipe(
-                      mergeMap((pathStat) =>
-                        pathStat.isDirectory()
-                          ? recursiveScanFolder({ dir: subDir, depth: entry.depth + 1 })
-                          : []
-                      ),
-                      catchError((subDirStatError) => [
-                        PluginDiscoveryError.invalidPluginPath(subDir, subDirStatError),
-                      ])
-                    )
-                  )
-                );
-              }
-              return [];
-            }
-            return [PluginDiscoveryError.invalidPluginPath(entry.dir, manifestStatError)];
-          })
-        );
+          }
+          return mapSubdirectories(entry.dir, (subDir) =>
+            recursiveScanFolder({ dir: subDir, depth: entry.depth + 1 })
+          );
+        });
       })
     );
   }
@@ -148,6 +121,57 @@ function processPluginSearchPaths$(
   );
 }
 
+/**
+ * Attempts to read manifest file in specified directory or calls `notFound` and returns results if not found. For any
+ * manifest files that cannot be read, a PluginDiscoveryError is added.
+ * @param dir
+ * @param notFound
+ */
+function findManifestInFolder(
+  dir: string,
+  notFound: () => never[] | Observable<string | PluginDiscoveryError>
+): string[] | Observable<string | PluginDiscoveryError> {
+  return fsStat$(resolve(dir, 'kibana.json')).pipe(
+    mergeMap((stats) => {
+      // `kibana.json` exists in given directory, we got a plugin
+      if (stats.isFile()) {
+        return [dir];
+      }
+      return [];
+    }),
+    catchError((manifestStatError) => {
+      // did not find manifest. recursively process sub directories until we reach max depth.
+      if (manifestStatError.code !== 'ENOENT') {
+        return [PluginDiscoveryError.invalidPluginPath(dir, manifestStatError)];
+      }
+      return notFound();
+    })
+  );
+}
+
+/**
+ * Finds all subdirectories in `dir` and executed `mapFunc` for each one. For any directories that cannot be read,
+ * a PluginDiscoveryError is added.
+ * @param dir
+ * @param mapFunc
+ */
+function mapSubdirectories(
+  dir: string,
+  mapFunc: (subDir: string) => Observable<string | PluginDiscoveryError>
+): Observable<string | PluginDiscoveryError> {
+  return fsReadDir$(dir).pipe(
+    mergeMap((subDirs: string[]) => subDirs.map((subDir) => resolve(dir, subDir))),
+    mergeMap((subDir) =>
+      fsStat$(subDir).pipe(
+        mergeMap((pathStat) => (pathStat.isDirectory() ? mapFunc(subDir) : [])),
+        catchError((subDirStatError) => [
+          PluginDiscoveryError.invalidPluginPath(subDir, subDirStatError),
+        ])
+      )
+    )
+  );
+}
+
 /**
  * Tries to load and parse the plugin manifest file located at the provided plugin
  * directory path and produces an error result if it fails to do so or plugin manifest