Update saved object delete to always delete the object

This method will no longer update a multi-namespace object to
remove it from its current namespace. Instead, it will always
delete the object, even if that object exists in multiple
namespaces.
Adds a new `force` option that is required if the object does exist
in multiple namespaces.

docs/development/core/server/kibana-plugin-core-server.savedobjectsdeleteoptions.force.md

@@ -0,0 +1,13 @@
+<!-- Do not edit this file. It is automatically generated by API Documenter. -->
+
+[Home](./index.md) &gt; [kibana-plugin-core-server](./kibana-plugin-core-server.md) &gt; [SavedObjectsDeleteOptions](./kibana-plugin-core-server.savedobjectsdeleteoptions.md) &gt; [force](./kibana-plugin-core-server.savedobjectsdeleteoptions.force.md)
+
+## SavedObjectsDeleteOptions.force property
+
+Force deletion of an object that exists in multiple namespaces
+
+<b>Signature:</b>
+
+```typescript
+force?: boolean;
+```

docs/development/core/server/kibana-plugin-core-server.savedobjectsdeleteoptions.md

@@ -15,5 +15,6 @@ export interface SavedObjectsDeleteOptions extends SavedObjectsBaseOptions
 
 | Property | Type | Description |
 | --- | --- | --- |
+| [force](./kibana-plugin-core-server.savedobjectsdeleteoptions.force.md) | <code>boolean</code> | Force deletion of an object that exists in multiple namespaces |
 | [refresh](./kibana-plugin-core-server.savedobjectsdeleteoptions.refresh.md) | <code>MutatingOperationRefreshSetting</code> | The Elasticsearch Refresh setting for this operation |
 

src/core/server/saved_objects/routes/delete.ts

@@ -29,11 +29,15 @@ export const registerDeleteRoute = (router: IRouter) => {
  type: schema.string(),
  id: schema.string(),
  }),
+ query: schema.object({
+ force: schema.maybe(schema.boolean()),
+ }),
  },
  },
  router.handleLegacyErrors(async (context, req, res) => {
  const { type, id } = req.params;
- const result = await context.core.savedObjects.client.delete(type, id);
+ const { force } = req.query;
+ const result = await context.core.savedObjects.client.delete(type, id, { force });
  return res.ok({ body: result });
  })
  );

src/core/server/saved_objects/service/lib/repository.test.js

@@ -1974,31 +1974,17 @@ describe('SavedObjectsRepository', () => {
  describe('client calls', () => {
  it(`should use the ES delete action when not using a multi-namespace type`, async () => {
  await deleteSuccess(type, id);
+ expect(client.get).not.toHaveBeenCalled();
  expect(client.delete).toHaveBeenCalledTimes(1);
  });
 
- it(`should use ES get action then delete action when using a multi-namespace type with no namespaces remaining`, async () => {
+ it(`should use ES get action then delete action when using a multi-namespace type`, async () => {
  await deleteSuccess(MULTI_NAMESPACE_TYPE, id);
  expect(client.get).toHaveBeenCalledTimes(1);
  expect(client.delete).toHaveBeenCalledTimes(1);
  });
 
- it(`should use ES get action then update action when using a multi-namespace type with one or more namespaces remaining`, async () => {
- const mockResponse = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id });
- mockResponse._source.namespaces = ['default', 'some-other-nameespace'];
- client.get.mockResolvedValueOnce(
- elasticsearchClientMock.createSuccessTransportRequestPromise(mockResponse)
- );
- client.update.mockResolvedValueOnce(
- elasticsearchClientMock.createSuccessTransportRequestPromise({ result: 'updated' })
- );
-
- await savedObjectsRepository.delete(MULTI_NAMESPACE_TYPE, id);
- expect(client.get).toHaveBeenCalledTimes(1);
- expect(client.update).toHaveBeenCalledTimes(1);
- });
-
- it(`includes the version of the existing document when type is multi-namespace`, async () => {
+ it(`includes the version of the existing document when using a multi-namespace type`, async () => {
  await deleteSuccess(MULTI_NAMESPACE_TYPE, id);
  const versionProperties = {
  if_seq_no: mockVersionProps._seq_no,
@@ -2092,19 +2078,18 @@ describe('SavedObjectsRepository', () => {
  expect(client.get).toHaveBeenCalledTimes(1);
  });
 
- it(`throws when ES is unable to find the document during update`, async () => {
- const mockResponse = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id });
- mockResponse._source.namespaces = ['default', 'some-other-nameespace'];
+ it(`throws when the type is multi-namespace and the document has multiple namespaces and the force option is not enabled`, async () => {
+ const response = getMockGetResponse({ type: MULTI_NAMESPACE_TYPE, id, namespace });
+ response._source.namespaces = [namespace, 'bar-namespace'];
  client.get.mockResolvedValueOnce(
- elasticsearchClientMock.createSuccessTransportRequestPromise(mockResponse)
+ elasticsearchClientMock.createSuccessTransportRequestPromise(response)
  );
- client.update.mockResolvedValueOnce(
- elasticsearchClientMock.createSuccessTransportRequestPromise({}, { statusCode: 404 })
+ await expect(
+ savedObjectsRepository.delete(MULTI_NAMESPACE_TYPE, id, { namespace })
+ ).rejects.toThrowError(
+ 'Unable to delete saved object that exists in multiple namespaces, use `force` option to delete it anyway'
  );
-
- await expectNotFoundError(MULTI_NAMESPACE_TYPE, id);
  expect(client.get).toHaveBeenCalledTimes(1);
- expect(client.update).toHaveBeenCalledTimes(1);
  });
 
  it(`throws when ES is unable to find the document during delete`, async () => {

src/core/server/saved_objects/service/lib/repository.ts

@@ -551,45 +551,18 @@ export class SavedObjectsRepository {
  throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
  }
 
- const { namespace, refresh = DEFAULT_REFRESH_SETTING } = options;
+ const { namespace, refresh = DEFAULT_REFRESH_SETTING, force } = options;
 
  const rawId = this._serializer.generateRawId(namespace, type, id);
  let preflightResult: SavedObjectsRawDoc | undefined;
 
  if (this._registry.isMultiNamespace(type)) {
  preflightResult = await this.preflightCheckIncludesNamespace(type, id, namespace);
- const existingNamespaces = getSavedObjectNamespaces(undefined, preflightResult);
- const remainingNamespaces = existingNamespaces?.filter(
- (x) => x !== getNamespaceString(namespace)
- );
-
- if (remainingNamespaces?.length) {
- // if there is 1 or more namespace remaining, update the saved object
- const time = this._getCurrentTime();
-
- const doc = {
- updated_at: time,
- namespaces: remainingNamespaces,
- };
-
- const { statusCode } = await this.client.update(
- {
- id: rawId,
- index: this.getIndexForType(type),
- ...getExpectedVersionProperties(undefined, preflightResult),
- refresh,
- body: {
- doc,
- },
- },
- { ignore: [404] }
+ const existingNamespaces = getSavedObjectNamespaces(undefined, preflightResult) ?? [];
+ if (!force && existingNamespaces.length > 1) {
+ throw SavedObjectsErrorHelpers.createBadRequestError(
+ 'Unable to delete saved object that exists in multiple namespaces, use `force` option to delete it anyway'
  );
-
- if (statusCode === 404) {
- // see "404s from missing index" above
- throw SavedObjectsErrorHelpers.createGenericNotFoundError(type, id);
- }
- return {};
  }
  }
 

src/core/server/saved_objects/service/saved_objects_client.ts

@@ -204,6 +204,8 @@ export interface SavedObjectsBulkUpdateOptions extends SavedObjectsBaseOptions {
 export interface SavedObjectsDeleteOptions extends SavedObjectsBaseOptions {
  /** The Elasticsearch Refresh setting for this operation */
  refresh?: MutatingOperationRefreshSetting;
+ /** Force deletion of an object that exists in multiple namespaces */
+ force?: boolean;
 }
 
 /**

src/core/server/server.api.md

@@ -2226,6 +2226,7 @@ export interface SavedObjectsDeleteFromNamespacesResponse {
 
 // @public (undocumented)
 export interface SavedObjectsDeleteOptions extends SavedObjectsBaseOptions {
+ force?: boolean;
  refresh?: MutatingOperationRefreshSetting;
 }
 

x-pack/test/saved_object_api_integration/common/suites/delete.ts

@@ -8,25 +8,26 @@ import { SuperTest } from 'supertest';
 import expect from '@kbn/expect/expect.js';
 import { SAVED_OBJECT_TEST_CASES as CASES } from '../lib/saved_object_test_cases';
 import { SPACES } from '../lib/spaces';
-import {
- createRequest,
- expectResponses,
- getUrlPrefix,
- getTestTitle,
-} from '../lib/saved_object_test_utils';
+import { expectResponses, getUrlPrefix, getTestTitle } from '../lib/saved_object_test_utils';
 import { ExpectResponseBody, TestCase, TestDefinition, TestSuite } from '../lib/types';
 
 export interface DeleteTestDefinition extends TestDefinition {
- request: { type: string; id: string };
+ request: { type: string; id: string; force?: boolean };
 }
 export type DeleteTestSuite = TestSuite<DeleteTestDefinition>;
 export interface DeleteTestCase extends TestCase {
+ force?: boolean;
  failure?: 403 | 404;
 }
 
 const DOES_NOT_EXIST = Object.freeze({ type: 'dashboard', id: 'does-not-exist' });
 export const TEST_CASES = Object.freeze({ ...CASES, DOES_NOT_EXIST });
 
+/**
+ * Test cases have additional properties that we don't want to send in HTTP Requests
+ */
+const createRequest = ({ type, id, force }: DeleteTestCase) => ({ type, id, force });
+
 export function deleteTestSuiteFactory(esArchiver: any, supertest: SuperTest<any>) {
  const expectForbidden = expectResponses.forbiddenTypes('delete');
  const expectResponseBody = (testCase: DeleteTestCase): ExpectResponseBody => async (
@@ -78,9 +79,10 @@ export function deleteTestSuiteFactory(esArchiver: any, supertest: SuperTest<any
 
  for (const test of tests) {
  it(`should return ${test.responseStatusCode} ${test.title}`, async () => {
- const { type, id } = test.request;
+ const { type, id, force } = test.request;
  await supertest
  .delete(`${getUrlPrefix(spaceId)}/api/saved_objects/${type}/${id}`)
+ .query({ ...(force && { force }) })
  .auth(user?.username, user?.password)
  .expect(test.responseStatusCode)
  .then(test.responseBody);

x-pack/test/saved_object_api_integration/security_and_spaces/apis/delete.ts

@@ -19,7 +19,7 @@ const {
  SPACE_1: { spaceId: SPACE_1_ID },
  SPACE_2: { spaceId: SPACE_2_ID },
 } = SPACES;
-const { fail404 } = testCaseFailures;
+const { fail400, fail404 } = testCaseFailures;
 
 const createTestCases = (spaceId: string) => {
  // for each permitted (non-403) outcome, if failure !== undefined then we expect
@@ -30,6 +30,13 @@ const createTestCases = (spaceId: string) => {
  { ...CASES.SINGLE_NAMESPACE_SPACE_2, ...fail404(spaceId !== SPACE_2_ID) },
  {
  ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1,
+ ...fail400(spaceId === DEFAULT_SPACE_ID || spaceId === SPACE_1_ID),
+ ...fail404(spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID),
+ },
+ // try to delete this object again, this time using the `force` option
+ {
+ ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1,
+ force: true,
  ...fail404(spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID),
  },
  { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail404(spaceId !== SPACE_1_ID) },

x-pack/test/saved_object_api_integration/security_only/apis/delete.ts

@@ -13,7 +13,7 @@ import {
  DeleteTestDefinition,
 } from '../../common/suites/delete';
 
-const { fail404 } = testCaseFailures;
+const { fail400, fail404 } = testCaseFailures;
 
 const createTestCases = () => {
  // for each permitted (non-403) outcome, if failure !== undefined then we expect
@@ -22,7 +22,9 @@ const createTestCases = () => {
  CASES.SINGLE_NAMESPACE_DEFAULT_SPACE,
  { ...CASES.SINGLE_NAMESPACE_SPACE_1, ...fail404() },
  { ...CASES.SINGLE_NAMESPACE_SPACE_2, ...fail404() },
- CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1,
+ { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, ...fail400() },
+ // try to delete this object again, this time using the `force` option
+ { ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1, force: true },
  { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail404() },
  { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_2, ...fail404() },
  CASES.NAMESPACE_AGNOSTIC,

x-pack/test/saved_object_api_integration/spaces_only/apis/delete.ts

@@ -14,7 +14,7 @@ const {
  SPACE_1: { spaceId: SPACE_1_ID },
  SPACE_2: { spaceId: SPACE_2_ID },
 } = SPACES;
-const { fail404 } = testCaseFailures;
+const { fail400, fail404 } = testCaseFailures;
 
 const createTestCases = (spaceId: string) => [
  // for each outcome, if failure !== undefined then we expect to receive
@@ -24,6 +24,13 @@ const createTestCases = (spaceId: string) => [
  { ...CASES.SINGLE_NAMESPACE_SPACE_2, ...fail404(spaceId !== SPACE_2_ID) },
  {
  ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1,
+ ...fail400(spaceId === DEFAULT_SPACE_ID || spaceId === SPACE_1_ID),
+ ...fail404(spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID),
+ },
+ // try to delete this object again, this time using the `force` option
+ {
+ ...CASES.MULTI_NAMESPACE_DEFAULT_AND_SPACE_1,
+ force: true,
  ...fail404(spaceId !== DEFAULT_SPACE_ID && spaceId !== SPACE_1_ID),
  },
  { ...CASES.MULTI_NAMESPACE_ONLY_SPACE_1, ...fail404(spaceId !== SPACE_1_ID) },