Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Security Solution][Ingest Manager][Endpoint] Optional ingest manager #71198

Merged
merged 9 commits into from
Jul 10, 2020
Merged

[Security Solution][Ingest Manager][Endpoint] Optional ingest manager #71198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cd269b3
Revert "Reverting ingestManager back to being required for now"
madirey Jul 9, 2020
dcf65a4
Disable package config component when ingestManager is disabled
madirey Jul 9, 2020
c15576c
Merge branch 'master' into optional-ingest-manager
elasticmachine Jul 9, 2020
b46b9e0
remove management plugin
kevinlog Jul 9, 2020
824d637
Merge branch 'master' into optional-ingest-manager
elasticmachine Jul 10, 2020
fdc3329
Merge branch 'optional-ingest-manager' of github.com:madirey/kibana i…
kevinlog Jul 10, 2020
19cc454
Merge branch 'master' of github.com:kevinlog/kibana into optional-ing…
kevinlog Jul 10, 2020
eb60b1b
correct plugin
kevinlog Jul 10, 2020
a4b6218
make ingest type optional
kevinlog Jul 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions x-pack/plugins/security_solution/kibana.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"embeddable",
"features",
"home",
"ingestManager",
"taskManager",
"inspector",
"licensing",
Expand All @@ -21,6 +20,7 @@
],
"optionalPlugins": [
"encryptedSavedObjects",
"ingestManager",
"ml",
"newsfeed",
"security",
Expand All @@ -33,6 +33,7 @@
"requiredBundles": [
"kibanaUtils",
"esUiShared",
"kibanaReact"
"kibanaReact",
"ingestManager"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a new feature from kibana platform, see #70911

This allows us to still load the shared code that we import from ingestManager without the plugin actually being loaded. So, since "ingestManager" is optional we can either load it or not and the Administration section will handle accordingly.

There are existing tests that already checked the behavior of the Admin section with or without ingestManager either within Space or if the plugin is loaded at all.

]
}
10 changes: 6 additions & 4 deletions x-pack/plugins/security_solution/public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,12 @@ export class Plugin implements IPlugin<PluginSetup, PluginStart, SetupPlugins, S

public start(core: CoreStart, plugins: StartPlugins) {
KibanaServices.init({ ...core, ...plugins, kibanaVersion: this.kibanaVersion });
plugins.ingestManager.registerPackageConfigComponent(
'endpoint',
ConfigureEndpointPackageConfig
);
if (plugins.ingestManager) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem with tests failing without this conditional should have been caught by typescript. Is it because we should have have also set the ingestManager StartPlugins type as optional? https://github.com/elastic/kibana/blob/master/x-pack/plugins/security_solution/public/types.ts#L35

@paul-tavares @XavierM @kevinlog thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peluja1012 yeah, that sounds like it needs to be set to optional in the Type

plugins.ingestManager.registerPackageConfigComponent(
'endpoint',
ConfigureEndpointPackageConfig
);
}

return {};
}
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/public/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export interface StartPlugins {
data: DataPublicPluginStart;
embeddable: EmbeddableStart;
inspector: InspectorStart;
ingestManager: IngestManagerStart;
ingestManager?: IngestManagerStart;
newsfeed?: NewsfeedStart;
triggers_actions_ui: TriggersActionsStart;
uiActions: UiActionsStart;
Expand Down
16 changes: 8 additions & 8 deletions x-pack/plugins/security_solution/server/endpoint/endpoint_app_context_services.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import { httpServerMock } from '../../../../../src/core/server/mocks';
import { EndpointAppContextService } from './endpoint_app_context_services';

describe('test endpoint app context services', () => {
// it('should return undefined on getAgentService if dependencies are not enabled', async () => {
// const endpointAppContextService = new EndpointAppContextService();
// expect(endpointAppContextService.getAgentService()).toEqual(undefined);
// });
// it('should return undefined on getManifestManager if dependencies are not enabled', async () => {
// const endpointAppContextService = new EndpointAppContextService();
// expect(endpointAppContextService.getManifestManager()).toEqual(undefined);
// });
it('should return undefined on getAgentService if dependencies are not enabled', async () => {
const endpointAppContextService = new EndpointAppContextService();
expect(endpointAppContextService.getAgentService()).toEqual(undefined);
});
it('should return undefined on getManifestManager if dependencies are not enabled', async () => {
const endpointAppContextService = new EndpointAppContextService();
expect(endpointAppContextService.getManifestManager()).toEqual(undefined);
});
it('should throw error on getScopedSavedObjectsClient if start is not called', async () => {
const endpointAppContextService = new EndpointAppContextService();
expect(() =>
Expand Down