Adds header check to client.delete

elastic · Jul 29, 2021 · 2671098 · 2671098
1 parent e0886b2
commit 2671098
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 17 deletions.
diff --git a/src/core/server/saved_objects/service/lib/repository.test.js b/src/core/server/saved_objects/service/lib/repository.test.js
@@ -2309,7 +2309,7 @@ describe('SavedObjectsRepository', () => {
  client.delete.mockResolvedValueOnce(
  elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'not_found' })
  );
- await expectNotFoundError(type, id);
+ await expectNotFoundEsUnavailableError(type, id);
  expect(client.delete).toHaveBeenCalledTimes(1);
  });
 
@@ -2319,7 +2319,7 @@ describe('SavedObjectsRepository', () => {
  error: { type: 'index_not_found_exception' },
  })
  );
- await expectNotFoundError(type, id);
+ await expectNotFoundEsUnavailableError(type, id);
  expect(client.delete).toHaveBeenCalledTimes(1);
  });
 
@@ -3257,7 +3257,7 @@ describe('SavedObjectsRepository', () => {
  expect(client.get).toHaveBeenCalledTimes(1);
  });
 
- it.skip(`throws when ES does not return the correct header when finding the document during get`, async () => {
+ it(`throws when ES does not return the correct header when finding the document during get`, async () => {
  client.get.mockResolvedValueOnce(
  elasticsearchClientMock.createSuccessTransportRequestPromise({ found: false })
  );

diff --git a/src/core/server/saved_objects/service/lib/repository.ts b/src/core/server/saved_objects/service/lib/repository.ts
@@ -648,7 +648,7 @@ export class SavedObjectsRepository {
  }
  }
 
- const { body, statusCode } = await this.client.delete(
+ const { body, statusCode, headers } = await this.client.delete(
  {
  id: rawId,
  index: this.getIndexForType(type),
@@ -665,9 +665,15 @@ export class SavedObjectsRepository {
 
  const deleteDocNotFound = body.result === 'not_found';
  const deleteIndexNotFound = body.error && body.error.type === 'index_not_found_exception';
+ const esHeaderFound = isEsHeaderValid(headers);
  if (deleteDocNotFound || deleteIndexNotFound) {
- // see "404s from missing index" above
- throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
+ if (esHeaderFound) {
+ // see "404s from missing index" above
+ throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
+ } else {
+ // throw if we can't verify the response is from Elasticsearch
+ throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id);
+ }
  }
 
  throw new Error(
@@ -968,6 +974,11 @@ export class SavedObjectsRepository {
  )
  : undefined;
 
+ // // initial header check to ensure the response is from Elasticsearch
+ // if (bulkGetResponse && !isEsHeaderValid(bulkGetResponse.headers)) {
+ // throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError();
+ // }
+
  return {
  saved_objects: expectedBulkGetResults.map((expectedResult) => {
  if (isLeft(expectedResult)) {
@@ -1018,15 +1029,9 @@ export class SavedObjectsRepository {
  { ignore: [404] }
  );
  const indexNotFound = statusCode === 404;
- const esHeaderFound = isEsHeaderValid(headers);
  // check if we have the elasticsearch header when doc.found is not true (false, undefined or null) and if we do, ensure it is Elasticsearch
- if (!isFoundGetResponse(body) && !esHeaderFound) {
+ if (!isFoundGetResponse(body) && !isEsHeaderValid(headers)) {
  throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id);
- // const notFoundError = SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
- // throw SavedObjectsErrorHelpers.decorateEsUnavailableError(
- // new Error(`${notFoundError.message}`),
- // `x-elastic-product not present or not recognized` // alternative: cannot verify response from elasticsearch
- // );
  }
  if (
  !isFoundGetResponse(body) ||
@@ -1253,18 +1258,17 @@ export class SavedObjectsRepository {
  _source_includes: ['namespace', 'namespaces', 'originId'],
  require_alias: true,
  })
- // TODO: clean this up and make it generic
  .then((res) => {
  const indexNotFound = res.statusCode === 404;
  const esHeaderFound = isEsHeaderValid(res.headers);
- // check if we have the elasticsearch header when doc.found is not true (false, undefined or null) and if we do, ensure it is Elasticsearch
+ // check if we have the elasticsearch header when doc.found is not true (false, undefined or null) and if we do, ensure it is Elasticsearchq
  if (indexNotFound && !esHeaderFound) {
  throw SavedObjectsErrorHelpers.createGenericNotFoundEsUnavailableError(type, id);
  }
  return res;
  })
  .catch((err) => {
- if (SavedObjectsErrorHelpers.isEsUnavailableError(err)) {
+ if (SavedObjectsErrorHelpers.isNotFoundEsUnavailableError(err)) {
  throw err;
  }
  if (SavedObjectsErrorHelpers.isNotFoundError(err)) {
@@ -2088,7 +2092,7 @@ export class SavedObjectsRepository {
  * @param id The ID of the saved object.
  * @param namespace The target namespace.
  * @returns Raw document from Elasticsearch.
- * @throws Will throw an error if the saved object is not found, or if it doesn't include the target namespace.
+ * @throws Will throw an error if the saved object is not found, if it doesn't include the target namespace or if the response is not identifiable as an Elasticsearch response.
  */
  private async preflightCheckIncludesNamespace(type: string, id: string, namespace?: string) {
  if (!this._registry.isMultiNamespace(type)) {