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

[8.0] Delete legacy URL aliases when objects are deleted or unshared (#117056) #117461

Merged
merged 1 commit into from
Nov 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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