Avoid data loss when cloud proxy returns 404 (#135542)

* Avoid potential data loss when cloud proxy returns 404

* Fail migration if we encounter an alias pointing to multiple indices

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Rudolf Meijering 2022-07-05 16:11:46 +02:00 committed by GitHub
parent 801bebae93
commit b631e65c26
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 191 additions and 26 deletions

View file

@ -8,7 +8,15 @@
import { catchRetryableEsClientErrors } from './catch_retryable_es_client_errors';
import { errors as EsErrors } from '@elastic/elasticsearch';
jest.mock('./catch_retryable_es_client_errors');
// Create a mock powered by the actual implementation
jest.mock('./catch_retryable_es_client_errors', () => ({
catchRetryableEsClientErrors: jest
.fn()
.mockImplementation(
jest.requireActual('./catch_retryable_es_client_errors').catchRetryableEsClientErrors
),
}));
import { elasticsearchClientMock } from '../../../elasticsearch/client/mocks';
import { fetchIndices } from './fetch_indices';
@ -16,16 +24,18 @@ describe('fetchIndices', () => {
beforeEach(() => {
jest.clearAllMocks();
});
const retryableError = new EsErrors.ResponseError(
elasticsearchClientMock.createApiResponse({
statusCode: 503,
body: { error: { type: 'es_type', reason: 'es_reason' } },
})
);
const client = elasticsearchClientMock.createInternalClient(
elasticsearchClientMock.createErrorTransportRequestPromise(retryableError)
);
it('calls catchRetryableEsClientErrors when the promise rejects', async () => {
const retryableError = new EsErrors.ResponseError(
elasticsearchClientMock.createApiResponse({
statusCode: 503,
body: { error: { type: 'es_type', reason: 'es_reason' } },
})
);
const client = elasticsearchClientMock.createInternalClient(
elasticsearchClientMock.createErrorTransportRequestPromise(retryableError)
);
const task = fetchIndices({ client, indices: ['my_index'] });
try {
await task();
@ -34,4 +44,21 @@ describe('fetchIndices', () => {
}
expect(catchRetryableEsClientErrors).toHaveBeenCalledWith(retryableError);
});
it('throws when cloud returns an incorrect 404 response', async () => {
const notFoundError = new EsErrors.ResponseError(
elasticsearchClientMock.createApiResponse({
statusCode: 404,
body: { ok: false, message: 'Unknown resource.' },
})
);
const client = elasticsearchClientMock.createInternalClient(
elasticsearchClientMock.createErrorTransportRequestPromise(notFoundError)
);
const task = fetchIndices({ client, indices: ['my_index'] });
expect(task()).rejects.toMatchInlineSnapshot(
`[ResponseError: {"ok":false,"message":"Unknown resource."}]`
);
});
});

View file

@ -41,7 +41,7 @@ export const fetchIndices =
index: indices,
ignore_unavailable: true, // Don't return an error for missing indices. Note this *will* include closed indices, the docs are misleading https://github.com/elastic/elasticsearch/issues/63607
},
{ ignore: [404], maxRetries: 0 }
{ maxRetries: 0 }
)
.then((body) => {
return Either.right(body);

View file

@ -99,9 +99,7 @@ describe('KibanaMigrator', () => {
it('throws if prepareMigrations is not called first', async () => {
const options = mockOptions();
options.client.cat.templates.mockResponse([], { statusCode: 404 });
options.client.indices.get.mockResponse({}, { statusCode: 404 });
options.client.indices.getAlias.mockResponse({}, { statusCode: 404 });
options.client.indices.get.mockResponse({}, { statusCode: 200 });
const migrator = new KibanaMigrator(options);
@ -112,8 +110,7 @@ describe('KibanaMigrator', () => {
it('only runs migrations once if called multiple times', async () => {
const options = mockOptions();
options.client.indices.get.mockResponse({}, { statusCode: 404 });
options.client.indices.getAlias.mockResponse({}, { statusCode: 404 });
options.client.indices.get.mockResponse({}, { statusCode: 200 });
options.client.cluster.getSettings.mockResponse(
{

View file

@ -6,10 +6,12 @@
* Side Public License, v 1.
*/
import { FetchIndexResponse } from '../actions/fetch_indices';
import {
addExcludedTypesToBoolQuery,
addMustClausesToBoolQuery,
addMustNotClausesToBoolQuery,
getAliases,
} from './helpers';
describe('addExcludedTypesToBoolQuery', () => {
@ -174,3 +176,57 @@ describe('addMustNotClausesToBoolQuery', () => {
});
});
});
describe('getAliases', () => {
it('returns a right record of alias to index name pairs', () => {
const indices: FetchIndexResponse = {
'.kibana_8.0.0_001': {
aliases: { '.kibana': {}, '.kibana_8.0.0': {} },
mappings: { properties: {} },
settings: {},
},
'.kibana_7.17.0_001': {
aliases: { '.kibana_7.17.0': {} },
mappings: { properties: {} },
settings: {},
},
};
expect(getAliases(indices)).toMatchInlineSnapshot(`
Object {
"_tag": "Right",
"right": Object {
".kibana": ".kibana_8.0.0_001",
".kibana_7.17.0": ".kibana_7.17.0_001",
".kibana_8.0.0": ".kibana_8.0.0_001",
},
}
`);
});
it('returns a left multiple_indices_per_alias when one alias points to multiple indices', () => {
const indices: FetchIndexResponse = {
'.kibana_8.0.0_001': {
aliases: { '.kibana': {}, '.kibana_8.0.0': {} },
mappings: { properties: {} },
settings: {},
},
'.kibana_7.17.0_001': {
aliases: { '.kibana': {}, '.kibana_7.17.0': {} },
mappings: { properties: {} },
settings: {},
},
};
expect(getAliases(indices)).toMatchInlineSnapshot(`
Object {
"_tag": "Left",
"left": Object {
"alias": ".kibana",
"indices": Array [
".kibana_8.0.0_001",
".kibana_7.17.0_001",
],
"type": "multiple_indices_per_alias",
},
}
`);
});
});

View file

@ -11,6 +11,7 @@ import type {
QueryDslBoolQuery,
QueryDslQueryContainer,
} from '@elastic/elasticsearch/lib/api/types';
import * as Either from 'fp-ts/lib/Either';
import type { State } from '../state';
import type { IndexMapping } from '../../mappings';
import type { FetchIndexResponse } from '../actions';
@ -152,12 +153,25 @@ export function indexVersion(indexName?: string): string | undefined {
/**
* Creates a record of alias -> index name pairs
*/
export function getAliases(indices: FetchIndexResponse) {
return Object.keys(indices).reduce((acc, index) => {
Object.keys(indices[index].aliases || {}).forEach((alias) => {
// TODO throw if multiple .kibana aliases point to the same index?
acc[alias] = index;
});
return acc;
}, {} as Record<string, string>);
export function getAliases(
indices: FetchIndexResponse
): Either.Either<
{ type: 'multiple_indices_per_alias'; alias: string; indices: string[] },
Record<string, string>
> {
const aliases = {} as Record<string, string>;
for (const index of Object.getOwnPropertyNames(indices)) {
for (const alias of Object.getOwnPropertyNames(indices[index].aliases || {})) {
if (aliases[alias] != null) {
return Either.left({
type: 'multiple_indices_per_alias',
alias,
indices: [aliases[alias], index],
});
}
aliases[alias] = index;
}
}
return Either.right(aliases);
}

View file

@ -324,6 +324,29 @@ describe('migrations v2 model', () => {
`"The .kibana alias is pointing to a newer version of Kibana: v7.12.0"`
);
});
test('INIT -> FATAL when .kibana points to multiple indices', () => {
const res: ResponseType<'INIT'> = Either.right({
'.kibana_7.12.0_001': {
aliases: {
'.kibana': {},
'.kibana_7.12.0': {},
},
mappings: { properties: {}, _meta: { migrationMappingPropertyHashes: {} } },
settings: {},
},
'.kibana_7.11.0_001': {
aliases: { '.kibana': {}, '.kibana_7.11.0': {} },
mappings: { properties: {}, _meta: { migrationMappingPropertyHashes: {} } },
settings: {},
},
});
const newState = model(initState, res) as FatalState;
expect(newState.controlState).toEqual('FATAL');
expect(newState.reason).toMatchInlineSnapshot(
`"The .kibana alias is pointing to multiple indices: .kibana_7.12.0_001,.kibana_7.11.0_001."`
);
});
test('INIT -> WAIT_FOR_YELLOW_SOURCE when .kibana points to an index with an invalid version', () => {
// If users tamper with our index version naming scheme we can no
// longer accurately detect a newer version. Older Kibana versions
@ -2072,6 +2095,30 @@ describe('migrations v2 model', () => {
expect(newState.retryCount).toEqual(0);
expect(newState.retryDelay).toEqual(0);
});
test('MARK_VERSION_INDEX_READY_CONFLICT -> FATAL if the current alias is pointing to a multiple indices', () => {
const res: ResponseType<'MARK_VERSION_INDEX_READY_CONFLICT'> = Either.right({
'.kibana_7.11.0_001': {
aliases: { '.kibana': {}, '.kibana_7.11.0': {} },
mappings: { properties: {}, _meta: { migrationMappingPropertyHashes: {} } },
settings: {},
},
'.kibana_7.12.0_001': {
aliases: {
'.kibana': {},
'.kibana_7.12.0': {},
},
mappings: { properties: {}, _meta: { migrationMappingPropertyHashes: {} } },
settings: {},
},
});
const newState = model(markVersionIndexConflictState, res) as FatalState;
expect(newState.controlState).toEqual('FATAL');
expect(newState.reason).toMatchInlineSnapshot(
`"The .kibana alias is pointing to multiple indices: .kibana_7.11.0_001,.kibana_7.12.0_001."`
);
expect(newState.retryCount).toEqual(0);
expect(newState.retryDelay).toEqual(0);
});
});
});
});

View file

@ -83,7 +83,19 @@ export const model = (currentState: State, resW: ResponseType<AllActionStates>):
} else if (Either.isRight(res)) {
// cluster routing allocation is enabled and we can continue with the migration as normal
const indices = res.right;
const aliases = getAliases(indices);
const aliasesRes = getAliases(indices);
if (Either.isLeft(aliasesRes)) {
return {
...stateP,
controlState: 'FATAL',
reason: `The ${
aliasesRes.left.alias
} alias is pointing to multiple indices: ${aliasesRes.left.indices.join(',')}.`,
};
}
const aliases = aliasesRes.right;
if (
// `.kibana` and the version specific aliases both exists and
@ -1069,7 +1081,19 @@ export const model = (currentState: State, resW: ResponseType<AllActionStates>):
const res = resW as ExcludeRetryableEsError<ResponseType<typeof stateP.controlState>>;
if (Either.isRight(res)) {
const indices = res.right;
const aliases = getAliases(indices);
const aliasesRes = getAliases(indices);
if (Either.isLeft(aliasesRes)) {
return {
...stateP,
controlState: 'FATAL',
reason: `The ${
aliasesRes.left.alias
} alias is pointing to multiple indices: ${aliasesRes.left.indices.join(',')}.`,
};
}
const aliases = aliasesRes.right;
if (
aliases[stateP.currentAlias] != null &&
aliases[stateP.versionAlias] != null &&