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

use NP deprecations in uiSettings #53755

Merged
merged 4 commits into from
Jan 6, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 0 additions & 1 deletion src/core/server/config/deprecation/core_deprecations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
11 changes: 0 additions & 11 deletions src/core/server/http/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
1 change: 0 additions & 1 deletion src/core/server/http/http_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ const createSetupContractMock = () => {
getAuthHeaders: jest.fn(),
},
isTlsEnabled: false,
config: {},
};
setupContract.createCookieSessionStorageFactory.mockResolvedValue(
sessionStorageMock.createFactory()
Expand Down
4 changes: 0 additions & 4 deletions src/core/server/http/http_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,6 @@ export class HttpService implements CoreService<InternalHttpServiceSetup, HttpSe
contextName: T,
provider: RequestHandlerContextProvider<T>
) => this.requestHandlerContext!.registerContext(pluginOpaqueId, contextName, provider),

config: {
defaultRoute: config.defaultRoute,
},
};

return contract;
Expand Down
10 changes: 0 additions & 10 deletions src/core/server/http/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,6 @@ export interface InternalHttpServiceSetup
contextName: T,
provider: RequestHandlerContextProvider<T>
) => RequestHandlerContextContainer;
config: {
/**
* @internalRemarks
* Deprecated part of the server config, provided until
* https://github.com/elastic/kibana/issues/40255
*
* @deprecated
* */
defaultRoute?: string;
};
}

/** @public */
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,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,
Expand Down
1 change: 1 addition & 0 deletions src/core/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,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);
Expand Down
25 changes: 21 additions & 4 deletions src/core/server/ui_settings/ui_settings_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof config.schema>;

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(
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure: do we want to introduce an equivalent to PluginConfigDescriptor for our core services namespaces ?

Something like

interface ServiceConfigDescriptor<T = any> {
  path: string;
  schema: PluginConfigSchema<T>;
  deprecations?: ConfigDeprecationProvider;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure we can do that.

{
defaultRoute: schema.maybe(
schema.string({
validate(value) {
if (!value.startsWith('/')) {
return 'must start with a slash';
}
},
})
),
},
{ allowUnknowns: true }
),
}),
deprecations,
};
22 changes: 0 additions & 22 deletions src/core/server/ui_settings/ui_settings_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand Down
18 changes: 3 additions & 15 deletions src/core/server/ui_settings/ui_settings_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ export class UiSettingsService
public async setup(deps: SetupDeps): Promise<InternalUiSettingsServiceSetup> {
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(),
Expand Down Expand Up @@ -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<string, any> = 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;
}
}
1 change: 0 additions & 1 deletion src/legacy/server/config/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({}),
Expand Down
2 changes: 1 addition & 1 deletion src/legacy/server/http/setup_default_route_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>('defaultRoute');
const qualifiedDefaultRoute = `${request.getBasePath()}${defaultRoute}`;

if (isRelativePath(qualifiedDefaultRoute, serverBasePath)) {
Expand Down