-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
move oss features registration to KP #66524
Changes from 7 commits
0816ade
c9ebc3a
a8af99a
670d7b0
ee39ebd
863d488
b7a7035
64af9c9
ec102c2
6634d5a
0b45f68
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
import { coreMock, savedObjectsServiceMock } from 'src/core/server/mocks'; | ||
|
||
import { Plugin } from './plugin'; | ||
const initContext = coreMock.createPluginInitializerContext(); | ||
const coreSetup = coreMock.createSetup(); | ||
const coreStart = coreMock.createStart(); | ||
const typeRegistry = savedObjectsServiceMock.createTypeRegistryMock(); | ||
typeRegistry.getAllTypes.mockReturnValue([ | ||
{ | ||
name: 'foo', | ||
hidden: false, | ||
mappings: { properties: {} }, | ||
namespaceType: 'single' as 'single', | ||
}, | ||
{ | ||
name: 'bar', | ||
hidden: true, | ||
mappings: { properties: {} }, | ||
namespaceType: 'agnostic' as 'agnostic', | ||
}, | ||
]); | ||
coreStart.savedObjects.getTypeRegistry.mockReturnValue(typeRegistry); | ||
|
||
describe('Features Plugin', () => { | ||
it('returns OSS + registered features', async () => { | ||
const plugin = new Plugin(initContext); | ||
const { registerFeature } = await plugin.setup(coreSetup, {}); | ||
registerFeature({ | ||
id: 'baz', | ||
name: 'baz', | ||
app: [], | ||
privileges: null, | ||
}); | ||
|
||
const { getFeatures } = await plugin.start(coreStart); | ||
|
||
expect(getFeatures().map(f => f.id)).toMatchInlineSnapshot(` | ||
Array [ | ||
"baz", | ||
"discover", | ||
"visualize", | ||
"dashboard", | ||
"dev_tools", | ||
"advancedSettings", | ||
"indexPatterns", | ||
"savedObjectsManagement", | ||
] | ||
`); | ||
}); | ||
|
||
it('returns OSS + registered features with timielion when available', async () => { | ||
const plugin = new Plugin(initContext); | ||
const { registerFeature } = await plugin.setup(coreSetup, { | ||
visTypeTimelion: { uiEnabled: true }, | ||
}); | ||
registerFeature({ | ||
id: 'baz', | ||
name: 'baz', | ||
app: [], | ||
privileges: null, | ||
}); | ||
|
||
const { getFeatures } = await plugin.start(coreStart); | ||
|
||
expect(getFeatures().map(f => f.id)).toMatchInlineSnapshot(` | ||
Array [ | ||
"baz", | ||
"discover", | ||
"visualize", | ||
"dashboard", | ||
"dev_tools", | ||
"advancedSettings", | ||
"indexPatterns", | ||
"savedObjectsManagement", | ||
"timelion", | ||
] | ||
`); | ||
}); | ||
|
||
it('registers not hidden saved objects types', async () => { | ||
const plugin = new Plugin(initContext); | ||
await plugin.setup(coreSetup, {}); | ||
const { getFeatures } = await plugin.start(coreStart); | ||
|
||
const soTypes = | ||
getFeatures().find(f => f.id === 'savedObjectsManagement')?.privileges?.all.savedObject.all || | ||
[]; | ||
|
||
expect(soTypes.includes('foo')).toBe(true); | ||
expect(soTypes.includes('bar')).toBe(false); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -6,13 +6,14 @@ | |||||
|
||||||
import { | ||||||
CoreSetup, | ||||||
CoreStart, | ||||||
SavedObjectsServiceStart, | ||||||
Logger, | ||||||
PluginInitializerContext, | ||||||
RecursiveReadonly, | ||||||
} from '../../../../src/core/server'; | ||||||
import { Capabilities as UICapabilities } from '../../../../src/core/server'; | ||||||
import { deepFreeze } from '../../../../src/core/server'; | ||||||
import { XPackInfo } from '../../../legacy/plugins/xpack_main/server/lib/xpack_info'; | ||||||
import { PluginSetupContract as TimelionSetupContract } from '../../../../src/plugins/vis_type_timelion/server'; | ||||||
import { FeatureRegistry } from './feature_registry'; | ||||||
import { Feature, FeatureConfig } from '../common/feature'; | ||||||
|
@@ -27,37 +28,19 @@ export interface PluginSetupContract { | |||||
registerFeature(feature: FeatureConfig): void; | ||||||
getFeatures(): Feature[]; | ||||||
getFeaturesUICapabilities(): UICapabilities; | ||||||
registerLegacyAPI: (legacyAPI: LegacyAPI) => void; | ||||||
} | ||||||
|
||||||
export interface PluginStartContract { | ||||||
getFeatures(): Feature[]; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Describes a set of APIs that are available in the legacy platform only and required by this plugin | ||||||
* to function properly. | ||||||
*/ | ||||||
export interface LegacyAPI { | ||||||
xpackInfo: Pick<XPackInfo, 'license'>; | ||||||
savedObjectTypes: string[]; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Represents Features Plugin instance that will be managed by the Kibana plugin system. | ||||||
*/ | ||||||
export class Plugin { | ||||||
private readonly logger: Logger; | ||||||
|
||||||
private readonly featureRegistry: FeatureRegistry = new FeatureRegistry(); | ||||||
|
||||||
private legacyAPI?: LegacyAPI; | ||||||
private readonly getLegacyAPI = () => { | ||||||
if (!this.legacyAPI) { | ||||||
throw new Error('Legacy API is not registered!'); | ||||||
} | ||||||
return this.legacyAPI; | ||||||
}; | ||||||
private isTimelionEnabled: boolean = false; | ||||||
|
||||||
constructor(private readonly initializerContext: PluginInitializerContext) { | ||||||
this.logger = this.initializerContext.logger.get(); | ||||||
|
@@ -67,33 +50,23 @@ export class Plugin { | |||||
core: CoreSetup, | ||||||
{ visTypeTimelion }: { visTypeTimelion?: TimelionSetupContract } | ||||||
): Promise<RecursiveReadonly<PluginSetupContract>> { | ||||||
this.isTimelionEnabled = visTypeTimelion !== undefined && visTypeTimelion.uiEnabled; | ||||||
|
||||||
defineRoutes({ | ||||||
router: core.http.createRouter(), | ||||||
featureRegistry: this.featureRegistry, | ||||||
getLegacyAPI: this.getLegacyAPI, | ||||||
}); | ||||||
|
||||||
return deepFreeze({ | ||||||
registerFeature: this.featureRegistry.register.bind(this.featureRegistry), | ||||||
getFeatures: this.featureRegistry.getAll.bind(this.featureRegistry), | ||||||
getFeaturesUICapabilities: () => uiCapabilitiesForFeatures(this.featureRegistry.getAll()), | ||||||
|
||||||
registerLegacyAPI: (legacyAPI: LegacyAPI) => { | ||||||
this.legacyAPI = legacyAPI; | ||||||
|
||||||
// Register OSS features. | ||||||
for (const feature of buildOSSFeatures({ | ||||||
savedObjectTypes: this.legacyAPI.savedObjectTypes, | ||||||
includeTimelion: visTypeTimelion !== undefined && visTypeTimelion.uiEnabled, | ||||||
})) { | ||||||
this.featureRegistry.register(feature); | ||||||
} | ||||||
}, | ||||||
}); | ||||||
} | ||||||
|
||||||
public start(): RecursiveReadonly<PluginStartContract> { | ||||||
this.logger.debug('Starting plugin'); | ||||||
public start(core: CoreStart): RecursiveReadonly<PluginStartContract> { | ||||||
this.registerOssFeatures(core.savedObjects); | ||||||
|
||||||
return deepFreeze({ | ||||||
getFeatures: this.featureRegistry.getAll.bind(this.featureRegistry), | ||||||
}); | ||||||
|
@@ -102,4 +75,21 @@ export class Plugin { | |||||
public stop() { | ||||||
this.logger.debug('Stopping plugin'); | ||||||
} | ||||||
|
||||||
private registerOssFeatures(savedObjects: SavedObjectsServiceStart) { | ||||||
const registry = savedObjects.getTypeRegistry(); | ||||||
const savedObjectTypes = registry | ||||||
.getAllTypes() | ||||||
.filter(t => !t.hidden) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pgayvallet I wrongly assumed that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When we use the type registry in the repository we need to know all types, but I think consumers of this API should usually only operate on types that are visible so I think it would make sense to make that the default. If we have a use case where this should be exposed outside of core we could have an option like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I named it kibana/src/legacy/server/saved_objects/saved_objects_mixin.js Lines 81 to 82 in 358d139
It shouldn't be a problem for a plugin to access / expose all types to a consumer. AFAIK, hidden types were already consumed in legacy, but for different usages, via the However I agree that 1/ the jsdoc for I will create a issue for that improvement There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. created #66818 |
||||||
.map(t => t.name); | ||||||
|
||||||
const features = buildOSSFeatures({ | ||||||
savedObjectTypes, | ||||||
includeTimelion: this.isTimelionEnabled, | ||||||
}); | ||||||
|
||||||
for (const feature of features) { | ||||||
this.featureRegistry.register(feature); | ||||||
} | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,6 @@ | |
|
||
import { schema } from '@kbn/config-schema'; | ||
import { IRouter } from '../../../../../src/core/server'; | ||
import { LegacyAPI } from '../plugin'; | ||
import { FeatureRegistry } from '../feature_registry'; | ||
|
||
/** | ||
|
@@ -15,10 +14,9 @@ import { FeatureRegistry } from '../feature_registry'; | |
export interface RouteDefinitionParams { | ||
router: IRouter; | ||
featureRegistry: FeatureRegistry; | ||
getLegacyAPI: () => LegacyAPI; | ||
} | ||
|
||
export function defineRoutes({ router, featureRegistry, getLegacyAPI }: RouteDefinitionParams) { | ||
export function defineRoutes({ router, featureRegistry }: RouteDefinitionParams) { | ||
router.get( | ||
{ | ||
path: '/api/features', | ||
|
@@ -37,7 +35,8 @@ export function defineRoutes({ router, featureRegistry, getLegacyAPI }: RouteDef | |
request.query.ignoreValidLicenses || | ||
!feature.validLicenses || | ||
!feature.validLicenses.length || | ||
getLegacyAPI().xpackInfo.license.isOneOf(feature.validLicenses) | ||
(context.licensing!.license.type && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you want the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want |
||
feature.validLicenses.includes(context.licensing!.license.type)) | ||
) | ||
.sort( | ||
(f1, f2) => | ||
|
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.
typo:
timielion