From 7cadce09313364ae9791bbf7d456ab2e501da28b Mon Sep 17 00:00:00 2001 From: restrry Date: Mon, 23 Dec 2019 10:34:09 +0300 Subject: [PATCH 1/2] use NP deprecation iunstead of manual one in uiSettings --- .../config/deprecation/core_deprecations.ts | 1 - src/core/server/http/http_config.ts | 11 -------- src/core/server/http/http_service.mock.ts | 1 - src/core/server/http/http_service.ts | 4 --- src/core/server/http/types.ts | 10 -------- ...gacy_object_to_config_adapter.test.ts.snap | 2 -- .../config/legacy_object_to_config_adapter.ts | 1 - src/core/server/server.ts | 1 + .../server/ui_settings/ui_settings_config.ts | 25 ++++++++++++++++--- .../ui_settings/ui_settings_service.test.ts | 22 ---------------- .../server/ui_settings/ui_settings_service.ts | 18 +++---------- src/legacy/server/config/schema.js | 1 - .../http/setup_default_route_provider.ts | 2 +- 13 files changed, 26 insertions(+), 73 deletions(-) diff --git a/src/core/server/config/deprecation/core_deprecations.ts b/src/core/server/config/deprecation/core_deprecations.ts index 6a401ec6625a2..36fe95e05cb53 100644 --- a/src/core/server/config/deprecation/core_deprecations.ts +++ b/src/core/server/config/deprecation/core_deprecations.ts @@ -97,7 +97,6 @@ export const coreDeprecationProvider: ConfigDeprecationProvider = ({ }) => [ unusedFromRoot('savedObjects.indexCheckTimeout'), unusedFromRoot('server.xsrf.token'), - unusedFromRoot('uiSettings.enabled'), renameFromRoot('optimize.lazy', 'optimize.watch'), renameFromRoot('optimize.lazyPort', 'optimize.watchPort'), renameFromRoot('optimize.lazyHost', 'optimize.watchHost'), diff --git a/src/core/server/http/http_config.ts b/src/core/server/http/http_config.ts index ef6a9c0a5f1a5..da90be2c8dd41 100644 --- a/src/core/server/http/http_config.ts +++ b/src/core/server/http/http_config.ts @@ -40,15 +40,6 @@ export const config = { validate: match(validBasePathRegex, "must start with a slash, don't end with one"), }) ), - defaultRoute: schema.maybe( - schema.string({ - validate(value) { - if (!value.startsWith('/')) { - return 'must start with a slash'; - } - }, - }) - ), cors: schema.conditional( schema.contextRef('dev'), true, @@ -136,7 +127,6 @@ export class HttpConfig { public basePath?: string; public rewriteBasePath: boolean; public publicDir: string; - public defaultRoute?: string; public ssl: SslConfig; public compression: { enabled: boolean; referrerWhitelist?: string[] }; public csp: ICspConfig; @@ -156,7 +146,6 @@ export class HttpConfig { this.rewriteBasePath = rawHttpConfig.rewriteBasePath; this.publicDir = env.staticFilesDir; this.ssl = new SslConfig(rawHttpConfig.ssl || {}); - this.defaultRoute = rawHttpConfig.defaultRoute; this.compression = rawHttpConfig.compression; this.csp = new CspConfig(rawCspConfig); } diff --git a/src/core/server/http/http_service.mock.ts b/src/core/server/http/http_service.mock.ts index 1668b409050b7..75a0a830ba9ad 100644 --- a/src/core/server/http/http_service.mock.ts +++ b/src/core/server/http/http_service.mock.ts @@ -63,7 +63,6 @@ const createSetupContractMock = () => { getAuthHeaders: jest.fn(), }, isTlsEnabled: false, - config: {}, }; setupContract.createCookieSessionStorageFactory.mockResolvedValue( sessionStorageMock.createFactory() diff --git a/src/core/server/http/http_service.ts b/src/core/server/http/http_service.ts index faeae0b559b6b..d707938aab149 100644 --- a/src/core/server/http/http_service.ts +++ b/src/core/server/http/http_service.ts @@ -107,10 +107,6 @@ export class HttpService implements CoreService ) => this.requestHandlerContext!.registerContext(pluginOpaqueId, contextName, provider), - - config: { - defaultRoute: config.defaultRoute, - }, }; return contract; diff --git a/src/core/server/http/types.ts b/src/core/server/http/types.ts index 92217515a22a1..9c8bfc073a524 100644 --- a/src/core/server/http/types.ts +++ b/src/core/server/http/types.ts @@ -250,16 +250,6 @@ export interface InternalHttpServiceSetup contextName: T, provider: RequestHandlerContextProvider ) => RequestHandlerContextContainer; - config: { - /** - * @internalRemarks - * Deprecated part of the server config, provided until - * https://github.com/elastic/kibana/issues/40255 - * - * @deprecated - * */ - defaultRoute?: string; - }; } /** @public */ diff --git a/src/core/server/legacy/config/__snapshots__/legacy_object_to_config_adapter.test.ts.snap b/src/core/server/legacy/config/__snapshots__/legacy_object_to_config_adapter.test.ts.snap index 0ebd8b8371628..3161dd06cf3b6 100644 --- a/src/core/server/legacy/config/__snapshots__/legacy_object_to_config_adapter.test.ts.snap +++ b/src/core/server/legacy/config/__snapshots__/legacy_object_to_config_adapter.test.ts.snap @@ -8,7 +8,6 @@ Object { "enabled": true, }, "cors": false, - "defaultRoute": undefined, "host": "host", "keepaliveTimeout": 5000, "maxPayload": 1000, @@ -32,7 +31,6 @@ Object { "enabled": true, }, "cors": false, - "defaultRoute": undefined, "host": "host", "keepaliveTimeout": 5000, "maxPayload": 1000, diff --git a/src/core/server/legacy/config/legacy_object_to_config_adapter.ts b/src/core/server/legacy/config/legacy_object_to_config_adapter.ts index ffcbfda4e024d..c5c1e1394c4a9 100644 --- a/src/core/server/legacy/config/legacy_object_to_config_adapter.ts +++ b/src/core/server/legacy/config/legacy_object_to_config_adapter.ts @@ -63,7 +63,6 @@ export class LegacyObjectToConfigAdapter extends ObjectToConfigAdapter { return { autoListen: configValue.autoListen, basePath: configValue.basePath, - defaultRoute: configValue.defaultRoute, cors: configValue.cors, host: configValue.host, maxPayload: configValue.maxPayloadBytes, diff --git a/src/core/server/server.ts b/src/core/server/server.ts index 5ca3800f3fb8f..447ab18c6ef39 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -242,6 +242,7 @@ export class Server { ]; this.configService.addDeprecationProvider(rootConfigPath, coreDeprecationProvider); + this.configService.addDeprecationProvider(uiSettingsConfig.path, uiSettingsConfig.deprecations); for (const [path, schema] of schemas) { await this.configService.setSchema(path, schema); diff --git a/src/core/server/ui_settings/ui_settings_config.ts b/src/core/server/ui_settings/ui_settings_config.ts index 702286f953ef1..6fdbdae95b6ed 100644 --- a/src/core/server/ui_settings/ui_settings_config.ts +++ b/src/core/server/ui_settings/ui_settings_config.ts @@ -18,15 +18,32 @@ */ import { schema, TypeOf } from '@kbn/config-schema'; +import { ConfigDeprecationProvider } from 'src/core/server'; + +const deprecations: ConfigDeprecationProvider = ({ unused, renameFromRoot }) => [ + unused('enabled'), + renameFromRoot('server.defaultRoute', 'uiSettings.overrides.defaultRoute'), +]; export type UiSettingsConfigType = TypeOf; export const config = { path: 'uiSettings', schema: schema.object({ - overrides: schema.object({}, { allowUnknowns: true }), - // Deprecation is implemented in LP. - // We define schema here not to fail on the validation step. - enabled: schema.maybe(schema.boolean()), + overrides: schema.object( + { + defaultRoute: schema.maybe( + schema.string({ + validate(value) { + if (!value.startsWith('/')) { + return 'must start with a slash'; + } + }, + }) + ), + }, + { allowUnknowns: true } + ), }), + deprecations, }; diff --git a/src/core/server/ui_settings/ui_settings_service.test.ts b/src/core/server/ui_settings/ui_settings_service.test.ts index d908a91a39c70..9b6a5ba0a4884 100644 --- a/src/core/server/ui_settings/ui_settings_service.test.ts +++ b/src/core/server/ui_settings/ui_settings_service.test.ts @@ -22,7 +22,6 @@ import { MockUiSettingsClientConstructor } from './ui_settings_service.test.mock import { UiSettingsService } from './ui_settings_service'; import { httpServiceMock } from '../http/http_service.mock'; -import { loggingServiceMock } from '../logging/logging_service.mock'; import { savedObjectsClientMock } from '../mocks'; import { mockCoreContext } from '../core_context.mock'; @@ -69,27 +68,6 @@ describe('uiSettings', () => { expect(MockUiSettingsClientConstructor.mock.calls[0][0].overrides).toEqual(overrides); }); - it('passes overrides with deprecated "server.defaultRoute"', async () => { - const service = new UiSettingsService(coreContext); - const httpSetupWithDefaultRoute = httpServiceMock.createSetupContract(); - httpSetupWithDefaultRoute.config.defaultRoute = '/defaultRoute'; - const setup = await service.setup({ http: httpSetupWithDefaultRoute }); - setup.asScopedToClient(savedObjectsClient); - - expect(MockUiSettingsClientConstructor.mock.calls[0][0].overrides).toEqual({ - ...overrides, - defaultRoute: '/defaultRoute', - }); - - expect(loggingServiceMock.collect(coreContext.logger).warn).toMatchInlineSnapshot(` - Array [ - Array [ - "Config key \\"server.defaultRoute\\" is deprecated. It has been replaced with \\"uiSettings.overrides.defaultRoute\\"", - ], - ] - `); - }); - it('passes a copy of set defaults to UiSettingsClient', async () => { const service = new UiSettingsService(coreContext); const setup = await service.setup(setupDeps); diff --git a/src/core/server/ui_settings/ui_settings_service.ts b/src/core/server/ui_settings/ui_settings_service.ts index db08c3cad85a2..942c2625ac8e7 100644 --- a/src/core/server/ui_settings/ui_settings_service.ts +++ b/src/core/server/ui_settings/ui_settings_service.ts @@ -56,7 +56,9 @@ export class UiSettingsService public async setup(deps: SetupDeps): Promise { registerRoutes(deps.http.createRouter('')); this.log.debug('Setting up ui settings service'); - this.overrides = await this.getOverrides(deps); + const config = await this.config$.pipe(first()).toPromise(); + this.overrides = config.overrides; + return { register: this.register.bind(this), asScopedToClient: this.getScopedClientFactory(), @@ -95,18 +97,4 @@ export class UiSettingsService this.uiSettingsDefaults.set(key, value); }); } - - private async getOverrides(deps: SetupDeps) { - const config = await this.config$.pipe(first()).toPromise(); - const overrides: Record = config.overrides; - // manually implemented deprecation until New platform Config service - // supports them https://github.com/elastic/kibana/issues/40255 - if (typeof deps.http.config.defaultRoute !== 'undefined') { - overrides.defaultRoute = deps.http.config.defaultRoute; - this.log.warn( - 'Config key "server.defaultRoute" is deprecated. It has been replaced with "uiSettings.overrides.defaultRoute"' - ); - } - return overrides; - } } diff --git a/src/legacy/server/config/schema.js b/src/legacy/server/config/schema.js index f886fd598f5c9..183904ff35985 100644 --- a/src/legacy/server/config/schema.js +++ b/src/legacy/server/config/schema.js @@ -70,7 +70,6 @@ export default () => server: Joi.object({ name: Joi.string().default(os.hostname()), - defaultRoute: Joi.string().regex(/^\//, `start with a slash`), customResponseHeaders: Joi.object() .unknown(true) .default({}), diff --git a/src/legacy/server/http/setup_default_route_provider.ts b/src/legacy/server/http/setup_default_route_provider.ts index 0e7bcf1f56f6f..9a580dd1c59bd 100644 --- a/src/legacy/server/http/setup_default_route_provider.ts +++ b/src/legacy/server/http/setup_default_route_provider.ts @@ -29,7 +29,7 @@ export function setupDefaultRouteProvider(server: Legacy.Server) { const uiSettings = request.getUiSettingsService(); - const defaultRoute = await uiSettings.get('defaultRoute'); + const defaultRoute = await uiSettings.get('defaultRoute'); const qualifiedDefaultRoute = `${request.getBasePath()}${defaultRoute}`; if (isRelativePath(qualifiedDefaultRoute, serverBasePath)) { From 6c06e701ee237b79a71f3cb2dcf67a338a1b0a2f Mon Sep 17 00:00:00 2001 From: restrry Date: Mon, 6 Jan 2020 13:41:11 +0300 Subject: [PATCH 2/2] add ServiceConfigDescriptor type --- src/core/server/internal_types.ts | 18 +++++++++ src/core/server/server.ts | 5 ++- .../server/ui_settings/ui_settings_config.ts | 39 ++++++++++--------- 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/src/core/server/internal_types.ts b/src/core/server/internal_types.ts index be4d830c55eab..ff68d1544d119 100644 --- a/src/core/server/internal_types.ts +++ b/src/core/server/internal_types.ts @@ -17,7 +17,10 @@ * under the License. */ +import { Type } from '@kbn/config-schema'; + import { CapabilitiesSetup, CapabilitiesStart } from './capabilities'; +import { ConfigDeprecationProvider } from './config'; import { ContextSetup } from './context'; import { InternalElasticsearchServiceSetup } from './elasticsearch'; import { InternalHttpServiceSetup } from './http'; @@ -47,3 +50,18 @@ export interface InternalCoreStart { savedObjects: InternalSavedObjectsServiceStart; uiSettings: InternalUiSettingsServiceStart; } + +/** + * @internal + */ +export interface ServiceConfigDescriptor { + path: string; + /** + * Schema to use to validate the configuration. + */ + schema: Type; + /** + * Provider for the {@link ConfigDeprecation} to apply to the plugin configuration. + */ + deprecations?: ConfigDeprecationProvider; +} diff --git a/src/core/server/server.ts b/src/core/server/server.ts index e5cda42d10d6c..4239e7e6c4254 100644 --- a/src/core/server/server.ts +++ b/src/core/server/server.ts @@ -257,7 +257,10 @@ export class Server { ]; this.configService.addDeprecationProvider(rootConfigPath, coreDeprecationProvider); - this.configService.addDeprecationProvider(uiSettingsConfig.path, uiSettingsConfig.deprecations); + this.configService.addDeprecationProvider( + uiSettingsConfig.path, + uiSettingsConfig.deprecations! + ); for (const [path, schema] of schemas) { await this.configService.setSchema(path, schema); diff --git a/src/core/server/ui_settings/ui_settings_config.ts b/src/core/server/ui_settings/ui_settings_config.ts index 6fdbdae95b6ed..a54d482a0296a 100644 --- a/src/core/server/ui_settings/ui_settings_config.ts +++ b/src/core/server/ui_settings/ui_settings_config.ts @@ -19,31 +19,34 @@ import { schema, TypeOf } from '@kbn/config-schema'; import { ConfigDeprecationProvider } from 'src/core/server'; +import { ServiceConfigDescriptor } from '../internal_types'; const deprecations: ConfigDeprecationProvider = ({ unused, renameFromRoot }) => [ unused('enabled'), renameFromRoot('server.defaultRoute', 'uiSettings.overrides.defaultRoute'), ]; -export type UiSettingsConfigType = TypeOf; +const configSchema = schema.object({ + overrides: schema.object( + { + defaultRoute: schema.maybe( + schema.string({ + validate(value) { + if (!value.startsWith('/')) { + return 'must start with a slash'; + } + }, + }) + ), + }, + { allowUnknowns: true } + ), +}); -export const config = { +export type UiSettingsConfigType = TypeOf; + +export const config: ServiceConfigDescriptor = { path: 'uiSettings', - schema: schema.object({ - overrides: schema.object( - { - defaultRoute: schema.maybe( - schema.string({ - validate(value) { - if (!value.startsWith('/')) { - return 'must start with a slash'; - } - }, - }) - ), - }, - { allowUnknowns: true } - ), - }), + schema: configSchema, deprecations, };