[7.0] Update Delete Remote Cluster API to support multiple comma-delimited clusters. (#34595)

x-pack/plugins/remote_clusters/public/store/actions/remove_clusters.js

@@ -20,49 +20,75 @@ import { closeDetailPanel } from './detail_panel';
 import { refreshClusters } from './refresh_clusters';
 import { getDetailPanelClusterName } from '../selectors';
 
+function getErrorTitle(count, name = null) {
+  if (count === 1) {
+    if (name) {
+      return i18n.translate('xpack.remoteClusters.removeAction.errorSingleNotificationTitle', {
+        defaultMessage: `Error removing remote cluster '{name}'`,
+        values: { name },
+      });
+    }
+  } else {
+    return i18n.translate('xpack.remoteClusters.removeAction.errorMultipleNotificationTitle', {
+      defaultMessage: `Error removing '{count}' remote clusters`,
+      values: { count },
+    });
+  }
+}
+
 export const removeClusters = (names) => async (dispatch, getState) => {
   dispatch({
     type: REMOVE_CLUSTERS_START,
   });
 
-  const removalSuccesses = [];
-  const removalErrors = [];
-  const removeClusterRequests = names.map(name => {
-    sendRemoveClusterRequest(name)
-      .then(() => removalSuccesses.push(name))
-      .catch(() => removalErrors.push(name));
-  });
+  let itemsDeleted = [];
+  let errors = [];
 
   await Promise.all([
-    ...removeClusterRequests,
-    // Wait at least half a second to avoid a weird flicker of the saving feedback.
+    sendRemoveClusterRequest(names.join(','))
+      .then((response) => {
+        ({ itemsDeleted, errors } = response.data);
+      }),
+    // Wait at least half a second to avoid a weird flicker of the saving feedback (only visible
+    // when requests resolve very quickly).
     new Promise(resolve => setTimeout(resolve, 500)),
-  ]);
+  ]).catch(error => {
+    const errorTitle = getErrorTitle(names.length, names[0]);
+    toastNotifications.addDanger({
+      title: errorTitle,
+      text: error.data.message,
+    });
+  });
 
-  if(removalErrors.length > 0) {
-    if (removalErrors.length === 1) {
-      toastNotifications.addDanger(i18n.translate('xpack.remoteClusters.removeAction.errorSingleNotificationTitle', {
-        defaultMessage: `Error removing remote cluster '{name}'`,
-        values: { name: removalErrors[0] },
-      }));
-    } else {
-      toastNotifications.addDanger(i18n.translate('xpack.remoteClusters.removeAction.errorMultipleNotificationTitle', {
-        defaultMessage: `Error removing '{count}' remote clusters`,
-        values: { count: removalErrors.length },
-      }));
-    }
+  if (errors.length > 0) {
+    const {
+      name,
+      error: {
+        output: {
+          payload: {
+            message,
+          },
+        },
+      },
+    } = errors[0];
+
+    const title = getErrorTitle(errors.length, name);
+    toastNotifications.addDanger({
+      title,
+      text: message,
+    });
   }
 
-  if(removalSuccesses.length > 0) {
-    if (removalSuccesses.length === 1) {
+  if (itemsDeleted.length > 0) {
+    if (itemsDeleted.length === 1) {
       toastNotifications.addSuccess(i18n.translate('xpack.remoteClusters.removeAction.successSingleNotificationTitle', {
         defaultMessage: `Remote cluster '{name}' was removed`,
-        values: { name: removalSuccesses[0] },
+        values: { name: itemsDeleted[0] },
       }));
     } else {
       toastNotifications.addSuccess(i18n.translate('xpack.remoteClusters.removeAction.successMultipleNotificationTitle', {
         defaultMessage: '{count} remote clusters were removed',
-        values: { count: names.length },
+        values: { count: itemsDeleted.length },
       }));
     }
   }

x-pack/plugins/remote_clusters/server/routes/api/remote_clusters/register_delete_route.js

@@ -18,45 +18,83 @@ export function registerDeleteRoute(server) {
   const licensePreRouting = licensePreRoutingFactory(server);
 
   server.route({
-    path: '/api/remote_clusters/{name}',
+    path: '/api/remote_clusters/{nameOrNames}',
     method: 'DELETE',
+    config: {
+      pre: [ licensePreRouting ]
+    },
     handler: async (request) => {
       const callWithRequest = callWithRequestFactory(server, request);
-      const { name } = request.params;
+      const { nameOrNames } = request.params;
+      const names = nameOrNames.split(',');
+
+      const itemsDeleted = [];
+      const errors = [];
 
-      // Check if cluster does exist
-      try {
-        const existingCluster = await doesClusterExist(callWithRequest, name);
-        if(!existingCluster) {
-          return wrapCustomError(new Error('There is no remote cluster with that name.'), 404);
+      // Validator that returns an error if the remote cluster does not exist.
+      const validateClusterDoesExist = async (name) => {
+        try {
+          const existingCluster = await doesClusterExist(callWithRequest, name);
+          if (!existingCluster) {
+            return wrapCustomError(new Error('There is no remote cluster with that name.'), 404);
+          }
+        } catch (error) {
+          return wrapCustomError(error, 400);
         }
-      } catch (err) {
-        return wrapCustomError(err, 400);
-      }
-
-      try {
-        const deleteClusterPayload = serializeCluster({ name });
-        const response = await callWithRequest('cluster.putSettings', { body: deleteClusterPayload });
-        const acknowledged = get(response, 'acknowledged');
-        const cluster = get(response, `persistent.cluster.remote.${name}`);
-
-        if (acknowledged && !cluster) {
-          return {};
+      };
+
+      // Send the request to delete the cluster and return an error if it could not be deleted.
+      const sendRequestToDeleteCluster = async (name) => {
+        try {
+          const body = serializeCluster({ name });
+          const response = await callWithRequest('cluster.putSettings', { body });
+          const acknowledged = get(response, 'acknowledged');
+          const cluster = get(response, `persistent.cluster.remote.${name}`);
+
+          if (acknowledged && !cluster) {
+            return null;
+          }
+
+          // If for some reason the ES response still returns the cluster information,
+          // return an error. This shouldn't happen.
+          return wrapCustomError(new Error('Unable to delete cluster, information still returned from ES.'), 400);
+        } catch (error) {
+          if (isEsError(error)) {
+            return wrapEsError(error);
+          }
+
+          return wrapUnknownError(error);
         }
+      };
 
-        // If for some reason the ES response still returns the cluster information,
-        // return an error. This shouldn't happen.
-        return wrapCustomError(new Error('Unable to delete cluster, information still returned from ES.'), 400);
-      } catch (err) {
-        if (isEsError(err)) {
-          return wrapEsError(err);
+      const deleteCluster = async (clusterName) => {
+        try {
+          // Validate that the cluster exists
+          let error = await validateClusterDoesExist(clusterName);
+
+          if (!error) {
+            // Delete the cluster
+            error = await sendRequestToDeleteCluster(clusterName);
+          }
+
+          if (error) {
+            throw error;
+          }
+
+          // If we are here, it means that everything went well...
+          itemsDeleted.push(clusterName);
+        } catch (error) {
+          errors.push({ name: clusterName, error });
         }
+      };
 
-        return wrapUnknownError(err);
-      }
-    },
-    config: {
-      pre: [ licensePreRouting ]
+      // Delete all our cluster in parallel
+      await Promise.all(names.map(deleteCluster));
+
+      return {
+        itemsDeleted,
+        errors,
+      };
     }
   });
 }

x-pack/plugins/remote_clusters/server/routes/api/remote_clusters/register_delete_route.test.js

@@ -48,11 +48,11 @@ describe('[API Routes] Remote Clusters Delete', () => {
     registerDeleteRoute(server);
     const response = await routeHandler({
       params: {
-        name: 'test_cluster'
+        nameOrNames: 'test_cluster'
       }
     });
 
-    expect(response).toEqual({});
+    expect(response).toEqual({ errors: [], itemsDeleted: ['test_cluster'] });
   });
 
   it('should return an error if the response does still contain cluster information', async () => {
@@ -74,23 +74,29 @@ describe('[API Routes] Remote Clusters Delete', () => {
     registerDeleteRoute(server);
     const response = await routeHandler({
       params: {
-        name: 'test_cluster'
+        nameOrNames: 'test_cluster'
       }
     });
 
-    expect(response).toEqual(wrapCustomError(new Error('Unable to delete cluster, information still returned from ES.'), 400));
+    expect(response.errors).toEqual([{
+      name: 'test_cluster',
+      error: wrapCustomError(new Error('Unable to delete cluster, information still returned from ES.'), 400),
+    }]);
   });
 
   it('should return an error if the cluster does not exist', async () => {
     doesClusterExist.mockReturnValueOnce(false);
     registerDeleteRoute(server);
     const response = await routeHandler({
       params: {
-        name: 'test_cluster'
+        nameOrNames: 'test_cluster'
       }
     });
 
-    expect(response).toEqual(wrapCustomError(new Error('There is no remote cluster with that name.'), 404));
+    expect(response.errors).toEqual([{
+      name: 'test_cluster',
+      error: wrapCustomError(new Error('There is no remote cluster with that name.'), 404),
+    }]);
   });
 
   it('should forward an ES error', async () => {
@@ -101,10 +107,13 @@ describe('[API Routes] Remote Clusters Delete', () => {
     registerDeleteRoute(server);
     const response = await routeHandler({
       params: {
-        name: 'test_cluster'
+        nameOrNames: 'test_cluster'
       }
     });
 
-    expect(response).toEqual(Boom.boomify(mockError));
+    expect(response.errors).toEqual([{
+      name: 'test_cluster',
+      error: Boom.boomify(mockError),
+    }]);
   });
 });

x-pack/test/api_integration/apis/management/remote_clusters/remote_clusters.js

@@ -48,6 +48,7 @@ export default function ({ getService }) {
           isConfiguredByNode: false,
         });
       });
+
       it('should not allow us to re-add an existing remote cluster', async () => {
         const uri = `${API_BASE_PATH}`;
 
@@ -142,7 +143,84 @@ export default function ({ getService }) {
           .set('kbn-xsrf', 'xxx')
           .expect(200);
 
-        expect(body).to.eql({});
+        expect(body).to.eql({
+          itemsDeleted: ['test_cluster'],
+          errors: [],
+        });
+      });
+
+      it('should allow us to delete multiple remote clusters', async () => {
+        // Create clusters to delete.
+        await supertest
+          .post(API_BASE_PATH)
+          .set('kbn-xsrf', 'xxx')
+          .send({
+            name: 'test_cluster1',
+            seeds: [
+              NODE_SEED
+            ],
+            skipUnavailable: true,
+          })
+          .expect(200);
+
+        await supertest
+          .post(API_BASE_PATH)
+          .set('kbn-xsrf', 'xxx')
+          .send({
+            name: 'test_cluster2',
+            seeds: [
+              NODE_SEED
+            ],
+            skipUnavailable: true,
+          })
+          .expect(200);
+
+        const uri = `${API_BASE_PATH}/test_cluster1,test_cluster2`;
+
+        const {
+          body: { itemsDeleted, errors }
+        } = await supertest
+          .delete(uri)
+          .set('kbn-xsrf', 'xxx')
+          .expect(200);
+
+        expect(errors).to.eql([]);
+
+        // The order isn't guaranteed, so we assert against individual names instead of asserting
+        // against the value of the array itself.
+        ['test_cluster1', 'test_cluster2'].forEach(clusterName => {
+          expect(itemsDeleted.includes(clusterName)).to.be(true);
+        });
+      });
+
+      it(`should tell us which remote clusters couldn't be deleted`, async () => {
+        const uri = `${API_BASE_PATH}/test_cluster_doesnt_exist`;
+
+        const { body } = await supertest
+          .delete(uri)
+          .set('kbn-xsrf', 'xxx')
+          .expect(200);
+
+        expect(body).to.eql({
+          itemsDeleted: [],
+          errors: [{
+            name: 'test_cluster_doesnt_exist',
+            error: {
+              isBoom: true,
+              isServer: false,
+              data: null,
+              output: {
+                statusCode: 404,
+                payload: {
+                  statusCode: 404,
+                  error: 'Not Found',
+                  message: 'There is no remote cluster with that name.',
+                },
+                headers: {},
+              },
+            },
+          }],
+        });
       });
     });
   });