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.

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

[Ingest Manager] Enable ingest manager plugin by default. #70955

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 3 deletions docs/settings/ingest-manager-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
experimental[]

You can configure `xpack.ingestManager` settings in your `kibana.yml`.
By default, {ingest-manager} is not enabled. You need to
enable it. To use {fleet}, you also need to configure {kib} and {es} hosts.
By default, {ingest-manager} is enabled. To use {fleet}, you also need to configure {kib} and {es} hosts.

See the {ingest-guide}/index.html[Ingest Management] docs for more information.

Expand All @@ -19,7 +18,7 @@ See the {ingest-guide}/index.html[Ingest Management] docs for more information.
[cols="2*<"]
|===
| `xpack.ingestManager.enabled` {ess-icon}
| Set to `true` to enable {ingest-manager}.
| Set to `true` (default) to enable {ingest-manager}.
| `xpack.ingestManager.fleet.enabled` {ess-icon}
| Set to `true` (default) to enable {fleet}.
|===
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/ingest_manager/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

## Plugin

- The plugin is disabled by default. See the TypeScript type for the [the available plugin configuration options](https://github.com/elastic/kibana/blob/master/x-pack/plugins/ingest_manager/common/types/index.ts#L9-L27)
- Setting `xpack.ingestManager.enabled=true` enables the plugin including the EPM and Fleet features. It also adds the `PACKAGE_CONFIG_API_ROUTES` and `AGENT_CONFIG_API_ROUTES` values in [`common/constants/routes.ts`](./common/constants/routes.ts)
- The plugin is enabled by default. See the TypeScript type for the [the available plugin configuration options](https://github.com/elastic/kibana/blob/master/x-pack/plugins/ingest_manager/common/types/index.ts#L9-L27)
- Adding `xpack.ingestManager.enabled=false` will disable the plugin including the EPM and Fleet features. It will also remove the `PACKAGE_CONFIG_API_ROUTES` and `AGENT_CONFIG_API_ROUTES` values in [`common/constants/routes.ts`](./common/constants/routes.ts)
- Adding `--xpack.ingestManager.fleet.enabled=false` will disable the Fleet API & UI
- [code for adding the routes](https://github.com/elastic/kibana/blob/1f27d349533b1c2865c10c45b2cf705d7416fb36/x-pack/plugins/ingest_manager/server/plugin.ts#L115-L133)
- [Integration tests](server/integration_tests/router.test.ts)
Expand Down
45 changes: 25 additions & 20 deletions x-pack/plugins/ingest_manager/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface IngestManagerSetup {}
*/
export interface IngestManagerStart {
registerPackageConfigComponent: typeof registerPackageConfigComponent;
success: Promise<true>;
isInitialized: () => Promise<true>;
}

export interface IngestManagerSetupDeps {
Expand Down Expand Up @@ -100,27 +100,32 @@ export class IngestManagerPlugin
}

public async start(core: CoreStart): Promise<IngestManagerStart> {
let successPromise: IngestManagerStart['success'];
try {
const permissionsResponse = await core.http.get<CheckPermissionsResponse>(
appRoutesService.getCheckPermissionsPath()
);

if (permissionsResponse?.success) {
successPromise = core.http
Copy link
Contributor

Choose a reason for hiding this comment

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

Moving from eager/guaranteed to lazy/optional is an important change and we should be on the watch for effects elsewhere.

Until now all the various services could be sure that the plugin setup had at least been started, if not completed, before the other services are used. This meant that some may rely on implied side-effects (default packages, templates, etc) or even race conditions instead of defining those dependencies.

When/if we find them we can add code which uses the global promise or more fine-grained checks, but I don't think we know where those changes will happen right now.

Copy link
Member Author

@nchaulet nchaulet Jul 13, 2020

Choose a reason for hiding this comment

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

Right now I think the only two consumer of this are the ingest manager plugin and endpoint plugin, it should work ( @elastic/endpoint-management could you confirm it's working for you as expected? ).

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not just about changing the shape returned from start. We are significantly changing the behavior. We cannot know for sure what services depend on setup having been called already, but we do know for sure that it was previously guaranteed to be and now it is not.

It not a deal breaker, but it also not insignificant not easily known (e.g statically analyzable)

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree the change is significant. But also consumer of ingest manager should probably use isInitialized to know if the plugin is correctly initialized, this will allow to statically know every consumers.

.post<PostIngestSetupResponse>(setupRouteService.getSetupPath())
.then(({ isInitialized }) =>
isInitialized ? Promise.resolve(true) : Promise.reject(new Error('Unknown setup error'))
);
} else {
throw new Error(permissionsResponse?.error || 'Unknown permissions error');
}
} catch (error) {
successPromise = Promise.reject(error);
}
let successPromise: ReturnType<IngestManagerStart['isInitialized']>;

return {
success: successPromise,
isInitialized: () => {
if (!successPromise) {
successPromise = Promise.resolve().then(async () => {
const permissionsResponse = await core.http.get<CheckPermissionsResponse>(
appRoutesService.getCheckPermissionsPath()
);

if (permissionsResponse?.success) {
return 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 || 'Unknown permissions error');
}
});
}

return successPromise;
},
registerPackageConfigComponent,
};
}
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/ingest_manager/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const config = {
fleet: true,
},
schema: schema.object({
enabled: schema.boolean({ defaultValue: false }),
enabled: schema.boolean({ defaultValue: true }),
registryUrl: schema.maybe(schema.uri()),
fleet: schema.object({
enabled: schema.boolean({ defaultValue: true }),
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/ingest_manager/server/routes/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { IRouter, RequestHandler } from 'src/core/server';
import { PLUGIN_ID, APP_API_ROUTES } from '../../constants';
import { APP_API_ROUTES } from '../../constants';
import { appContextService } from '../../services';
import { CheckPermissionsResponse } from '../../../common';

Expand Down Expand Up @@ -37,7 +37,7 @@ export const registerRoutes = (router: IRouter) => {
{
path: APP_API_ROUTES.CHECK_PERMISSIONS_PATTERN,
validate: {},
options: { tags: [`access:${PLUGIN_ID}-read`] },
options: { tags: [] },
},
getCheckPermissionsHandler
);
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/public/app/home/setup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const Setup: React.FunctionComponent<{
});
};

ingestManager.success.catch((error: Error) => displayToastWithModal(error.message));
ingestManager.isInitialized().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 @@ -59,6 +59,9 @@ export const depsStartMock: () => DepsStartMock = () => {

return {
data: dataMock,
ingestManager: { success: Promise.resolve(true), registerPackageConfigComponent },
ingestManager: {
isInitialized: () => Promise.resolve(true),
registerPackageConfigComponent,
},
};
};