[ILM] Fix set replicas serialization and validation (#85233)

* added fix for serializing replicas and updated validation to correctly check for non negative nr

* added tests and fixed incorrect use of warm phase setting in cold phase

* remove unused import

* clean up use of Boolean and rename nrOfReplicas -> numberOfReplicas

* fix comment

x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/cold_phase/cold_phase.tsx

@@ -141,9 +141,8 @@ export const ColdPhase: FunctionComponent = () => {
                         'xpack.indexLifecycleMgmt.editPolicy.coldPhase.numberOfReplicas.switchLabel',
                         { defaultMessage: 'Set replicas' }
                       ),
-                      initialValue: Boolean(
-                        policy.phases.cold?.actions?.allocate?.number_of_replicas
-                      ),
+                      initialValue:
+                        policy.phases.cold?.actions?.allocate?.number_of_replicas != null,
                     }}
                     fullWidth
                   >

x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/forcemerge_field.tsx

@@ -24,7 +24,7 @@ export const ForcemergeField: React.FunctionComponent<Props> = ({ phase }) => {
   const { policy } = useEditPolicyContext();
 
   const initialToggleValue = useMemo<boolean>(() => {
-    return Boolean(policy.phases[phase]?.actions?.forcemerge);
+    return policy.phases[phase]?.actions?.forcemerge != null;
   }, [policy, phase]);
 
   return (

x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase/warm_phase.tsx

@@ -162,7 +162,7 @@ export const WarmPhase: FunctionComponent = () => {
                   'xpack.indexLifecycleMgmt.editPolicy.warmPhase.numberOfReplicas.switchLabel',
                   { defaultMessage: 'Set replicas' }
                 ),
-                initialValue: Boolean(policy.phases.warm?.actions?.allocate?.number_of_replicas),
+                initialValue: policy.phases.warm?.actions?.allocate?.number_of_replicas != null,
               }}
               fullWidth
             >
@@ -203,7 +203,7 @@ export const WarmPhase: FunctionComponent = () => {
                   'data-test-subj': 'shrinkSwitch',
                   label: i18nTexts.shrinkLabel,
                   'aria-label': i18nTexts.shrinkLabel,
-                  initialValue: Boolean(policy.phases.warm?.actions?.shrink),
+                  initialValue: policy.phases.warm?.actions?.shrink != null,
                 }}
                 fullWidth
               >

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

@@ -56,11 +56,17 @@ const originalPolicy: SerializedPolicy = {
         shrink: { number_of_shards: 12 },
         allocate: {
           number_of_replicas: 3,
+          include: {
+            some: 'value',
+          },
+          exclude: {
+            some: 'value',
+          },
         },
         set_priority: {
           priority: 10,
         },
-        migrate: { enabled: false },
+        migrate: { enabled: true },
       },
     },
     cold: {
@@ -133,7 +139,8 @@ describe('deserializer and serializer', () => {
 
     const copyOfThisTestPolicy = cloneDeep(thisTestPolicy);
 
-    expect(serializer(deserializer(thisTestPolicy))).toEqual(thisTestPolicy);
+    const _formInternal = deserializer(thisTestPolicy);
+    expect(serializer(_formInternal)).toEqual(thisTestPolicy);
 
     // Assert that the policy we passed in is unaltered after deserialization and serialization
     expect(thisTestPolicy).not.toBe(copyOfThisTestPolicy);
@@ -247,4 +254,40 @@ describe('deserializer and serializer', () => {
       },
     });
   });
+
+  it('sets all known allocate options correctly', () => {
+    formInternal.phases.warm!.actions.allocate!.number_of_replicas = 0;
+    formInternal._meta.warm.dataTierAllocationType = 'node_attrs';
+    formInternal._meta.warm.allocationNodeAttribute = 'some:value';
+
+    expect(serializer(formInternal).phases.warm!.actions.allocate).toEqual({
+      number_of_replicas: 0,
+      require: {
+        some: 'value',
+      },
+      include: {
+        some: 'value',
+      },
+      exclude: {
+        some: 'value',
+      },
+    });
+  });
+
+  it('sets allocate and migrate actions when defined together', () => {
+    formInternal.phases.warm!.actions.allocate!.number_of_replicas = 0;
+    formInternal._meta.warm.dataTierAllocationType = 'none';
+    // This should not be set...
+    formInternal._meta.warm.allocationNodeAttribute = 'some:value';
+
+    const result = serializer(formInternal);
+
+    expect(result.phases.warm!.actions.allocate).toEqual({
+      number_of_replicas: 0,
+    });
+
+    expect(result.phases.warm!.actions.migrate).toEqual({
+      enabled: false,
+    });
+  });
 });

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

@@ -202,7 +202,7 @@ export const schema: FormSchema<FormInternal> = {
             }),
             validations: [
               {
-                validator: ifExistsNumberGreaterThanZero,
+                validator: ifExistsNumberNonNegative,
               },
             ],
             serializer: serializers.stringToNumber,
@@ -273,7 +273,7 @@ export const schema: FormSchema<FormInternal> = {
             }),
             validations: [
               {
-                validator: ifExistsNumberGreaterThanZero,
+                validator: ifExistsNumberNonNegative,
               },
             ],
             serializer: serializers.stringToNumber,

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

@@ -11,18 +11,33 @@ import { SerializedActionWithAllocation } from '../../../../../../common/types';
 import { DataAllocationMetaFields } from '../../types';
 
 export const serializeMigrateAndAllocateActions = (
+  /**
+   * Form metadata about what tier allocation strategy to use and custom node
+   * allocation information.
+   */
   { dataTierAllocationType, allocationNodeAttribute }: DataAllocationMetaFields,
-  newActions: SerializedActionWithAllocation = {},
-  originalActions: SerializedActionWithAllocation = {}
+  /**
+   * The new configuration merged with old configuration to ensure we don't lose
+   * any fields.
+   */
+  mergedActions: SerializedActionWithAllocation = {},
+  /**
+   * The actions from the policy for a given phase when it was loaded.
+   */
+  originalActions: SerializedActionWithAllocation = {},
+  /**
+   * The number of replicas value to set in the allocate action.
+   */
+  numberOfReplicas?: number
 ): SerializedActionWithAllocation => {
-  const { allocate, migrate, ...otherActions } = newActions;
+  const { allocate, migrate, ...otherActions } = mergedActions;
 
   // First copy over all non-allocate and migrate actions.
   const actions: SerializedActionWithAllocation = { ...otherActions };
 
-  // The UI only knows about include, exclude and require, so copy over all other values.
+  // The UI only knows about include, exclude, require and number_of_replicas so copy over all other values.
   if (allocate) {
-    const { include, exclude, require, ...otherSettings } = allocate;
+    const { include, exclude, require, number_of_replicas: __, ...otherSettings } = allocate;
     if (!isEmpty(otherSettings)) {
       actions.allocate = { ...otherSettings };
     }
@@ -69,5 +84,13 @@ export const serializeMigrateAndAllocateActions = (
       break;
     default:
   }
+
+  if (numberOfReplicas != null) {
+    actions.allocate = {
+      ...actions.allocate,
+      number_of_replicas: numberOfReplicas,
+    };
+  }
+
   return actions;
 };

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

@@ -95,7 +95,8 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => (
       warmPhase.actions = serializeMigrateAndAllocateActions(
         _meta.warm,
         warmPhase.actions,
-        originalPolicy?.phases.warm?.actions
+        originalPolicy?.phases.warm?.actions,
+        updatedPolicy.phases.warm?.actions?.allocate?.number_of_replicas
       );
 
       if (!updatedPolicy.phases.warm?.actions?.forcemerge) {
@@ -129,7 +130,8 @@ export const createSerializer = (originalPolicy?: SerializedPolicy) => (
       coldPhase.actions = serializeMigrateAndAllocateActions(
         _meta.cold,
         coldPhase.actions,
-        originalPolicy?.phases.cold?.actions
+        originalPolicy?.phases.cold?.actions,
+        updatedPolicy.phases.cold?.actions?.allocate?.number_of_replicas
       );
 
       if (_meta.cold.freezeEnabled) {