Skip to content

Commit

Permalink
[Event Log] Fixing call to update index alias to hidden (elastic#122882)
Browse files Browse the repository at this point in the history 
* Adding await

* Changing the way alias updates are made

* Updating unit tests

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
@ymao1 @kibanamachine
ymao1 and kibanamachine committed Jan 18, 2022
1 parent 52d4475 commit bd57aa5
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 62 deletions.
43 changes: 14 additions & 29 deletions x-pack/plugins/event_log/server/es/cluster_client_adapter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand All @@ -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: [
Expand All @@ -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',
Expand All @@ -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"`
);
});
});
Expand Down
13 changes: 7 additions & 6 deletions x-pack/plugins/event_log/server/es/cluster_client_adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -281,15 +282,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',
Expand All @@ -299,7 +300,7 @@ export class ClusterClientAdapter<TDoc extends { body: AliasAny; index: string }
return {
add: {
...existingAliasOptions,
index: indexName,
index: aliasData.indexName,
alias: aliasName,
is_hidden: true,
},
Expand All @@ -309,7 +310,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}`
);
}
}
Expand Down
100 changes: 85 additions & 15 deletions x-pack/plugins/event_log/server/es/init.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { contextMock } from './context.mock';
import { initializeEs } from './init';
import { initializeEs, parseIndexAliases } from './init';

describe('initializeEs', () => {
let esContext = contextMock.create();
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -310,6 +309,9 @@ describe('initializeEs', () => {
'foo-bar': {
is_write_index: true,
},
'bar-foo': {
is_write_index: true,
},
},
};
esContext.esAdapter.getExistingIndexAliases.mockResolvedValue({
Expand All @@ -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();
});
Expand Down Expand Up @@ -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,
},
]);
});
});
48 changes: 36 additions & 12 deletions x-pack/plugins/event_log/server/es/init.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

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';

Expand All @@ -33,6 +34,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;
Expand All @@ -56,7 +73,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
Expand Down Expand Up @@ -93,7 +110,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?.index?.hidden;

Expand Down Expand Up @@ -125,22 +142,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 estypes.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}`
);
}
});
Expand Down

0 comments on commit bd57aa5

Please sign in to comment.