[Upgrade Assistant] Remove "boom" from reindex service (#58715)

* Removed Boom from reindex-service

The reindex service had logic inside it for mapping errors
to HTTP. This has been moved to the route handlers and also
removed Boom.

There is one more instance of Boom use inside of reindex-actions
but that comes from Saved Objects which should probably be removed
at a later stage.

* Fix import path

Specify the full relative import path to the kibana's core
server folder

* Remove unnecessary if statement

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Jean-Louis Leysens 2020-03-02 11:08:19 +01:00 committed by GitHub
parent 37d18a7d58
commit ac5e7aa81e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 106 additions and 30 deletions

View file

@ -0,0 +1,35 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import {
AccessForbidden,
IndexNotFound,
CannotCreateIndex,
ReindexTaskCannotBeDeleted,
ReindexTaskFailed,
ReindexAlreadyInProgress,
MultipleReindexJobsFound,
} from './error_symbols';
export class ReindexError extends Error {
constructor(message: string, public readonly symbol: symbol) {
super(message);
}
}
export const createErrorFactory = (symbol: symbol) => (message: string) => {
return new ReindexError(message, symbol);
};
export const error = {
indexNotFound: createErrorFactory(IndexNotFound),
accessForbidden: createErrorFactory(AccessForbidden),
cannotCreateIndex: createErrorFactory(CannotCreateIndex),
reindexTaskFailed: createErrorFactory(ReindexTaskFailed),
reindexTaskCannotBeDeleted: createErrorFactory(ReindexTaskCannotBeDeleted),
reindexAlreadyInProgress: createErrorFactory(ReindexAlreadyInProgress),
multipleReindexJobsFound: createErrorFactory(MultipleReindexJobsFound),
};

View file

@ -0,0 +1,15 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
export const AccessForbidden = Symbol('AccessForbidden');
export const IndexNotFound = Symbol('IndexNotFound');
export const CannotCreateIndex = Symbol('CannotCreateIndex');
export const ReindexTaskFailed = Symbol('ReindexTaskFailed');
export const ReindexTaskCannotBeDeleted = Symbol('ReindexTaskCannotBeDeleted');
export const ReindexAlreadyInProgress = Symbol('ReindexAlreadyInProgress');
export const MultipleReindexJobsFound = Symbol('MultipleReindexJobsFound');

View file

@ -3,8 +3,6 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import Boom from 'boom';
import { APICaller, Logger } from 'src/core/server';
import { first } from 'rxjs/operators';
@ -24,6 +22,8 @@ import {
import { ReindexActions } from './reindex_actions';
import { LicensingPluginSetup } from '../../../../licensing/server';
import { error } from './error';
const VERSION_REGEX = new RegExp(/^([1-9]+)\.([0-9]+)\.([0-9]+)/);
const ML_INDICES = ['.ml-state', '.ml-anomalies', '.ml-config'];
const WATCHER_INDICES = ['.watches', '.triggered-watches'];
@ -284,7 +284,7 @@ export const reindexServiceFactory = (
const flatSettings = await actions.getFlatSettings(indexName);
if (!flatSettings) {
throw Boom.notFound(`Index ${indexName} does not exist.`);
throw error.indexNotFound(`Index ${indexName} does not exist.`);
}
const { settings, mappings } = transformFlatSettings(flatSettings);
@ -298,7 +298,7 @@ export const reindexServiceFactory = (
});
if (!createIndex.acknowledged) {
throw Boom.badImplementation(`Index could not be created: ${newIndexName}`);
throw error.cannotCreateIndex(`Index could not be created: ${newIndexName}`);
}
return actions.updateReindexOp(reindexOp, {
@ -363,7 +363,7 @@ export const reindexServiceFactory = (
if (taskResponse.task.status.created < count) {
// Include the entire task result in the error message. This should be guaranteed
// to be JSON-serializable since it just came back from Elasticsearch.
throw Boom.badData(`Reindexing failed: ${JSON.stringify(taskResponse)}`);
throw error.reindexTaskFailed(`Reindexing failed: ${JSON.stringify(taskResponse)}`);
}
// Update the status
@ -380,7 +380,7 @@ export const reindexServiceFactory = (
});
if (deleteTaskResp.result !== 'deleted') {
throw Boom.badImplementation(`Could not delete reindexing task ${taskId}`);
throw error.reindexTaskCannotBeDeleted(`Could not delete reindexing task ${taskId}`);
}
return reindexOp;
@ -414,7 +414,7 @@ export const reindexServiceFactory = (
});
if (!aliasResponse.acknowledged) {
throw Boom.badImplementation(`Index aliases could not be created.`);
throw error.cannotCreateIndex(`Index aliases could not be created.`);
}
return actions.updateReindexOp(reindexOp, {
@ -520,7 +520,7 @@ export const reindexServiceFactory = (
async createReindexOperation(indexName: string) {
const indexExists = await callAsUser('indices.exists', { index: indexName });
if (!indexExists) {
throw Boom.notFound(`Index ${indexName} does not exist in this cluster.`);
throw error.indexNotFound(`Index ${indexName} does not exist in this cluster.`);
}
const existingReindexOps = await actions.findReindexOperations(indexName);
@ -533,7 +533,9 @@ export const reindexServiceFactory = (
// Delete the existing one if it failed or was cancelled to give a chance to retry.
await actions.deleteReindexOp(existingOp);
} else {
throw Boom.badImplementation(`A reindex operation already in-progress for ${indexName}`);
throw error.reindexAlreadyInProgress(
`A reindex operation already in-progress for ${indexName}`
);
}
}
@ -547,7 +549,9 @@ export const reindexServiceFactory = (
if (findResponse.total === 0) {
return null;
} else if (findResponse.total > 1) {
throw Boom.badImplementation(`More than one reindex operation found for ${indexName}`);
throw error.multipleReindexJobsFound(
`More than one reindex operation found for ${indexName}`
);
}
return findResponse.saved_objects[0];

View file

@ -5,7 +5,12 @@
*/
import { schema } from '@kbn/config-schema';
import { Logger, ElasticsearchServiceSetup, SavedObjectsClient } from 'src/core/server';
import {
Logger,
ElasticsearchServiceSetup,
SavedObjectsClient,
kibanaResponseFactory,
} from '../../../../../src/core/server';
import { ReindexStatus } from '../../common/types';
import { versionCheckHandlerWrapper } from '../lib/es_version_precheck';
import { reindexServiceFactory, ReindexWorker } from '../lib/reindexing';
@ -13,6 +18,16 @@ import { CredentialStore } from '../lib/reindexing/credential_store';
import { reindexActionsFactory } from '../lib/reindexing/reindex_actions';
import { RouteDependencies } from '../types';
import { LicensingPluginSetup } from '../../../licensing/server';
import { ReindexError } from '../lib/reindexing/error';
import {
AccessForbidden,
IndexNotFound,
CannotCreateIndex,
ReindexAlreadyInProgress,
ReindexTaskCannotBeDeleted,
ReindexTaskFailed,
MultipleReindexJobsFound,
} from '../lib/reindexing/error_symbols';
interface CreateReindexWorker {
logger: Logger;
@ -33,6 +48,29 @@ export function createReindexWorker({
return new ReindexWorker(savedObjects, credentialStore, adminClient, logger, licensing);
}
const mapAnyErrorToKibanaHttpResponse = (e: any) => {
if (e instanceof ReindexError) {
switch (e.symbol) {
case AccessForbidden:
return kibanaResponseFactory.forbidden({ body: e.message });
case IndexNotFound:
return kibanaResponseFactory.notFound({ body: e.message });
case CannotCreateIndex:
case ReindexTaskCannotBeDeleted:
return kibanaResponseFactory.internalError({ body: e.message });
case ReindexTaskFailed:
// Bad data
return kibanaResponseFactory.customError({ body: e.message, statusCode: 422 });
case ReindexAlreadyInProgress:
case MultipleReindexJobsFound:
return kibanaResponseFactory.badRequest({ body: e.message });
default:
// nothing matched
}
}
return kibanaResponseFactory.internalError({ body: e });
};
export function registerReindexIndicesRoutes(
{ credentialStore, router, licensing, log }: RouteDependencies,
getWorker: () => ReindexWorker
@ -94,7 +132,7 @@ export function registerReindexIndicesRoutes(
return response.ok({ body: reindexOp.attributes });
} catch (e) {
return response.internalError({ body: e });
return mapAnyErrorToKibanaHttpResponse(e);
}
}
)
@ -150,15 +188,7 @@ export function registerReindexIndicesRoutes(
},
});
} catch (e) {
if (!e.isBoom) {
return response.internalError({ body: e });
}
return response.customError({
body: {
message: e.message,
},
statusCode: e.statusCode,
});
return mapAnyErrorToKibanaHttpResponse(e);
}
}
)
@ -201,15 +231,7 @@ export function registerReindexIndicesRoutes(
return response.ok({ body: { acknowledged: true } });
} catch (e) {
if (!e.isBoom) {
return response.internalError({ body: e });
}
return response.customError({
body: {
message: e.message,
},
statusCode: e.statusCode,
});
return mapAnyErrorToKibanaHttpResponse(e);
}
}
)