[8.x] [UA] Remove `delete` from `.tasks` permissi…

…on check (elastic#203379) (elastic#203417) # Backport This will backport the following commits from `main` to `8.x`: - [[UA] Remove `delete` from `.tasks` permission check (elastic#203379)](elastic#203379)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jean-Louis Leysens <[email protected]>
pgayvallet · Dec 9, 2024 · 2131153 · 2131153
1 parent 2e79baa
commit 2131153
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 33 deletions.
diff --git a/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.test.ts b/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.test.ts
@@ -26,6 +26,7 @@ import { esIndicesStateCheck } from '../es_indices_state_check';
 import { versionService } from '../version';
 
 import { ReindexService, reindexServiceFactory } from './reindex_service';
+import { error } from './error';
 
 const asApiResponse = <T>(body: T): TransportResult<T> =>
   ({
@@ -111,7 +112,7 @@ describe('reindexService', () => {
             },
             {
               names: ['.tasks'],
-              privileges: ['read', 'delete'],
+              privileges: ['read'],
             },
           ],
         },
@@ -141,7 +142,7 @@ describe('reindexService', () => {
             },
             {
               names: ['.tasks'],
-              privileges: ['read', 'delete'],
+              privileges: ['read'],
             },
           ],
         },
@@ -611,11 +612,11 @@ describe('reindexService', () => {
           });
         });
 
-        it('fails if docs created is less than count in source index', async () => {
+        it('does not throw if task doc deletion returns a bad result', async () => {
           clusterClient.asCurrentUser.tasks.get.mockResponseOnce({
             completed: true,
             // @ts-expect-error not full interface
-            task: { status: { created: 95, total: 95 } },
+            task: { status: { created: 100, total: 100 } },
           });
 
           clusterClient.asCurrentUser.count.mockResponseOnce(
@@ -625,11 +626,51 @@ describe('reindexService', () => {
             }
           );
 
+          clusterClient.asCurrentUser.delete.mockResponseOnce({
+            // @ts-expect-error not known result
+            result: '!?',
+          });
+
           const updatedOp = await service.processNextStep(reindexOp);
-          expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.reindexStarted);
-          expect(updatedOp.attributes.status).toEqual(ReindexStatus.failed);
-          expect(updatedOp.attributes.errorMessage).not.toBeNull();
-          expect(log.error).toHaveBeenCalledWith(expect.any(String));
+          expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.reindexCompleted);
+          expect(updatedOp.attributes.reindexTaskPercComplete).toEqual(1);
+          expect(clusterClient.asCurrentUser.delete).toHaveBeenCalledWith({
+            index: '.tasks',
+            id: 'xyz',
+          });
+          expect(log.warn).toHaveBeenCalledTimes(1);
+          expect(log.warn).toHaveBeenCalledWith(
+            error.reindexTaskCannotBeDeleted(
+              `Could not delete reindexing task xyz, got response "!?"`
+            )
+          );
+        });
+
+        it('does not throw if task doc deletion throws', async () => {
+          clusterClient.asCurrentUser.tasks.get.mockResponseOnce({
+            completed: true,
+            // @ts-expect-error not full interface
+            task: { status: { created: 100, total: 100 } },
+          });
+
+          clusterClient.asCurrentUser.count.mockResponseOnce(
+            // @ts-expect-error not full interface
+            {
+              count: 100,
+            }
+          );
+
+          clusterClient.asCurrentUser.delete.mockRejectedValue(new Error('FAILED!'));
+
+          const updatedOp = await service.processNextStep(reindexOp);
+          expect(updatedOp.attributes.lastCompletedStep).toEqual(ReindexStep.reindexCompleted);
+          expect(updatedOp.attributes.reindexTaskPercComplete).toEqual(1);
+          expect(clusterClient.asCurrentUser.delete).toHaveBeenCalledWith({
+            index: '.tasks',
+            id: 'xyz',
+          });
+          expect(log.warn).toHaveBeenCalledTimes(1);
+          expect(log.warn).toHaveBeenCalledWith(new Error('FAILED!'));
         });
       });
 

diff --git a/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts b/x-pack/plugins/upgrade_assistant/server/lib/reindexing/reindex_service.ts
@@ -308,14 +308,21 @@ export const reindexServiceFactory = (
       });
     }
 
-    // Delete the task from ES .tasks index
-    const deleteTaskResp = await esClient.delete({
-      index: '.tasks',
-      id: taskId,
-    });
-
-    if (deleteTaskResp.result !== 'deleted') {
-      throw error.reindexTaskCannotBeDeleted(`Could not delete reindexing task ${taskId}`);
+    try {
+      // Delete the task from ES .tasks index
+      const deleteTaskResp = await esClient.delete({
+        index: '.tasks',
+        id: taskId,
+      });
+      if (deleteTaskResp.result !== 'deleted') {
+        log.warn(
+          error.reindexTaskCannotBeDeleted(
+            `Could not delete reindexing task ${taskId}, got response "${deleteTaskResp.result}"`
+          )
+        );
+      }
+    } catch (e) {
+      log.warn(e);
     }
 
     return reindexOp;
@@ -396,24 +403,21 @@ export const reindexServiceFactory = (
         names.push(sourceName);
       }
 
-      // Otherwise, query for required privileges for this index.
-      const body = {
-        cluster: ['manage'],
-        index: [
-          {
-            names,
-            allow_restricted_indices: true,
-            privileges: ['all'],
-          },
-          {
-            names: ['.tasks'],
-            privileges: ['read', 'delete'],
-          },
-        ],
-      } as any;
-
       const resp = await esClient.security.hasPrivileges({
-        body,
+        body: {
+          cluster: ['manage'],
+          index: [
+            {
+              names,
+              allow_restricted_indices: true,
+              privileges: ['all'],
+            },
+            {
+              names: ['.tasks'],
+              privileges: ['read'],
+            },
+          ],
+        },
       });
 
       return resp.has_all_requested;