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

Spaces NP Migration - Moving server to the LegacyAPI model #45382

Merged
merged 22 commits into from
Sep 18, 2019
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
3dcd3ef
moving to the legacy api model
legrego Sep 6, 2019
f58b05d
update onPostAuth error handling
legrego Sep 11, 2019
115ea0c
cleaning up onRequest interceptor tests
legrego Sep 11, 2019
1b619a6
updating UI Capabilities tests to expect redirection to new space sel…
legrego Sep 11, 2019
4962e51
pass status code correctly
legrego Sep 11, 2019
2e4f6f4
update snapshot...
legrego Sep 11, 2019
b4939e3
remove unused interface
legrego Sep 12, 2019
b2ed48f
removing ts-ignore
legrego Sep 12, 2019
940742d
marking plugin fields readonly
legrego Sep 12, 2019
1099e3f
removing unnecessary types
legrego Sep 12, 2019
6d26d9f
improving type safety of wrapError
legrego Sep 12, 2019
ea16257
simplifying legacy config access
legrego Sep 12, 2019
87d7ae7
moving interceptor tests into an integration_tests folder for better …
legrego Sep 12, 2019
7ae92e2
Merge branch 'master' of github.com:elastic/kibana into np/spaces-leg…
legrego Sep 12, 2019
3d8729b
remove hard-coded test credentials
legrego Sep 12, 2019
abe5cb2
Revert "moving interceptor tests into an integration_tests folder for…
legrego Sep 13, 2019
4c30bfc
removing spaces enabled check from usage collector
legrego Sep 16, 2019
81629bc
removing unnecessary types
legrego Sep 16, 2019
0a8ba3f
moving legacy router into LegacyAPI
legrego Sep 16, 2019
48b0091
improve handling when navigating to the root of a non-default space
legrego Sep 16, 2019
42c617f
Merge branch 'master' of github.com:elastic/kibana into np/spaces-leg…
legrego Sep 16, 2019
5770f84
Merge branch 'master' into np/spaces-legacy-api
elasticmachine Sep 18, 2019
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
6 changes: 6 additions & 0 deletions src/core/server/http/http_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { OnPreAuthToolkit } from './lifecycle/on_pre_auth';
import { AuthToolkit } from './lifecycle/auth';
import { sessionStorageMock } from './cookie_session_storage.mocks';
import { IRouter } from './router';
import { OnPostAuthToolkit } from './lifecycle/on_post_auth';

type ServiceSetupMockType = jest.Mocked<HttpServiceSetup> & {
basePath: jest.Mocked<HttpServiceSetup['basePath']>;
Expand Down Expand Up @@ -89,6 +90,10 @@ const createOnPreAuthToolkitMock = (): jest.Mocked<OnPreAuthToolkit> => ({
rewriteUrl: jest.fn(),
});

const createOnPostAuthToolkitMock = (): jest.Mocked<OnPostAuthToolkit> => ({
next: jest.fn(),
});

const createAuthToolkitMock = (): jest.Mocked<AuthToolkit> => ({
authenticated: jest.fn(),
});
Expand All @@ -98,6 +103,7 @@ export const httpServiceMock = {
createBasePath: createBasePathMock,
createSetupContract: createSetupContractMock,
createOnPreAuthToolkit: createOnPreAuthToolkitMock,
createOnPostAuthToolkit: createOnPostAuthToolkitMock,
createAuthToolkit: createAuthToolkitMock,
createRouter: createRouterMock,
};
77 changes: 38 additions & 39 deletions x-pack/legacy/plugins/spaces/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import * as Rx from 'rxjs';
import { resolve } from 'path';
import KbnServer, { Server } from 'src/legacy/server/kbn_server';
import { CoreSetup, PluginInitializerContext } from 'src/core/server';
import { createOptionalPlugin } from '../../server/lib/optional_plugin';
// @ts-ignore
import { AuditLogger } from '../../server/lib/audit_logger';
Expand All @@ -16,14 +17,9 @@ import { getActiveSpace } from './server/lib/get_active_space';
import { getSpaceSelectorUrl } from './server/lib/get_space_selector_url';
import { migrateToKibana660 } from './server/lib/migrations';
import { plugin } from './server/new_platform';
import {
SpacesInitializerContext,
SpacesCoreSetup,
SpacesHttpServiceSetup,
} from './server/new_platform/plugin';
import { initSpacesRequestInterceptors } from './server/lib/request_interceptors';
import { SecurityPlugin } from '../security';
import { SpacesServiceSetup } from './server/new_platform/spaces_service/spaces_service';
import { initSpaceSelectorView } from './server/routes/views';

export interface SpacesPlugin {
getSpaceId: SpacesServiceSetup['getSpaceId'];
Expand Down Expand Up @@ -122,8 +118,7 @@ export const spaces = (kibana: Record<string, any>) =>

async init(server: Server) {
const kbnServer = (server as unknown) as KbnServer;
const initializerContext = ({
legacyConfig: server.config(),
Copy link
Member Author

Choose a reason for hiding this comment

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

legacyConfig moves to LegacyAPI.legacyConfig

const initializerContext = {
config: {
create: () => {
return Rx.of({
Expand All @@ -140,29 +135,15 @@ export const spaces = (kibana: Record<string, any>) =>
);
},
},
} as unknown) as SpacesInitializerContext;
} as PluginInitializerContext;

const spacesHttpService: SpacesHttpServiceSetup = {
...kbnServer.newPlatform.setup.core.http,
route: server.route.bind(server),
};

const core: SpacesCoreSetup = {
http: spacesHttpService,
elasticsearch: kbnServer.newPlatform.setup.core.elasticsearch,
savedObjects: server.savedObjects,
usage: server.usage,
tutorial: {
addScopedTutorialContextFactory: server.addScopedTutorialContextFactory,
const core: CoreSetup = ({
Copy link
Member Author

Choose a reason for hiding this comment

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

Now that all legacy requirements have moved to LegacyAPI, we can remove SpacesCoreSetup in favor of the real CoreSetup provided by NP

...kbnServer.newPlatform.setup.core,
http: {
...kbnServer.newPlatform.setup.core.http,
route: server.route,
legrego marked this conversation as resolved.
Show resolved Hide resolved
},
capabilities: {
registerCapabilitiesModifier: server.registerCapabilitiesModifier,
},
auditLogger: {
create: (pluginId: string) =>
new AuditLogger(server, pluginId, server.config(), server.plugins.xpack_main.info),
},
};
} as unknown) as CoreSetup;

const plugins = {
xpackMain: server.plugins.xpack_main,
Expand All @@ -177,20 +158,38 @@ export const spaces = (kibana: Record<string, any>) =>
spaces: this,
};

const { spacesService, log } = await plugin(initializerContext).setup(core, plugins);
const legacyRouter = server.route.bind(server);
Copy link
Member Author

Choose a reason for hiding this comment

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

The only component that couldn't move into LegacyAPI is the legacy router, because the route definitions are created before the legacy API is initialized. This is very short lived though, as my next PR will move the server code to a real NP plugin, which will use the NP router.

Copy link
Member

Choose a reason for hiding this comment

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

Btw, aren't you registering routes in setupLegacyComponents that is called from registerLegacyAPI?


const { spacesService, registerLegacyAPI } = await plugin(
initializerContext,
legacyRouter
).setup(core, plugins);

initSpacesRequestInterceptors({
config: initializerContext.legacyConfig,
http: core.http,
getHiddenUiAppById: server.getHiddenUiAppById,
onPostAuth: handler => {
server.ext('onPostAuth', handler);
const config = server.config();

registerLegacyAPI({
legacyConfig: {
serverBasePath: config.get('server.basePath'),
serverDefaultRoute: config.get('server.defaultRoute'),
kibanaIndex: config.get('kibana.index'),
spacesEnabled: config.get('xpack.spaces.enabled'),
legrego marked this conversation as resolved.
Show resolved Hide resolved
},
savedObjects: server.savedObjects,
usage: server.usage,
tutorial: {
addScopedTutorialContextFactory: server.addScopedTutorialContextFactory,
},
capabilities: {
registerCapabilitiesModifier: server.registerCapabilitiesModifier,
},
auditLogger: {
create: (pluginId: string) =>
new AuditLogger(server, pluginId, server.config(), server.plugins.xpack_main.info),
},
log,
spacesService,
xpackMain: plugins.xpackMain,
});

initSpaceSelectorView(server);

server.expose('getSpaceId', (request: any) => spacesService.getSpaceId(request));
server.expose('spaceIdToNamespace', spacesService.spaceIdToNamespace);
server.expose('namespaceToSpaceId', spacesService.namespaceToSpaceId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
import { i18n } from '@kbn/i18n';

import { first } from 'rxjs/operators';
import { ElasticsearchServiceSetup, SavedObjectsService } from 'src/core/server';
import { SavedObjectsService, CoreSetup } from 'src/core/server';
import { DEFAULT_SPACE_ID } from '../../common/constants';

interface Deps {
elasticsearch: ElasticsearchServiceSetup;
elasticsearch: CoreSetup['elasticsearch'];
savedObjects: SavedObjectsService;
}

Expand Down
6 changes: 3 additions & 3 deletions x-pack/legacy/plugins/spaces/server/lib/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
// @ts-ignore
import { boomify } from 'boom';

import { boomify, isBoom } from 'boom';

export function wrapError(error: any) {
if (error.isBoom) {
if (isBoom(error)) {
return error;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,14 @@

import { getSpaceSelectorUrl } from './get_space_selector_url';

const buildServerConfig = (serverBasePath?: string) => {
return {
get: (key: string) => {
if (key === 'server.basePath') {
return serverBasePath;
}
throw new Error(`unexpected config request: ${key}`);
},
};
};

describe('getSpaceSelectorUrl', () => {
it('returns / when no server base path is defined', () => {
const serverConfig = buildServerConfig();
expect(getSpaceSelectorUrl(serverConfig)).toEqual('/');
expect(getSpaceSelectorUrl('')).toEqual('/spaces/space_selector');
});

it('returns the server base path when defined', () => {
const serverConfig = buildServerConfig('/my/server/base/path');
expect(getSpaceSelectorUrl(serverConfig)).toEqual('/my/server/base/path');
expect(getSpaceSelectorUrl('/my/server/base/path')).toEqual(
'/my/server/base/path/spaces/space_selector'
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

interface Config {
get(key: string): string | boolean | number | null | undefined;
}

export function getSpaceSelectorUrl(serverConfig: Config) {
return serverConfig.get('server.basePath') || '/';
export function getSpaceSelectorUrl(serverBasePath: string = '') {
return `${serverBasePath}/spaces/space_selector`;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { getSpacesUsageCollector, UsageStats } from './get_spaces_usage_collector';
import { LegacyAPI } from '../new_platform/plugin';

function getServerMock(customization?: any) {
class MockUsageCollector {
Expand Down Expand Up @@ -43,13 +44,6 @@ function getServerMock(customization?: any) {
log: () => {
return;
},
config: () => ({
get: (key: string) => {
if (key === 'xpack.spaces.enabled') {
return true;
}
},
}),
usage: {
collectorSet: {
makeUsageCollector: (options: any) => {
Expand Down Expand Up @@ -79,17 +73,19 @@ const defaultCallClusterMock = jest.fn().mockResolvedValue({
},
});

const legacyConfig = {
spacesEnabled: true,
kibanaIndex: '.kibana',
} as LegacyAPI['legacyConfig'];

test('sets enabled to false when spaces is turned off', async () => {
legrego marked this conversation as resolved.
Show resolved Hide resolved
const mockConfigGet = jest.fn(key => {
if (key === 'xpack.spaces.enabled') {
return false;
} else if (key.indexOf('xpack.spaces') >= 0) {
throw new Error('Unknown config key!');
}
});
const serverMock = getServerMock({ config: () => ({ get: mockConfigGet }) });
const config = {
...legacyConfig,
spacesEnabled: false,
};
const serverMock = getServerMock();
const { fetch: getSpacesUsage } = getSpacesUsageCollector({
config: serverMock.config(),
config,
usage: serverMock.usage,
xpackMain: serverMock.plugins.xpack_main,
});
Expand All @@ -106,7 +102,7 @@ describe('with a basic license', () => {
.fn()
.mockReturnValue('basic');
const { fetch: getSpacesUsage } = getSpacesUsageCollector({
config: serverWithBasicLicenseMock.config(),
config: legacyConfig,
usage: serverWithBasicLicenseMock.usage,
xpackMain: serverWithBasicLicenseMock.plugins.xpack_main,
});
Expand Down Expand Up @@ -142,7 +138,7 @@ describe('with no license', () => {
serverWithNoLicenseMock.plugins.xpack_main.info.isAvailable = jest.fn().mockReturnValue(false);

const { fetch: getSpacesUsage } = getSpacesUsageCollector({
config: serverWithNoLicenseMock.config(),
config: legacyConfig,
usage: serverWithNoLicenseMock.usage,
xpackMain: serverWithNoLicenseMock.plugins.xpack_main,
});
Expand Down Expand Up @@ -175,7 +171,7 @@ describe('with platinum license', () => {
.fn()
.mockReturnValue('platinum');
const { fetch: getSpacesUsage } = getSpacesUsageCollector({
config: serverWithPlatinumLicenseMock.config(),
config: legacyConfig,
usage: serverWithPlatinumLicenseMock.usage,
xpackMain: serverWithPlatinumLicenseMock.plugins.xpack_main,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { KibanaConfig } from 'src/legacy/server/kbn_server';
import { get } from 'lodash';
import { CallAPIOptions } from 'src/core/server';
import { XPackMainPlugin } from '../../../xpack_main/xpack_main';
// @ts-ignore
import { KIBANA_STATS_TYPE_MONITORING } from '../../../monitoring/common/constants';
import { KIBANA_SPACES_STATS_TYPE } from '../../common/constants';
import { LegacyAPI } from '../new_platform/plugin';

type CallCluster = <T = unknown>(
endpoint: string,
Expand Down Expand Up @@ -114,7 +114,7 @@ export interface UsageStats {
}

interface CollectorDeps {
config: KibanaConfig;
config: LegacyAPI['legacyConfig'];
legrego marked this conversation as resolved.
Show resolved Hide resolved
usage: { collectorSet: any };
xpackMain: XPackMainPlugin;
}
Expand All @@ -131,12 +131,12 @@ export function getSpacesUsageCollector(deps: CollectorDeps) {
fetch: async (callCluster: CallCluster) => {
const xpackInfo = deps.xpackMain.info;
const available = xpackInfo && xpackInfo.isAvailable(); // some form of spaces is available for all valid licenses
const enabled = deps.config.get('xpack.spaces.enabled');
const enabled = deps.config.spacesEnabled;
const spacesAvailableAndEnabled = Boolean(available && enabled);

const usageStats = await getSpacesUsage(
callCluster,
deps.config.get('kibana.index'),
deps.config.kibanaIndex,
deps.xpackMain,
spacesAvailableAndEnabled
);
Expand Down
Loading