[6.x] Ignore config from disabled plugins (#16474) (#16558)

* [server/plugins] collect disabled plugin specs

* [pluginsDiscovery] fix pluralization of disabledPlugin$

* [pluginDiscovery] fix invalid assertion

* [server/config] ignore config from disabled plugins

src/plugin_discovery/README.md

@@ -1,6 +1,6 @@
 # Plugin Discovery
 
-The plugin discovery module defines the core plugin loading logic used by the Kibana server. It exports functions for 
+The plugin discovery module defines the core plugin loading logic used by the Kibana server. It exports functions for
 
 
 ## `findPluginSpecs(settings, [config])`
@@ -23,7 +23,7 @@ If you *never* subscribe to any of the Observables then plugin discovery won't a
  - `deprecation$: Observable<string>`: emits deprecation warnings that are produces when reading each [`PluginPack`][PluginPack]'s configuration
  - `extendedConfig$: Observable<Config>`: emits the [`Config`][Config] service that was passed to `findPluginSpecs()` (or created internally if none was passed) after it has been extended with the configuration from each plugin
  - `spec$: Observable<PluginSpec>`: emits every *enabled* [`PluginSpec`][PluginSpec] defined by the discovered [`PluginPack`][PluginPack]s
- - `disabledSpecs$: Observable<PluginSpec>`: emits every *disabled* [`PluginSpec`][PluginSpec] defined by the discovered [`PluginPack`][PluginPack]s
+ - `disabledSpec$: Observable<PluginSpec>`: emits every *disabled* [`PluginSpec`][PluginSpec] defined by the discovered [`PluginPack`][PluginPack]s
  - `invalidVersionSpec$: Observable<PluginSpec>`: emits every [`PluginSpec`][PluginSpec] who's required kibana version does not match the version exposed by `config.get('pkg.version')`
 
 ### example
@@ -48,7 +48,7 @@ const { pack$, invalidDirectoryError$ } = findPluginSpecs(settings);
 const packs = await Observable.merge(
   pack$.toArray(),
 
-  // if we ever get an InvalidDirectoryError, throw it 
+  // if we ever get an InvalidDirectoryError, throw it
   // into the stream so that all streams are unsubscribed,
   // the discovery process is aborted, and the promise rejects
   invalidDirectoryError$.map(error => {
@@ -73,25 +73,25 @@ const {
 Observable.merge(
   pack$
     .do(pluginPack => console.log('Found plugin pack', pluginPack)),
-    
+
   invalidDirectoryError$
     .do(error => console.log('Invalid directory error', error)),
-    
+
   invalidPackError$
     .do(error => console.log('Invalid plugin pack error', error)),
-    
+
   deprecation$
     .do(msg => console.log('DEPRECATION:', msg)),
-    
+
   extendedConfig$
     .do(config => console.log('config service extended by plugins', config)),
-    
+
   spec$
     .do(pluginSpec => console.log('enabled plugin spec found', spec)),
-    
+
   disabledSpec$
     .do(pluginSpec => console.log('disabled plugin spec found', spec)),
-    
+
   invalidVersionSpec$
     .do(pluginSpec => console.log('plugin spec with invalid version found', spec)),
 )
@@ -115,14 +115,14 @@ reducer(
   acc: any,
   // the exported value, found at `uiExports[type]` or `uiExports[type][i]`
   // in the PluginSpec config.
-  spec: any, 
+  spec: any,
   // the key in `uiExports` where this export was found
   type: string,
   // the PluginSpec which exported this spec
   pluginSpec: PluginSpec
 )
 ```
-  
+
 ## `new PluginPack(options)` class
 
 Only exported so that `PluginPack` instances can be created in tests and used in place of on-disk plugin fixtures. Use `findPluginSpecs()`, or the cached result of a call to `findPluginSpecs()` (like `kbnServer.pluginSpecs`) any time you might need access to `PluginPack` objects in distributed code.
@@ -137,4 +137,4 @@ Only exported so that `PluginPack` instances can be created in tests and used in
 [PluginSpec]: ./plugin_spec/plugin_spec.js "PluginSpec class definition"
 [Errors]: ./errors.js "PluginDiscover specific error types"
 [KbnServer]: ../server/kbn_server.js "KbnServer class definition"
-[Config]: ../server/config/config.js "KbnServer/Config class definition"
+[Config]: ../server/config/config.js "KbnServer/Config class definition"

src/plugin_discovery/find_plugin_specs.js

@@ -124,7 +124,7 @@ export function findPluginSpecs(settings, config = defaultConfig(settings)) {
       .mergeMap(result => result.enabledSpecs),
 
     // all disabled PluginSpec objects
-    disabledSpecs$: extendConfig$
+    disabledSpec$: extendConfig$
       .mergeMap(result => result.disabledSpecs),
 
     // all PluginSpec objects that were disabled because their version was incompatible

src/server/config/__tests__/complete.js

@@ -14,7 +14,8 @@ describe('server/config completeMixin()', function () {
   const setup = (options = {}) => {
     const {
       settings = {},
-      configValues = {}
+      configValues = {},
+      disabledPluginSpecs = [],
     } = options;
 
     const server = {
@@ -28,7 +29,8 @@ describe('server/config completeMixin()', function () {
     const kbnServer = {
       settings,
       server,
-      config
+      config,
+      disabledPluginSpecs
     };
 
     const callCompleteMixin = () => {
@@ -129,7 +131,9 @@ describe('server/config completeMixin()', function () {
         }
       });
 
-      expect(callCompleteMixin).to.throwError('"unused" not applied');
+      expect(callCompleteMixin).to.throwError(error => {
+        expect(error.message).to.contain('"unused" setting was not applied');
+      });
     });
 
     describe('error thrown', () => {
@@ -197,4 +201,27 @@ describe('server/config completeMixin()', function () {
       sinon.assert.calledOnce(transformDeprecations);
     });
   });
+
+  describe('disabled plugins', () => {
+    it('ignores config for plugins that are disabled', () => {
+      const { callCompleteMixin } = setup({
+        settings: {
+          foo: {
+            bar: {
+              unused: true
+            }
+          }
+        },
+        disabledPluginSpecs: [
+          {
+            id: 'foo',
+            getConfigPrefix: () => 'foo.bar'
+          }
+        ],
+        configValues: {}
+      });
+
+      expect(callCompleteMixin).to.not.throwError();
+    });
+  });
 });

src/server/config/complete.js

@@ -1,13 +1,20 @@
 import { difference } from 'lodash';
 import { transformDeprecations } from './transform_deprecations';
-import { formatListAsProse, getFlattenedObject } from '../../utils';
+import { unset, formatListAsProse, getFlattenedObject } from '../../utils';
 
 const getFlattenedKeys = object => (
   Object.keys(getFlattenedObject(object))
 );
 
-const getUnusedConfigKeys = (settings, configValues) => {
-  const inputKeys = getFlattenedKeys(transformDeprecations(settings));
+const getUnusedConfigKeys = (disabledPluginSpecs, rawSettings, configValues) => {
+  const settings = transformDeprecations(rawSettings);
+
+  // remove config values from disabled plugins
+  for (const spec of disabledPluginSpecs) {
+    unset(settings, spec.getConfigPrefix());
+  }
+
+  const inputKeys = getFlattenedKeys(settings);
   const appliedKeys = getFlattenedKeys(configValues);
 
   if (inputKeys.includes('env')) {
@@ -26,7 +33,7 @@ export default function (kbnServer, server, config) {
     return kbnServer.config;
   });
 
-  const unusedKeys = getUnusedConfigKeys(kbnServer.settings, config.get())
+  const unusedKeys = getUnusedConfigKeys(kbnServer.disabledPluginSpecs, kbnServer.settings, config.get())
     .map(key => `"${key}"`);
 
   if (!unusedKeys.length) {
@@ -40,7 +47,7 @@ export default function (kbnServer, server, config) {
   const error = new Error(
     `${formatListAsProse(unusedKeys)} ${desc} not applied. ` +
     'Check for spelling errors and ensure that expected ' +
-    'plugins are installed and enabled.'
+    'plugins are installed.'
   );
 
   error.code = 'InvalidConfig';

src/server/plugins/scan_mixin.js

@@ -11,6 +11,7 @@ export async function scanMixin(kbnServer, server, config) {
     deprecation$,
     invalidVersionSpec$,
     spec$,
+    disabledSpec$,
   } = findPluginSpecs(kbnServer.settings, config);
 
   const logging$ = Observable.merge(
@@ -53,10 +54,20 @@ export async function scanMixin(kbnServer, server, config) {
     })
   );
 
-  kbnServer.pluginSpecs = await spec$
-    .merge(logging$.ignoreElements())
+  const enabledSpecs$ = spec$
     .toArray()
-    .toPromise();
+    .do(specs => {
+      kbnServer.pluginSpecs = specs;
+    });
+
+  const disabledSpecs$ = disabledSpec$
+    .toArray()
+    .do(specs => {
+      kbnServer.disabledPluginSpecs = specs;
+    });
+
+  // await completion of enabledSpecs$, disabledSpecs$, and logging$
+  await Observable.merge(logging$, enabledSpecs$, disabledSpecs$).toPromise();
 
   kbnServer.plugins = kbnServer.pluginSpecs.map(spec => (
     new Plugin(kbnServer, spec)