Skip to content

Commit

Permalink
[Upgrade Assistant] Remove "boom" from reindex service (#58715)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
jloleysens and elasticmachine authored Mar 2, 2020
1 parent 37d18a7 commit ac5e7aa
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 30 deletions.
35 changes: 35 additions & 0 deletions x-pack/plugins/upgrade_assistant/server/lib/reindexing/error.ts
Original file line number Diff line number Diff line change
@@ -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),
};
Original file line number Diff line number Diff line change
@@ -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');
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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'];
Expand Down Expand Up @@ -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);
Expand All @@ -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, {
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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);
Expand All @@ -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}`
);
}
}

Expand All @@ -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];
Expand Down
62 changes: 42 additions & 20 deletions x-pack/plugins/upgrade_assistant/server/routes/reindex_indices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,29 @@
*/

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';
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;
Expand All @@ -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
Expand Down Expand Up @@ -94,7 +132,7 @@ export function registerReindexIndicesRoutes(

return response.ok({ body: reindexOp.attributes });
} catch (e) {
return response.internalError({ body: e });
return mapAnyErrorToKibanaHttpResponse(e);
}
}
)
Expand Down Expand Up @@ -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);
}
}
)
Expand Down Expand Up @@ -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);
}
}
)
Expand Down

0 comments on commit ac5e7aa

Please sign in to comment.