From 400f6e0a3ee9bc24328251c48d1305d7e8e7c5c6 Mon Sep 17 00:00:00 2001 From: ymao1 Date: Tue, 18 Jan 2022 11:33:39 -0500 Subject: [PATCH] [8.0] [Event Log] Fixing call to update index alias to hidden (#122882) (#123237) * [Event Log] Fixing call to update index alias to hidden (#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 bd57aa55b0788494fc4395fc8f71c3a423eb5cf4) # Conflicts: # x-pack/plugins/event_log/server/es/init.ts * Fixing types check --- .../server/es/cluster_client_adapter.test.ts | 43 +++----- .../server/es/cluster_client_adapter.ts | 13 +-- .../plugins/event_log/server/es/init.test.ts | 100 +++++++++++++++--- x-pack/plugins/event_log/server/es/init.ts | 53 +++++++--- 4 files changed, 143 insertions(+), 66 deletions(-) diff --git a/x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts b/x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts index 314a6b9a31ef89..0d4f4136b42edd 100644 --- a/x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts +++ b/x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts @@ -454,13 +454,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: [ @@ -477,18 +473,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: [ @@ -502,8 +491,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', @@ -517,15 +506,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"` ); }); }); diff --git a/x-pack/plugins/event_log/server/es/cluster_client_adapter.ts b/x-pack/plugins/event_log/server/es/cluster_client_adapter.ts index 7246e1ed972ecf..dd3146d6018862 100644 --- a/x-pack/plugins/event_log/server/es/cluster_client_adapter.ts +++ b/x-pack/plugins/event_log/server/es/cluster_client_adapter.ts @@ -15,6 +15,7 @@ import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; 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; @@ -281,15 +282,15 @@ export class ClusterClientAdapter { 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', @@ -299,7 +300,7 @@ export class ClusterClientAdapter { 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, + }, + ]); + }); +}); diff --git a/x-pack/plugins/event_log/server/es/init.ts b/x-pack/plugins/event_log/server/es/init.ts index 7641404c484cef..8e684a8c94f727 100644 --- a/x-pack/plugins/event_log/server/es/init.ts +++ b/x-pack/plugins/event_log/server/es/init.ts @@ -5,12 +5,10 @@ * 2.0. */ -import { - IndicesAlias, - IndicesIndexStatePrefixedSettings, -} from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; +import { IndicesIndexStatePrefixedSettings } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { asyncForEach } from '@kbn/std'; +import { groupBy } from 'lodash'; import { getIlmPolicy, getIndexTemplate } from './documents'; import { EsContext } from './context'; @@ -37,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; @@ -60,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 @@ -97,7 +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 @@ -131,22 +145,29 @@ class EsInitializationSteps { // should not block the rest of initialization, log the error and move on 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}` ); } });