Delete legacy URL aliases when objects are deleted or unshared #117056
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| /* | ||
| * 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. | ||
| */ | ||
|
|
||
| // 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() }; | ||
| }); | ||
|
|
||
| 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' }); | ||
|
|
||
| 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) | ||
| ); | ||
|
|
||
| expect(() => deleteLegacyUrlAliases(params)).rejects.toThrowError( | ||
| `Failed to delete legacy URL aliases for ${type}/${id}: es_type, es_reason` | ||
| ); | ||
| expect(params.client.updateByQuery).toHaveBeenCalledTimes(1); | ||
| }); | ||
|
|
||
| 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() | ||
| ); | ||
| }); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| /* | ||
| * 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 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: { | ||
| 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', | ||
| ...getSearchDsl(mappings, registry, { | ||
jportner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| type: LEGACY_URL_ALIAS_TYPE, | ||
| kueryNode, | ||
| }), | ||
| }, | ||
| }, | ||
| { ignore: [404] } | ||
| ); | ||
| } catch (err) { | ||
| const { error } = err.body; | ||
|
There was a problem hiding this comment. We're handling expected ES errors here, but an unexpected error may not have a There was a problem hiding this comment. Good call, I found this function that the core ES client module provides: kibana/src/core/server/elasticsearch/client/configure_client.ts Lines 83 to 93 in 2cd007e I'll reuse it here |
||
| throwError(type, id, `${error.type}, ${error.reason}`); | ||
| } | ||
| } | ||
|
|
||
| function throwError(type: string, id: string, message: string) { | ||
| throw new Error(`Failed to delete legacy URL aliases for ${type}/${id}: ${message}`); | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't even aware of that tbh...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, aliases are only created when objects in non-default spaces are converted, and I also designed
resolvewith that assumption in mind. Technically we could have chosen to support aliases in the default space (with adefault:prefix in the raw ID) but there wasn't any practical reason to do so.