[Upgrade Assistant] Fix potentially long URI (#74485)

* _cluster/state/metadata/* and added a comment to function

* add another comment regarding why we are asking for *

* update jest test

* refactor and clean up use of cluster status to get index state

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
Jean-Louis Leysens 2020-08-14 17:14:03 +02:00 committed by GitHub
parent 458bf9fb0d
commit 344e1de7e1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 108 additions and 220 deletions

View file

@ -0,0 +1,41 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { getIndexState } from './get_index_state';
import { ResolveIndexResponseFromES } from './types';
describe('getIndexState', () => {
const indexName1 = 'indexName';
const indexName2 = 'indexName2';
const response: ResolveIndexResponseFromES = {
indices: [
{
name: indexName2,
aliases: ['.security'],
attributes: ['open'],
},
{
name: indexName1,
attributes: ['closed'],
},
],
aliases: [
{
name: '.security',
indices: ['.security-7'],
},
],
data_streams: [],
};
it('correctly extracts state', () => {
expect(getIndexState(indexName1, response)).toBe('closed');
expect(getIndexState(indexName2, response)).toBe('open');
});
it('throws if the index name cannot be found', () => {
expect(() => getIndexState('nonExistent', response)).toThrow('not found');
});
});

View file

@ -0,0 +1,24 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { ResolveIndexResponseFromES } from './types';
/**
* Throws if the index name is not found in the resolved indices response
*
* @param indexName Assume this is an index name, not an alias
* @param resolvedResponse The response from _resolve/index/<indices>
*/
export const getIndexState = (
indexName: string,
resolvedResponse: ResolveIndexResponseFromES
): 'open' | 'closed' => {
const index = resolvedResponse.indices.find((i) => i.name === indexName);
if (index) {
return index.attributes.includes('closed') ? 'closed' : 'open';
}
throw new Error(`${indexName} not found!`);
};

View file

@ -1,53 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { getIndexStateFromClusterState } from './get_index_state_from_cluster_state';
import { ClusterStateAPIResponse } from './types';
describe('getIndexStateFromClusterState', () => {
const indexName = 'indexName';
const clusterState: ClusterStateAPIResponse = {
metadata: {
indices: {},
cluster_coordination: {} as any,
cluster_uuid: 'test',
templates: {} as any,
},
cluster_name: 'test',
cluster_uuid: 'test',
};
afterEach(() => {
clusterState.metadata.indices = {};
});
it('correctly extracts state from cluster state', () => {
clusterState.metadata.indices[indexName] = { state: 'open' } as any;
clusterState.metadata.indices.aTotallyDifferentIndex = { state: 'close' } as any;
expect(getIndexStateFromClusterState(indexName, clusterState)).toBe('open');
});
it('correctly extracts state from aliased index in cluster state', () => {
clusterState.metadata.indices.aTotallyDifferentName = {
state: 'close',
aliases: [indexName, 'test'],
} as any;
clusterState.metadata.indices.aTotallyDifferentName1 = {
state: 'open',
aliases: ['another', 'test'],
} as any;
expect(getIndexStateFromClusterState(indexName, clusterState)).toBe('close');
});
it('throws if the index name cannot be found in the cluster state', () => {
expect(() => getIndexStateFromClusterState(indexName, clusterState)).toThrow('not found');
clusterState.metadata.indices.aTotallyDifferentName1 = {
state: 'open',
aliases: ['another', 'test'],
} as any;
expect(() => getIndexStateFromClusterState(indexName, clusterState)).toThrow('not found');
});
});

View file

@ -1,28 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { ClusterStateAPIResponse } from './types';
const checkAllAliases = (
indexName: string,
clusterState: ClusterStateAPIResponse
): 'open' | 'close' => {
for (const index of Object.values(clusterState.metadata.indices)) {
if (index.aliases?.some((alias) => alias === indexName)) {
return index.state;
}
}
throw new Error(`${indexName} not found in cluster state!`);
};
export const getIndexStateFromClusterState = (
indexName: string,
clusterState: ClusterStateAPIResponse
): 'open' | 'close' =>
clusterState.metadata.indices[indexName]
? clusterState.metadata.indices[indexName].state
: checkAllAliases(indexName, clusterState);

View file

@ -184,41 +184,17 @@ export interface UpgradeAssistantStatus {
indices: EnrichedDeprecationInfo[];
}
export interface ClusterStateIndexAPIResponse {
state: 'open' | 'close';
settings: {
index: {
verified_before_close: string;
search: {
throttled: string;
};
number_of_shards: string;
provided_name: string;
frozen: string;
creation_date: string;
number_of_replicas: string;
uuid: string;
version: {
created: string;
};
};
};
mappings: any;
aliases: string[];
}
export interface ClusterStateAPIResponse {
cluster_name: string;
cluster_uuid: string;
metadata: {
cluster_uuid: string;
cluster_coordination: {
term: number;
last_committed_config: string[];
last_accepted_config: string[];
voting_config_exclusions: [];
};
templates: any;
indices: { [indexName: string]: ClusterStateIndexAPIResponse };
};
export interface ResolveIndexResponseFromES {
indices: Array<{
name: string;
// per https://github.com/elastic/elasticsearch/pull/57626
attributes: Array<'open' | 'closed' | 'hidden' | 'frozen'>;
aliases?: string[];
data_stream?: string;
}>;
aliases: Array<{
name: string;
indices: string[];
}>;
data_streams: Array<{ name: string; backing_indices: string[]; timestamp_field: string }>;
}

View file

@ -5,28 +5,27 @@
*/
import { LegacyAPICaller } from 'kibana/server';
import { getIndexStateFromClusterState } from '../../common/get_index_state_from_cluster_state';
import { ClusterStateAPIResponse } from '../../common/types';
import { getIndexState } from '../../common/get_index_state';
import { ResolveIndexResponseFromES } from '../../common/types';
type StatusCheckResult = Record<string, 'open' | 'close'>;
type StatusCheckResult = Record<string, 'open' | 'closed'>;
export const esIndicesStateCheck = async (
callAsUser: LegacyAPICaller,
indices: string[]
): Promise<StatusCheckResult> => {
// According to https://www.elastic.co/guide/en/elasticsearch/reference/7.6/cluster-state.html
// The response from this call is considered internal and subject to change. We have an API
// integration test for asserting that the current ES version still returns what we expect.
// This lives in x-pack/test/upgrade_assistant_integration
const clusterState: ClusterStateAPIResponse = await callAsUser('cluster.state', {
index: indices,
metric: 'metadata',
const response: ResolveIndexResponseFromES = await callAsUser('transport.request', {
method: 'GET',
path: `/_resolve/index/*`,
query: {
expand_wildcards: 'all',
},
});
const result: StatusCheckResult = {};
indices.forEach((index) => {
result[index] = getIndexStateFromClusterState(index, clusterState);
result[index] = getIndexState(index, response);
});
return result;

View file

@ -6,33 +6,25 @@
import _ from 'lodash';
import { elasticsearchServiceMock } from 'src/core/server/mocks';
import { getUpgradeAssistantStatus } from './es_migration_apis';
import { DeprecationAPIResponse } from 'src/legacy/core_plugins/elasticsearch';
import { getUpgradeAssistantStatus } from './es_migration_apis';
import fakeDeprecations from './__fixtures__/fake_deprecations.json';
const fakeIndexNames = Object.keys(fakeDeprecations.index_settings);
describe('getUpgradeAssistantStatus', () => {
const resolvedIndices = {
indices: fakeIndexNames.map((f) => ({ name: f, attributes: ['open'] })),
};
let deprecationsResponse: DeprecationAPIResponse;
const dataClient = elasticsearchServiceMock.createLegacyScopedClusterClient();
(dataClient.callAsCurrentUser as jest.Mock).mockImplementation(async (api, { path, index }) => {
(dataClient.callAsCurrentUser as jest.Mock).mockImplementation(async (api, { path }) => {
if (path === '/_migration/deprecations') {
return deprecationsResponse;
} else if (api === 'cluster.state') {
return {
metadata: {
indices: {
...index.reduce((acc: any, i: any) => {
return {
...acc,
[i]: {
state: 'open',
},
};
}, {}),
},
},
};
} else if (path === '/_resolve/index/*') {
return resolvedIndices;
} else if (api === 'indices.getMapping') {
return {};
} else {

View file

@ -34,7 +34,7 @@ export async function getUpgradeAssistantStatus(
indices.forEach((indexData) => {
indexData.blockerForReindexing =
indexStates[indexData.index!] === 'close' ? 'index-closed' : undefined;
indexStates[indexData.index!] === 'closed' ? 'index-closed' : undefined;
});
}

View file

@ -346,8 +346,8 @@ export const reindexServiceFactory = (
// Where possible, derive reindex options at the last moment before reindexing
// to prevent them from becoming stale as they wait in the queue.
const indicesState = await esIndicesStateCheck(callAsUser, [indexName]);
const openAndClose = indicesState[indexName] === 'close';
if (indicesState[indexName] === 'close') {
const shouldOpenAndClose = indicesState[indexName] === 'closed';
if (shouldOpenAndClose) {
log.debug(`Detected closed index ${indexName}, opening...`);
await callAsUser('indices.open', { index: indexName });
}
@ -369,7 +369,7 @@ export const reindexServiceFactory = (
...(reindexOptions ?? {}),
// Indicate to downstream states whether we opened a closed index that should be
// closed again.
openAndClose,
openAndClose: shouldOpenAndClose,
},
});
};

View file

@ -9,6 +9,5 @@ export default function ({ loadTestFile }) {
this.tags('ciGroup7');
loadTestFile(require.resolve('./reindexing'));
loadTestFile(require.resolve('./status'));
});
}

View file

@ -8,7 +8,7 @@ import expect from '@kbn/expect';
import { ReindexStatus, REINDEX_OP_TYPE } from '../../../plugins/upgrade_assistant/common/types';
import { generateNewIndexName } from '../../../plugins/upgrade_assistant/server/lib/reindexing/index_settings';
import { getIndexStateFromClusterState } from '../../../plugins/upgrade_assistant/common/get_index_state_from_cluster_state';
import { getIndexState } from '../../../plugins/upgrade_assistant/common/get_index_state';
export default function ({ getService }) {
const supertest = getService('supertest');
@ -190,7 +190,7 @@ export default function ({ getService }) {
expect(result.body.enqueued.length).to.equal(3);
expect(result.body.errors.length).to.equal(0);
const [{ newIndexName: newTest1Name }] = result.body.enqueued;
const [{ newIndexName: nameOfIndexThatShouldBeClosed }] = result.body.enqueued;
await assertQueueState(test1, 3);
await waitForReindexToComplete(test1);
@ -204,16 +204,12 @@ export default function ({ getService }) {
await assertQueueState(undefined, 0);
// Check that the closed index is still closed after reindexing
const clusterStateResponse = await es.cluster.state({
index: newTest1Name,
metric: 'metadata',
const resolvedIndices = await es.transport.request({
path: `_resolve/index/${nameOfIndexThatShouldBeClosed}`,
});
const test1ReindexedState = getIndexStateFromClusterState(
newTest1Name,
clusterStateResponse
);
expect(test1ReindexedState).to.be('close');
const test1ReindexedState = getIndexState(nameOfIndexThatShouldBeClosed, resolvedIndices);
expect(test1ReindexedState).to.be('closed');
} finally {
await cleanupReindex(test1);
await cleanupReindex(test2);

View file

@ -1,58 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import expect from '@kbn/expect';
import { FtrProviderContext } from '../../api_integration/ftr_provider_context';
import { ClusterStateAPIResponse } from '../../../plugins/upgrade_assistant/common/types';
import { getIndexStateFromClusterState } from '../../../plugins/upgrade_assistant/common/get_index_state_from_cluster_state';
// eslint-disable-next-line import/no-default-export
export default function ({ getService }: FtrProviderContext) {
const esArchiver = getService('esArchiver');
const es = getService('es');
describe('status and _cluster/state contract', () => {
beforeEach(async () => {
await es.indices.open({ index: '7.0-data' });
});
afterEach(async () => {
await es.indices.open({ index: '7.0-data' });
});
// According to https://www.elastic.co/guide/en/elasticsearch/reference/7.6/cluster-state.html
// The response from this call is considered internal and subject to change. We check that
// the contract has not changed in this integration test.
it('the _cluster/state endpoint is still what we expect', async () => {
await esArchiver.load('upgrade_assistant/reindex');
await es.indices.close({ index: '7.0-data' });
const result = await es.cluster.state<ClusterStateAPIResponse>({
index: '7.0-data',
metric: 'metadata',
});
try {
if (getIndexStateFromClusterState('7.0-data', result.body) === 'close') {
return;
}
} catch (e) {
expect().fail(
`Can no longer access index open/closed state. Please update Upgrade Assistant checkup. (${e.message})`
);
return;
}
expect().fail(
`The response contract for _cluster/state metadata has changed. Please update Upgrade Assistant checkup. Received ${JSON.stringify(
result,
null,
2
)}.
Expected body.metadata.indices['7.0-data'].state to be "close".`
);
});
});
}