[APM] Agent remote config: validation for Java agent configs

x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingFormRow.tsx

@@ -18,7 +18,7 @@ import {
 } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { SettingDefinition } from '../../../../../../../../../../plugins/apm/common/agent_configuration/setting_definitions/types';
-import { isValid } from '../../../../../../../../../../plugins/apm/common/agent_configuration/setting_definitions';
+import { validateSetting } from '../../../../../../../../../../plugins/apm/common/agent_configuration/setting_definitions';
 import {
   amountAndUnitToString,
   amountAndUnitToObject
@@ -93,7 +93,10 @@ function FormRow({
             <EuiFieldNumber
               placeholder={setting.placeholder}
               value={(amount as unknown) as number}
-              min={'min' in setting ? setting.min : 1}
+              // Min and max settings are string in the duration type, representing the amount and unit e.g.: '1s'.
+              // becasue of that only uses it when it's defined as number.
+              min={typeof setting.min === 'number' ? setting.min : undefined}
+              max={typeof setting.max === 'number' ? setting.max : undefined}
               onChange={e =>
                 onChange(
                   setting.key,
@@ -137,7 +140,8 @@ export function SettingFormRow({
   value?: string;
   onChange: (key: string, value: string) => void;
 }) {
-  const isInvalid = value != null && value !== '' && !isValid(setting, value);
+  const { isValid, message } = validateSetting(setting, value);
+  const isInvalid = value != null && value !== '' && !isValid;
 
   return (
     <EuiDescribedFormGroup
@@ -170,11 +174,7 @@ export function SettingFormRow({
         </>
       }
     >
-      <EuiFormRow
-        label={setting.key}
-        error={setting.validationError}
-        isInvalid={isInvalid}
-      >
+      <EuiFormRow label={setting.key} error={message} isInvalid={isInvalid}>
         <FormRow onChange={onChange} setting={setting} value={value} />
       </EuiFormRow>
     </EuiDescribedFormGroup>

x-pack/legacy/plugins/apm/public/components/app/Settings/AgentConfigurations/AgentConfigurationCreateEdit/SettingsPage/SettingsPage.tsx

@@ -29,7 +29,7 @@ import { AgentConfigurationIntake } from '../../../../../../../../../../plugins/
 import {
   filterByAgent,
   settingDefinitions,
-  isValid
+  validateSetting
 } from '../../../../../../../../../../plugins/apm/common/agent_configuration/setting_definitions';
 import { saveConfig } from './saveConfig';
 import { useApmPluginContext } from '../../../../../../hooks/useApmPluginContext';
@@ -79,7 +79,7 @@ export function SettingsPage({
         // every setting must be valid for the form to be valid
         .every(def => {
           const value = newConfig.settings[def.key];
-          return isValid(def, value);
+          return validateSetting(def, value).isValid;
         })
     );
   }, [newConfig.settings]);

x-pack/plugins/apm/common/agent_configuration/runtime_types/agent_configuration_intake_rt.ts

@@ -6,11 +6,11 @@
 
 import * as t from 'io-ts';
 import { settingDefinitions } from '../setting_definitions';
+import { SettingValidation } from '../setting_definitions/types';
 
 // retrieve validation from config definitions settings and validate on the server
 const knownSettings = settingDefinitions.reduce<
-  // TODO: is it possible to get rid of any?
-  Record<string, t.Type<any, string, unknown>>
+  Record<string, SettingValidation>
 >((acc, { key, validation }) => {
   acc[key] = validation;
   return acc;

x-pack/plugins/apm/common/agent_configuration/runtime_types/bytes_rt.test.ts

@@ -4,25 +4,17 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { bytesRt } from './bytes_rt';
+import { getBytesRt } from './bytes_rt';
 import { isRight } from 'fp-ts/lib/Either';
 
 describe('bytesRt', () => {
+  const bytesRt = getBytesRt({
+    min: 0,
+    units: ['b', 'mb', 'kb']
+  });
+
   describe('it should not accept', () => {
-    [
-      undefined,
-      null,
-      '',
-      0,
-      'foo',
-      true,
-      false,
-      '100',
-      'mb',
-      '0kb',
-      '5gb',
-      '6tb'
-    ].map(input => {
+    ['mb', '-1kb', '5gb', '6tb'].map(input => {
       it(`${JSON.stringify(input)}`, () => {
         expect(isRight(bytesRt.decode(input))).toBe(false);
       });

x-pack/plugins/apm/common/agent_configuration/runtime_types/bytes_rt.ts

@@ -6,28 +6,51 @@
 
 import * as t from 'io-ts';
 import { either } from 'fp-ts/lib/Either';
+import { i18n } from '@kbn/i18n';
 import { amountAndUnitToObject } from '../amount_and_unit';
+import { getRangeType } from './get_range_type';
 
 export const BYTE_UNITS = ['b', 'kb', 'mb'];
 
-export const bytesRt = new t.Type<string, string, unknown>(
-  'bytesRt',
-  t.string.is,
-  (input, context) => {
-    return either.chain(t.string.validate(input, context), inputAsString => {
-      const { amount, unit } = amountAndUnitToObject(inputAsString);
-      const amountAsInt = parseInt(amount, 10);
-      const isValidUnit = BYTE_UNITS.includes(unit);
-      const isValid = amountAsInt > 0 && isValidUnit;
+export function getBytesRt({
+  min = -Infinity,
+  max = Infinity,
+  units
+}: {
+  min?: number;
+  max?: number;
+  units: string[];
+}) {
+  const message = i18n.translate('xpack.apm.agentConfig.bytes.errorText', {
+    defaultMessage: `{rangeType, select,
+        between {Must be between {min} and {max} with unit: {units}}
+        gt {Must be greater than {min} with unit: {units}}
+        lt {Must be less than {max} with unit: {units}}
+        other {Must be an integer with unit: {units}}
+      }`,
+    values: {
+      min,
+      max,
+      units: units.join(', '),
+      rangeType: getRangeType(min, max)
+    }
+  });
 
-      return isValid
-        ? t.success(inputAsString)
-        : t.failure(
-            input,
-            context,
-            `Must have numeric amount and a valid unit (${BYTE_UNITS})`
-          );
-    });
-  },
-  t.identity
-);
+  return new t.Type<string, string, unknown>(
+    'bytesRt',
+    t.string.is,
+    (input, context) => {
+      return either.chain(t.string.validate(input, context), inputAsString => {
+        const { amount, unit } = amountAndUnitToObject(inputAsString);
+        const amountAsInt = parseInt(amount, 10);
+        const isValidUnit = units.includes(unit);
+        const isValid = amountAsInt >= min && amountAsInt <= max && isValidUnit;
+
+        return isValid
+          ? t.success(inputAsString)
+          : t.failure(input, context, message);
+      });
+    },
+    t.identity
+  );
+}

x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.test.ts

@@ -4,62 +4,92 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-/*
- * 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 { durationRt, getDurationRt } from './duration_rt';
+import { getDurationRt } from './duration_rt';
 import { isRight } from 'fp-ts/lib/Either';
 
-describe('durationRt', () => {
-  describe('it should not accept', () => {
-    [
-      undefined,
-      null,
-      '',
-      0,
-      'foo',
-      true,
-      false,
-      '100',
-      's',
-      'm',
-      '0ms',
-      '-1ms'
-    ].map(input => {
-      it(`${JSON.stringify(input)}`, () => {
-        expect(isRight(durationRt.decode(input))).toBe(false);
+describe('getDurationRt', () => {
+  const units = ['ms', 's', 'm'];
+  describe('must be at least 1m', () => {
+    const customDurationRt = getDurationRt({ min: '1m', units });
+    describe('it should not accept', () => {
+      [
+        undefined,
+        null,
+        '',
+        0,
+        'foo',
+        true,
+        false,
+        '0m',
+        '-1m',
+        '1ms',
+        '1s'
+      ].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(customDurationRt.decode(input))).toBeFalsy();
+        });
       });
     });
-  });
-
-  describe('it should accept', () => {
-    ['1000ms', '2s', '3m', '1s'].map(input => {
-      it(`${JSON.stringify(input)}`, () => {
-        expect(isRight(durationRt.decode(input))).toBe(true);
+    describe('it should accept', () => {
+      ['1m', '2m', '1000m'].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(customDurationRt.decode(input))).toBeTruthy();
+        });
       });
     });
   });
-});
 
-describe('getDurationRt', () => {
-  const customDurationRt = getDurationRt({ min: -1 });
-  describe('it should not accept', () => {
-    [undefined, null, '', 0, 'foo', true, false, '100', 's', 'm', '-2ms'].map(
-      input => {
+  describe('must be between 1ms and 1s', () => {
+    const customDurationRt = getDurationRt({ min: '1ms', max: '1s', units });
+
+    describe('it should not accept', () => {
+      [
+        undefined,
+        null,
+        '',
+        0,
+        'foo',
+        true,
+        false,
+        '-1s',
+        '0s',
+        '2s',
+        '1001ms',
+        '0ms',
+        '-1ms',
+        '0m',
+        '1m'
+      ].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(customDurationRt.decode(input))).toBeFalsy();
+        });
+      });
+    });
+    describe('it should accept', () => {
+      ['1s', '1ms', '50ms', '1000ms'].map(input => {
         it(`${JSON.stringify(input)}`, () => {
-          expect(isRight(customDurationRt.decode(input))).toBe(false);
+          expect(isRight(customDurationRt.decode(input))).toBeTruthy();
         });
-      }
-    );
+      });
+    });
   });
+  describe('must be max 1m', () => {
+    const customDurationRt = getDurationRt({ max: '1m', units });
 
-  describe('it should accept', () => {
-    ['1000ms', '2s', '3m', '1s', '-1s', '0ms'].map(input => {
-      it(`${JSON.stringify(input)}`, () => {
-        expect(isRight(customDurationRt.decode(input))).toBe(true);
+    describe('it should not accept', () => {
+      [undefined, null, '', 0, 'foo', true, false, '2m', '61s', '60001ms'].map(
+        input => {
+          it(`${JSON.stringify(input)}`, () => {
+            expect(isRight(customDurationRt.decode(input))).toBeFalsy();
+          });
+        }
+      );
+    });
+    describe('it should accept', () => {
+      ['1m', '0m', '-1m', '60s', '6000ms', '1ms', '1s'].map(input => {
+        it(`${JSON.stringify(input)}`, () => {
+          expect(isRight(customDurationRt.decode(input))).toBeTruthy();
+        });
       });
     });
   });

x-pack/plugins/apm/common/agent_configuration/runtime_types/duration_rt.ts

@@ -6,32 +6,68 @@
 
 import * as t from 'io-ts';
 import { either } from 'fp-ts/lib/Either';
+import moment, { unitOfTime } from 'moment';
+import { i18n } from '@kbn/i18n';
 import { amountAndUnitToObject } from '../amount_and_unit';
+import { getRangeType } from './get_range_type';
 
-export const DURATION_UNITS = ['ms', 's', 'm'];
+function getDuration({ amount, unit }: { amount: string; unit: string }) {
+  return moment.duration(parseInt(amount, 10), unit as unitOfTime.Base);
+}
+
+export function getDurationRt({
+  min,
+  max,
+  units
+}: {
+  min?: string;
+  max?: string;
+  units: string[];
+}) {
+  const minAmountAndUnit = min && amountAndUnitToObject(min);
+  const maxAmountAndUnit = max && amountAndUnitToObject(max);
 
-export function getDurationRt({ min }: { min: number }) {
+  const message = i18n.translate('xpack.apm.agentConfig.duration.errorText', {
+    defaultMessage: `{rangeType, select,
+      between {Must be between {min} and {max}}
+      gt {Must be greater than {min}}
+      lt {Must be less than {max}}
+      other {Must be an integer}
+    }`,
+    values: {
+      min,
+      max,
+      rangeType: getRangeType(
+        minAmountAndUnit ? parseInt(minAmountAndUnit.amount, 10) : undefined,
+        maxAmountAndUnit ? parseInt(maxAmountAndUnit.amount, 10) : undefined
+      )
+    }
+  });
   return new t.Type<string, string, unknown>(
     'durationRt',
     t.string.is,
     (input, context) => {
       return either.chain(t.string.validate(input, context), inputAsString => {
         const { amount, unit } = amountAndUnitToObject(inputAsString);
-        const amountAsInt = parseInt(amount, 10);
-        const isValidUnit = DURATION_UNITS.includes(unit);
-        const isValid = amountAsInt >= min && isValidUnit;
+        const inputDuration = getDuration({ amount, unit });
+
+        const minDuration = minAmountAndUnit
+          ? getDuration(minAmountAndUnit)
+          : inputDuration;
 
-        return isValid
+        const maxDuration = maxAmountAndUnit
+          ? getDuration(maxAmountAndUnit)
+          : inputDuration;
+
+        const isValidUnit = units.includes(unit);
+        const isValidAmount =
+          inputDuration >= minDuration && inputDuration <= maxDuration;
+
+        return isValidUnit && isValidAmount
           ? t.success(inputAsString)
-          : t.failure(
-              input,
-              context,
-              `Must have numeric amount and a valid unit (${DURATION_UNITS})`
-            );
+          : t.failure(input, context, message);
       });
     },
     t.identity
   );
 }
-
-export const durationRt = getDurationRt({ min: 1 });