Skip to content

Commit

Permalink
Delete legacy URL aliases when objects are deleted or unshared (elast…
Browse files Browse the repository at this point in the history
  • Loading branch information
jportner authored and kibanamachine committed Nov 4, 2021
1 parent a29bbc0 commit 967291e
Show file tree
Hide file tree
Showing 27 changed files with 895 additions and 55 deletions.
3 changes: 2 additions & 1 deletion src/core/server/saved_objects/object_types/registration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ const legacyUrlAliasType: SavedObjectsType = {
dynamic: false,
properties: {
sourceId: { type: 'keyword' },
targetType: { type: 'keyword' },
targetNamespace: { type: 'keyword' },
targetType: { type: 'keyword' },
targetId: { type: 'keyword' },
resolveCounter: { type: 'long' },
disabled: { type: 'boolean' },
// other properties exist, but we aren't querying or aggregating on those, so we don't need to specify them (because we use `dynamic: false` above)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
* Side Public License, v 1.
*/

import type { findLegacyUrlAliases } from './find_legacy_url_aliases';
import type { findLegacyUrlAliases } from './legacy_url_aliases';
import type * as InternalUtils from './internal_utils';

export const mockFindLegacyUrlAliases = jest.fn() as jest.MockedFunction<
typeof findLegacyUrlAliases
>;

jest.mock('./find_legacy_url_aliases', () => {
jest.mock('./legacy_url_aliases', () => {
return { findLegacyUrlAliases: mockFindLegacyUrlAliases };
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import type { ISavedObjectTypeRegistry } from '../../saved_objects_type_registry';
import type { SavedObjectsSerializer } from '../../serialization';
import type { SavedObject, SavedObjectsBaseOptions } from '../../types';
import { findLegacyUrlAliases } from './find_legacy_url_aliases';
import { findLegacyUrlAliases } from './legacy_url_aliases';
import { getRootFields } from './included_fields';
import {
getObjectKey,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* 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 type { getErrorMessage } from '../../../../elasticsearch';

export const mockGetEsErrorMessage = jest.fn() as jest.MockedFunction<typeof getErrorMessage>;

jest.mock('../../../../elasticsearch', () => {
return { getErrorMessage: mockGetEsErrorMessage };
});

// Mock these functions to return empty results, as this simplifies test cases and we don't need to exercise alternate code paths for these
jest.mock('@kbn/es-query', () => {
return { nodeTypes: { function: { buildNode: jest.fn() } } };
});
jest.mock('../search_dsl', () => {
return { getSearchDsl: jest.fn() };
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/*
* 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 { mockGetEsErrorMessage } from './delete_legacy_url_aliases.test.mock'; // Note: importing this file applies default mocks for other functions too

import { errors as EsErrors } from '@elastic/elasticsearch';

import { elasticsearchClientMock } from '../../../../elasticsearch/client/mocks';
import { typeRegistryMock } from '../../../saved_objects_type_registry.mock';
import { ALL_NAMESPACES_STRING } from '../utils';
import { deleteLegacyUrlAliases } from './delete_legacy_url_aliases';
import type { DeleteLegacyUrlAliasesParams } from './delete_legacy_url_aliases';

type SetupParams = Pick<
DeleteLegacyUrlAliasesParams,
'type' | 'id' | 'namespaces' | 'deleteBehavior'
>;

describe('deleteLegacyUrlAliases', () => {
function setup(setupParams: SetupParams) {
return {
mappings: { properties: {} }, // doesn't matter, only used as an argument to getSearchDsl which is mocked
registry: typeRegistryMock.create(), // doesn't matter, only used as an argument to getSearchDsl which is mocked
client: elasticsearchClientMock.createElasticsearchClient(),
getIndexForType: jest.fn(), // doesn't matter
...setupParams,
};
}

const type = 'obj-type';
const id = 'obj-id';

it('throws an error if namespaces includes the "all namespaces" string', async () => {
const namespaces = [ALL_NAMESPACES_STRING];
const params = setup({ type, id, namespaces, deleteBehavior: 'inclusive' });

await expect(() => deleteLegacyUrlAliases(params)).rejects.toThrowError(
`Failed to delete legacy URL aliases for ${type}/${id}: "namespaces" cannot include the * string`
);
expect(params.client.updateByQuery).not.toHaveBeenCalled();
});

it('throws an error if updateByQuery fails', async () => {
const namespaces = ['space-a', 'space-b'];
const params = setup({ type, id, namespaces, deleteBehavior: 'inclusive' });
const esError = new EsErrors.ResponseError(
elasticsearchClientMock.createApiResponse({
statusCode: 500,
body: { error: { type: 'es_type', reason: 'es_reason' } },
})
);
params.client.updateByQuery.mockResolvedValueOnce(
elasticsearchClientMock.createErrorTransportRequestPromise(esError)
);
mockGetEsErrorMessage.mockClear();
mockGetEsErrorMessage.mockReturnValue('Oh no!');

await expect(() => deleteLegacyUrlAliases(params)).rejects.toThrowError(
`Failed to delete legacy URL aliases for ${type}/${id}: Oh no!`
);
expect(params.client.updateByQuery).toHaveBeenCalledTimes(1);
expect(mockGetEsErrorMessage).toHaveBeenCalledTimes(1);
expect(mockGetEsErrorMessage).toHaveBeenCalledWith(esError);
});

describe('deleteBehavior "inclusive"', () => {
const deleteBehavior = 'inclusive' as const;

it('when filtered namespaces is not empty, returns early', async () => {
const namespaces = ['default'];
const params = setup({ type, id, namespaces, deleteBehavior });

await deleteLegacyUrlAliases(params);
expect(params.client.updateByQuery).not.toHaveBeenCalled();
});

it('when filtered namespaces is not empty, calls updateByQuery with expected script params', async () => {
const namespaces = ['space-a', 'default', 'space-b'];
const params = setup({ type, id, namespaces, deleteBehavior });

await deleteLegacyUrlAliases(params);
expect(params.client.updateByQuery).toHaveBeenCalledTimes(1);
expect(params.client.updateByQuery).toHaveBeenCalledWith(
expect.objectContaining({
body: expect.objectContaining({
script: expect.objectContaining({
params: {
namespaces: ['space-a', 'space-b'], // 'default' is filtered out
matchTargetNamespaceOp: 'delete',
notMatchTargetNamespaceOp: 'noop',
},
}),
}),
}),
expect.anything()
);
});
});

describe('deleteBehavior "exclusive"', () => {
const deleteBehavior = 'exclusive' as const;

it('when filtered namespaces is empty, calls updateByQuery with expected script params', async () => {
const namespaces = ['default'];
const params = setup({ type, id, namespaces, deleteBehavior });

await deleteLegacyUrlAliases(params);
expect(params.client.updateByQuery).toHaveBeenCalledTimes(1);
expect(params.client.updateByQuery).toHaveBeenCalledWith(
expect.objectContaining({
body: expect.objectContaining({
script: expect.objectContaining({
params: {
namespaces: [], // 'default' is filtered out
matchTargetNamespaceOp: 'noop',
notMatchTargetNamespaceOp: 'delete',
},
}),
}),
}),
expect.anything()
);
});

it('when filtered namespaces is not empty, calls updateByQuery with expected script params', async () => {
const namespaces = ['space-a', 'default', 'space-b'];
const params = setup({ type, id, namespaces, deleteBehavior });

await deleteLegacyUrlAliases(params);
expect(params.client.updateByQuery).toHaveBeenCalledTimes(1);
expect(params.client.updateByQuery).toHaveBeenCalledWith(
expect.objectContaining({
body: expect.objectContaining({
script: expect.objectContaining({
params: {
namespaces: ['space-a', 'space-b'], // 'default' is filtered out
matchTargetNamespaceOp: 'noop',
notMatchTargetNamespaceOp: 'delete',
},
}),
}),
}),
expect.anything()
);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* 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 * as esKuery from '@kbn/es-query';

import { getErrorMessage as getEsErrorMessage } from '../../../../elasticsearch';
import type { ISavedObjectTypeRegistry } from '../../../saved_objects_type_registry';
import type { IndexMapping } from '../../../mappings';
import { LEGACY_URL_ALIAS_TYPE } from '../../../object_types';
import type { RepositoryEsClient } from '../repository_es_client';
import { getSearchDsl } from '../search_dsl';
import { ALL_NAMESPACES_STRING, DEFAULT_NAMESPACE_STRING } from '../utils';

/** @internal */
export interface DeleteLegacyUrlAliasesParams {
mappings: IndexMapping;
registry: ISavedObjectTypeRegistry;
client: RepositoryEsClient;
getIndexForType: (type: string) => string;
/** The object type. */
type: string;
/** The object ID. */
id: string;
/**
* The namespaces to include or exclude when searching for legacy URL alias targets (depends on the `deleteBehavior` parameter).
* Note that using `namespaces: [], deleteBehavior: 'exclusive'` will delete all aliases for this object in all spaces.
*/
namespaces: string[];
/**
* If this is equal to 'inclusive', all aliases with a `targetNamespace` in the `namespaces` array will be deleted.
* If this is equal to 'exclusive', all aliases with a `targetNamespace` _not_ in the `namespaces` array will be deleted.
*/
deleteBehavior: 'inclusive' | 'exclusive';
}

/**
* Deletes legacy URL aliases that point to a given object.
*
* Note that aliases are only created when an object is converted to become share-capable, and each targetId is deterministically generated
* with uuidv5 -- this means that the chances of there actually being _multiple_ legacy URL aliases that target a given type/ID are slim to
* none. However, we don't always know exactly what space an alias could be in (if an object exists in multiple spaces, or in all spaces),
* so the most straightforward way for us to ensure that aliases are reliably deleted is to use updateByQuery, which is what this function
* does.
*
* @internal
*/
export async function deleteLegacyUrlAliases(params: DeleteLegacyUrlAliasesParams) {
const { mappings, registry, client, getIndexForType, type, id, namespaces, deleteBehavior } =
params;

if (namespaces.includes(ALL_NAMESPACES_STRING)) {
throwError(type, id, '"namespaces" cannot include the * string');
}

// Legacy URL aliases cannot exist in the default space; filter that out
const filteredNamespaces = namespaces.filter(
(namespace) => namespace !== DEFAULT_NAMESPACE_STRING
);
if (!filteredNamespaces.length && deleteBehavior === 'inclusive') {
// nothing to do, return early
return;
}

const { buildNode } = esKuery.nodeTypes.function;
const match1 = buildNode('is', `${LEGACY_URL_ALIAS_TYPE}.targetType`, type);
const match2 = buildNode('is', `${LEGACY_URL_ALIAS_TYPE}.targetId`, id);
const kueryNode = buildNode('and', [match1, match2]);

try {
await client.updateByQuery(
{
index: getIndexForType(LEGACY_URL_ALIAS_TYPE),
refresh: false, // This could be called many times in succession, intentionally do not wait for a refresh
body: {
...getSearchDsl(mappings, registry, {
type: LEGACY_URL_ALIAS_TYPE,
kueryNode,
}),
script: {
// Intentionally use one script source with variable params to take advantage of ES script caching
source: `
if (params['namespaces'].indexOf(ctx._source['${LEGACY_URL_ALIAS_TYPE}']['targetNamespace']) > -1) {
ctx.op = params['matchTargetNamespaceOp'];
} else {
ctx.op = params['notMatchTargetNamespaceOp'];
}
`,
params: {
namespaces: filteredNamespaces,
matchTargetNamespaceOp: deleteBehavior === 'inclusive' ? 'delete' : 'noop',
notMatchTargetNamespaceOp: deleteBehavior === 'inclusive' ? 'noop' : 'delete',
},
lang: 'painless',
},
conflicts: 'proceed',
},
},
{ ignore: [404] }
);
} catch (err) {
const errorMessage = getEsErrorMessage(err);
throwError(type, id, `${errorMessage}`);
}
}

function throwError(type: string, id: string, message: string) {
throw new Error(`Failed to delete legacy URL aliases for ${type}/${id}: ${message}`);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@

import type { DeeplyMockedKeys } from '@kbn/utility-types/jest';

import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../object_types';
import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../../object_types';
import type { CreatePointInTimeFinderFn, PointInTimeFinder } from '../point_in_time_finder';
import { savedObjectsPointInTimeFinderMock } from '../point_in_time_finder.mock';
import type { ISavedObjectsRepository } from '../repository';
import { savedObjectsRepositoryMock } from '../repository.mock';
import { findLegacyUrlAliases } from './find_legacy_url_aliases';
import type { CreatePointInTimeFinderFn, PointInTimeFinder } from './point_in_time_finder';
import { savedObjectsPointInTimeFinderMock } from './point_in_time_finder.mock';
import type { ISavedObjectsRepository } from './repository';
import { savedObjectsRepositoryMock } from './repository.mock';

describe('findLegacyUrlAliases', () => {
let savedObjectsMock: jest.Mocked<ISavedObjectsRepository>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
*/

import * as esKuery from '@kbn/es-query';
import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../object_types';
import { getObjectKey } from './internal_utils';
import type { CreatePointInTimeFinderFn } from './point_in_time_finder';
import { LegacyUrlAlias, LEGACY_URL_ALIAS_TYPE } from '../../../object_types';
import { getObjectKey } from '../internal_utils';
import type { CreatePointInTimeFinderFn } from '../point_in_time_finder';

interface FindLegacyUrlAliasesObject {
type: string;
Expand Down Expand Up @@ -68,6 +68,7 @@ export async function findLegacyUrlAliases(

function createAliasKueryFilter(objects: Array<{ type: string; id: string }>) {
const { buildNode } = esKuery.nodeTypes.function;
// Note: these nodes include '.attributes' for type-level fields because these are eventually passed to `validateConvertFilterToKueryNode`, which requires it
const kueryNodes = objects.reduce<unknown[]>((acc, { type, id }) => {
const match1 = buildNode('is', `${LEGACY_URL_ALIAS_TYPE}.attributes.targetType`, type);
const match2 = buildNode('is', `${LEGACY_URL_ALIAS_TYPE}.attributes.sourceId`, id);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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.
*/

export { findLegacyUrlAliases } from './find_legacy_url_aliases';

export { deleteLegacyUrlAliases } from './delete_legacy_url_aliases';
export type { DeleteLegacyUrlAliasesParams } from './delete_legacy_url_aliases';
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
* Side Public License, v 1.
*/

import type { findLegacyUrlAliases } from './find_legacy_url_aliases';
import type { findLegacyUrlAliases } from './legacy_url_aliases';
import type * as InternalUtils from './internal_utils';

export const mockFindLegacyUrlAliases = jest.fn() as jest.MockedFunction<
typeof findLegacyUrlAliases
>;

jest.mock('./find_legacy_url_aliases', () => {
jest.mock('./legacy_url_aliases', () => {
return { findLegacyUrlAliases: mockFindLegacyUrlAliases };
});

Expand Down
Loading

0 comments on commit 967291e

Please sign in to comment.