Expose whitelisted config values to client-side plugin #50641
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,25 +17,33 @@ | |
| * under the License. | ||
| */ | ||
|
|
||
| import { Observable, of } from 'rxjs'; | ||
| import { filter, first, map, mergeMap, tap, toArray } from 'rxjs/operators'; | ||
| import { CoreService } from '../../types'; | ||
| import { CoreContext } from '../core_context'; | ||
|
|
||
| import { Logger } from '../logging'; | ||
| import { discover, PluginDiscoveryError, PluginDiscoveryErrorType } from './discovery'; | ||
| import { PluginWrapper } from './plugin'; | ||
| import { | ||
| DiscoveredPlugin, | ||
| DiscoveredPluginInternal, | ||
| PluginConfigDescriptor, | ||
| PluginName, | ||
| } from './types'; | ||
| import { PluginsConfig, PluginsConfigType } from './plugins_config'; | ||
| import { PluginsSystem } from './plugins_system'; | ||
| import { InternalCoreSetup } from '../internal_types'; | ||
| import { IConfigService } from '../config'; | ||
| import { pick } from '../../utils'; | ||
|
|
||
| /** @public */ | ||
| export interface PluginsServiceSetup { | ||
| contracts: Map<PluginName, unknown>; | ||
| uiPlugins: { | ||
| public: Map<PluginName, DiscoveredPlugin>; | ||
| internal: Map<PluginName, DiscoveredPluginInternal>; | ||
| config: Map<PluginName, Observable<unknown> | null>; | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -54,11 +62,14 @@ export interface PluginsServiceStartDeps {} // eslint-disable-line @typescript-e | |
| export class PluginsService implements CoreService<PluginsServiceSetup, PluginsServiceStart> { | ||
| private readonly log: Logger; | ||
| private readonly pluginsSystem: PluginsSystem; | ||
| private readonly configService: IConfigService; | ||
| private readonly config$: Observable<PluginsConfig>; | ||
| private readonly pluginConfigDescriptors = new Map<PluginName, PluginConfigDescriptor>(); | ||
|
|
||
| constructor(private readonly coreContext: CoreContext) { | ||
| this.log = coreContext.logger.get('plugins-service'); | ||
| this.pluginsSystem = new PluginsSystem(coreContext); | ||
| this.configService = coreContext.configService; | ||
| this.config$ = coreContext.configService | ||
| .atPath<PluginsConfigType>('plugins') | ||
| .pipe(map(rawConfig => new PluginsConfig(rawConfig, coreContext.env))); | ||
|
|
@@ -82,17 +93,20 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS | |
|
|
||
| const config = await this.config$.pipe(first()).toPromise(); | ||
|
|
||
| let contracts = new Map<PluginName, unknown>(); | ||
| if (!config.initialize || this.coreContext.env.isDevClusterMaster) { | ||
| this.log.info('Plugin initialization disabled.'); | ||
| } else { | ||
| contracts = await this.pluginsSystem.setupPlugins(deps); | ||
| } | ||
|
|
||
| const uiPlugins = this.pluginsSystem.uiPlugins(); | ||
| return { | ||
| contracts, | ||
| uiPlugins: { | ||
| ...uiPlugins, | ||
| config: this.generateUiPluginsConfigs(uiPlugins.public), | ||
| }, | ||
|
There was a problem hiding this comment. uiPlugins are directly serialized in injected metadatas, so I added the configuration in a distinct property instead of trying to merge in the uiPlugins structs |
||
| }; | ||
| } | ||
|
|
||
|
|
@@ -107,6 +121,27 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS | |
| await this.pluginsSystem.stopPlugins(); | ||
| } | ||
|
|
||
| private generateUiPluginsConfigs( | ||
| uiPlugins: Map<string, DiscoveredPlugin> | ||
| ): Map<PluginName, Observable<unknown>> { | ||
|
Comment on lines
+122
to
+124
There was a problem hiding this comment. The observable is currently not of use, as we |
||
| return new Map( | ||
| [...uiPlugins].map(([pluginId, plugin]) => { | ||
| const configDescriptor = this.pluginConfigDescriptors.get(pluginId); | ||
| if (configDescriptor && configDescriptor.exposeToBrowser) { | ||
| return [ | ||
| pluginId, | ||
| this.configService | ||
| .atPath(plugin.configPath) | ||
| .pipe( | ||
| map((config: any) => pick(config || {}, configDescriptor.exposeToBrowser || [])) | ||
|
There was a problem hiding this comment. nit: |
||
| ), | ||
| ]; | ||
| } | ||
| return [pluginId, of({})]; | ||
|
There was a problem hiding this comment. is this necessary with your check here? daf09e3#diff-ce5eebb41b6315c38446e8a68a4ca0aeR242 There was a problem hiding this comment. I was filtering null values after the map call using |
||
| }) | ||
| ); | ||
| } | ||
|
|
||
| private async handleDiscoveryErrors(error$: Observable<PluginDiscoveryError>) { | ||
| // At this stage we report only errors that can occur when new platform plugin | ||
| // manifest is present, otherwise we can't be sure that the plugin is for the new | ||
|
|
@@ -140,6 +175,7 @@ export class PluginsService implements CoreService<PluginsServiceSetup, PluginsS | |
| mergeMap(async plugin => { | ||
| const configDescriptor = plugin.getConfigDescriptor(); | ||
| if (configDescriptor) { | ||
| this.pluginConfigDescriptors.set(plugin.name, configDescriptor); | ||
| await this.coreContext.configService.setSchema( | ||
| plugin.configPath, | ||
| configDescriptor.schema | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional: The name collision probability is minimal. But to simplify reading, shouldn't we handle it separately?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, wasn't sure which one was the best approach. Both works with me, I can change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a87b7b0 separates configs from plugins