[Event Log] Fixing call to update index alias to hidden (elastic#122882)

* Adding await

* Changing the way alias updates are made

* Updating unit tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit bd57aa5)

# Conflicts:
#	x-pack/plugins/event_log/server/es/init.ts

x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts

@@ -455,13 +455,9 @@ describe('getExistingIndexAliases', () => {
 
 describe('setIndexAliasToHidden', () => {
   test('should call cluster with given index name and aliases', async () => {
-    await clusterClientAdapter.setIndexAliasToHidden('foo-bar-000001', {
-      aliases: {
-        'foo-bar': {
-          is_write_index: true,
-        },
-      },
-    });
+    await clusterClientAdapter.setIndexAliasToHidden('foo-bar', [
+      { alias: 'foo-bar', indexName: 'foo-bar-000001', is_write_index: true },
+    ]);
     expect(clusterClient.indices.updateAliases).toHaveBeenCalledWith({
       body: {
         actions: [
@@ -478,18 +474,11 @@ describe('setIndexAliasToHidden', () => {
     });
   });
 
-  test('should update multiple aliases at once and preserve existing alias settings', async () => {
-    await clusterClientAdapter.setIndexAliasToHidden('foo-bar-000001', {
-      aliases: {
-        'foo-bar': {
-          is_write_index: true,
-        },
-        'foo-b': {
-          index_routing: 'index',
-          routing: 'route',
-        },
-      },
-    });
+  test('should update multiple indices for an alias at once and preserve existing alias settings', async () => {
+    await clusterClientAdapter.setIndexAliasToHidden('foo-bar', [
+      { alias: 'foo-bar', indexName: 'foo-bar-000001', is_write_index: true },
+      { alias: 'foo-bar', indexName: 'foo-bar-000002', index_routing: 'index', routing: 'route' },
+    ]);
     expect(clusterClient.indices.updateAliases).toHaveBeenCalledWith({
       body: {
         actions: [
@@ -503,8 +492,8 @@ describe('setIndexAliasToHidden', () => {
           },
           {
             add: {
-              index: 'foo-bar-000001',
-              alias: 'foo-b',
+              index: 'foo-bar-000002',
+              alias: 'foo-bar',
               is_hidden: true,
               index_routing: 'index',
               routing: 'route',
@@ -518,15 +507,11 @@ describe('setIndexAliasToHidden', () => {
   test('should throw error when call cluster throws an error', async () => {
     clusterClient.indices.updateAliases.mockRejectedValue(new Error('Fail'));
     await expect(
-      clusterClientAdapter.setIndexAliasToHidden('foo-bar-000001', {
-        aliases: {
-          'foo-bar': {
-            is_write_index: true,
-          },
-        },
-      })
+      clusterClientAdapter.setIndexAliasToHidden('foo-bar', [
+        { alias: 'foo-bar', indexName: 'foo-bar-000001', is_write_index: true },
+      ])
     ).rejects.toThrowErrorMatchingInlineSnapshot(
-      `"error setting existing index aliases for index foo-bar-000001 to is_hidden: Fail"`
+      `"error setting existing index aliases for alias foo-bar to is_hidden: Fail"`
     );
   });
 });

x-pack/plugins/event_log/server/es/cluster_client_adapter.ts

@@ -15,6 +15,7 @@ import { estypes } from '@elastic/elasticsearch';
 import { fromKueryExpression, toElasticsearchQuery } from '@kbn/es-query';
 import { IEvent, IValidatedEvent, SAVED_OBJECT_REL_PRIMARY } from '../types';
 import { FindOptionsType } from '../event_log_client';
+import { ParsedIndexAlias } from './init';
 
 export const EVENT_BUFFER_TIME = 1000; // milliseconds
 export const EVENT_BUFFER_LENGTH = 100;
@@ -272,15 +273,15 @@ export class ClusterClientAdapter<TDoc extends { body: AliasAny; index: string }
   }
 
   public async setIndexAliasToHidden(
-    indexName: string,
-    currentAliases: estypes.IndicesGetAliasIndexAliases
+    aliasName: string,
+    currentAliasData: ParsedIndexAlias[]
   ): Promise<void> {
     try {
       const esClient = await this.elasticsearchClientPromise;
       await esClient.indices.updateAliases({
         body: {
-          actions: Object.keys(currentAliases.aliases).map((aliasName) => {
-            const existingAliasOptions = pick(currentAliases.aliases[aliasName], [
+          actions: currentAliasData.map((aliasData) => {
+            const existingAliasOptions = pick(aliasData, [
               'is_write_index',
               'filter',
               'index_routing',
@@ -290,7 +291,7 @@ export class ClusterClientAdapter<TDoc extends { body: AliasAny; index: string }
             return {
               add: {
                 ...existingAliasOptions,
-                index: indexName,
+                index: aliasData.indexName,
                 alias: aliasName,
                 is_hidden: true,
               },
@@ -300,7 +301,7 @@ export class ClusterClientAdapter<TDoc extends { body: AliasAny; index: string }
       });
     } catch (err) {
       throw new Error(
-        `error setting existing index aliases for index ${indexName} to is_hidden: ${err.message}`
+        `error setting existing index aliases for alias ${aliasName} to is_hidden: ${err.message}`
       );
     }
   }

x-pack/plugins/event_log/server/es/init.test.ts

@@ -6,7 +6,7 @@
  */
 
 import { contextMock } from './context.mock';
-import { initializeEs } from './init';
+import { initializeEs, parseIndexAliases } from './init';
 
 describe('initializeEs', () => {
   let esContext = contextMock.create();
@@ -267,11 +267,10 @@ describe('initializeEs', () => {
     });
 
     await initializeEs(esContext);
-    expect(esContext.esAdapter.getExistingIndexAliases).toHaveBeenCalled();
-    expect(esContext.esAdapter.setIndexAliasToHidden).toHaveBeenCalledWith(
-      'foo-bar-000001',
-      testAliases
-    );
+    expect(esContext.esAdapter.getExistingIndexAliases).toHaveBeenCalledTimes(1);
+    expect(esContext.esAdapter.setIndexAliasToHidden).toHaveBeenCalledWith('foo-bar', [
+      { alias: 'foo-bar', indexName: 'foo-bar-000001', is_write_index: true },
+    ]);
   });
 
   test(`should not update existing index aliases if any exist and are already hidden`, async () => {
@@ -310,6 +309,9 @@ describe('initializeEs', () => {
         'foo-bar': {
           is_write_index: true,
         },
+        'bar-foo': {
+          is_write_index: true,
+        },
       },
     };
     esContext.esAdapter.getExistingIndexAliases.mockResolvedValue({
@@ -321,17 +323,18 @@ describe('initializeEs', () => {
 
     await initializeEs(esContext);
     expect(esContext.esAdapter.getExistingIndexAliases).toHaveBeenCalled();
-    expect(esContext.esAdapter.setIndexAliasToHidden).toHaveBeenCalledWith(
-      'foo-bar-000001',
-      testAliases
-    );
-    expect(esContext.esAdapter.setIndexAliasToHidden).toHaveBeenCalledWith(
-      'foo-bar-000002',
-      testAliases
-    );
+    expect(esContext.esAdapter.setIndexAliasToHidden).toHaveBeenCalledTimes(2);
+    expect(esContext.esAdapter.setIndexAliasToHidden).toHaveBeenCalledWith('foo-bar', [
+      { alias: 'foo-bar', indexName: 'foo-bar-000001', is_write_index: true },
+      { alias: 'foo-bar', indexName: 'foo-bar-000002', is_write_index: true },
+    ]);
+    expect(esContext.esAdapter.setIndexAliasToHidden).toHaveBeenCalledWith('bar-foo', [
+      { alias: 'bar-foo', indexName: 'foo-bar-000001', is_write_index: true },
+      { alias: 'bar-foo', indexName: 'foo-bar-000002', is_write_index: true },
+    ]);
     expect(esContext.logger.error).toHaveBeenCalledTimes(1);
     expect(esContext.logger.error).toHaveBeenCalledWith(
-      `error setting existing \"foo-bar-000001\" index aliases - Fail`
+      `error setting existing \"foo-bar\" index aliases - Fail`
     );
     expect(esContext.esAdapter.doesIlmPolicyExist).toHaveBeenCalled();
   });
@@ -384,3 +387,70 @@ describe('initializeEs', () => {
     expect(esContext.esAdapter.createIndex).not.toHaveBeenCalled();
   });
 });
+
+describe('parseIndexAliases', () => {
+  test('should parse IndicesGetAliasResponse into desired format', () => {
+    const indexGetAliasResponse = {
+      '.kibana-event-log-7.15.2-000003': {
+        aliases: {
+          '.kibana-event-log-7.15.2': {
+            is_write_index: true,
+          },
+          another_alias: {
+            is_write_index: true,
+          },
+        },
+      },
+      '.kibana-event-log-7.15.2-000002': {
+        aliases: {
+          '.kibana-event-log-7.15.2': {
+            is_write_index: false,
+          },
+        },
+      },
+      '.kibana-event-log-7.15.2-000001': {
+        aliases: {
+          '.kibana-event-log-7.15.2': {
+            is_write_index: false,
+          },
+        },
+      },
+      '.kibana-event-log-8.0.0-000001': {
+        aliases: {
+          '.kibana-event-log-8.0.0': {
+            is_write_index: true,
+            is_hidden: true,
+          },
+        },
+      },
+    };
+    expect(parseIndexAliases(indexGetAliasResponse)).toEqual([
+      {
+        alias: '.kibana-event-log-7.15.2',
+        indexName: '.kibana-event-log-7.15.2-000003',
+        is_write_index: true,
+      },
+      {
+        alias: 'another_alias',
+        indexName: '.kibana-event-log-7.15.2-000003',
+        is_write_index: true,
+      },
+      {
+        alias: '.kibana-event-log-7.15.2',
+        indexName: '.kibana-event-log-7.15.2-000002',
+        is_write_index: false,
+      },
+      {
+        alias: '.kibana-event-log-7.15.2',
+        indexName: '.kibana-event-log-7.15.2-000001',
+        is_write_index: false,
+      },
+      {
+        alias: '.kibana-event-log-8.0.0',
+        indexName: '.kibana-event-log-8.0.0-000001',
+        is_hidden: true,
+        is_write_index: true,
+      },
+    ]);
+  });
+});

x-pack/plugins/event_log/server/es/init.ts

@@ -6,8 +6,9 @@
  */
 
 import { asyncForEach } from '@kbn/std';
-import { IndicesAlias, IndicesIndexStatePrefixedSettings } from '@elastic/elasticsearch/api/types';
+import { IndicesIndexStatePrefixedSettings } from '@elastic/elasticsearch/api/types';
 import { estypes } from '@elastic/elasticsearch';
+import { groupBy } from 'lodash';
 import { getIlmPolicy, getIndexTemplate } from './documents';
 import { EsContext } from './context';
 
@@ -34,6 +35,22 @@ async function initializeEsResources(esContext: EsContext) {
   await steps.createInitialIndexIfNotExists();
 }
 
+export interface ParsedIndexAlias extends estypes.IndicesAliasDefinition {
+  indexName: string;
+  alias: string;
+  is_hidden?: boolean;
+}
+
+export function parseIndexAliases(aliasInfo: estypes.IndicesGetAliasResponse): ParsedIndexAlias[] {
+  return Object.keys(aliasInfo).flatMap((indexName: string) =>
+    Object.keys(aliasInfo[indexName].aliases).map((alias: string) => ({
+      ...aliasInfo[indexName].aliases[alias],
+      indexName,
+      alias,
+    }))
+  );
+}
+
 class EsInitializationSteps {
   constructor(private readonly esContext: EsContext) {
     this.esContext = esContext;
@@ -57,7 +74,7 @@ class EsInitializationSteps {
       this.esContext.logger.error(`error getting existing index templates - ${err.message}`);
     }
 
-    asyncForEach(Object.keys(indexTemplates), async (indexTemplateName: string) => {
+    await asyncForEach(Object.keys(indexTemplates), async (indexTemplateName: string) => {
       try {
         const hidden: string | boolean = indexTemplates[indexTemplateName]?.settings?.index?.hidden;
         // Check to see if this index template is hidden
@@ -94,8 +111,7 @@ class EsInitializationSteps {
       // should not block the rest of initialization, log the error and move on
       this.esContext.logger.error(`error getting existing indices - ${err.message}`);
     }
-
-    asyncForEach(Object.keys(indices), async (indexName: string) => {
+    await asyncForEach(Object.keys(indices), async (indexName: string) => {
       try {
         const hidden: string | boolean | undefined = (
           indices[indexName]?.settings as IndicesIndexStatePrefixedSettings
@@ -130,22 +146,28 @@ class EsInitializationSteps {
       this.esContext.logger.error(`error getting existing index aliases - ${err.message}`);
     }
 
-    asyncForEach(Object.keys(indexAliases), async (indexName: string) => {
+    // Flatten the results so we can group by index alias
+    const parsedAliasData = parseIndexAliases(indexAliases);
+
+    // Group by index alias name
+    const indexAliasData = groupBy(parsedAliasData, 'alias');
+
+    await asyncForEach(Object.keys(indexAliasData), async (aliasName: string) => {
       try {
-        const aliases = indexAliases[indexName]?.aliases;
-        const hasNotHiddenAliases: boolean = Object.keys(aliases).some((alias: string) => {
-          return (aliases[alias] as IndicesAlias)?.is_hidden !== true;
-        });
-
-        if (hasNotHiddenAliases) {
-          this.esContext.logger.debug(`setting existing "${indexName}" index aliases to hidden.`);
-          await this.esContext.esAdapter.setIndexAliasToHidden(indexName, indexAliases[indexName]);
+        const aliasData = indexAliasData[aliasName];
+        const isNotHidden = aliasData.some((data) => data.is_hidden !== true);
+        if (isNotHidden) {
+          this.esContext.logger.debug(`setting existing "${aliasName}" index alias to hidden.`);
+          await this.esContext.esAdapter.setIndexAliasToHidden(
+            aliasName,
+            indexAliasData[aliasName]
+          );
         }
       } catch (err) {
         // errors when trying to set existing index aliases to is_hidden
         // should not block the rest of initialization, log the error and move on
         this.esContext.logger.error(
-          `error setting existing "${indexName}" index aliases - ${err.message}`
+          `error setting existing "${aliasName}" index aliases - ${err.message}`
         );
       }
     });