Skip to content

Commit

Permalink
Add enforcement logic to prevent cross-solution plugin dependencies.
Browse files Browse the repository at this point in the history
  • Loading branch information
lukeelmers committed May 15, 2024
1 parent abd031f commit 7077867
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export function discover({
manifest,
opaqueId: initializerContext.opaqueId,
initializerContext,
categoryInfo: pkg.getPluginCategories(),
});
})
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@ jest.doMock(join('plugin-with-wrong-initializer-path', 'server'), () => ({ plugi
virtual: true,
});

const OSS_PLUGIN_PATH_POSIX = '/kibana/src/plugins/ossPlugin';
const OSS_PLUGIN_PATH_WINDOWS = 'C:\\kibana\\src\\plugins\\ossPlugin';
const XPACK_PLUGIN_PATH_POSIX = '/kibana/x-pack/plugins/xPackPlugin';
const XPACK_PLUGIN_PATH_WINDOWS = 'C:\\kibana\\x-pack\\plugins\\xPackPlugin';
const OSS_PLUGIN_PATH_POSIX = '/kibana/src/foo/ossPlugin';
const OSS_PLUGIN_PATH_WINDOWS = 'C:\\kibana\\src\\foo\\ossPlugin';
const XPACK_PLUGIN_PATH_POSIX = '/kibana/x-pack/foo/xPackPlugin';
const XPACK_PLUGIN_PATH_WINDOWS = 'C:\\kibana\\x-pack\\foo\\xPackPlugin';

function createPluginManifest(manifestProps: Partial<PluginManifest> = {}): PluginManifest {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import typeDetect from 'type-detect';
import { firstValueFrom, Subject } from 'rxjs';
import { isPromise } from '@kbn/std';
import { isConfigSchema } from '@kbn/config-schema';
import type { PluginCategoryInfo } from '@kbn/repo-packages';
import type { Logger } from '@kbn/logging';
import { type PluginOpaqueId, PluginType } from '@kbn/core-base-common';
import type {
Expand All @@ -24,8 +25,8 @@ import type {
} from '@kbn/core-plugins-server';
import type { CorePreboot, CoreSetup, CoreStart } from '@kbn/core-lifecycle-server';

const OSS_PATH_REGEX = /[\/|\\]src[\/|\\]plugins[\/|\\]/; // Matches src/plugins directory on POSIX and Windows
const XPACK_PATH_REGEX = /[\/|\\]x-pack[\/|\\]plugins[\/|\\]/; // Matches x-pack/plugins directory on POSIX and Windows
const OSS_PATH_REGEX = /[\/|\\]src[\/|\\]/; // Matches src/ directory on POSIX and Windows
const XPACK_PATH_REGEX = /[\/|\\]x-pack[\/|\\]/; // Matches x-pack/ directory on POSIX and Windows

/**
* Lightweight wrapper around discovered plugin that is responsible for instantiating
Expand All @@ -51,6 +52,7 @@ export class PluginWrapper<
public readonly requiredBundles: PluginManifest['requiredBundles'];
public readonly includesServerPlugin: PluginManifest['server'];
public readonly includesUiPlugin: PluginManifest['ui'];
public readonly categoryInfo?: PluginCategoryInfo;

private readonly log: Logger;
private readonly initializerContext: PluginInitializerContext;
Expand All @@ -69,6 +71,7 @@ export class PluginWrapper<
readonly manifest: PluginManifest;
readonly opaqueId: PluginOpaqueId;
readonly initializerContext: PluginInitializerContext;
readonly categoryInfo?: PluginCategoryInfo;
}
) {
this.path = params.path;
Expand All @@ -85,6 +88,7 @@ export class PluginWrapper<
this.runtimePluginDependencies = params.manifest.runtimePluginDependencies;
this.includesServerPlugin = params.manifest.server;
this.includesUiPlugin = params.manifest.ui;
this.categoryInfo = params.categoryInfo;
}

public async init() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,14 @@ export class PluginsService
plugin: PluginWrapper,
pluginEnableStatuses: Map<PluginName, { plugin: PluginWrapper; isEnabled: boolean }>
) {
const { name, manifest, requiredBundles, requiredPlugins } = plugin;
const {
name,
manifest,
optionalPlugins,
requiredBundles,
requiredPlugins,
runtimePluginDependencies,
} = plugin;

// validate that `requiredBundles` ids point to a discovered plugin which `includesUiPlugin`
for (const requiredBundleId of requiredBundles) {
Expand Down Expand Up @@ -403,6 +410,44 @@ export class PluginsService
}
}
}

// validate that non-platform plugins do not have dependencies on each other
for (const id of [
...requiredPlugins,
...requiredBundles,
...optionalPlugins,
...runtimePluginDependencies,
]) {
const dependency = pluginEnableStatuses.get(id);
const pluginTier = this.getPluginTier(plugin);
const dependencyPluginTier = this.getPluginTier(dependency?.plugin);
if (
dependency &&
!this.isPublicPlatformPlugin(dependency.plugin) &&
pluginTier !== dependencyPluginTier
) {
throw new Error(
`Plugin or bundle with id "${id}" is required by plugin "${name}", which is prohibited. Plugin "${name}" may only have dependencies on public platform plugins, or other ${pluginTier} plugins.`
);
}
}
}

private getPluginTier(plugin?: PluginWrapper) {
if (plugin?.categoryInfo?.platform) return 'platform';
if (plugin?.categoryInfo?.observability) return 'observability';
if (plugin?.categoryInfo?.search) return 'search';
if (plugin?.categoryInfo?.security) return 'security';
return 'unknown';
}

private isPublicPlatformPlugin(plugin: PluginWrapper) {
const isPlatformPlugin = this.getPluginTier(plugin) === 'platform';
// TODO: find a nicer way to do this
const isInternalPlatformPlugin =
plugin.path.includes('src/platform/internal') ||
plugin.path.includes('x-pack/platform/internal');
return isPlatformPlugin && !isInternalPlatformPlugin;
}

private shouldEnablePlugin(
Expand Down
9 changes: 9 additions & 0 deletions packages/kbn-repo-packages/modern/package.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,19 @@ class Package {
const oss = !dir.startsWith('x-pack/');
const example = dir.startsWith('examples/') || dir.startsWith('x-pack/examples/');
const testPlugin = dir.startsWith('test/') || dir.startsWith('x-pack/test/');
const platform = dir.startsWith('src/platform/') || dir.startsWith('x-pack/platform/');
const observability =
dir.startsWith('src/observability/') || dir.startsWith('x-pack/observability/');
const search = dir.startsWith('src/search/') || dir.startsWith('x-pack/search/');
const security = dir.startsWith('src/security/') || dir.startsWith('x-pack/security/');
return {
oss,
example,
testPlugin,
platform,
observability,
search,
security,
};
}

Expand Down
8 changes: 8 additions & 0 deletions packages/kbn-repo-packages/modern/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,4 +194,12 @@ export interface PluginCategoryInfo {
example: boolean;
/** is this a test plugin? */
testPlugin: boolean;
/** is this a platform plugin? */
platform: boolean;
/** is this a observability plugin? */
observability: boolean;
/** is this a search plugin? */
search: boolean;
/** is this a security plugin? */
security: boolean;
}

0 comments on commit 7077867

Please sign in to comment.