From 809b32f9b2a8887cb39140f0b4db9271ceecd585 Mon Sep 17 00:00:00 2001 From: JImmy Kuang Date: Thu, 19 Dec 2019 10:13:44 -0800 Subject: [PATCH 1/8] Kibana should allow a min_age setting of 0ms in ILM policy phases --- .../sections/edit_policy/components/min_age_input.js | 2 +- .../public/store/selectors/lifecycle.js | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/x-pack/legacy/plugins/index_lifecycle_management/public/sections/edit_policy/components/min_age_input.js b/x-pack/legacy/plugins/index_lifecycle_management/public/sections/edit_policy/components/min_age_input.js index 0ed28bbaa905f..b4c9f4e958cd2 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/public/sections/edit_policy/components/min_age_input.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/public/sections/edit_policy/components/min_age_input.js @@ -131,7 +131,7 @@ export const MinAgeInput = props => { onChange={async e => { setPhaseData(PHASE_ROLLOVER_MINIMUM_AGE, e.target.value); }} - min={1} + min={0} /> diff --git a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js index 026845c78ee66..b287b74a7aebb 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js @@ -84,6 +84,11 @@ export const positiveNumbersAboveZeroErrorMessage = i18n.translate( } ); +export const positiveNumbersEqualAboveZeroErrorMessage = + i18n.translate('xpack.indexLifecycleMgmt.editPolicy.positiveNumberAboveZeroRequiredError', { + defaultMessage: 'Only numbers equal to or above 0 are allowed.' + }); + export const validatePhase = (type, phase, errors) => { const phaseErrors = {}; @@ -123,11 +128,12 @@ export const validatePhase = (type, phase, errors) => { } else if ( (numberedAttribute === PHASE_ROLLOVER_MINIMUM_AGE || numberedAttribute === PHASE_PRIMARY_SHARD_COUNT) && - phase[numberedAttribute] < 1 + phase[numberedAttribute] < 0 ) { - phaseErrors[numberedAttribute] = [positiveNumbersAboveZeroErrorMessage]; + phaseErrors[numberedAttribute] = [positiveNumbersEqualAboveZeroErrorMessage]; } } + } if (phase[PHASE_ROLLOVER_ENABLED]) { if ( @@ -161,6 +167,7 @@ export const validatePhase = (type, phase, errors) => { } if (phase[PHASE_FORCE_MERGE_ENABLED]) { + if (!isNumber(phase[PHASE_FORCE_MERGE_SEGMENTS])) { phaseErrors[PHASE_FORCE_MERGE_SEGMENTS] = [numberRequiredMessage]; } else if (phase[PHASE_FORCE_MERGE_SEGMENTS] < 1) { From 231071a189a116afa963ca607110fdb161cbbddf Mon Sep 17 00:00:00 2001 From: JImmy Kuang Date: Fri, 20 Dec 2019 15:29:23 -0800 Subject: [PATCH 2/8] Fixed jest by adding >= 0 in lifecycle.js line 133 --- .../public/store/selectors/lifecycle.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js index b287b74a7aebb..a9c3218731afb 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js @@ -84,10 +84,12 @@ export const positiveNumbersAboveZeroErrorMessage = i18n.translate( } ); -export const positiveNumbersEqualAboveZeroErrorMessage = - i18n.translate('xpack.indexLifecycleMgmt.editPolicy.positiveNumberAboveZeroRequiredError', { - defaultMessage: 'Only numbers equal to or above 0 are allowed.' - }); +export const positiveNumbersEqualAboveZeroErrorMessage = i18n.translate( + 'xpack.indexLifecycleMgmt.editPolicy.positiveNumberAboveZeroRequiredError', + { + defaultMessage: 'Only numbers equal to or above 0 are allowed.', + } +); export const validatePhase = (type, phase, errors) => { const phaseErrors = {}; @@ -128,12 +130,11 @@ export const validatePhase = (type, phase, errors) => { } else if ( (numberedAttribute === PHASE_ROLLOVER_MINIMUM_AGE || numberedAttribute === PHASE_PRIMARY_SHARD_COUNT) && - phase[numberedAttribute] < 0 + phase[numberedAttribute] <= 0 ) { phaseErrors[numberedAttribute] = [positiveNumbersEqualAboveZeroErrorMessage]; } } - } if (phase[PHASE_ROLLOVER_ENABLED]) { if ( @@ -167,7 +168,6 @@ export const validatePhase = (type, phase, errors) => { } if (phase[PHASE_FORCE_MERGE_ENABLED]) { - if (!isNumber(phase[PHASE_FORCE_MERGE_SEGMENTS])) { phaseErrors[PHASE_FORCE_MERGE_SEGMENTS] = [numberRequiredMessage]; } else if (phase[PHASE_FORCE_MERGE_SEGMENTS] < 1) { From 3d23c4eab481464e0388301b67807fa28d0f7640 Mon Sep 17 00:00:00 2001 From: JImmy Kuang Date: Fri, 20 Dec 2019 22:13:05 -0800 Subject: [PATCH 3/8] 1) Updated the jest conditions to allow equal to and above 0. 2) Set default values of [PHASE_ROLLOVER_MINIMUM_AGE]: 0 in [cold,warm,delete_phase].js. 3) Set phase[numberedAttribute] <= 0 in lifecycle.js --- .../__jest__/components/edit_policy.test.js | 22 ++++++------------- .../public/store/defaults/cold_phase.js | 2 +- .../public/store/defaults/delete_phase.js | 2 +- .../public/store/defaults/warm_phase.js | 2 +- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js b/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js index 9463eccb93a02..4a61d71bfa7cd 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js @@ -20,7 +20,7 @@ import sinon from 'sinon'; import { findTestSubject } from '@elastic/eui/lib/test'; import { positiveNumbersAboveZeroErrorMessage, - numberRequiredMessage, + positiveNumbersEqualAboveZeroErrorMessage, positiveNumberRequiredMessage, maximumAgeRequiredMessage, maximumSizeRequiredMessage, @@ -238,22 +238,14 @@ describe('edit policy', () => { }); }); describe('warm phase', () => { - test('should show number required error when trying to save empty warm phase', () => { - const rendered = mountWithIntl(component); - noRollover(rendered); - setPolicyName(rendered, 'mypolicy'); - activatePhase(rendered, 'warm'); - save(rendered); - expectedErrorMessages(rendered, [numberRequiredMessage]); - }); - test('should show positive number required above zero error when trying to save warm phase with 0 for after', () => { + test('should show positive number required equal to or above zero error when trying to save warm phase with 0 for after', () => { const rendered = mountWithIntl(component); noRollover(rendered); setPolicyName(rendered, 'mypolicy'); activatePhase(rendered, 'warm'); setPhaseAfter(rendered, 'warm', 0); save(rendered); - expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); + expectedErrorMessages(rendered, [positiveNumbersEqualAboveZeroErrorMessage]); }); test('should show positive number required error when trying to save warm phase with -1 for after', () => { const rendered = mountWithIntl(component); @@ -383,14 +375,14 @@ describe('edit policy', () => { }); }); describe('cold phase', () => { - test('should show positive number required error when trying to save cold phase with 0 for after', () => { + test('should show positive number required error when trying to save cold phase with less than 0 for after', () => { const rendered = mountWithIntl(component); noRollover(rendered); setPolicyName(rendered, 'mypolicy'); activatePhase(rendered, 'cold'); setPhaseAfter(rendered, 'cold', 0); save(rendered); - expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); + expectedErrorMessages(rendered, [positiveNumbersEqualAboveZeroErrorMessage]); }); test('should show positive number required error when trying to save cold phase with -1 for after', () => { const rendered = mountWithIntl(component); @@ -464,14 +456,14 @@ describe('edit policy', () => { }); }); describe('delete phase', () => { - test('should show positive number required error when trying to save delete phase with 0 for after', () => { + test('should show positive number required error when trying to save delete phase with less than 0 for after', () => { const rendered = mountWithIntl(component); noRollover(rendered); setPolicyName(rendered, 'mypolicy'); activatePhase(rendered, 'delete'); setPhaseAfter(rendered, 'delete', 0); save(rendered); - expectedErrorMessages(rendered, [positiveNumbersAboveZeroErrorMessage]); + expectedErrorMessages(rendered, [positiveNumbersEqualAboveZeroErrorMessage]); }); test('should show positive number required error when trying to save delete phase with -1 for after', () => { const rendered = mountWithIntl(component); diff --git a/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/cold_phase.js b/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/cold_phase.js index b0af0e6547803..a8f7fd3f4bdfa 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/cold_phase.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/cold_phase.js @@ -17,7 +17,7 @@ import { export const defaultColdPhase = { [PHASE_ENABLED]: false, [PHASE_ROLLOVER_ALIAS]: '', - [PHASE_ROLLOVER_MINIMUM_AGE]: '', + [PHASE_ROLLOVER_MINIMUM_AGE]: 0, [PHASE_ROLLOVER_MINIMUM_AGE_UNITS]: 'd', [PHASE_NODE_ATTRS]: '', [PHASE_REPLICA_COUNT]: '', diff --git a/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/delete_phase.js b/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/delete_phase.js index 5a44539ff90f8..b5296cd83fabd 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/delete_phase.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/delete_phase.js @@ -15,7 +15,7 @@ export const defaultDeletePhase = { [PHASE_ENABLED]: false, [PHASE_ROLLOVER_ENABLED]: false, [PHASE_ROLLOVER_ALIAS]: '', - [PHASE_ROLLOVER_MINIMUM_AGE]: '', + [PHASE_ROLLOVER_MINIMUM_AGE]: 0, [PHASE_ROLLOVER_MINIMUM_AGE_UNITS]: 'd', }; export const defaultEmptyDeletePhase = defaultDeletePhase; diff --git a/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/warm_phase.js b/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/warm_phase.js index d3dc55178b253..f02ac2096675f 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/warm_phase.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/public/store/defaults/warm_phase.js @@ -23,7 +23,7 @@ export const defaultWarmPhase = { [PHASE_ROLLOVER_ALIAS]: '', [PHASE_FORCE_MERGE_SEGMENTS]: '', [PHASE_FORCE_MERGE_ENABLED]: false, - [PHASE_ROLLOVER_MINIMUM_AGE]: '', + [PHASE_ROLLOVER_MINIMUM_AGE]: 0, [PHASE_ROLLOVER_MINIMUM_AGE_UNITS]: 'd', [PHASE_NODE_ATTRS]: '', [PHASE_SHRINK_ENABLED]: false, From 6393ae21df61785189ff4672de09e4b504b295c9 Mon Sep 17 00:00:00 2001 From: JImmy Kuang Date: Sat, 21 Dec 2019 17:08:09 -0800 Subject: [PATCH 4/8] Fix Internationalization check, line 81 was the same as line 74 --- .../public/store/selectors/lifecycle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js index a9c3218731afb..034edebbb625f 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js @@ -85,7 +85,7 @@ export const positiveNumbersAboveZeroErrorMessage = i18n.translate( ); export const positiveNumbersEqualAboveZeroErrorMessage = i18n.translate( - 'xpack.indexLifecycleMgmt.editPolicy.positiveNumberAboveZeroRequiredError', + 'xpack.indexLifecycleMgmt.editPolicy.positiveNumberEqualAboveZeroRequiredError', { defaultMessage: 'Only numbers equal to or above 0 are allowed.', } From 595ab8f4e8da079cacb77ebd5f1d14d2a99b8590 Mon Sep 17 00:00:00 2001 From: JImmy Kuang Date: Fri, 27 Dec 2019 09:57:13 -0800 Subject: [PATCH 5/8] Updated <=0 to < 0 as this will not throw error line 133 lifecycle.js --- .../public/store/selectors/lifecycle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js index 034edebbb625f..b768d7293ee35 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js @@ -130,7 +130,7 @@ export const validatePhase = (type, phase, errors) => { } else if ( (numberedAttribute === PHASE_ROLLOVER_MINIMUM_AGE || numberedAttribute === PHASE_PRIMARY_SHARD_COUNT) && - phase[numberedAttribute] <= 0 + phase[numberedAttribute] < 0 ) { phaseErrors[numberedAttribute] = [positiveNumbersEqualAboveZeroErrorMessage]; } From 1827fab6c0f5bda33d6d705f0d22ded934267473 Mon Sep 17 00:00:00 2001 From: JImmy Kuang Date: Fri, 27 Dec 2019 12:04:20 -0800 Subject: [PATCH 6/8] Remove comments --- .../__jest__/components/edit_policy.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js b/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js index 4a61d71bfa7cd..cb3b88c26cdf6 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js @@ -238,7 +238,7 @@ describe('edit policy', () => { }); }); describe('warm phase', () => { - test('should show positive number required equal to or above zero error when trying to save warm phase with 0 for after', () => { + test('', () => { const rendered = mountWithIntl(component); noRollover(rendered); setPolicyName(rendered, 'mypolicy'); @@ -375,7 +375,7 @@ describe('edit policy', () => { }); }); describe('cold phase', () => { - test('should show positive number required error when trying to save cold phase with less than 0 for after', () => { + test('', () => { const rendered = mountWithIntl(component); noRollover(rendered); setPolicyName(rendered, 'mypolicy'); @@ -456,7 +456,7 @@ describe('edit policy', () => { }); }); describe('delete phase', () => { - test('should show positive number required error when trying to save delete phase with less than 0 for after', () => { + test('', () => { const rendered = mountWithIntl(component); noRollover(rendered); setPolicyName(rendered, 'mypolicy'); From 2f7c197502d27f80ac4fcffd19af4410fcfc4276 Mon Sep 17 00:00:00 2001 From: JImmy Kuang Date: Fri, 27 Dec 2019 14:14:27 -0800 Subject: [PATCH 7/8] Updated the edit_policy.js test cases to accept 0 timing phase --- .../__jest__/components/edit_policy.test.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js b/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js index cb3b88c26cdf6..3d4bf05d964a2 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js @@ -20,7 +20,6 @@ import sinon from 'sinon'; import { findTestSubject } from '@elastic/eui/lib/test'; import { positiveNumbersAboveZeroErrorMessage, - positiveNumbersEqualAboveZeroErrorMessage, positiveNumberRequiredMessage, maximumAgeRequiredMessage, maximumSizeRequiredMessage, @@ -238,14 +237,14 @@ describe('edit policy', () => { }); }); describe('warm phase', () => { - test('', () => { + test('should allow 0 for phase timing', () => { const rendered = mountWithIntl(component); noRollover(rendered); setPolicyName(rendered, 'mypolicy'); activatePhase(rendered, 'warm'); setPhaseAfter(rendered, 'warm', 0); save(rendered); - expectedErrorMessages(rendered, [positiveNumbersEqualAboveZeroErrorMessage]); + expectedErrorMessages(rendered, []); }); test('should show positive number required error when trying to save warm phase with -1 for after', () => { const rendered = mountWithIntl(component); @@ -375,14 +374,14 @@ describe('edit policy', () => { }); }); describe('cold phase', () => { - test('', () => { + test('should allow 0 for phase timing', () => { const rendered = mountWithIntl(component); noRollover(rendered); setPolicyName(rendered, 'mypolicy'); activatePhase(rendered, 'cold'); setPhaseAfter(rendered, 'cold', 0); save(rendered); - expectedErrorMessages(rendered, [positiveNumbersEqualAboveZeroErrorMessage]); + expectedErrorMessages(rendered, []); }); test('should show positive number required error when trying to save cold phase with -1 for after', () => { const rendered = mountWithIntl(component); @@ -456,14 +455,14 @@ describe('edit policy', () => { }); }); describe('delete phase', () => { - test('', () => { + test('should allow 0 for phase timing', () => { const rendered = mountWithIntl(component); noRollover(rendered); setPolicyName(rendered, 'mypolicy'); activatePhase(rendered, 'delete'); setPhaseAfter(rendered, 'delete', 0); save(rendered); - expectedErrorMessages(rendered, [positiveNumbersEqualAboveZeroErrorMessage]); + expectedErrorMessages(rendered, []); }); test('should show positive number required error when trying to save delete phase with -1 for after', () => { const rendered = mountWithIntl(component); From 3b52342ee608a159bf83115e2340c3f002e092de Mon Sep 17 00:00:00 2001 From: JImmy Kuang Date: Thu, 9 Jan 2020 11:02:16 -0800 Subject: [PATCH 8/8] Add back test cases to edit_policy.test.js and removed operational conditions from lifecycle.js & the positiveNumbersEqualAboveZeroErrorMessage variable --- .../__jest__/components/edit_policy.test.js | 10 ++++++++++ .../public/store/selectors/lifecycle.js | 13 ------------- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js b/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js index 3d4bf05d964a2..2c0ea7fe699b8 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.js @@ -21,6 +21,7 @@ import { findTestSubject } from '@elastic/eui/lib/test'; import { positiveNumbersAboveZeroErrorMessage, positiveNumberRequiredMessage, + numberRequiredMessage, maximumAgeRequiredMessage, maximumSizeRequiredMessage, policyNameRequiredMessage, @@ -237,6 +238,15 @@ describe('edit policy', () => { }); }); describe('warm phase', () => { + test('should show number required error when trying to save empty warm phase', () => { + const rendered = mountWithIntl(component); + noRollover(rendered); + setPolicyName(rendered, 'mypolicy'); + activatePhase(rendered, 'warm'); + setPhaseAfter(rendered, 'warm', ''); + save(rendered); + expectedErrorMessages(rendered, [numberRequiredMessage]); + }); test('should allow 0 for phase timing', () => { const rendered = mountWithIntl(component); noRollover(rendered); diff --git a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js index b768d7293ee35..750a7feb19c3d 100644 --- a/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js +++ b/x-pack/legacy/plugins/index_lifecycle_management/public/store/selectors/lifecycle.js @@ -84,13 +84,6 @@ export const positiveNumbersAboveZeroErrorMessage = i18n.translate( } ); -export const positiveNumbersEqualAboveZeroErrorMessage = i18n.translate( - 'xpack.indexLifecycleMgmt.editPolicy.positiveNumberEqualAboveZeroRequiredError', - { - defaultMessage: 'Only numbers equal to or above 0 are allowed.', - } -); - export const validatePhase = (type, phase, errors) => { const phaseErrors = {}; @@ -127,12 +120,6 @@ export const validatePhase = (type, phase, errors) => { phaseErrors[numberedAttribute] = [numberRequiredMessage]; } else if (phase[numberedAttribute] < 0) { phaseErrors[numberedAttribute] = [positiveNumberRequiredMessage]; - } else if ( - (numberedAttribute === PHASE_ROLLOVER_MINIMUM_AGE || - numberedAttribute === PHASE_PRIMARY_SHARD_COUNT) && - phase[numberedAttribute] < 0 - ) { - phaseErrors[numberedAttribute] = [positiveNumbersEqualAboveZeroErrorMessage]; } } }