From ccee9d2c3fb126407f2a164762290c867ce0e598 Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Sat, 14 Aug 2021 08:27:45 -0700 Subject: [PATCH 01/10] Improves not found response handling in the saved objects repository --- src/core/server/elasticsearch/client/mocks.ts | 4 +- .../saved_objects/service/lib/errors.ts | 6 +- .../service/lib/internal_utils.test.ts | 38 ++++++ .../service/lib/internal_utils.ts | 18 +++ .../saved_objects/service/lib/repository.ts | 118 +++++++++++++++--- 5 files changed, 162 insertions(+), 22 deletions(-) diff --git a/src/core/server/elasticsearch/client/mocks.ts b/src/core/server/elasticsearch/client/mocks.ts index 848d9c204bfbf..dc5dfc0e6410b 100644 --- a/src/core/server/elasticsearch/client/mocks.ts +++ b/src/core/server/elasticsearch/client/mocks.ts @@ -142,7 +142,7 @@ export type MockedTransportRequestPromise = TransportRequestPromise & { const createSuccessTransportRequestPromise = ( body: T, { statusCode = 200 }: { statusCode?: number } = {}, - headers?: Record + headers: Record = { 'x-elastic-product': 'Elasticsearch' } ): MockedTransportRequestPromise> => { const response = createApiResponse({ body, statusCode, headers }); const promise = Promise.resolve(response); @@ -163,7 +163,7 @@ function createApiResponse>( return { body: {} as any, statusCode: 200, - headers: {}, + headers: { 'x-elastic-product': 'Elasticsearch' }, warnings: [], meta: {} as any, ...opts, diff --git a/src/core/server/saved_objects/service/lib/errors.ts b/src/core/server/saved_objects/service/lib/errors.ts index c1e1e9589b9ae..92fbd78fe7b94 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 availablefor 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/internal_utils.test.ts b/src/core/server/saved_objects/service/lib/internal_utils.test.ts index d1fd067990f07..a1624071701e6 100644 --- a/src/core/server/saved_objects/service/lib/internal_utils.test.ts +++ b/src/core/server/saved_objects/service/lib/internal_utils.test.ts @@ -13,6 +13,7 @@ import { getBulkOperationError, getSavedObjectFromSource, rawDocExistsInNamespace, + isNotFoundFromUnsupportedServer, } from './internal_utils'; import { ALL_NAMESPACES_STRING } from './utils'; @@ -241,3 +242,40 @@ describe('#rawDocExistsInNamespace', () => { }); }); }); + +describe('#isNotFoundFromUnsupportedServer', () => { + it('returns true with not found response from unsupported server', () => { + const rawResponse = { + statusCode: 404, + headers: {}, + }; + + const result = isNotFoundFromUnsupportedServer(rawResponse); + expect(result).toBeTruthy(); + }); + + 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).toBeFalsy(); + }); + + it('returns false when not a 404', async () => { + const rawResponse = { + statusCode: 200, + headers: { 'x-elastic-product': 'Elasticsearch' }, + }; + + const result = isNotFoundFromUnsupportedServer(rawResponse); + expect(result).toBeFalsy(); + }); + + it('returns false when there is no response', async () => { + const result = isNotFoundFromUnsupportedServer(); + expect(result).toBeFalsy(); + }); +}); diff --git a/src/core/server/saved_objects/service/lib/internal_utils.ts b/src/core/server/saved_objects/service/lib/internal_utils.ts index feaaea15649c7..f892f46f187e9 100644 --- a/src/core/server/saved_objects/service/lib/internal_utils.ts +++ b/src/core/server/saved_objects/service/lib/internal_utils.ts @@ -7,12 +7,14 @@ */ import type { Payload } from '@hapi/boom'; +import { ApiResponse } from '@elastic/elasticsearch'; import type { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import type { SavedObjectsRawDoc, SavedObjectsRawDocSource } from '../../serialization'; import type { SavedObject } from '../../types'; import { decodeRequestVersion, encodeHitVersion } from '../../version'; import { SavedObjectsErrorHelpers } from './errors'; import { ALL_NAMESPACES_STRING, SavedObjectsUtils } from './utils'; +import { isSupportedEsServer } from '../../../elasticsearch'; /** * Checks the raw response of a bulk operation and returns an error if necessary. @@ -141,3 +143,19 @@ export function rawDocExistsInNamespace( namespaces?.includes(ALL_NAMESPACES_STRING); return existsInNamespace ?? false; } + +type NotFoundServerCheckResponse = Partial>; +/** + * 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 = ( + response?: NotFoundServerCheckResponse | undefined +) => { + return response && response.statusCode === 404 && !isSupportedEsServer(response.headers!); +}; diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 17b0f10ef67c8..1eb48c8255b8d 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -74,6 +74,7 @@ import { getExpectedVersionProperties, getSavedObjectFromSource, rawDocExistsInNamespace, + isNotFoundFromUnsupportedServer, } from './internal_utils'; import { ALL_NAMESPACES_STRING, @@ -335,11 +336,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); + // fail fast if we can't be sure a 404 response is from Elasticsearch + if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(id, type); + } return this._rawToSavedObject({ ...raw, ...body, @@ -419,7 +424,16 @@ export class SavedObjectsRepository { { ignore: [404] } ) : undefined; - + // fail fast if we can't be sure 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 +602,16 @@ export class SavedObjectsRepository { { ignore: [404] } ) : undefined; - + // fail fast if we can't be sure 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)) { @@ -703,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, @@ -731,6 +754,10 @@ export class SavedObjectsRepository { }, { ignore: [404] } ); + // fail fast if we can't be sure a 404 response is from Elasticsearch + if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } return body; } @@ -875,10 +902,17 @@ 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) { + // first need to ensure the 404 is a valid Elasticsearch response + 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 { @@ -974,7 +1008,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)) { @@ -1098,7 +1141,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 || @@ -1129,7 +1183,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 = @@ -1438,7 +1500,16 @@ export class SavedObjectsRepository { } ) : undefined; - + // fail fast if we can't verify a 404 response is from Elasticsearch + if ( + bulkGetResponse !== undefined && + isNotFoundFromUnsupportedServer({ + statusCode: bulkGetResponse.statusCode, + headers: bulkGetResponse.headers, + }) + ) { + throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); + } let bulkUpdateRequestIndexCounter = 0; const bulkUpdateParams: object[] = []; const expectedBulkUpdateResults: Either[] = expectedBulkGetResults.map( @@ -1579,7 +1650,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, @@ -1612,7 +1683,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, @@ -1877,11 +1951,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 { @@ -2060,7 +2138,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), @@ -2076,6 +2154,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); } @@ -2107,9 +2188,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); } @@ -2134,6 +2213,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; From 25d65a24c3d9cd30ee977ad308c91a446c74b13b Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Sat, 14 Aug 2021 11:19:40 -0700 Subject: [PATCH 02/10] Updates repository unit tests --- .../supported_server_response_check.ts | 2 +- ...collect_multi_namespace_references.test.ts | 18 ++ .../lib/collect_multi_namespace_references.ts | 7 +- .../service/lib/internal_utils.test.ts | 5 - .../service/lib/internal_utils.ts | 10 +- .../service/lib/repository.test.js | 249 ++++++++++++++---- .../saved_objects/service/lib/repository.ts | 22 +- .../service/lib/update_objects_spaces.test.ts | 38 +++ .../service/lib/update_objects_spaces.ts | 12 + 9 files changed, 292 insertions(+), 71 deletions(-) diff --git a/src/core/server/elasticsearch/supported_server_response_check.ts b/src/core/server/elasticsearch/supported_server_response_check.ts index 6fe812bc58518..54e4e2be0ab9c 100644 --- a/src/core/server/elasticsearch/supported_server_response_check.ts +++ b/src/core/server/elasticsearch/supported_server_response_check.ts @@ -12,6 +12,6 @@ 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'; }; 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..490641b9e5a25 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 { isSupportedEsServer } 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,10 @@ async function getObjectsAndReferences({ { body: { docs: makeBulkGetDocs(bulkGetObjects) } }, { ignore: [404] } ); + // exit early if we can't verify a 404 response is from Elasticsearch + if (bulkGetResponse.statusCode === 404 && !isSupportedEsServer(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/internal_utils.test.ts b/src/core/server/saved_objects/service/lib/internal_utils.test.ts index a1624071701e6..0d65850ca50e3 100644 --- a/src/core/server/saved_objects/service/lib/internal_utils.test.ts +++ b/src/core/server/saved_objects/service/lib/internal_utils.test.ts @@ -273,9 +273,4 @@ describe('#isNotFoundFromUnsupportedServer', () => { const result = isNotFoundFromUnsupportedServer(rawResponse); expect(result).toBeFalsy(); }); - - it('returns false when there is no response', async () => { - const result = isNotFoundFromUnsupportedServer(); - expect(result).toBeFalsy(); - }); }); diff --git a/src/core/server/saved_objects/service/lib/internal_utils.ts b/src/core/server/saved_objects/service/lib/internal_utils.ts index f892f46f187e9..420035c59b1ea 100644 --- a/src/core/server/saved_objects/service/lib/internal_utils.ts +++ b/src/core/server/saved_objects/service/lib/internal_utils.ts @@ -144,7 +144,6 @@ export function rawDocExistsInNamespace( return existsInNamespace ?? false; } -type NotFoundServerCheckResponse = Partial>; /** * Check to ensure that a 404 response does not come from Elasticsearch * @@ -154,8 +153,9 @@ type NotFoundServerCheckResponse = Partial { - return response && response.statusCode === 404 && !isSupportedEsServer(response.headers!); +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/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js index eead42db1ec58..00a7eaf13e2a2 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 bulkCreateMgetError = 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 bulkCreateMgetError(objects); + expect(client.mget).toHaveBeenCalledTimes(1); + expect(client.bulk).toHaveBeenCalledTimes(0); + }); }); describe('migration', () => { @@ -1011,6 +1041,21 @@ describe('SavedObjectsRepository', () => { }); }; + const bulkGetMgetError = 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 bulkGetMgetError([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 bulkUpdateMgetError = 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 bulkUpdateMgetError(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 expectNotFoundUnsupportedProductError = 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 expectNotFoundUnsupportedProductError(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 1eb48c8255b8d..b969b9800175e 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -341,7 +341,7 @@ export class SavedObjectsRepository { ? await this.client.index(requestParams) : await this.client.create(requestParams); - // fail fast if we can't be sure a 404 response is from Elasticsearch + // throw if we can't verify a 404 response is from Elasticsearch if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(id, type); } @@ -424,12 +424,12 @@ export class SavedObjectsRepository { { ignore: [404] } ) : undefined; - // fail fast if we can't be sure a 404 response is from Elasticsearch + // throw if we can't verify a 404 response is from Elasticsearch if ( bulkGetResponse && isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: bulkGetResponse.headers, + headers: { ...bulkGetResponse.headers }, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); @@ -602,12 +602,12 @@ export class SavedObjectsRepository { { ignore: [404] } ) : undefined; - // fail fast if we can't be sure a 404 response is from Elasticsearch + // throw if we can't verify a 404 response is from Elasticsearch if ( bulkGetResponse && isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: bulkGetResponse.headers, + headers: { ...bulkGetResponse.headers }, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); @@ -754,7 +754,7 @@ export class SavedObjectsRepository { }, { ignore: [404] } ); - // fail fast if we can't be sure a 404 response is from Elasticsearch + // throw if we can't verify a 404 response is from Elasticsearch if (isNotFoundFromUnsupportedServer({ statusCode, headers })) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); } @@ -1013,7 +1013,7 @@ export class SavedObjectsRepository { bulkGetResponse && isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: bulkGetResponse.headers, + headers: { ...bulkGetResponse.headers }, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); @@ -1144,7 +1144,7 @@ export class SavedObjectsRepository { if ( isNotFoundFromUnsupportedServer({ statusCode: aliasResponse.statusCode, - headers: aliasResponse.headers, + headers: { ...aliasResponse.headers }, }) ) { // throw if we cannot verify the response is from Elasticsearch @@ -1187,7 +1187,7 @@ export class SavedObjectsRepository { if ( isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: bulkGetResponse.headers, + headers: { ...bulkGetResponse.headers }, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); @@ -1502,10 +1502,10 @@ export class SavedObjectsRepository { : undefined; // fail fast if we can't verify a 404 response is from Elasticsearch if ( - bulkGetResponse !== undefined && + bulkGetResponse && isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: bulkGetResponse.headers, + headers: { ...bulkGetResponse.headers }, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); 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..05b71474e9def 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 @@ -8,6 +8,7 @@ import type { estypes } from '@elastic/elasticsearch'; import intersection from 'lodash/intersection'; +import { isSupportedEsServer } from 'src/core/server/elasticsearch'; import type { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import type { SavedObjectsRawDocSource, SavedObjectsSerializer } from '../../serialization'; @@ -22,6 +23,7 @@ import { getBulkOperationError, getExpectedVersionProperties, rawDocExistsInNamespace, + isNotFoundFromUnsupportedServer, } from './internal_utils'; import { DEFAULT_REFRESH_SETTING } from './repository'; import type { RepositoryEsClient } from './repository_es_client'; @@ -190,6 +192,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[] = []; From 05b91aea4e575060fd56ef8466ff0e564ad648ea Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Mon, 16 Aug 2021 07:47:24 -0700 Subject: [PATCH 03/10] removes unused types --- src/core/server/saved_objects/service/lib/internal_utils.ts | 1 - .../server/saved_objects/service/lib/update_objects_spaces.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/internal_utils.ts b/src/core/server/saved_objects/service/lib/internal_utils.ts index 420035c59b1ea..98243769af658 100644 --- a/src/core/server/saved_objects/service/lib/internal_utils.ts +++ b/src/core/server/saved_objects/service/lib/internal_utils.ts @@ -7,7 +7,6 @@ */ import type { Payload } from '@hapi/boom'; -import { ApiResponse } from '@elastic/elasticsearch'; import type { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import type { SavedObjectsRawDoc, SavedObjectsRawDocSource } from '../../serialization'; import type { SavedObject } from '../../types'; 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 05b71474e9def..ea33c9429ae9c 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 @@ -8,7 +8,6 @@ import type { estypes } from '@elastic/elasticsearch'; import intersection from 'lodash/intersection'; -import { isSupportedEsServer } from 'src/core/server/elasticsearch'; import type { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry'; import type { SavedObjectsRawDocSource, SavedObjectsSerializer } from '../../serialization'; From bb1dfa23448c97e956e69ffea3737c4279616e58 Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Mon, 16 Aug 2021 10:46:56 -0700 Subject: [PATCH 04/10] Uses healper function to validate response --- .../lib/collect_multi_namespace_references.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) 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 490641b9e5a25..c57f7f1f8020c 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,14 +7,17 @@ */ import * as esKuery from '@kbn/es-query'; -import { isSupportedEsServer } 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 { + getSavedObjectFromSource, + rawDocExistsInNamespace, + isNotFoundFromUnsupportedServer, +} from './internal_utils'; import type { ISavedObjectsPointInTimeFinder, SavedObjectsCreatePointInTimeFinderOptions, @@ -200,7 +203,12 @@ async function getObjectsAndReferences({ { ignore: [404] } ); // exit early if we can't verify a 404 response is from Elasticsearch - if (bulkGetResponse.statusCode === 404 && !isSupportedEsServer(bulkGetResponse.headers)) { + if ( + isNotFoundFromUnsupportedServer({ + statusCode: bulkGetResponse.statusCode, + headers: bulkGetResponse.headers, + }) + ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); } const newObjectsToGet = new Set(); From 01961662bd5e4290ced96631561c036d7073125a Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Mon, 16 Aug 2021 11:39:27 -0700 Subject: [PATCH 05/10] Updates API docs --- ...serrorhelpers.creategenericnotfoundesunavailableerror.md | 6 +++--- src/core/server/server.api.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) 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/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) From 6def7f8fd0547c384a3b7d66bdfabcd7be8b51f5 Mon Sep 17 00:00:00 2001 From: "Christiane (Tina) Heiligers" Date: Mon, 16 Aug 2021 11:50:00 -0700 Subject: [PATCH 06/10] Apply suggestions from code review --- src/core/server/saved_objects/service/lib/errors.ts | 2 +- src/core/server/saved_objects/service/lib/repository.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/server/saved_objects/service/lib/errors.ts b/src/core/server/saved_objects/service/lib/errors.ts index 92fbd78fe7b94..7412e744f19e7 100644 --- a/src/core/server/saved_objects/service/lib/errors.ts +++ b/src/core/server/saved_objects/service/lib/errors.ts @@ -204,7 +204,7 @@ export class SavedObjectsErrorHelpers { } public static createGenericNotFoundEsUnavailableError( - // type and id not availablefor all operations (e.g. mget) + // type and id not available in all operations (e.g. mget) type: string | null = null, id: string | null = null ) { diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index b969b9800175e..09a956f0f0a82 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -909,7 +909,6 @@ export class SavedObjectsRepository { } ); if (statusCode === 404) { - // first need to ensure the 404 is a valid Elasticsearch response if (!isSupportedEsServer(headers)) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); } From bfa095e5c66cb589ab10ab7a74ebe90329b82418 Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Mon, 16 Aug 2021 16:31:44 -0700 Subject: [PATCH 07/10] moves helper method to elasticsearch service --- src/core/server/elasticsearch/index.ts | 5 ++- .../supported_server_response_check.test.ts | 41 +++++++++++++++++++ .../supported_server_response_check.ts | 16 ++++++++ .../lib/collect_multi_namespace_references.ts | 7 +--- .../service/lib/internal_utils.test.ts | 33 --------------- .../service/lib/internal_utils.ts | 17 -------- .../saved_objects/service/lib/repository.ts | 3 +- .../service/lib/update_objects_spaces.ts | 2 +- 8 files changed, 65 insertions(+), 59 deletions(-) create mode 100644 src/core/server/elasticsearch/supported_server_response_check.test.ts diff --git a/src/core/server/elasticsearch/index.ts b/src/core/server/elasticsearch/index.ts index 62bb30452bb98..84a973eada8e5 100644 --- a/src/core/server/elasticsearch/index.ts +++ b/src/core/server/elasticsearch/index.ts @@ -38,4 +38,7 @@ export type { DeleteDocumentResponse, } from './client'; export { getRequestDebugMeta, getErrorMessage } from './client'; -export { isSupportedEsServer } from './supported_server_response_check'; +export { + isSupportedEsServer, + isNotFoundFromUnsupportedServer, +} 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 54e4e2be0ab9c..85235d04caf5c 100644 --- a/src/core/server/elasticsearch/supported_server_response_check.ts +++ b/src/core/server/elasticsearch/supported_server_response_check.ts @@ -15,3 +15,19 @@ export const PRODUCT_RESPONSE_HEADER = 'x-elastic-product'; 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.ts b/src/core/server/saved_objects/service/lib/collect_multi_namespace_references.ts index c57f7f1f8020c..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,17 +7,14 @@ */ 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, - isNotFoundFromUnsupportedServer, -} from './internal_utils'; +import { getSavedObjectFromSource, rawDocExistsInNamespace } from './internal_utils'; import type { ISavedObjectsPointInTimeFinder, SavedObjectsCreatePointInTimeFinderOptions, diff --git a/src/core/server/saved_objects/service/lib/internal_utils.test.ts b/src/core/server/saved_objects/service/lib/internal_utils.test.ts index 0d65850ca50e3..d1fd067990f07 100644 --- a/src/core/server/saved_objects/service/lib/internal_utils.test.ts +++ b/src/core/server/saved_objects/service/lib/internal_utils.test.ts @@ -13,7 +13,6 @@ import { getBulkOperationError, getSavedObjectFromSource, rawDocExistsInNamespace, - isNotFoundFromUnsupportedServer, } from './internal_utils'; import { ALL_NAMESPACES_STRING } from './utils'; @@ -242,35 +241,3 @@ describe('#rawDocExistsInNamespace', () => { }); }); }); - -describe('#isNotFoundFromUnsupportedServer', () => { - it('returns true with not found response from unsupported server', () => { - const rawResponse = { - statusCode: 404, - headers: {}, - }; - - const result = isNotFoundFromUnsupportedServer(rawResponse); - expect(result).toBeTruthy(); - }); - - 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).toBeFalsy(); - }); - - it('returns false when not a 404', async () => { - const rawResponse = { - statusCode: 200, - headers: { 'x-elastic-product': 'Elasticsearch' }, - }; - - const result = isNotFoundFromUnsupportedServer(rawResponse); - expect(result).toBeFalsy(); - }); -}); diff --git a/src/core/server/saved_objects/service/lib/internal_utils.ts b/src/core/server/saved_objects/service/lib/internal_utils.ts index 98243769af658..feaaea15649c7 100644 --- a/src/core/server/saved_objects/service/lib/internal_utils.ts +++ b/src/core/server/saved_objects/service/lib/internal_utils.ts @@ -13,7 +13,6 @@ import type { SavedObject } from '../../types'; import { decodeRequestVersion, encodeHitVersion } from '../../version'; import { SavedObjectsErrorHelpers } from './errors'; import { ALL_NAMESPACES_STRING, SavedObjectsUtils } from './utils'; -import { isSupportedEsServer } from '../../../elasticsearch'; /** * Checks the raw response of a bulk operation and returns an error if necessary. @@ -142,19 +141,3 @@ export function rawDocExistsInNamespace( namespaces?.includes(ALL_NAMESPACES_STRING); return existsInNamespace ?? false; } - -/** - * 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/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index 09a956f0f0a82..a12bb264718c4 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 { @@ -74,7 +74,6 @@ import { getExpectedVersionProperties, getSavedObjectFromSource, rawDocExistsInNamespace, - isNotFoundFromUnsupportedServer, } from './internal_utils'; import { ALL_NAMESPACES_STRING, 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 ea33c9429ae9c..c9652b60bfb85 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 @@ -22,10 +22,10 @@ import { getBulkOperationError, getExpectedVersionProperties, rawDocExistsInNamespace, - isNotFoundFromUnsupportedServer, } 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. From 3651a2ec170bb114208c846d01c0472bb5439275 Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Mon, 16 Aug 2021 16:50:24 -0700 Subject: [PATCH 08/10] Removes unnecessary header spread --- src/core/server/elasticsearch/client/mocks.ts | 5 +++-- .../saved_objects/service/lib/repository.test.js | 16 ++++++++-------- .../saved_objects/service/lib/repository.ts | 12 ++++++------ .../service/lib/update_objects_spaces.ts | 2 +- 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/core/server/elasticsearch/client/mocks.ts b/src/core/server/elasticsearch/client/mocks.ts index dc5dfc0e6410b..a0f18eb356827 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 = { 'x-elastic-product': 'Elasticsearch' } + 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: { 'x-elastic-product': 'Elasticsearch' }, + headers: { PRODUCT_RESPONSE_HEADER: 'Elasticsearch' }, warnings: [], meta: {} as any, ...opts, 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 00a7eaf13e2a2..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,7 +652,7 @@ describe('SavedObjectsRepository', () => { }); }; - const bulkCreateMgetError = async (objects, options) => { + const unsupportedProductBulkCreateMgetError = async (objects, options) => { const multiNamespaceObjects = objects.filter( ({ type, id }) => registry.isMultiNamespace(type) && id ); @@ -785,7 +785,7 @@ describe('SavedObjectsRepository', () => { it(`throws when ES mget action returns 404 with missing Elasticsearch header`, async () => { const objects = [obj1, { ...obj2, type: MULTI_NAMESPACE_ISOLATED_TYPE }]; - await bulkCreateMgetError(objects); + await unsupportedProductBulkCreateMgetError(objects); expect(client.mget).toHaveBeenCalledTimes(1); expect(client.bulk).toHaveBeenCalledTimes(0); }); @@ -1041,7 +1041,7 @@ describe('SavedObjectsRepository', () => { }); }; - const bulkGetMgetError = async (objects, options) => { + const unsupportedProductBulkGetMgetError = async (objects, options) => { const response = getMockMgetResponse(objects, options?.namespace); client.mget.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise( @@ -1094,7 +1094,7 @@ describe('SavedObjectsRepository', () => { it(`throws when ES mget action responds with a 404 and a missing Elasticsearch product header`, async () => { const getId = (type, id) => `${type}:${id}`; - await bulkGetMgetError([obj1, obj2]); // returns 404 without required product header + await unsupportedProductBulkGetMgetError([obj1, obj2]); // returns 404 without required product header _expectClientCallArgs([obj1, obj2], { getId }); }); }); @@ -1501,7 +1501,7 @@ describe('SavedObjectsRepository', () => { saved_objects: [expectSuccess(obj1), expectErrorNotFound(_obj), expectSuccess(obj2)], }); }; - const bulkUpdateMgetError = async (objects, options, includeOriginId) => { + const unsupportedProductBulkUpdateMgetError = async (objects, options, includeOriginId) => { const multiNamespaceObjects = objects.filter(({ type }) => registry.isMultiNamespace(type)); if (multiNamespaceObjects?.length) { const response = getMockMgetResponse(multiNamespaceObjects, options?.namespace); @@ -1526,7 +1526,7 @@ describe('SavedObjectsRepository', () => { 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 bulkUpdateMgetError(objects); + await unsupportedProductBulkUpdateMgetError(objects); expect(client.mget).toHaveBeenCalledTimes(1); }); @@ -4436,7 +4436,7 @@ describe('SavedObjectsRepository', () => { ); }; - const expectNotFoundUnsupportedProductError = async (type, options) => { + const unsupportedProductExpectNotFoundError = async (type, options) => { const results = generateResults(); client.openPointInTime.mockResolvedValueOnce( elasticsearchClientMock.createSuccessTransportRequestPromise( @@ -4471,7 +4471,7 @@ describe('SavedObjectsRepository', () => { }); it(`throws on 404 with missing Elasticsearch product header`, async () => { - await expectNotFoundUnsupportedProductError(type); + await unsupportedProductExpectNotFoundError(type); expect(client.openPointInTime).toHaveBeenCalledTimes(1); }); }); diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts index a12bb264718c4..deca7b47708d7 100644 --- a/src/core/server/saved_objects/service/lib/repository.ts +++ b/src/core/server/saved_objects/service/lib/repository.ts @@ -428,7 +428,7 @@ export class SavedObjectsRepository { bulkGetResponse && isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: { ...bulkGetResponse.headers }, + headers: bulkGetResponse.headers, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); @@ -606,7 +606,7 @@ export class SavedObjectsRepository { bulkGetResponse && isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: { ...bulkGetResponse.headers }, + headers: bulkGetResponse.headers, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); @@ -1011,7 +1011,7 @@ export class SavedObjectsRepository { bulkGetResponse && isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: { ...bulkGetResponse.headers }, + headers: bulkGetResponse.headers, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); @@ -1142,7 +1142,7 @@ export class SavedObjectsRepository { if ( isNotFoundFromUnsupportedServer({ statusCode: aliasResponse.statusCode, - headers: { ...aliasResponse.headers }, + headers: aliasResponse.headers, }) ) { // throw if we cannot verify the response is from Elasticsearch @@ -1185,7 +1185,7 @@ export class SavedObjectsRepository { if ( isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: { ...bulkGetResponse.headers }, + headers: bulkGetResponse.headers, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id); @@ -1503,7 +1503,7 @@ export class SavedObjectsRepository { bulkGetResponse && isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: { ...bulkGetResponse.headers }, + headers: bulkGetResponse.headers, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); 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 c9652b60bfb85..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 @@ -196,7 +196,7 @@ export async function updateObjectsSpaces({ bulkGetResponse && isNotFoundFromUnsupportedServer({ statusCode: bulkGetResponse.statusCode, - headers: { ...bulkGetResponse.headers }, + headers: bulkGetResponse.headers, }) ) { throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(); From 0cb74e06d5043bdcc7170aae0cee77fa88549140 Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Mon, 16 Aug 2021 17:12:21 -0700 Subject: [PATCH 09/10] can't use the const --- src/core/server/elasticsearch/client/mocks.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/server/elasticsearch/client/mocks.ts b/src/core/server/elasticsearch/client/mocks.ts index a0f18eb356827..dc5dfc0e6410b 100644 --- a/src/core/server/elasticsearch/client/mocks.ts +++ b/src/core/server/elasticsearch/client/mocks.ts @@ -11,7 +11,6 @@ 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 @@ -143,7 +142,7 @@ export type MockedTransportRequestPromise = TransportRequestPromise & { const createSuccessTransportRequestPromise = ( body: T, { statusCode = 200 }: { statusCode?: number } = {}, - headers: Record = { PRODUCT_RESPONSE_HEADER: 'Elasticsearch' } + headers: Record = { 'x-elastic-product': 'Elasticsearch' } ): MockedTransportRequestPromise> => { const response = createApiResponse({ body, statusCode, headers }); const promise = Promise.resolve(response); @@ -164,7 +163,7 @@ function createApiResponse>( return { body: {} as any, statusCode: 200, - headers: { PRODUCT_RESPONSE_HEADER: 'Elasticsearch' }, + headers: { 'x-elastic-product': 'Elasticsearch' }, warnings: [], meta: {} as any, ...opts, From 810fbdcf54f5fc3463743e226d3cdfcbedc76841 Mon Sep 17 00:00:00 2001 From: Christiane Heiligers Date: Tue, 17 Aug 2021 08:20:26 -0700 Subject: [PATCH 10/10] uses product response header in createApiResponse --- src/core/server/elasticsearch/client/mocks.ts | 5 +++-- src/core/server/elasticsearch/index.ts | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/server/elasticsearch/client/mocks.ts b/src/core/server/elasticsearch/client/mocks.ts index dc5dfc0e6410b..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 = { 'x-elastic-product': 'Elasticsearch' } + 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: { 'x-elastic-product': 'Elasticsearch' }, + 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 84a973eada8e5..f50e3a0f72860 100644 --- a/src/core/server/elasticsearch/index.ts +++ b/src/core/server/elasticsearch/index.ts @@ -41,4 +41,5 @@ export { getRequestDebugMeta, getErrorMessage } from './client'; export { isSupportedEsServer, isNotFoundFromUnsupportedServer, + PRODUCT_RESPONSE_HEADER, } from './supported_server_response_check';