Skip to content

Commit

Permalink
Revert config sync API changes
Browse files Browse the repository at this point in the history
  • Loading branch information
afharo committed Apr 19, 2021
1 parent 2e1402b commit ef2929c
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 20 deletions.
3 changes: 2 additions & 1 deletion src/core/server/http/http_server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { Readable } from 'stream';
import { RequestHandlerContext } from 'kibana/server';
import { KBN_CERT_PATH, KBN_KEY_PATH } from '@kbn/dev-utils';
import moment from 'moment';
import { of } from 'rxjs';

const cookieOptions = {
name: 'sid',
Expand Down Expand Up @@ -81,7 +82,7 @@ beforeEach(() => {
},
} as HttpConfig;

server = new HttpServer(loggingService, 'tests', config.shutdownTimeout);
server = new HttpServer(loggingService, 'tests', of(config.shutdownTimeout));
});

afterEach(async () => {
Expand Down
7 changes: 5 additions & 2 deletions src/core/server/http/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {
} from '@kbn/server-http-tools';

import type { Duration } from 'moment';
import { Observable } from 'rxjs';
import { take } from 'rxjs/operators';
import { Logger, LoggerFactory } from '../logging';
import { HttpConfig } from './http_config';
import { adoptToHapiAuthFormat, AuthenticationHandler } from './lifecycle/auth';
Expand Down Expand Up @@ -92,7 +94,7 @@ export class HttpServer {
constructor(
private readonly logger: LoggerFactory,
private readonly name: string,
private readonly shutdownTimeout: Duration
private readonly shutdownTimeout$: Observable<Duration>
) {
this.authState = new AuthStateStorage(() => this.authRegistered);
this.authRequestHeaders = new AuthHeadersStorage();
Expand Down Expand Up @@ -231,7 +233,8 @@ export class HttpServer {
if (hasStarted) {
this.log.debug('stopping http server');

await this.server.stop({ timeout: this.shutdownTimeout.asMilliseconds() });
const shutdownTimeout = await this.shutdownTimeout$.pipe(take(1)).toPromise();
await this.server.stop({ timeout: shutdownTimeout.asMilliseconds() });

this.log.debug(`http server stopped`);

Expand Down
19 changes: 9 additions & 10 deletions src/core/server/http/http_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const logger = loggingSystemMock.create();
const env = Env.createDefault(REPO_ROOT, getEnvOptions());
const coreId = Symbol();

const createConfigService = async (value: Partial<HttpConfigType> = {}) => {
const createConfigService = (value: Partial<HttpConfigType> = {}) => {
const configService = new ConfigService(
{
getConfig$: () =>
Expand All @@ -39,7 +39,6 @@ const createConfigService = async (value: Partial<HttpConfigType> = {}) => {
configService.setSchema(config.path, config.schema);
configService.setSchema(cspConfig.path, cspConfig.schema);
configService.setSchema(externalUrlConfig.path, externalUrlConfig.schema);
await configService.validate();
return configService;
};
const contextSetup = contextServiceMock.createSetupContract();
Expand All @@ -58,7 +57,7 @@ afterEach(() => {
});

test('creates and sets up http server', async () => {
const configService = await createConfigService({
const configService = createConfigService({
host: 'example.org',
port: 1234,
});
Expand Down Expand Up @@ -86,7 +85,7 @@ test('creates and sets up http server', async () => {
});

test('spins up notReady server until started if configured with `autoListen:true`', async () => {
const configService = await createConfigService();
const configService = createConfigService();
const httpServer = {
isListening: () => false,
setup: jest.fn().mockReturnValue({}),
Expand Down Expand Up @@ -138,7 +137,7 @@ test('spins up notReady server until started if configured with `autoListen:true
});

test('logs error if already set up', async () => {
const configService = await createConfigService();
const configService = createConfigService();

const httpServer = {
isListening: () => true,
Expand All @@ -156,7 +155,7 @@ test('logs error if already set up', async () => {
});

test('stops http server', async () => {
const configService = await createConfigService();
const configService = createConfigService();

const httpServer = {
isListening: () => false,
Expand All @@ -179,7 +178,7 @@ test('stops http server', async () => {
});

test('stops not ready server if it is running', async () => {
const configService = await createConfigService();
const configService = createConfigService();
const mockHapiServer = {
start: jest.fn(),
stop: jest.fn(),
Expand All @@ -203,7 +202,7 @@ test('stops not ready server if it is running', async () => {
});

test('register route handler', async () => {
const configService = await createConfigService();
const configService = createConfigService();

const registerRouterMock = jest.fn();
const httpServer = {
Expand All @@ -226,7 +225,7 @@ test('register route handler', async () => {
});

test('returns http server contract on setup', async () => {
const configService = await createConfigService();
const configService = createConfigService();
const httpServer = { server: fakeHapiServer, options: { someOption: true } };

mockHttpServer.mockImplementation(() => ({
Expand All @@ -244,7 +243,7 @@ test('returns http server contract on setup', async () => {
});

test('does not start http server if configured with `autoListen:false`', async () => {
const configService = await createConfigService({
const configService = createConfigService({
autoListen: false,
});
const httpServer = {
Expand Down
11 changes: 4 additions & 7 deletions src/core/server/http/http_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { Observable, Subscription, combineLatest } from 'rxjs';
import { Observable, Subscription, combineLatest, of } from 'rxjs';
import { first, map } from 'rxjs/operators';
import { Server } from '@hapi/hapi';
import { pick } from '@kbn/std';
Expand Down Expand Up @@ -69,11 +69,8 @@ export class HttpService
configService.atPath<CspConfigType>(cspConfig.path),
configService.atPath<ExternalUrlConfigType>(externalUrlConfig.path),
]).pipe(map(([http, csp, externalUrl]) => new HttpConfig(http, csp, externalUrl)));
this.httpServer = new HttpServer(
logger,
'Kibana',
configService.atPathSync<HttpConfigType>(httpConfig.path).shutdownTimeout
);
const shutdownTimeout$ = this.config$.pipe(map(({ shutdownTimeout }) => shutdownTimeout));
this.httpServer = new HttpServer(logger, 'Kibana', shutdownTimeout$);
this.httpsRedirectServer = new HttpsRedirectServer(logger.get('http', 'redirect', 'server'));
}

Expand Down Expand Up @@ -183,7 +180,7 @@ export class HttpService

private async runNotReadyServer(config: HttpConfig) {
this.log.debug('starting NotReady server');
const httpServer = new HttpServer(this.logger, 'NotReady', config.shutdownTimeout);
const httpServer = new HttpServer(this.logger, 'NotReady', of(config.shutdownTimeout));
const { server } = await httpServer.setup(config);
this.notReadyServer = server;
// use hapi server while KibanaResponseFactory doesn't allow specifying custom headers
Expand Down

0 comments on commit ef2929c

Please sign in to comment.