[Snapshot Restore] Fix error when deleting snapshots behind reverse p…

…roxy (elastic#66147)
alisonelizabeth · May 13, 2020 · 44df595 · 44df595
1 parent 29dcaaf
commit 44df595
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 28 deletions.
diff --git a/x-pack/plugins/snapshot_restore/public/application/services/http/snapshot_requests.ts b/x-pack/plugins/snapshot_restore/public/application/services/http/snapshot_requests.ts
@@ -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(

diff --git a/x-pack/plugins/snapshot_restore/server/routes/api/snapshots.test.ts b/x-pack/plugins/snapshot_restore/server/routes/api/snapshots.test.ts
@@ -210,14 +210,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 () => {

diff --git a/x-pack/plugins/snapshot_restore/server/routes/api/snapshots.ts b/x-pack/plugins/snapshot_restore/server/routes/api/snapshots.ts
@@ -169,17 +169,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[];
@@ -188,17 +190,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 }))

diff --git a/x-pack/plugins/snapshot_restore/server/services/license.ts b/x-pack/plugins/snapshot_restore/server/services/license.ts
@@ -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();