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

[Snapshot Restore] Fix error when deleting snapshots behind reverse proxy #66147

Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,9 @@ export const deleteSnapshots = async (
snapshotIds: Array<{ snapshot: string; repository: string }>
) => {
const result = await sendRequest({
path: `${API_BASE_PATH}snapshots/${snapshotIds
.map(({ snapshot, repository }) => encodeURIComponent(`${repository}/${snapshot}`))
.join(',')}`,
method: 'delete',
path: `${API_BASE_PATH}snapshots/bulk_delete`,
method: 'post',
body: snapshotIds,
});

uiMetricService.trackUiMetric(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,19 @@ describe('[Snapshot and Restore API Routes] Snapshots', () => {
});

describe('deleteHandler()', () => {
const ids = ['fooRepository/snapshot-1', 'barRepository/snapshot-2'];

const mockRequest: RequestMock = {
method: 'delete',
path: addBasePath('snapshots/{ids}'),
params: {
ids: ids.join(','),
},
method: 'post',
path: addBasePath('snapshots/bulk_delete'),
body: [
{
repository: 'fooRepository',
snapshot: 'snapshot-1',
},
{
repository: 'barRepository',
snapshot: 'snapshot-2',
},
],
};

it('should return successful ES responses', async () => {
Expand Down
28 changes: 13 additions & 15 deletions x-pack/plugins/snapshot_restore/server/routes/api/snapshots.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,17 +179,19 @@ export function registerSnapshotsRoutes({
})
);

const deleteParamsSchema = schema.object({
ids: schema.string(),
});
const deleteSchema = schema.arrayOf(
schema.object({
repository: schema.string(),
snapshot: schema.string(),
})
);

// DELETE one or multiple snapshots
router.delete(
{ path: addBasePath('snapshots/{ids}'), validate: { params: deleteParamsSchema } },
router.post(
{ path: addBasePath('snapshots/bulk_delete'), validate: { body: deleteSchema } },
license.guardApiRoute(async (ctx, req, res) => {
const { callAsCurrentUser } = ctx.snapshotRestore!.client;
const { ids } = req.params as TypeOf<typeof deleteParamsSchema>;
const snapshotIds = ids.split(',');

const response: {
itemsDeleted: Array<{ snapshot: string; repository: string }>;
errors: any[];
Expand All @@ -198,17 +200,13 @@ export function registerSnapshotsRoutes({
errors: [],
};

const snapshots = req.body;

try {
// We intentially perform deletion requests sequentially (blocking) instead of in parallel (non-blocking)
// because there can only be one snapshot deletion task performed at a time (ES restriction).
for (let i = 0; i < snapshotIds.length; i++) {
// IDs come in the format of `repository-name/snapshot-name`
// Extract the two parts by splitting at last occurrence of `/` in case
// repository name contains '/` (from older versions)
const id = snapshotIds[i];
const indexOfDivider = id.lastIndexOf('/');
const snapshot = id.substring(indexOfDivider + 1);
const repository = id.substring(0, indexOfDivider);
for (let i = 0; i < snapshots.length; i++) {
const { snapshot, repository } = snapshots[i];

await callAsCurrentUser('snapshot.delete', { snapshot, repository })
.then(() => response.itemsDeleted.push({ snapshot, repository }))
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/snapshot_restore/server/services/license.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ export class License {
});
}

guardApiRoute(handler: RequestHandler) {
guardApiRoute<P, Q, B>(handler: RequestHandler<P, Q, B>) {
const license = this;

return function licenseCheck(
ctx: RequestHandlerContext,
request: KibanaRequest,
request: KibanaRequest<P, Q, B>,
response: KibanaResponseFactory
) {
const licenseStatus = license.getStatus();
Expand Down