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

Removes custom header check on 404 responses, includes es client ProductNotSupportedError in EsUnavailableError conditional #116029

Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 3 additions & 1 deletion src/core/server/elasticsearch/client/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import type { DeeplyMockedKeys } from '@kbn/utility-types/jest';
import type { PublicKeys } from '@kbn/utility-types';
import { ElasticsearchClient } from './types';
import { ICustomClusterClient } from './cluster_client';
import { PRODUCT_RESPONSE_HEADER } from '../supported_server_response_check';

const omittedProps = [
'diagnostic',
Expand All @@ -23,6 +22,9 @@ const omittedProps = [
'helpers',
] as Array<PublicKeys<KibanaClient>>;

// the product header expected in every response from es
const PRODUCT_RESPONSE_HEADER = 'x-elastic-product';

// use jest.requireActual() to prevent weird errors when people mock @elastic/elasticsearch
const { Client: UnmockedClient } = jest.requireActual('@elastic/elasticsearch');
const createInternalClientMock = (res?: Promise<unknown>): DeeplyMockedKeys<KibanaClient> => {
Expand Down
5 changes: 0 additions & 5 deletions src/core/server/elasticsearch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,3 @@ export type {
ElasticsearchErrorDetails,
} from './client';
export { getRequestDebugMeta, getErrorMessage } from './client';
export {
isSupportedEsServer,
isNotFoundFromUnsupportedServer,
PRODUCT_RESPONSE_HEADER,
} from './supported_server_response_check';

This file was deleted.

35 changes: 0 additions & 35 deletions src/core/server/elasticsearch/supported_server_response_check.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -319,23 +319,6 @@ describe('collectMultiNamespaceReferences', () => {
// obj3 is excluded from the results
]);
});
it(`handles 404 responses that don't come from Elasticsearch`, async () => {
const createEsUnavailableNotFoundError = () => {
return SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError();
};
const obj1 = { type: MULTI_NAMESPACE_OBJ_TYPE_1, id: 'id-1' };
const params = setup([obj1]);
client.mget.mockReturnValueOnce(
elasticsearchClientMock.createSuccessTransportRequestPromise(
{ docs: [] },
{ statusCode: 404 },
{}
)
);
await expect(() => collectMultiNamespaceReferences(params)).rejects.toThrowError(
createEsUnavailableNotFoundError()
);
});

describe('legacy URL aliases', () => {
it('uses the PointInTimeFinder to search for legacy URL aliases', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@
*/

import * as esKuery from '@kbn/es-query';
import { isNotFoundFromUnsupportedServer } from '../../../elasticsearch';
import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../object_types';
import type { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry';
import type { SavedObjectsSerializer } from '../../serialization';
import type { SavedObject, SavedObjectsBaseOptions } from '../../types';
import { SavedObjectsErrorHelpers } from './errors';
import { getRootFields } from './included_fields';
import { getSavedObjectFromSource, rawDocExistsInNamespace } from './internal_utils';
import type {
Expand Down Expand Up @@ -199,15 +197,6 @@ async function getObjectsAndReferences({
{ body: { docs: makeBulkGetDocs(bulkGetObjects) } },
{ ignore: [404] }
);
// exit early if we can't verify a 404 response is from Elasticsearch
if (
isNotFoundFromUnsupportedServer({
statusCode: bulkGetResponse.statusCode,
headers: bulkGetResponse.headers,
})
) {
throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError();
}
const newObjectsToGet = new Set<string>();
for (let i = 0; i < bulkGetObjects.length; i++) {
// For every element in bulkGetObjects, there should be a matching element in bulkGetResponse.body.docs
Expand Down
22 changes: 10 additions & 12 deletions src/core/server/saved_objects/service/lib/decorate_es_error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ describe('savedObjectsClient/decorateEsError', () => {
expect(SavedObjectsErrorHelpers.isEsUnavailableError(error)).toBe(true);
});

it('makes ProductNotSupportedError a SavedObjectsClient/EsUnavailable error', () => {
const error = new esErrors.ProductNotSupportedError(
'reason',
elasticsearchClientMock.createApiResponse()
);
expect(SavedObjectsErrorHelpers.isEsUnavailableError(error)).toBe(false);
expect(decorateEsError(error)).toBe(error);
expect(SavedObjectsErrorHelpers.isEsUnavailableError(error)).toBe(true);
});

it('makes Conflict a SavedObjectsClient/Conflict error', () => {
const error = new esErrors.ResponseError(
elasticsearchClientMock.createApiResponse({ statusCode: 409 })
Expand Down Expand Up @@ -109,18 +119,6 @@ describe('savedObjectsClient/decorateEsError', () => {
expect(SavedObjectsErrorHelpers.isNotFoundError(genericError)).toBe(true);
});

it('makes NotFound errors generic NotFoundEsUnavailableError errors when response is from unsupported server', () => {
const error = new esErrors.ResponseError(
// explicitly override the headers
elasticsearchClientMock.createApiResponse({ statusCode: 404, headers: {} })
);
expect(SavedObjectsErrorHelpers.isNotFoundError(error)).toBe(false);
const genericError = decorateEsError(error);
expect(genericError).not.toBe(error);
expect(SavedObjectsErrorHelpers.isNotFoundError(genericError)).toBe(false);
expect(SavedObjectsErrorHelpers.isEsUnavailableError(genericError)).toBe(true);
});

it('if saved objects index does not exist makes NotFound a SavedObjectsClient/generalError', () => {
const error = new esErrors.ResponseError(
elasticsearchClientMock.createApiResponse({
Expand Down
12 changes: 5 additions & 7 deletions src/core/server/saved_objects/service/lib/decorate_es_error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { errors as esErrors } from '@elastic/elasticsearch';
import { get } from 'lodash';
import { isSupportedEsServer, ElasticsearchErrorDetails } from '../../../elasticsearch';
import { ElasticsearchErrorDetails } from '../../../elasticsearch';

const responseErrors = {
isServiceUnavailable: (statusCode?: number) => statusCode === 503,
Expand All @@ -20,7 +20,8 @@ const responseErrors = {
isBadRequest: (statusCode?: number) => statusCode === 400,
isTooManyRequests: (statusCode?: number) => statusCode === 429,
};
const { ConnectionError, NoLivingConnectionsError, TimeoutError } = esErrors;
const { ConnectionError, NoLivingConnectionsError, TimeoutError, ProductNotSupportedError } =
esErrors;
const SCRIPT_CONTEXT_DISABLED_REGEX = /(?:cannot execute scripts using \[)([a-z]*)(?:\] context)/;
const INLINE_SCRIPTS_DISABLED_MESSAGE = 'cannot execute [inline] scripts';

Expand All @@ -30,6 +31,7 @@ type EsErrors =
| esErrors.ConnectionError
| esErrors.NoLivingConnectionsError
| esErrors.TimeoutError
| esErrors.ProductNotSupportedError
| esErrors.ResponseError;

export function decorateEsError(error: EsErrors) {
Expand All @@ -42,6 +44,7 @@ export function decorateEsError(error: EsErrors) {
error instanceof ConnectionError ||
error instanceof NoLivingConnectionsError ||
error instanceof TimeoutError ||
error instanceof ProductNotSupportedError ||
responseErrors.isServiceUnavailable(error.statusCode)
) {
return SavedObjectsErrorHelpers.decorateEsUnavailableError(error, reason);
Expand Down Expand Up @@ -70,11 +73,6 @@ export function decorateEsError(error: EsErrors) {
if (match && match.length > 0) {
return SavedObjectsErrorHelpers.decorateIndexAliasNotFoundError(error, match[1]);
}
// Throw EsUnavailable error if the 404 is not from elasticsearch
// Needed here to verify Product support for any non-ignored 404 responses from calls to ES
if (!isSupportedEsServer(error?.meta?.headers)) {
return SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError();
}
return SavedObjectsErrorHelpers.createGenericNotFoundError();
}

Expand Down
41 changes: 0 additions & 41 deletions src/core/server/saved_objects/service/lib/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,45 +439,4 @@ describe('savedObjectsClient/errorTypes', () => {
});
});
});

describe('NotFoundEsUnavailableError', () => {
it('makes an error identifiable as an EsUnavailable error', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError('foo', 'bar');
expect(SavedObjectsErrorHelpers.isEsUnavailableError(error)).toBe(true);
});

it('returns a boom error', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError('foo', 'bar');
expect(error).toHaveProperty('isBoom', true);
});

it('decorates the error message with the saved object that was not found', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError('foo', 'bar');
expect(error.output.payload).toHaveProperty(
'message',
'x-elastic-product not present or not recognized: Saved object [foo/bar] not found'
);
});

describe('error.output', () => {
it('specifies the saved object that was not found', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(
'foo',
'bar'
);
expect(error.output.payload).toHaveProperty(
'message',
'x-elastic-product not present or not recognized: Saved object [foo/bar] not found'
);
});

it('sets statusCode to 503', () => {
const error = SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(
'foo',
'bar'
);
expect(error.output).toHaveProperty('statusCode', 503);
});
});
});
});
12 changes: 0 additions & 12 deletions src/core/server/saved_objects/service/lib/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,16 +202,4 @@ export class SavedObjectsErrorHelpers {
public static isGeneralError(error: Error | DecoratedError) {
return isSavedObjectsClientError(error) && error[code] === CODE_GENERAL_ERROR;
}

public static createGenericNotFoundEsUnavailableError(
// type and id not available in all operations (e.g. mget)
type: string | null = null,
id: string | null = null
) {
const notFoundError = this.createGenericNotFoundError(type, id);
return this.decorateEsUnavailableError(
new Error(`${notFoundError.message}`),
`x-elastic-product not present or not recognized`
);
}
}
Loading