[6.x] Delete objects belonging to removed space (#23640) (#24033)

Backports the following commits to 6.x:
 - Delete objects belonging to removed space  (#23640)
This commit is contained in:
Larry Gregory 2018-10-15 19:05:57 +01:00 committed by GitHub
parent 2f7797b7b1
commit 4a4c1e5985
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 398 additions and 77 deletions

View file

@ -0,0 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`SavedObjectsRepository #deleteByNamespace requires namespace to be a string 1`] = `"namespace is required, and must be a string"`;
exports[`SavedObjectsRepository #deleteByNamespace requires namespace to be defined 1`] = `"namespace is required, and must be a string"`;

View file

@ -18,7 +18,7 @@
*/
import { omit } from 'lodash';
import { getRootType } from '../../../mappings';
import { getRootType, getRootPropertiesObjects } from '../../../mappings';
import { getSearchDsl } from './search_dsl';
import { includedFields } from './included_fields';
import { decorateEsError } from './decorate_es_error';
@ -245,6 +245,38 @@ export class SavedObjectsRepository {
);
}
/**
* Deletes all objects from the provided namespace.
*
* @param {string} namespace
* @returns {promise} - { took, timed_out, total, deleted, batches, version_conflicts, noops, retries, failures }
*/
async deleteByNamespace(namespace) {
if (!namespace || typeof namespace !== 'string') {
throw new TypeError(`namespace is required, and must be a string`);
}
const allTypes = Object.keys(getRootPropertiesObjects(this._mappings));
const typesToDelete = allTypes.filter(type => !this._schema.isNamespaceAgnostic(type));
const esOptions = {
index: this._index,
ignore: [404],
refresh: 'wait_for',
body: {
conflicts: 'proceed',
...getSearchDsl(this._mappings, this._schema, {
namespace,
type: typesToDelete,
})
}
};
return await this._writeToCluster('deleteByQuery', esOptions);
}
/**
* @param {object} [options={}]
* @property {(string|Array<string>)} [options.type]

View file

@ -162,6 +162,21 @@ describe('SavedObjectsRepository', () => {
}
};
const deleteByQueryResults = {
took: 27,
timed_out: false,
total: 23,
deleted: 23,
batches: 1,
version_conflicts: 0,
noops: 0,
retries: { bulk: 0, search: 0 },
throttled_millis: 0,
requests_per_second: -1,
throttled_until_millis: 0,
failures: []
};
const mappings = {
doc: {
properties: {
@ -171,6 +186,20 @@ describe('SavedObjectsRepository', () => {
type: 'keyword'
}
}
},
'dashboard': {
properties: {
otherField: {
type: 'keyword'
}
}
},
'globaltype': {
properties: {
yetAnotherField: {
type: 'keyword'
}
}
}
}
}
@ -708,6 +737,43 @@ describe('SavedObjectsRepository', () => {
});
});
describe('#deleteByNamespace', () => {
it('requires namespace to be defined', async () => {
callAdminCluster.returns(deleteByQueryResults);
expect(savedObjectsRepository.deleteByNamespace()).rejects.toThrowErrorMatchingSnapshot();
sinon.assert.notCalled(callAdminCluster);
sinon.assert.notCalled(onBeforeWrite);
});
it('requires namespace to be a string', async () => {
callAdminCluster.returns(deleteByQueryResults);
expect(savedObjectsRepository.deleteByNamespace(['namespace-1', 'namespace-2'])).rejects.toThrowErrorMatchingSnapshot();
sinon.assert.notCalled(callAdminCluster);
sinon.assert.notCalled(onBeforeWrite);
});
it('constructs a deleteByQuery call using all types that are namespace aware', async () => {
callAdminCluster.returns(deleteByQueryResults);
const result = await savedObjectsRepository.deleteByNamespace('my-namespace');
expect(result).toEqual(deleteByQueryResults);
sinon.assert.calledOnce(callAdminCluster);
sinon.assert.calledOnce(onBeforeWrite);
sinon.assert.calledWithExactly(getSearchDsl, mappings, schema, {
namespace: 'my-namespace',
type: ['index-pattern', 'dashboard']
});
sinon.assert.calledWithExactly(callAdminCluster, 'deleteByQuery', {
body: { conflicts: 'proceed' },
ignore: [404],
index: '.kibana-test',
refresh: 'wait_for'
});
});
});
describe('#find', () => {
it('waits until migrations are complete before proceeding', async () => {
migrator.awaitMigration = sinon.spy(async () => sinon.assert.notCalled(callAdminCluster));

View file

@ -2,52 +2,92 @@
exports[`ConfirmDeleteModal renders as expected 1`] = `
<EuiOverlayMask>
<EuiConfirmModal
buttonColor="danger"
cancelButtonText="Cancel"
confirmButtonText="Delete space"
defaultFocusedButton="cancel"
onCancel={[MockFunction]}
onConfirm={[Function]}
title="Delete space 'My Space'"
<EuiModal
className="euiModal--confirmation"
maxWidth={true}
onClose={[MockFunction]}
>
<p>
Deleting a space permanently removes the space and all of its contents. You can't undo this action.
</p>
<EuiFormRow
describedByIds={Array []}
error="Space names do not match."
fullWidth={false}
hasEmptyLabelSpace={false}
isInvalid={false}
label="Confirm space name"
>
<EuiFieldText
compressed={false}
fullWidth={false}
isLoading={false}
onChange={[Function]}
value=""
/>
</EuiFormRow>
<EuiCallOut
color="warning"
size="m"
>
<EuiModalHeader>
<EuiModalHeaderTitle
data-test-subj="confirmModalTitleText"
>
Delete space
'My Space'
</EuiModalHeaderTitle>
</EuiModalHeader>
<EuiModalBody>
<EuiText
data-test-subj="confirmModalBodyText"
grow={true}
>
You are about to delete your current space
<span>
(
<p>
Deleting a space permanently removes the space and
<strong>
My Space
all of its contents
</strong>
)
</span>
. You will be redirected to choose a different space if you continue.
. You can't undo this action.
</p>
<EuiFormRow
describedByIds={Array []}
error="Space names do not match."
fullWidth={false}
hasEmptyLabelSpace={false}
isInvalid={false}
label="Confirm space name"
>
<EuiFieldText
compressed={false}
disabled={false}
fullWidth={false}
isLoading={false}
onChange={[Function]}
value=""
/>
</EuiFormRow>
<EuiCallOut
color="warning"
size="m"
>
<EuiText
grow={true}
>
You are about to delete your current space
<span>
(
<strong>
My Space
</strong>
)
</span>
. You will be redirected to choose a different space if you continue.
</EuiText>
</EuiCallOut>
</EuiText>
</EuiCallOut>
</EuiConfirmModal>
</EuiModalBody>
<EuiModalFooter>
<EuiButtonEmpty
color="primary"
data-test-subj="confirmModalCancelButton"
iconSide="left"
isDisabled={false}
onClick={[MockFunction]}
type="button"
>
Cancel
</EuiButtonEmpty>
<EuiButton
color="danger"
data-test-subj="confirmModalConfirmButton"
fill={true}
iconSide="left"
isLoading={false}
onClick={[Function]}
type="button"
>
Delete space and all contents
</EuiButton>
</EuiModalFooter>
</EuiModal>
</EuiOverlayMask>
`;

View file

@ -5,11 +5,18 @@
*/
import {
EuiButton,
EuiButtonEmpty,
EuiCallOut,
// @ts-ignore
EuiConfirmModal,
EuiFieldText,
EuiFormRow,
EuiModal,
EuiModalBody,
EuiModalFooter,
EuiModalHeader,
EuiModalHeaderTitle,
EuiOverlayMask,
EuiText,
} from '@elastic/eui';
@ -29,12 +36,14 @@ interface Props {
interface State {
confirmSpaceName: string;
error: boolean | null;
deleteInProgress: boolean;
}
export class ConfirmDeleteModal extends Component<Props, State> {
public state = {
confirmSpaceName: '',
error: null,
deleteInProgress: false,
};
public render() {
@ -57,32 +66,59 @@ export class ConfirmDeleteModal extends Component<Props, State> {
);
}
// This is largely the same as the built-in EuiConfirmModal component, but we needed the ability
// to disable the buttons since this could be a long-running operation
return (
<EuiOverlayMask>
<EuiConfirmModal
buttonColor={'danger'}
cancelButtonText={'Cancel'}
confirmButtonText={'Delete space'}
onCancel={onCancel}
onConfirm={this.onConfirm}
title={`Delete space '${space.name}'`}
defaultFocusedButton={'cancel'}
>
<p>
Deleting a space permanently removes the space and all of its contents. You can't undo
this action.
</p>
<EuiModal onClose={onCancel} className={'euiModal--confirmation'}>
<EuiModalHeader>
<EuiModalHeaderTitle data-test-subj="confirmModalTitleText">
Delete space {`'${space.name}'`}
</EuiModalHeaderTitle>
</EuiModalHeader>
<EuiModalBody>
<EuiText data-test-subj="confirmModalBodyText">
<p>
Deleting a space permanently removes the space and{' '}
<strong>all of its contents</strong>. You can't undo this action.
</p>
<EuiFormRow
label={'Confirm space name'}
isInvalid={!!this.state.error}
error={'Space names do not match.'}
>
<EuiFieldText value={this.state.confirmSpaceName} onChange={this.onSpaceNameChange} />
</EuiFormRow>
<EuiFormRow
label={'Confirm space name'}
isInvalid={!!this.state.error}
error={'Space names do not match.'}
>
<EuiFieldText
value={this.state.confirmSpaceName}
onChange={this.onSpaceNameChange}
disabled={this.state.deleteInProgress}
/>
</EuiFormRow>
{warning}
</EuiConfirmModal>
{warning}
</EuiText>
</EuiModalBody>
<EuiModalFooter>
<EuiButtonEmpty
data-test-subj="confirmModalCancelButton"
onClick={onCancel}
isDisabled={this.state.deleteInProgress}
>
Cancel
</EuiButtonEmpty>
<EuiButton
data-test-subj="confirmModalConfirmButton"
onClick={this.onConfirm}
fill
color={'danger'}
isLoading={this.state.deleteInProgress}
>
Delete space and all contents
</EuiButton>
</EuiModalFooter>
</EuiModal>
</EuiOverlayMask>
);
}
@ -105,7 +141,16 @@ export class ConfirmDeleteModal extends Component<Props, State> {
const needsRedirect = isDeletingCurrentSpace(this.props.space, this.props.spacesNavState);
const spacesManager = this.props.spacesManager;
this.setState({
deleteInProgress: true,
});
await this.props.onConfirm();
this.setState({
deleteInProgress: false,
});
if (needsRedirect) {
spacesManager.redirectToSpaceSelector();
}

View file

@ -1072,7 +1072,9 @@ describe('#delete', () => {
const mockCallWithRequestRepository = {
get: jest.fn().mockReturnValue(notReservedSavedObject),
delete: jest.fn(),
deleteByNamespace: jest.fn(),
};
const request = Symbol();
const client = new SpacesClient(
@ -1088,6 +1090,7 @@ describe('#delete', () => {
expect(mockCallWithRequestRepository.get).toHaveBeenCalledWith('space', id);
expect(mockCallWithRequestRepository.delete).toHaveBeenCalledWith('space', id);
expect(mockCallWithRequestRepository.deleteByNamespace).toHaveBeenCalledWith(id);
expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0);
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0);
});
@ -1127,7 +1130,9 @@ describe('#delete', () => {
const mockCallWithRequestRepository = {
get: jest.fn().mockReturnValue(notReservedSavedObject),
delete: jest.fn(),
deleteByNamespace: jest.fn(),
};
const request = Symbol();
const client = new SpacesClient(
@ -1144,6 +1149,7 @@ describe('#delete', () => {
expect(mockAuthorization.mode.useRbacForRequest).toHaveBeenCalledWith(request);
expect(mockCallWithRequestRepository.get).toHaveBeenCalledWith('space', id);
expect(mockCallWithRequestRepository.delete).toHaveBeenCalledWith('space', id);
expect(mockCallWithRequestRepository.deleteByNamespace).toHaveBeenCalledWith(id);
expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0);
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledTimes(0);
});
@ -1226,7 +1232,9 @@ describe('#delete', () => {
const mockInternalRepository = {
get: jest.fn().mockReturnValue(notReservedSavedObject),
delete: jest.fn(),
deleteByNamespace: jest.fn(),
};
const request = Symbol();
const client = new SpacesClient(
mockAuditLogger as any,
@ -1246,6 +1254,7 @@ describe('#delete', () => {
);
expect(mockInternalRepository.get).toHaveBeenCalledWith('space', id);
expect(mockInternalRepository.delete).toHaveBeenCalledWith('space', id);
expect(mockInternalRepository.deleteByNamespace).toHaveBeenCalledWith(id);
expect(mockAuditLogger.spacesAuthorizationFailure).toHaveBeenCalledTimes(0);
expect(mockAuditLogger.spacesAuthorizationSuccess).toHaveBeenCalledWith(username, 'delete');
});

View file

@ -156,6 +156,8 @@ export class SpacesClient {
}
await repository.delete('space', id);
await repository.deleteByNamespace(id);
}
private useRbac(): boolean {

View file

@ -118,6 +118,7 @@ export function createTestHandler(initApiFn: (server: any, preCheckLicenseImpl:
delete: jest.fn((type: string, id: string) => {
return {};
}),
deleteByNamespace: jest.fn(),
};
server.savedObjects = {

View file

@ -49,3 +49,56 @@
}
}
}
{
"type": "doc",
"value": {
"index": ".kibana",
"type": "doc",
"id": "space_2:dashboard:my_dashboard",
"source": {
"type": "dashboard",
"updated_at": "2017-09-21T18:49:16.270Z",
"namespace": "space_2",
"dashboard": {
"description": "Space 2",
"title": "This is the second test space"
}
}
}
}
{
"type": "doc",
"value": {
"index": ".kibana",
"type": "doc",
"id": "space_1:dashboard:my_dashboard",
"source": {
"type": "dashboard",
"updated_at": "2017-09-21T18:49:16.270Z",
"namespace": "space_1",
"dashboard": {
"description": "Space 1",
"title": "This is the second test space"
}
}
}
}
{
"type": "doc",
"value": {
"index": ".kibana",
"type": "doc",
"id": "dashboard:my_dashboard",
"source": {
"type": "dashboard",
"updated_at": "2017-09-21T18:49:16.270Z",
"dashboard": {
"description": "Default Space",
"title": "This is the default test space"
}
}
}
}

View file

@ -55,6 +55,9 @@
}
}
},
"namespace": {
"type": "keyword"
},
"dashboard": {
"properties": {
"description": {
@ -305,4 +308,4 @@
},
"aliases": {}
}
}
}

View file

@ -11,7 +11,7 @@ export const createUsersAndRoles = async (es: any, supertest: SuperTest<any>) =>
elasticsearch: {
indices: [
{
names: ['.kibana'],
names: ['.kibana*'],
privileges: ['manage', 'read', 'index', 'delete'],
},
],
@ -22,7 +22,7 @@ export const createUsersAndRoles = async (es: any, supertest: SuperTest<any>) =>
elasticsearch: {
indices: [
{
names: ['.kibana'],
names: ['.kibana*'],
privileges: ['read', 'view_index_metadata'],
},
],
@ -33,7 +33,7 @@ export const createUsersAndRoles = async (es: any, supertest: SuperTest<any>) =>
elasticsearch: {
indices: [
{
names: ['.kibana'],
names: ['.kibana*'],
privileges: ['manage', 'read', 'index', 'delete'],
},
],
@ -47,7 +47,7 @@ export const createUsersAndRoles = async (es: any, supertest: SuperTest<any>) =>
elasticsearch: {
indices: [
{
names: ['.kibana'],
names: ['.kibana*'],
privileges: ['read', 'view_index_metadata'],
},
],

View file

@ -25,7 +25,7 @@ interface DeleteTestDefinition {
tests: DeleteTests;
}
export function deleteTestSuiteFactory(esArchiver: any, supertest: SuperTest<any>) {
export function deleteTestSuiteFactory(es: any, esArchiver: any, supertest: SuperTest<any>) {
const createExpectLegacyForbidden = (username: string, action: string) => (resp: {
[key: string]: any;
}) => {
@ -40,8 +40,75 @@ export function deleteTestSuiteFactory(esArchiver: any, supertest: SuperTest<any
expect(resp.body).to.eql(expectedResult);
};
const expectEmptyResult = (resp: { [key: string]: any }) => {
const expectEmptyResult = async (resp: { [key: string]: any }) => {
expect(resp.body).to.eql('');
// Query ES to ensure that we deleted everything we expected, and nothing we didn't
// Grouping first by namespace, then by saved object type
const response = await es.search({
index: '.kibana',
body: {
size: 0,
aggs: {
count: {
terms: {
field: 'namespace',
missing: 'default',
size: 10,
},
aggs: {
countByType: {
terms: {
field: 'type',
missing: 'UNKNOWN',
size: 10,
},
},
},
},
},
},
});
const buckets = response.aggregations.count.buckets;
// Space 2 deleted, all others should exist
const expectedBuckets = [
{
key: 'default',
doc_count: 3,
countByType: {
doc_count_error_upper_bound: 0,
sum_other_doc_count: 0,
buckets: [
{
key: 'space',
doc_count: 2,
},
{
key: 'dashboard',
doc_count: 1,
},
],
},
},
{
doc_count: 1,
key: 'space_1',
countByType: {
doc_count_error_upper_bound: 0,
sum_other_doc_count: 0,
buckets: [
{
key: 'dashboard',
doc_count: 1,
},
],
},
},
];
expect(buckets).to.eql(expectedBuckets);
};
const expectNotFound = (resp: { [key: string]: any }) => {

View file

@ -13,6 +13,7 @@ import { deleteTestSuiteFactory } from '../../common/suites/delete';
export default function deleteSpaceTestSuite({ getService }: TestInvoker) {
const supertestWithoutAuth = getService('supertestWithoutAuth');
const esArchiver = getService('esArchiver');
const es = getService('es');
const {
deleteTest,
@ -21,7 +22,7 @@ export default function deleteSpaceTestSuite({ getService }: TestInvoker) {
expectEmptyResult,
expectNotFound,
expectReservedSpaceResult,
} = deleteTestSuiteFactory(esArchiver, supertestWithoutAuth);
} = deleteTestSuiteFactory(es, esArchiver, supertestWithoutAuth);
describe('delete', () => {
[

View file

@ -3,11 +3,7 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { createUsersAndRoles } from '../../common/lib/create_users_and_roles';
import { TestInvoker } from '../../common/lib/types';

View file

@ -12,13 +12,14 @@ import { deleteTestSuiteFactory } from '../../common/suites/delete';
export default function deleteSpaceTestSuite({ getService }: TestInvoker) {
const supertestWithoutAuth = getService('supertestWithoutAuth');
const esArchiver = getService('esArchiver');
const es = getService('es');
const {
deleteTest,
expectEmptyResult,
expectReservedSpaceResult,
expectNotFound,
} = deleteTestSuiteFactory(esArchiver, supertestWithoutAuth);
} = deleteTestSuiteFactory(es, esArchiver, supertestWithoutAuth);
describe('delete', () => {
[