Skip to content

Commit

Permalink
[Ingest Manager] Do not await in start. Return a Promise (#69505)
Browse files Browse the repository at this point in the history
  1. Do not `await` in the public `start` lifecycle method. Fixes #66125
PR based on #66125 (comment) & #66125 (comment)
  2. Change `success` to be Promise
  • Loading branch information
John Schulz committed Jun 24, 2020
1 parent 83cf82c commit 8f82f59
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export * from './fleet_setup';
export * from './epm';
export * from './enrollment_api_key';
export * from './install_script';
export * from './ingest_setup';
export * from './output';
export * from './settings';
export * from './app';
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* 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.
*/

export interface PostIngestSetupResponse {
isInitialized: boolean;
}
35 changes: 20 additions & 15 deletions x-pack/plugins/ingest_manager/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { i18n } from '@kbn/i18n';
import { DEFAULT_APP_CATEGORIES } from '../../../../src/core/public';
import { DataPublicPluginSetup, DataPublicPluginStart } from '../../../../src/plugins/data/public';
import { LicensingPluginSetup } from '../../licensing/public';
import { PLUGIN_ID } from '../common/constants';
import { PLUGIN_ID, CheckPermissionsResponse, PostIngestSetupResponse } from '../common';

import { IngestManagerConfigType } from '../common/types';
import { setupRouteService, appRoutesService } from '../common';
Expand All @@ -28,10 +28,7 @@ export type IngestManagerSetup = void;
*/
export interface IngestManagerStart {
registerDatasource: typeof registerDatasource;
success: boolean;
error?: {
message: string;
};
success: Promise<true>;
}

export interface IngestManagerSetupDeps {
Expand Down Expand Up @@ -78,21 +75,29 @@ export class IngestManagerPlugin
}

public async start(core: CoreStart): Promise<IngestManagerStart> {
let successPromise: IngestManagerStart['success'];
try {
const permissionsResponse = await core.http.get(appRoutesService.getCheckPermissionsPath());
if (permissionsResponse.success) {
const { isInitialized: success } = await core.http.post(setupRouteService.getSetupPath());
return { success, registerDatasource };
const permissionsResponse = await core.http.get<CheckPermissionsResponse>(
appRoutesService.getCheckPermissionsPath()
);

if (permissionsResponse?.success) {
successPromise = core.http
.post<PostIngestSetupResponse>(setupRouteService.getSetupPath())
.then(({ isInitialized }) =>
isInitialized ? Promise.resolve(true) : Promise.reject(new Error('Unknown setup error'))
);
} else {
throw new Error(permissionsResponse.error);
throw new Error(permissionsResponse?.error || 'Unknown permissions error');
}
} catch (error) {
return {
success: false,
error: { message: error.body?.message || 'Unknown error' },
registerDatasource,
};
successPromise = Promise.reject(error);
}

return {
success: successPromise,
registerDatasource,
};
}

public stop() {}
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/ingest_manager/server/routes/setup/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { RequestHandler } from 'src/core/server';
import { TypeOf } from '@kbn/config-schema';
import { outputService, appContextService } from '../../services';
import { GetFleetStatusResponse } from '../../../common';
import { GetFleetStatusResponse, PostIngestSetupResponse } from '../../../common';
import { setupIngestManager, setupFleet } from '../../services/setup';
import { PostFleetSetupRequestSchema } from '../../types';
import { IngestManagerError, getHTTPResponseCode } from '../../errors';
Expand Down Expand Up @@ -83,9 +83,10 @@ export const ingestManagerSetupHandler: RequestHandler = async (context, request
const callCluster = context.core.elasticsearch.legacy.client.callAsCurrentUser;
const logger = appContextService.getLogger();
try {
const body: PostIngestSetupResponse = { isInitialized: true };
await setupIngestManager(soClient, callCluster);
return response.ok({
body: { isInitialized: true },
body,
});
} catch (e) {
if (e instanceof IngestManagerError) {
Expand Down
15 changes: 1 addition & 14 deletions x-pack/plugins/security_solution/public/app/home/setup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,7 @@ export const Setup: React.FunctionComponent<{
});
};

const displayToast = () => {
notifications.toasts.addDanger({
title,
text: defaultText,
});
};

if (!ingestManager.success) {
if (ingestManager.error) {
displayToastWithModal(ingestManager.error.message);
} else {
displayToast();
}
}
ingestManager.success.catch((error: Error) => displayToastWithModal(error.message));
}, [ingestManager, notifications.toasts]);

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,6 @@ export const depsStartMock: () => DepsStartMock = () => {

return {
data: dataMock,
ingestManager: { success: true, registerDatasource },
ingestManager: { success: Promise.resolve(true), registerDatasource },
};
};
7 changes: 5 additions & 2 deletions x-pack/test/security_solution_endpoint/apps/endpoint/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
*/
import { FtrProviderContext } from '../../ftr_provider_context';

export default function ({ loadTestFile }: FtrProviderContext) {
export default function ({ loadTestFile, getService }: FtrProviderContext) {
describe('endpoint', function () {
this.tags('ciGroup7');

const ingestManager = getService('ingestManager');
before(async () => {
await ingestManager.setup();
});
loadTestFile(require.resolve('./endpoint_list'));
loadTestFile(require.resolve('./policy_list'));
loadTestFile(require.resolve('./policy_details'));
Expand Down
2 changes: 2 additions & 0 deletions x-pack/test/security_solution_endpoint/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { services as apiIntegrationServices } from '../../api_integration/services';
import { services as xPackFunctionalServices } from '../../functional/services';
import { EndpointPolicyTestResourcesProvider } from './endpoint_policy';

export const services = {
...xPackFunctionalServices,
ingestManager: apiIntegrationServices.ingestManager,
policyTestResources: EndpointPolicyTestResourcesProvider,
};

0 comments on commit 8f82f59

Please sign in to comment.