[ILM] Policy form should not throw away data

x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/constants.ts

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

x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts

@@ -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';
 
@@ -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 () => {

x-pack/plugins/index_lifecycle_management/__jest__/components/edit_policy.test.tsx

@@ -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',

x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer.ts

@@ -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),
     },

x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/deserializer_and_serializer.test.ts

@@ -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();
+  });
+});

x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts

@@ -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> = {