Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improves not found response handling in the saved objects repository #108749

Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
<b>Signature:</b>

```typescript
static createGenericNotFoundEsUnavailableError(type: string, id: string): DecoratedError;
static createGenericNotFoundEsUnavailableError(type?: string | null, id?: string | null): DecoratedError;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| type | <code>string</code> | |
| id | <code>string</code> | |
| type | <code>string &#124; null</code> | |
| id | <code>string &#124; null</code> | |

<b>Returns:</b>

Expand Down
5 changes: 3 additions & 2 deletions src/core/server/elasticsearch/client/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<unknown>
Expand Down Expand Up @@ -142,7 +143,7 @@ export type MockedTransportRequestPromise<T> = TransportRequestPromise<T> & {
const createSuccessTransportRequestPromise = <T>(
body: T,
{ statusCode = 200 }: { statusCode?: number } = {},
headers?: Record<string, string | string[]>
headers: Record<string, string | string[]> = { [PRODUCT_RESPONSE_HEADER]: 'Elasticsearch' }
): MockedTransportRequestPromise<ApiResponse<T>> => {
const response = createApiResponse({ body, statusCode, headers });
const promise = Promise.resolve(response);
Expand All @@ -163,7 +164,7 @@ function createApiResponse<TResponse = Record<string, any>>(
return {
body: {} as any,
statusCode: 200,
headers: {},
headers: { [PRODUCT_RESPONSE_HEADER]: 'Elasticsearch' },
warnings: [],
meta: {} as any,
...opts,
Expand Down
6 changes: 5 additions & 1 deletion src/core/server/elasticsearch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Original file line number Diff line number Diff line change
@@ -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);
});
});
18 changes: 17 additions & 1 deletion src/core/server/elasticsearch/supported_server_response_check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> | null) => {
export const isSupportedEsServer = (headers: Record<string, string | string[]> | 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<string, string | string[]> | null;
}): boolean => {
return args.statusCode === 404 && !isSupportedEsServer(args.headers);
};
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down Expand Up @@ -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 () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<string>();
for (let i = 0; i < bulkGetObjects.length; i++) {
// For every element in bulkGetObjects, there should be a matching element in bulkGetResponse.body.docs
Expand Down
6 changes: 5 additions & 1 deletion src/core/server/saved_objects/service/lib/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`),
Expand Down
Loading