diff --git a/docs/development/core/server/kibana-plugin-core-server.savedobjectserrorhelpers.creategenericnotfoundesunavailableerror.md b/docs/development/core/server/kibana-plugin-core-server.savedobjectserrorhelpers.creategenericnotfoundesunavailableerror.md index e05f9466aa9ee..e17877a537d00 100644 --- a/docs/development/core/server/kibana-plugin-core-server.savedobjectserrorhelpers.creategenericnotfoundesunavailableerror.md +++ b/docs/development/core/server/kibana-plugin-core-server.savedobjectserrorhelpers.creategenericnotfoundesunavailableerror.md @@ -7,15 +7,15 @@ Signature: ```typescript -static createGenericNotFoundEsUnavailableError(type: string, id: string): DecoratedError; +static createGenericNotFoundEsUnavailableError(type?: string | null, id?: string | null): DecoratedError; ``` ## Parameters | Parameter | Type | Description | | --- | --- | --- | -| type | string | | -| id | string | | +| type | string | null | | +| id | string | null | | Returns: diff --git a/src/core/server/elasticsearch/client/mocks.ts b/src/core/server/elasticsearch/client/mocks.ts index 848d9c204bfbf..26a68df81f24e 100644 --- a/src/core/server/elasticsearch/client/mocks.ts +++ b/src/core/server/elasticsearch/client/mocks.ts @@ -11,6 +11,7 @@ import { TransportRequestPromise } from '@elastic/elasticsearch/lib/Transport'; import type { DeeplyMockedKeys } from '@kbn/utility-types/jest'; import { ElasticsearchClient } from './types'; import { ICustomClusterClient } from './cluster_client'; +import { PRODUCT_RESPONSE_HEADER } from '../supported_server_response_check'; const createInternalClientMock = ( res?: MockedTransportRequestPromise @@ -142,7 +143,7 @@ export type MockedTransportRequestPromise = TransportRequestPromise & { const createSuccessTransportRequestPromise = ( body: T, { statusCode = 200 }: { statusCode?: number } = {}, - headers?: Record + headers: Record = { [PRODUCT_RESPONSE_HEADER]: 'Elasticsearch' } ): MockedTransportRequestPromise> => { const response = createApiResponse({ body, statusCode, headers }); const promise = Promise.resolve(response); @@ -163,7 +164,7 @@ function createApiResponse>( return { body: {} as any, statusCode: 200, - headers: {}, + headers: { [PRODUCT_RESPONSE_HEADER]: 'Elasticsearch' }, warnings: [], meta: {} as any, ...opts, diff --git a/src/core/server/elasticsearch/index.ts b/src/core/server/elasticsearch/index.ts index 62bb30452bb98..f50e3a0f72860 100644 --- a/src/core/server/elasticsearch/index.ts +++ b/src/core/server/elasticsearch/index.ts @@ -38,4 +38,8 @@ export type { DeleteDocumentResponse, } from './client'; export { getRequestDebugMeta, getErrorMessage } from './client'; -export { isSupportedEsServer } from './supported_server_response_check'; +export { + isSupportedEsServer, + isNotFoundFromUnsupportedServer, + PRODUCT_RESPONSE_HEADER, +} from './supported_server_response_check'; diff --git a/src/core/server/elasticsearch/supported_server_response_check.test.ts b/src/core/server/elasticsearch/supported_server_response_check.test.ts new file mode 100644 index 0000000000000..589e947142fc3 --- /dev/null +++ b/src/core/server/elasticsearch/supported_server_response_check.test.ts @@ -0,0 +1,41 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { isNotFoundFromUnsupportedServer } from './supported_server_response_check'; + +describe('#isNotFoundFromUnsupportedServer', () => { + it('returns true with not found response from unsupported server', () => { + const rawResponse = { + statusCode: 404, + headers: {}, + }; + + const result = isNotFoundFromUnsupportedServer(rawResponse); + expect(result).toBe(true); + }); + + it('returns false with not found response from supported server', async () => { + const rawResponse = { + statusCode: 404, + headers: { 'x-elastic-product': 'Elasticsearch' }, + }; + + const result = isNotFoundFromUnsupportedServer(rawResponse); + expect(result).toBe(false); + }); + + it('returns false when not a 404', async () => { + const rawResponse = { + statusCode: 200, + headers: { 'x-elastic-product': 'Elasticsearch' }, + }; + + const result = isNotFoundFromUnsupportedServer(rawResponse); + expect(result).toBe(false); + }); +}); diff --git a/src/core/server/elasticsearch/supported_server_response_check.ts b/src/core/server/elasticsearch/supported_server_response_check.ts index 6fe812bc58518..85235d04caf5c 100644 --- a/src/core/server/elasticsearch/supported_server_response_check.ts +++ b/src/core/server/elasticsearch/supported_server_response_check.ts @@ -12,6 +12,22 @@ export const PRODUCT_RESPONSE_HEADER = 'x-elastic-product'; * @returns boolean */ // This check belongs to the elasticsearch service as a dedicated helper method. -export const isSupportedEsServer = (headers: Record | null) => { +export const isSupportedEsServer = (headers: Record | null) => { return !!headers && headers[PRODUCT_RESPONSE_HEADER] === 'Elasticsearch'; }; + +/** + * Check to ensure that a 404 response does not come from Elasticsearch + * + * WARNING: This is a hack to work around for 404 responses returned from a proxy. + * We're aiming to minimise the risk of data loss when consumers act on Not Found errors + * + * @param response response from elasticsearch client call + * @returns boolean 'true' if the status code is 404 and the Elasticsearch product header is missing/unexpected value + */ +export const isNotFoundFromUnsupportedServer = (args: { + statusCode: number | null; + headers: Record | null; +}): boolean => { + return args.statusCode === 404 && !isSupportedEsServer(args.headers); +}; diff --git a/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.ts b/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.ts index 45794e25d00a6..fe97208a6168d 100644 --- a/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.ts +++ b/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.test.ts @@ -25,6 +25,7 @@ import { savedObjectsPointInTimeFinderMock } from './point_in_time_finder.mock'; import { savedObjectsRepositoryMock } from './repository.mock'; import { PointInTimeFinder } from './point_in_time_finder'; import { ISavedObjectsRepository } from './repository'; +import { SavedObjectsErrorHelpers } from './errors'; const SPACES = ['default', 'another-space']; const VERSION_PROPS = { _seq_no: 1, _primary_term: 1 }; @@ -318,6 +319,23 @@ 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 () => { diff --git a/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.ts b/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.ts index b82dfec9467c3..7acbaaea1f5d7 100644 --- a/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.ts +++ b/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.ts @@ -7,11 +7,12 @@ */ 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 { @@ -198,6 +199,15 @@ 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(); for (let i = 0; i < bulkGetObjects.length; i++) { // For every element in bulkGetObjects, there should be a matching element in bulkGetResponse.body.docs diff --git a/src/core/server/saved_objects/service/lib/errors.ts b/src/core/server/saved_objects/service/lib/errors.ts index c1e1e9589b9ae..7412e744f19e7 100644 --- a/src/core/server/saved_objects/service/lib/errors.ts +++ b/src/core/server/saved_objects/service/lib/errors.ts @@ -203,7 +203,11 @@ export class SavedObjectsErrorHelpers { return isSavedObjectsClientError(error) && error[code] === CODE_GENERAL_ERROR; } - public static createGenericNotFoundEsUnavailableError(type: string, id: string) { + 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}`), diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index eead42db1ec58..efae5bd737020 100644 --- a/src/core/server/saved_objects/service/lib/repository.test.js +++ b/src/core/server/saved_objects/service/lib/repository.test.js @@ -652,6 +652,29 @@ describe('SavedObjectsRepository', () => { }); }; + const unsupportedProductBulkCreateMgetError = async (objects, options) => { + const multiNamespaceObjects = objects.filter( + ({ type, id }) => registry.isMultiNamespace(type) && id + ); + if (multiNamespaceObjects?.length) { + const response = getMockMgetResponse(multiNamespaceObjects, options?.namespace); + client.mget.mockResolvedValue( + elasticsearchClientMock.createSuccessTransportRequestPromise( + { ...response }, + { statusCode: 404 }, + {} + ) + ); + } + const response = getMockBulkCreateResponse(objects, options?.namespace); + client.bulk.mockResolvedValue( + elasticsearchClientMock.createSuccessTransportRequestPromise(response) + ); + await expect(savedObjectsRepository.bulkCreate(objects, options)).rejects.toThrowError( + createGenericNotFoundEsUnavailableError() + ); + }; + it(`throws when options.namespace is '*'`, async () => { await expect( savedObjectsRepository.bulkCreate([obj3], { namespace: ALL_NAMESPACES_STRING }) @@ -759,6 +782,13 @@ describe('SavedObjectsRepository', () => { const expectedErrorResult = { type: obj3.type, id: obj3.id, error: 'Oh no, a bulk error!' }; await bulkCreateError(obj3, true, expectedErrorResult); }); + + it(`throws when ES mget action returns 404 with missing Elasticsearch header`, async () => { + const objects = [obj1, { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE }]; + await unsupportedProductBulkCreateMgetError(objects); + expect(client.mget).toHaveBeenCalledTimes(1); + expect(client.bulk).toHaveBeenCalledTimes(0); + }); }); describe('migration', () => { @@ -1011,6 +1041,21 @@ describe('SavedObjectsRepository', () => { }); }; + const unsupportedProductBulkGetMgetError = async (objects, options) => { + const response = getMockMgetResponse(objects, options?.namespace); + client.mget.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise( + { ...response }, + { statusCode: 404 }, + {} + ) + ); + await expect(bulkGet(objects, options)).rejects.toThrowError( + createGenericNotFoundEsUnavailableError() + ); + expect(client.mget).toHaveBeenCalledTimes(1); + }; + it(`throws when options.namespace is '*'`, async () => { const obj = { type: 'dashboard', id: 'three' }; await expect( @@ -1046,6 +1091,12 @@ describe('SavedObjectsRepository', () => { response.docs[1].namespaces = ['bar-namespace']; await bulkGetErrorNotFound([obj1, obj, obj2], { namespace }, response); }); + + it(`throws when ES mget action responds with a 404 and a missing Elasticsearch product header`, async () => { + const getId = (type, id) => `${type}:${id}`; + await unsupportedProductBulkGetMgetError([obj1, obj2]); // returns 404 without required product header + _expectClientCallArgs([obj1, obj2], { getId }); + }); }); describe('returns', () => { @@ -1450,6 +1501,34 @@ describe('SavedObjectsRepository', () => { saved_objects: [expectSuccess(obj1), expectErrorNotFound(_obj), expectSuccess(obj2)], }); }; + const unsupportedProductBulkUpdateMgetError = async (objects, options, includeOriginId) => { + const multiNamespaceObjects = objects.filter(({ type }) => registry.isMultiNamespace(type)); + if (multiNamespaceObjects?.length) { + const response = getMockMgetResponse(multiNamespaceObjects, options?.namespace); + client.mget.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise( + { ...response }, + { statusCode: 404 }, + {} + ) + ); + } + const response = getMockBulkUpdateResponse(objects, options?.namespace, includeOriginId); + client.bulk.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise(response) + ); + + await expect(savedObjectsRepository.bulkUpdate(objects, options)).rejects.toThrowError( + createGenericNotFoundEsUnavailableError() + ); + expect(client.mget).toHaveBeenCalledTimes(multiNamespaceObjects?.length ? 1 : 0); + }; + + it(`throws when ES mget action responds with a 404 and a missing Elasticsearch product header`, async () => { + const objects = [obj1, { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE }]; + await unsupportedProductBulkUpdateMgetError(objects); + expect(client.mget).toHaveBeenCalledTimes(1); + }); it(`throws when options.namespace is '*'`, async () => { await expect( @@ -1651,6 +1730,24 @@ describe('SavedObjectsRepository', () => { savedObjectsRepository.checkConflicts([obj1], { namespace: ALL_NAMESPACES_STRING }) ).rejects.toThrowError(createBadRequestError('"options.namespace" cannot be "*"')); }); + + it(`throws when not found responses aren't from Elasticsearch`, async () => { + const checkConflictsMgetError = async (objects, options) => { + const response = getMockMgetResponse(objects, options?.namespace); + client.mget.mockResolvedValue( + elasticsearchClientMock.createSuccessTransportRequestPromise( + { ...response }, + { statusCode: 404 }, + {} + ) + ); + await expect(checkConflicts(objects, options)).rejects.toThrowError( + createGenericNotFoundEsUnavailableError() + ); + expect(client.mget).toHaveBeenCalledTimes(1); + }; + await checkConflictsMgetError([obj1, obj2], { namespace: 'default' }); + }); }); describe('returns', () => { @@ -2228,11 +2325,7 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise( - { found: false }, - undefined, - { 'x-elastic-product': 'Elasticsearch' } - ) + elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }, undefined) ); await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -2240,11 +2333,7 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the index during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise( - {}, - { statusCode: 404 }, - { 'x-elastic-product': 'Elasticsearch' } - ) + elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) ); await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -2252,14 +2341,18 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during get with missing Elasticsearch header`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }) + elasticsearchClientMock.createSuccessTransportRequestPromise( + { found: false }, + { statusCode: 404 }, + {} + ) ); await expectNotFoundEsUnavailableError(MULTI_NAMESPACE_ISOLATED_TYPE, id); }); it(`throws when ES is unable to find the index during get with missing Elasticsearch header`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }, {}) ); await expectNotFoundEsUnavailableError(MULTI_NAMESPACE_ISOLATED_TYPE, id); }); @@ -2305,7 +2398,11 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during delete`, async () => { client.delete.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'not_found' }) + elasticsearchClientMock.createSuccessTransportRequestPromise( + { result: 'not_found' }, + {}, + {} + ) ); await expectNotFoundEsUnavailableError(type, id); expect(client.delete).toHaveBeenCalledTimes(1); @@ -2313,9 +2410,13 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the index during delete`, async () => { client.delete.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ - error: { type: 'index_not_found_exception' }, - }) + elasticsearchClientMock.createSuccessTransportRequestPromise( + { + error: { type: 'index_not_found_exception' }, + }, + {}, + {} + ) ); await expectNotFoundEsUnavailableError(type, id); expect(client.delete).toHaveBeenCalledTimes(1); @@ -2572,6 +2673,22 @@ describe('SavedObjectsRepository', () => { savedObjectsRepository.removeReferencesTo(type, id, defaultOptions) ).rejects.toThrowError(createConflictError(type, id)); }); + + it(`throws on 404 with missing Elasticsearch header`, async () => { + client.updateByQuery.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise( + { + updated: updatedCount, + }, + { statusCode: 404 }, + {} + ) + ); + await expect( + savedObjectsRepository.removeReferencesTo(type, id, defaultOptions) + ).rejects.toThrowError(createGenericNotFoundEsUnavailableError(type, id)); + expect(client.updateByQuery).toHaveBeenCalledTimes(1); + }); }); }); @@ -2748,6 +2865,21 @@ describe('SavedObjectsRepository', () => { }); describe('errors', () => { + const findNotSupportedServerError = async (options, namespace) => { + const expectedSearchResults = generateSearchResults(namespace); + client.search.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise( + { ...expectedSearchResults }, + { statusCode: 404 }, + {} + ) + ); + await expect(savedObjectsRepository.find(options)).rejects.toThrowError( + createGenericNotFoundEsUnavailableError() + ); + expect(getSearchDslNS.getSearchDsl).toHaveBeenCalledTimes(1); + expect(client.search).toHaveBeenCalledTimes(1); + }; it(`throws when type is not defined`, async () => { await expect(savedObjectsRepository.find({})).rejects.toThrowError( 'options.type must be a string or an array of strings' @@ -2828,6 +2960,11 @@ describe('SavedObjectsRepository', () => { expect(getSearchDslNS.getSearchDsl).not.toHaveBeenCalled(); expect(client.search).not.toHaveBeenCalled(); }); + + it(`throws when ES is unable to find with missing Elasticsearch`, async () => { + await findNotSupportedServerError({ type }); + expect(client.search).toHaveBeenCalledTimes(1); + }); }); describe('returns', () => { @@ -3204,6 +3341,7 @@ describe('SavedObjectsRepository', () => { createGenericNotFoundEsUnavailableError(type, id) ); }; + it(`throws when options.namespace is '*'`, async () => { await expect( savedObjectsRepository.get(type, id, { namespace: ALL_NAMESPACES_STRING }) @@ -3222,11 +3360,7 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise( - { found: false }, - undefined, - { 'x-elastic-product': 'Elasticsearch' } - ) + elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }, undefined) ); await expectNotFoundError(type, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -3234,11 +3368,7 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the index during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise( - {}, - { statusCode: 404 }, - { 'x-elastic-product': 'Elasticsearch' } - ) + elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) ); await expectNotFoundError(type, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -3257,7 +3387,11 @@ describe('SavedObjectsRepository', () => { it(`throws when ES does not return the correct header when finding the document during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }) + elasticsearchClientMock.createSuccessTransportRequestPromise( + { found: false }, + undefined, + {} + ) ); await expectNotFoundEsUnavailableError(type, id); @@ -3367,8 +3501,7 @@ describe('SavedObjectsRepository', () => { client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise( { found: false }, - undefined, - { 'x-elastic-product': 'Elasticsearch' } + undefined ) // for actual target ); @@ -3421,10 +3554,16 @@ describe('SavedObjectsRepository', () => { describe('because alias is not used', () => { const expectExactMatchResult = async (aliasResult) => { const options = { namespace }; - client.update.mockResolvedValueOnce(aliasResult); // for alias object + if (!aliasResult.body) { + client.update.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise({}, { ...aliasResult }) + ); + } else { + client.update.mockResolvedValueOnce(aliasResult); // for alias object + } const response = getMockGetResponse({ type, id }, options.namespace); client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise(response) // for actual target + elasticsearchClientMock.createSuccessTransportRequestPromise({ ...response }) // for actual target ); const result = await savedObjectsRepository.resolve(type, id, options); @@ -3909,8 +4048,7 @@ describe('SavedObjectsRepository', () => { client.get.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise( { ...mockGetResponse }, - { statusCode: 200 }, - { 'x-elastic-product': 'Elasticsearch' } + { statusCode: 200 } ) ); } @@ -3932,8 +4070,7 @@ describe('SavedObjectsRepository', () => { }, }, }, - { statusCode: 200 }, - { 'x-elastic-product': 'Elasticsearch' } + { statusCode: 200 } ) ); const result = await savedObjectsRepository.update(type, id, attributes, options); @@ -4144,11 +4281,7 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise( - { found: false }, - undefined, - { 'x-elastic-product': 'Elasticsearch' } - ) + elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }, undefined) ); await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -4156,11 +4289,7 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the index during get`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise( - {}, - { statusCode: 404 }, - { 'x-elastic-product': 'Elasticsearch' } - ) + elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) ); await expectNotFoundError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -4168,15 +4297,19 @@ describe('SavedObjectsRepository', () => { it(`throws when ES is unable to find the document during get with missing Elasticsearch header`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false }) + elasticsearchClientMock.createSuccessTransportRequestPromise( + { found: false }, + { statusCode: 404 }, + {} + ) ); await expectNotFoundEsUnavailableError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); }); - it(`throws when ES is unable to find the index during get with missing Elasticsearch`, async () => { + it(`throws when ES is unable to find the index during get with missing Elasticsearch header`, async () => { client.get.mockResolvedValueOnce( - elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) + elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }, {}) ); await expectNotFoundEsUnavailableError(MULTI_NAMESPACE_ISOLATED_TYPE, id); expect(client.get).toHaveBeenCalledTimes(1); @@ -4303,6 +4436,21 @@ describe('SavedObjectsRepository', () => { ); }; + const unsupportedProductExpectNotFoundError = async (type, options) => { + const results = generateResults(); + client.openPointInTime.mockResolvedValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise( + { ...results }, + { statusCode: 404 }, + {} + ) + ); + await expect( + savedObjectsRepository.openPointInTimeForType(type, options) + ).rejects.toThrowError(createGenericNotFoundEsUnavailableError()); + expect(client.openPointInTime).toHaveBeenCalledTimes(1); + }; + it(`throws when ES is unable to find the index`, async () => { client.openPointInTime.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 }) @@ -4321,6 +4469,11 @@ describe('SavedObjectsRepository', () => { await test(HIDDEN_TYPE); await test(['unknownType', HIDDEN_TYPE]); }); + + it(`throws on 404 with missing Elasticsearch product header`, async () => { + await unsupportedProductExpectNotFoundError(type); + expect(client.openPointInTime).toHaveBeenCalledTimes(1); + }); }); describe('returns', () => { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 2e8e3189fae2d..365fc6a3734e4 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -14,7 +14,7 @@ import { REPOSITORY_RESOLVE_OUTCOME_STATS, } from '../../../core_usage_data'; import type { ElasticsearchClient } from '../../../elasticsearch/'; -import { isSupportedEsServer } from '../../../elasticsearch'; +import { isSupportedEsServer, isNotFoundFromUnsupportedServer } from '../../../elasticsearch'; import type { Logger } from '../../../logging'; import { getRootPropertiesObjects, IndexMapping } from '../../mappings'; import { @@ -335,11 +335,15 @@ export class SavedObjectsRepository { require_alias: true, }; - const { body } = + const { body, statusCode, headers } = id && overwrite ? await this.client.index(requestParams) : await this.client.create(requestParams); + // throw if we can't verify a 404 response is from Elasticsearch + if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(id, type); + } return this._rawToSavedObject({ ...raw, ...body, @@ -419,7 +423,16 @@ export class SavedObjectsRepository { { ignore: [404] } ) : undefined; - + // throw if we can't verify a 404 response is from Elasticsearch + if ( + bulkGetResponse && + isNotFoundFromUnsupportedServer({ + statusCode: bulkGetResponse.statusCode, + headers: bulkGetResponse.headers, + }) + ) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } let bulkRequestIndexCounter = 0; const bulkCreateParams: object[] = []; const expectedBulkResults: Either[] = expectedResults.map((expectedBulkGetResult) => { @@ -588,7 +601,16 @@ export class SavedObjectsRepository { { ignore: [404] } ) : undefined; - + // throw if we can't verify a 404 response is from Elasticsearch + if ( + bulkGetResponse && + isNotFoundFromUnsupportedServer({ + statusCode: bulkGetResponse.statusCode, + headers: bulkGetResponse.headers, + }) + ) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } const errors: SavedObjectsCheckConflictsResponse['errors'] = []; expectedBulkGetResults.forEach((expectedResult) => { if (isLeft(expectedResult)) { @@ -704,7 +726,7 @@ export class SavedObjectsRepository { const allTypes = Object.keys(getRootPropertiesObjects(this._mappings)); const typesToUpdate = allTypes.filter((type) => !this._registry.isNamespaceAgnostic(type)); - const { body } = await this.client.updateByQuery( + const { body, statusCode, headers } = await this.client.updateByQuery( { index: this.getIndicesForTypes(typesToUpdate), refresh: options.refresh, @@ -732,6 +754,10 @@ export class SavedObjectsRepository { }, { ignore: [404] } ); + // throw if we can't verify a 404 response is from Elasticsearch + if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } return body; } @@ -876,10 +902,16 @@ export class SavedObjectsRepository { }, }; - const { body, statusCode } = await this.client.search(esOptions, { - ignore: [404], - }); + const { body, statusCode, headers } = await this.client.search( + esOptions, + { + ignore: [404], + } + ); if (statusCode === 404) { + if (!isSupportedEsServer(headers)) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } // 404 is only possible here if the index is missing, which // we don't want to leak, see "404s from missing index" above return { @@ -975,7 +1007,16 @@ export class SavedObjectsRepository { { ignore: [404] } ) : undefined; - + // fail fast if we can't verify a 404 is from Elasticsearch + if ( + bulkGetResponse && + isNotFoundFromUnsupportedServer({ + statusCode: bulkGetResponse.statusCode, + headers: bulkGetResponse.headers, + }) + ) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } return { saved_objects: expectedBulkGetResults.map((expectedResult) => { if (isLeft(expectedResult)) { @@ -1099,7 +1140,18 @@ export class SavedObjectsRepository { }, { ignore: [404] } ); - + if ( + isNotFoundFromUnsupportedServer({ + statusCode: aliasResponse.statusCode, + headers: aliasResponse.headers, + }) + ) { + // throw if we cannot verify the response is from Elasticsearch + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError( + LEGACY_URL_ALIAS_TYPE, + rawAliasId + ); + } if ( aliasResponse.statusCode === 404 || aliasResponse.body.get?.found === false || @@ -1130,7 +1182,15 @@ export class SavedObjectsRepository { }, { ignore: [404] } ); - + // exit early if a 404 isn't from elasticsearch + if ( + isNotFoundFromUnsupportedServer({ + statusCode: bulkGetResponse.statusCode, + headers: bulkGetResponse.headers, + }) + ) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); + } const exactMatchDoc = bulkGetResponse?.body.docs[0]; const aliasMatchDoc = bulkGetResponse?.body.docs[1]; const foundExactMatch = @@ -1439,7 +1499,16 @@ export class SavedObjectsRepository { } ) : undefined; - + // fail fast if we can't verify a 404 response is from Elasticsearch + if ( + bulkGetResponse && + isNotFoundFromUnsupportedServer({ + statusCode: bulkGetResponse.statusCode, + headers: bulkGetResponse.headers, + }) + ) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } let bulkUpdateRequestIndexCounter = 0; const bulkUpdateParams: object[] = []; const expectedBulkUpdateResults: Either[] = expectedBulkGetResults.map( @@ -1580,7 +1649,7 @@ export class SavedObjectsRepository { // we need to target all SO indices as all types of objects may have references to the given SO. const targetIndices = this.getIndicesForTypes(allTypes); - const { body } = await this.client.updateByQuery( + const { body, statusCode, headers } = await this.client.updateByQuery( { index: targetIndices, refresh, @@ -1613,7 +1682,10 @@ export class SavedObjectsRepository { }, { ignore: [404] } ); - + // fail fast if we can't verify a 404 is from Elasticsearch + if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); + } if (body.failures?.length) { throw SavedObjectsErrorHelpers.createConflictError( type, @@ -1878,11 +1950,15 @@ export class SavedObjectsRepository { ...(preference ? { preference } : {}), }; - const { body, statusCode } = await this.client.openPointInTime(esOptions, { + const { body, statusCode, headers } = await this.client.openPointInTime(esOptions, { ignore: [404], }); if (statusCode === 404) { - throw SavedObjectsErrorHelpers.createGenericNotFoundError(); + if (!isSupportedEsServer(headers)) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } else { + throw SavedObjectsErrorHelpers.createGenericNotFoundError(); + } } return { @@ -2061,7 +2137,7 @@ export class SavedObjectsRepository { throw new Error(`Cannot make preflight get request for non-multi-namespace type '${type}'.`); } - const { body, statusCode } = await this.client.get( + const { body, statusCode, headers } = await this.client.get( { id: this._serializer.generateRawId(undefined, type, id), index: this.getIndexForType(type), @@ -2077,6 +2153,9 @@ export class SavedObjectsRepository { throw SavedObjectsErrorHelpers.createConflictError(type, id); } return getSavedObjectNamespaces(namespace, body); + } else if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { + // checking if the 404 is from Elasticsearch + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); } return getSavedObjectNamespaces(namespace); } @@ -2108,9 +2187,7 @@ export class SavedObjectsRepository { const indexFound = statusCode !== 404; // check if we have the elasticsearch header when index is not found and if we do, ensure it is Elasticsearch - const esServerSupported = isSupportedEsServer(headers); - - if (!isFoundGetResponse(body) && !esServerSupported) { + if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); } @@ -2135,6 +2212,7 @@ export class SavedObjectsRepository { return { saved_object: object, outcome: 'exactMatch' }; } catch (err) { if (SavedObjectsErrorHelpers.isNotFoundError(err)) { + // 404 responses already confirmed to be valid Elasticsearch responses await this.incrementResolveOutcomeStats(REPOSITORY_RESOLVE_OUTCOME_STATS.NOT_FOUND); } throw err; diff --git a/src/core/server/saved_objects/service/lib/update_objects_spaces.test.ts b/src/core/server/saved_objects/service/lib/update_objects_spaces.test.ts index 11dbe6149878c..ba15fbabfba6b 100644 --- a/src/core/server/saved_objects/service/lib/update_objects_spaces.test.ts +++ b/src/core/server/saved_objects/service/lib/update_objects_spaces.test.ts @@ -23,6 +23,7 @@ import type { UpdateObjectsSpacesParams, } from './update_objects_spaces'; import { updateObjectsSpaces } from './update_objects_spaces'; +import { SavedObjectsErrorHelpers } from './errors'; type SetupParams = Partial< Pick @@ -105,6 +106,32 @@ describe('#updateObjectsSpaces', () => { }) ); } + /** Mocks the saved objects client so as to test unsupported server responding with 404 */ + function mockMgetResultsNotFound(...results: Array<{ found: boolean }>) { + client.mget.mockReturnValueOnce( + elasticsearchClientMock.createSuccessTransportRequestPromise( + { + docs: results.map((x) => + x.found + ? { + _id: 'doesnt-matter', + _index: 'doesnt-matter', + _source: { namespaces: [EXISTING_SPACE] }, + ...VERSION_PROPS, + found: true, + } + : { + _id: 'doesnt-matter', + _index: 'doesnt-matter', + found: false, + } + ), + }, + { statusCode: 404 }, + {} + ) + ); + } /** Asserts that mget is called for the given objects */ function expectMgetArgs(...objects: SavedObjectsUpdateObjectsSpacesObject[]) { @@ -240,6 +267,17 @@ describe('#updateObjectsSpaces', () => { { ...obj7, spaces: [EXISTING_SPACE, 'foo-space'] }, ]); }); + + it('throws when mget not found response is missing the Elasticsearch header', async () => { + const objects = [{ type: SHAREABLE_OBJ_TYPE, id: 'id-1' }]; + const spacesToAdd = ['foo-space']; + const params = setup({ objects, spacesToAdd }); + mockMgetResultsNotFound({ found: true }); + + await expect(() => updateObjectsSpaces(params)).rejects.toThrowError( + SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError() + ); + }); }); // Note: these test cases do not include requested objects that will result in errors (those are covered above) diff --git a/src/core/server/saved_objects/service/lib/update_objects_spaces.ts b/src/core/server/saved_objects/service/lib/update_objects_spaces.ts index 3131d0240f96b..666b7b98b42e5 100644 --- a/src/core/server/saved_objects/service/lib/update_objects_spaces.ts +++ b/src/core/server/saved_objects/service/lib/update_objects_spaces.ts @@ -25,6 +25,7 @@ import { } from './internal_utils'; import { DEFAULT_REFRESH_SETTING } from './repository'; import type { RepositoryEsClient } from './repository_es_client'; +import { isNotFoundFromUnsupportedServer } from '../../../elasticsearch'; /** * An object that should have its spaces updated. @@ -190,6 +191,16 @@ export async function updateObjectsSpaces({ ) : undefined; + // fail fast if we can't verify a 404 response is from Elasticsearch + if ( + bulkGetResponse && + isNotFoundFromUnsupportedServer({ + statusCode: bulkGetResponse.statusCode, + headers: bulkGetResponse.headers, + }) + ) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } const time = new Date().toISOString(); let bulkOperationRequestIndexCounter = 0; const bulkOperationParams: estypes.BulkOperationContainer[] = []; diff --git a/src/core/server/server.api.md b/src/core/server/server.api.md index ac5fe9a5d8dbb..adbc06c92d45c 100644 --- a/src/core/server/server.api.md +++ b/src/core/server/server.api.md @@ -2528,7 +2528,7 @@ export class SavedObjectsErrorHelpers { // (undocumented) static createGenericNotFoundError(type?: string | null, id?: string | null): DecoratedError; // (undocumented) - static createGenericNotFoundEsUnavailableError(type: string, id: string): DecoratedError; + static createGenericNotFoundEsUnavailableError(type?: string | null, id?: string | null): DecoratedError; // (undocumented) static createIndexAliasNotFoundError(alias: string): DecoratedError; // (undocumented)