Skip to content

Commit

Permalink
[ILM] Policy form should not throw away data (#83077) (#83893)
Browse files Browse the repository at this point in the history
* fix ilm policy deserialization

* reorder expected jest object to match actual

* fix removal of wait for snapshot if it is not on the form

* add client integration test for policy serialization of unknown fields

* save on a few chars

* added unit test for deserializer and serializer

* Implement feedback

- move serializer function around a little bit
- move serialize migrate and allocate function out of serializer
  file

* Updated serialization unit test coverage

- removed the "forcemergeEnabled" meta field that was not being
  used
- added test cases for deleting of values from policies

* fixed minor issue in how serialization tests are being set up

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
jloleysens and kibanamachine authored Nov 23, 2020
1 parent cacd199 commit 968b6e5
Show file tree
Hide file tree
Showing 11 changed files with 550 additions and 193 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,41 @@ export const POLICY_WITH_NODE_ROLE_ALLOCATION: PolicyFromES = {
},
name: POLICY_NAME,
};

export const POLICY_WITH_KNOWN_AND_UNKNOWN_FIELDS = ({
version: 1,
modified_date: Date.now().toString(),
policy: {
foo: 'bar',
phases: {
hot: {
min_age: '0ms',
actions: {
rollover: {
unknown_setting: 123,
max_size: '50gb',
},
},
},
warm: {
actions: {
my_unfollow_action: {},
set_priority: {
priority: 22,
unknown_setting: true,
},
},
},
delete: {
wait_for_snapshot: {
policy: SNAPSHOT_POLICY_NAME,
},
delete: {
delete_searchable_snapshot: true,
},
},
},
name: POLICY_NAME,
},
name: POLICY_NAME,
} as any) as PolicyFromES;
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
POLICY_WITH_INCLUDE_EXCLUDE,
POLICY_WITH_NODE_ATTR_AND_OFF_ALLOCATION,
POLICY_WITH_NODE_ROLE_ALLOCATION,
POLICY_WITH_KNOWN_AND_UNKNOWN_FIELDS,
getDefaultHotPhasePolicy,
} from './constants';

Expand All @@ -31,6 +32,70 @@ describe('<EditPolicy />', () => {
server.restore();
});

describe('serialization', () => {
/**
* We assume that policies that populate this form are loaded directly from ES and so
* are valid according to ES. There may be settings in the policy created through the ILM
* API that the UI does not cater for, like the unfollow action. We do not want to overwrite
* the configuration for these actions in the UI.
*/
it('preserves policy settings it did not configure', async () => {
httpRequestsMockHelpers.setLoadPolicies([POLICY_WITH_KNOWN_AND_UNKNOWN_FIELDS]);
httpRequestsMockHelpers.setLoadSnapshotPolicies([]);
httpRequestsMockHelpers.setListNodes({
nodesByRoles: {},
nodesByAttributes: { test: ['123'] },
isUsingDeprecatedDataRoleConfig: false,
});

await act(async () => {
testBed = await setup();
});

const { component, actions } = testBed;
component.update();

// Set max docs to test whether we keep the unknown fields in that object after serializing
await actions.hot.setMaxDocs('1000');
// Remove the delete phase to ensure that we also correctly remove data
await actions.delete.enable(false);
await actions.savePolicy();

const latestRequest = server.requests[server.requests.length - 1];
const entirePolicy = JSON.parse(JSON.parse(latestRequest.requestBody).body);

expect(entirePolicy).toEqual({
foo: 'bar', // Made up value
name: 'my_policy',
phases: {
hot: {
actions: {
rollover: {
max_docs: 1000,
max_size: '50gb',
unknown_setting: 123, // Made up setting that should stay preserved
},
set_priority: {
priority: 100,
},
},
min_age: '0ms',
},
warm: {
actions: {
my_unfollow_action: {}, // Made up action
set_priority: {
priority: 22,
unknown_setting: true,
},
},
min_age: '0ms',
},
},
});
});
});

describe('hot phase', () => {
describe('serialization', () => {
beforeEach(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,12 @@ describe('edit policy', () => {
phases: {
hot: {
actions: {
set_priority: {
priority: 100,
},
rollover: {
max_size: '50gb',
max_age: '30d',
max_size: '50gb',
},
set_priority: {
priority: 100,
},
},
min_age: '0ms',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@ export const deserializer = (policy: SerializedPolicy): FormInternal => {
const _meta: FormInternal['_meta'] = {
hot: {
useRollover: Boolean(hot?.actions?.rollover),
forceMergeEnabled: Boolean(hot?.actions?.forcemerge),
bestCompression: hot?.actions?.forcemerge?.index_codec === 'best_compression',
},
warm: {
enabled: Boolean(warm),
warmPhaseOnRollover: Boolean(warm?.min_age === '0ms'),
forceMergeEnabled: Boolean(warm?.actions?.forcemerge),
bestCompression: warm?.actions?.forcemerge?.index_codec === 'best_compression',
dataTierAllocationType: determineDataTierAllocationType(warm?.actions),
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
/*
* 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 { setAutoFreeze } from 'immer';
import { cloneDeep } from 'lodash';
import { SerializedPolicy } from '../../../../../common/types';
import { deserializer } from './deserializer';
import { createSerializer } from './serializer';
import { FormInternal } from '../types';

const isObject = (v: unknown): v is { [key: string]: any } =>
Object.prototype.toString.call(v) === '[object Object]';

const unknownValue = { some: 'value' };

const populateWithUnknownEntries = (v: unknown) => {
if (isObject(v)) {
for (const key of Object.keys(v)) {
if (['require', 'include', 'exclude'].includes(key)) continue; // this will generate an invalid policy
populateWithUnknownEntries(v[key]);
}
v.unknown = unknownValue;
return;
}
if (Array.isArray(v)) {
v.forEach(populateWithUnknownEntries);
}
};

const originalPolicy: SerializedPolicy = {
name: 'test',
phases: {
hot: {
actions: {
rollover: {
max_age: '1d',
max_size: '10gb',
max_docs: 1000,
},
forcemerge: {
index_codec: 'best_compression',
max_num_segments: 22,
},
set_priority: {
priority: 1,
},
},
min_age: '12ms',
},
warm: {
min_age: '12ms',
actions: {
shrink: { number_of_shards: 12 },
allocate: {
number_of_replicas: 3,
},
set_priority: {
priority: 10,
},
migrate: { enabled: false },
},
},
cold: {
min_age: '30ms',
actions: {
allocate: {
number_of_replicas: 12,
require: { test: 'my_value' },
include: { test: 'my_value' },
exclude: { test: 'my_value' },
},
freeze: {},
set_priority: {
priority: 12,
},
},
},
delete: {
min_age: '33ms',
actions: {
delete: {
delete_searchable_snapshot: true,
},
wait_for_snapshot: {
policy: 'test',
},
},
},
},
};

describe('deserializer and serializer', () => {
let policy: SerializedPolicy;
let serializer: ReturnType<typeof createSerializer>;
let formInternal: FormInternal;

// So that we can modify produced form objects
beforeAll(() => setAutoFreeze(false));
// This is the default in dev, so change back to true (https://github.com/immerjs/immer/blob/master/docs/freezing.md)
afterAll(() => setAutoFreeze(true));

beforeEach(() => {
policy = cloneDeep(originalPolicy);
formInternal = deserializer(policy);
// Because the policy object is not deepCloned by the form lib we
// clone here so that we can mutate the policy and preserve the
// original reference in the createSerializer
serializer = createSerializer(cloneDeep(policy));
});

it('preserves any unknown policy settings', () => {
const thisTestPolicy = cloneDeep(originalPolicy);
// We populate all levels of the policy with entries our UI does not know about
populateWithUnknownEntries(thisTestPolicy);
serializer = createSerializer(thisTestPolicy);

const copyOfThisTestPolicy = cloneDeep(thisTestPolicy);

expect(serializer(deserializer(thisTestPolicy))).toEqual(thisTestPolicy);

// Assert that the policy we passed in is unaltered after deserialization and serialization
expect(thisTestPolicy).not.toBe(copyOfThisTestPolicy);
expect(thisTestPolicy).toEqual(copyOfThisTestPolicy);
});

it('removes all phases if they were disabled in the form', () => {
formInternal._meta.warm.enabled = false;
formInternal._meta.cold.enabled = false;
formInternal._meta.delete.enabled = false;

expect(serializer(formInternal)).toEqual({
name: 'test',
phases: {
hot: policy.phases.hot, // We expect to see only the hot phase
},
});
});

it('removes the forcemerge action if it is disabled in the form', () => {
delete formInternal.phases.hot!.actions.forcemerge;
delete formInternal.phases.warm!.actions.forcemerge;

const result = serializer(formInternal);

expect(result.phases.hot!.actions.forcemerge).toBeUndefined();
expect(result.phases.warm!.actions.forcemerge).toBeUndefined();
});

it('removes set priority if it is disabled in the form', () => {
delete formInternal.phases.hot!.actions.set_priority;
delete formInternal.phases.warm!.actions.set_priority;
delete formInternal.phases.cold!.actions.set_priority;

const result = serializer(formInternal);

expect(result.phases.hot!.actions.set_priority).toBeUndefined();
expect(result.phases.warm!.actions.set_priority).toBeUndefined();
expect(result.phases.cold!.actions.set_priority).toBeUndefined();
});

it('removes freeze setting in the cold phase if it is disabled in the form', () => {
formInternal._meta.cold.freezeEnabled = false;

const result = serializer(formInternal);

expect(result.phases.cold!.actions.freeze).toBeUndefined();
});

it('removes node attribute allocation when it is not selected in the form', () => {
// Change from 'node_attrs' to 'node_roles'
formInternal._meta.cold.dataTierAllocationType = 'node_roles';

const result = serializer(formInternal);

expect(result.phases.cold!.actions.allocate!.number_of_replicas).toBe(12);
expect(result.phases.cold!.actions.allocate!.require).toBeUndefined();
expect(result.phases.cold!.actions.allocate!.include).toBeUndefined();
expect(result.phases.cold!.actions.allocate!.exclude).toBeUndefined();
});

it('removes forcemerge and rollover config when rollover is disabled in hot phase', () => {
formInternal._meta.hot.useRollover = false;

const result = serializer(formInternal);

expect(result.phases.hot!.actions.rollover).toBeUndefined();
expect(result.phases.hot!.actions.forcemerge).toBeUndefined();
});

it('removes min_age from warm when rollover is enabled', () => {
formInternal._meta.hot.useRollover = true;
formInternal._meta.warm.warmPhaseOnRollover = true;

const result = serializer(formInternal);

expect(result.phases.warm!.min_age).toBeUndefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { i18nTexts } from '../i18n_texts';
const { emptyField, numberGreaterThanField } = fieldValidators;

const serializers = {
stringToNumber: (v: string): any => (v ? parseInt(v, 10) : undefined),
stringToNumber: (v: string): any => (v != null ? parseInt(v, 10) : undefined),
};

export const schema: FormSchema<FormInternal> = {
Expand Down
Loading

0 comments on commit 968b6e5

Please sign in to comment.