Skip to content

Commit

Permalink
Add deprecation warning when inline scripting is disabled (#111865) (#…
Browse files Browse the repository at this point in the history
…111932)

* initial implementation

* extract isScriptingDisabled to own file

* add unit tests for scripting deprecation

* add unit tests
  • Loading branch information
pgayvallet authored Sep 13, 2021
1 parent 3ee2b29 commit ae7e262
Show file tree
Hide file tree
Showing 11 changed files with 406 additions and 13 deletions.
18 changes: 18 additions & 0 deletions src/core/server/elasticsearch/deprecations/deprecation_provider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import type { RegisterDeprecationsConfig } from '../../deprecations';
import { getScriptingDisabledDeprecations } from './scripting_disabled_deprecation';

export const getElasticsearchDeprecationsProvider = (): RegisterDeprecationsConfig => {
return {
getDeprecations: async (context) => {
return [...(await getScriptingDisabledDeprecations({ esClient: context.esClient }))];
},
};
};
9 changes: 9 additions & 0 deletions src/core/server/elasticsearch/deprecations/index.ts
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
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export { getElasticsearchDeprecationsProvider } from './deprecation_provider';
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { estypes } from '@elastic/elasticsearch';
import { elasticsearchServiceMock } from '../../elasticsearch/elasticsearch_service.mock';
import { isInlineScriptingDisabled } from './is_scripting_disabled';

describe('isInlineScriptingDisabled', () => {
let client: ReturnType<typeof elasticsearchServiceMock.createElasticsearchClient>;

beforeEach(() => {
client = elasticsearchServiceMock.createElasticsearchClient();
});

const mockSettingsValue = (settings: estypes.ClusterGetSettingsResponse) => {
client.cluster.getSettings.mockReturnValue(
elasticsearchServiceMock.createSuccessTransportRequestPromise(settings)
);
};

it('returns `false` if all settings are empty', async () => {
mockSettingsValue({
transient: {},
persistent: {},
defaults: {},
});

expect(await isInlineScriptingDisabled({ client })).toEqual(false);
});

it('returns `false` if `defaults.script.allowed_types` is `inline`', async () => {
mockSettingsValue({
transient: {},
persistent: {},
defaults: {
'script.allowed_types': ['inline'],
},
});

expect(await isInlineScriptingDisabled({ client })).toEqual(false);
});

it('returns `true` if `defaults.script.allowed_types` is `none`', async () => {
mockSettingsValue({
transient: {},
persistent: {},
defaults: {
'script.allowed_types': ['none'],
},
});

expect(await isInlineScriptingDisabled({ client })).toEqual(true);
});

it('returns `true` if `defaults.script.allowed_types` is `stored`', async () => {
mockSettingsValue({
transient: {},
persistent: {},
defaults: {
'script.allowed_types': ['stored'],
},
});

expect(await isInlineScriptingDisabled({ client })).toEqual(true);
});

it('respect the persistent->defaults priority', async () => {
mockSettingsValue({
transient: {},
persistent: {
'script.allowed_types': ['inline'],
},
defaults: {
'script.allowed_types': ['stored'],
},
});

expect(await isInlineScriptingDisabled({ client })).toEqual(false);
});

it('respect the transient->persistent priority', async () => {
mockSettingsValue({
transient: {
'script.allowed_types': ['stored'],
},
persistent: {
'script.allowed_types': ['inline'],
},
defaults: {},
});

expect(await isInlineScriptingDisabled({ client })).toEqual(true);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { ElasticsearchClient } from '../../elasticsearch';

const scriptAllowedTypesKey = 'script.allowed_types';

export const isInlineScriptingDisabled = async ({
client,
}: {
client: ElasticsearchClient;
}): Promise<boolean> => {
const { body: settings } = await client.cluster.getSettings({
include_defaults: true,
flat_settings: true,
});

// priority: transient -> persistent -> default
const scriptAllowedTypes: string[] =
settings.transient[scriptAllowedTypesKey] ??
settings.persistent[scriptAllowedTypesKey] ??
settings.defaults![scriptAllowedTypesKey] ??
[];

// when unspecified, the setting as a default `[]` value that means that both scriptings are allowed.
const scriptAllowed = scriptAllowedTypes.length === 0 || scriptAllowedTypes.includes('inline');

return !scriptAllowed;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export const isInlineScriptingDisabledMock = jest.fn();
jest.doMock('./is_scripting_disabled', () => ({
isInlineScriptingDisabled: isInlineScriptingDisabledMock,
}));
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { isInlineScriptingDisabledMock } from './scripting_disabled_deprecation.test.mocks';
import { elasticsearchServiceMock } from '../../elasticsearch/elasticsearch_service.mock';
import { getScriptingDisabledDeprecations } from './scripting_disabled_deprecation';

describe('getScriptingDisabledDeprecations', () => {
let esClient: ReturnType<typeof elasticsearchServiceMock.createScopedClusterClient>;

beforeEach(() => {
esClient = elasticsearchServiceMock.createScopedClusterClient();
});

afterEach(() => {
isInlineScriptingDisabledMock.mockReset();
});

it('calls `isInlineScriptingDisabled` with the correct arguments', async () => {
await getScriptingDisabledDeprecations({
esClient,
});

expect(isInlineScriptingDisabledMock).toHaveBeenCalledTimes(1);
expect(isInlineScriptingDisabledMock).toHaveBeenCalledWith({
client: esClient.asInternalUser,
});
});

it('returns no deprecations if scripting is not disabled', async () => {
isInlineScriptingDisabledMock.mockResolvedValue(false);

const deprecations = await getScriptingDisabledDeprecations({
esClient,
});

expect(deprecations).toHaveLength(0);
});

it('returns a deprecation if scripting is disabled', async () => {
isInlineScriptingDisabledMock.mockResolvedValue(true);

const deprecations = await getScriptingDisabledDeprecations({
esClient,
});

expect(deprecations).toHaveLength(1);
expect(deprecations[0]).toEqual({
title: expect.any(String),
message: expect.any(String),
level: 'critical',
requireRestart: false,
correctiveActions: {
manualSteps: expect.any(Array),
},
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { i18n } from '@kbn/i18n';
import type { DeprecationsDetails } from '../../deprecations';
import { IScopedClusterClient } from '../../elasticsearch';
import { isInlineScriptingDisabled } from './is_scripting_disabled';

interface GetScriptingDisabledDeprecations {
esClient: IScopedClusterClient;
}

export const getScriptingDisabledDeprecations = async ({
esClient,
}: GetScriptingDisabledDeprecations): Promise<DeprecationsDetails[]> => {
const deprecations: DeprecationsDetails[] = [];
if (await isInlineScriptingDisabled({ client: esClient.asInternalUser })) {
deprecations.push({
title: i18n.translate('core.elasticsearch.deprecations.scriptingDisabled.title', {
defaultMessage: 'Inline scripting is disabled on elasticsearch',
}),
message: i18n.translate('core.elasticsearch.deprecations.scriptingDisabled.message', {
defaultMessage:
'Starting in 8.0, Kibana will require inline scripting to be enabled,' +
'and will fail to start otherwise.',
}),
level: 'critical',
requireRestart: false,
correctiveActions: {
manualSteps: [
i18n.translate('core.elasticsearch.deprecations.scriptingDisabled.manualSteps.1', {
defaultMessage: 'Set `script.allowed_types=inline` in your elasticsearch config ',
}),
],
},
});
}
return deprecations;
};
39 changes: 31 additions & 8 deletions src/core/server/elasticsearch/elasticsearch_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,32 +28,39 @@ import { loggingSystemMock } from '../logging/logging_system.mock';
import { httpServiceMock } from '../http/http_service.mock';
import { executionContextServiceMock } from '../execution_context/execution_context_service.mock';
import { configSchema, ElasticsearchConfig } from './elasticsearch_config';
import { ElasticsearchService } from './elasticsearch_service';
import { ElasticsearchService, SetupDeps } from './elasticsearch_service';
import { elasticsearchClientMock } from './client/mocks';
import { deprecationsServiceMock } from '../deprecations/deprecations_service.mock';
import { duration } from 'moment';
import { isValidConnection as isValidConnectionMock } from './is_valid_connection';
import { pollEsNodesVersion as pollEsNodesVersionMocked } from './version_check/ensure_es_version';

const { pollEsNodesVersion: pollEsNodesVersionActual } = jest.requireActual(
'./version_check/ensure_es_version'
);

const delay = async (durationMs: number) =>
await new Promise((resolve) => setTimeout(resolve, durationMs));

let elasticsearchService: ElasticsearchService;
const configService = configServiceMock.create();
const setupDeps = {
http: httpServiceMock.createInternalSetupContract(),
executionContext: executionContextServiceMock.createInternalSetupContract(),
};

let elasticsearchService: ElasticsearchService;
let env: Env;
let coreContext: CoreContext;

let mockClusterClientInstance: ReturnType<typeof elasticsearchClientMock.createCustomClusterClient>;

let mockConfig$: BehaviorSubject<any>;
let setupDeps: SetupDeps;
let deprecationsSetup: ReturnType<typeof deprecationsServiceMock.createInternalSetupContract>;

beforeEach(() => {
deprecationsSetup = deprecationsServiceMock.createInternalSetupContract();

setupDeps = {
http: httpServiceMock.createInternalSetupContract(),
executionContext: executionContextServiceMock.createInternalSetupContract(),
deprecations: deprecationsSetup,
};

env = Env.createDefault(REPO_ROOT, getEnvOptions());

mockConfig$ = new BehaviorSubject({
Expand Down Expand Up @@ -174,6 +181,22 @@ describe('#setup', () => {
);
});

it('registers its deprecation provider', async () => {
const registry = deprecationsServiceMock.createSetupContract();

deprecationsSetup.getRegistry.mockReturnValue(registry);

await elasticsearchService.setup(setupDeps);

expect(deprecationsSetup.getRegistry).toHaveBeenCalledTimes(1);
expect(deprecationsSetup.getRegistry).toHaveBeenCalledWith('elasticsearch');

expect(registry.registerDeprecations).toHaveBeenCalledTimes(1);
expect(registry.registerDeprecations).toHaveBeenCalledWith({
getDeprecations: expect.any(Function),
});
});

it('esNodeVersionCompatibility$ only starts polling when subscribed to', async (done) => {
const mockedClient = mockClusterClientInstance.asInternalUser;
mockedClient.nodes.info.mockImplementation(() =>
Expand Down
10 changes: 9 additions & 1 deletion src/core/server/elasticsearch/elasticsearch_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { ClusterClient, ElasticsearchClientConfig } from './client';
import { ElasticsearchConfig, ElasticsearchConfigType } from './elasticsearch_config';
import type { InternalHttpServiceSetup, GetAuthHeaders } from '../http';
import type { InternalExecutionContextSetup, IExecutionContext } from '../execution_context';
import type { InternalDeprecationsServiceSetup } from '../deprecations';
import {
InternalElasticsearchServicePreboot,
InternalElasticsearchServiceSetup,
Expand All @@ -27,9 +28,11 @@ import type { NodesVersionCompatibility } from './version_check/ensure_es_versio
import { pollEsNodesVersion } from './version_check/ensure_es_version';
import { calculateStatus$ } from './status';
import { isValidConnection } from './is_valid_connection';
import { getElasticsearchDeprecationsProvider } from './deprecations';

interface SetupDeps {
export interface SetupDeps {
http: InternalHttpServiceSetup;
deprecations: InternalDeprecationsServiceSetup;
executionContext: InternalExecutionContextSetup;
}

Expand Down Expand Up @@ -78,6 +81,10 @@ export class ElasticsearchService
this.executionContextClient = deps.executionContext;
this.client = this.createClusterClient('data', config);

deps.deprecations
.getRegistry('elasticsearch')
.registerDeprecations(getElasticsearchDeprecationsProvider());

const esNodesCompatibility$ = pollEsNodesVersion({
internalClient: this.client.asInternalUser,
log: this.log,
Expand All @@ -96,6 +103,7 @@ export class ElasticsearchService
status$: calculateStatus$(esNodesCompatibility$),
};
}

public async start(): Promise<InternalElasticsearchServiceStart> {
if (!this.client || !this.esNodesCompatibility$) {
throw new Error('ElasticsearchService needs to be setup before calling start');
Expand Down
Loading

0 comments on commit ae7e262

Please sign in to comment.