Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 18 commits into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -92,12 +92,14 @@ function FormRow({
<EuiFlexItem grow={false}>
<EuiFieldNumber
placeholder={setting.placeholder}
value={(amount as unknown) as number}
min={'min' in setting ? setting.min : 1}
value={amount}
onChange={e =>
onChange(
setting.key,
amountAndUnitToString({ amount: e.target.value, unit })
amountAndUnitToString({
amount: parseInt(e.target.value, 10),
unit
})
)
}
/>
Expand Down Expand Up @@ -137,7 +139,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
Expand Down Expand Up @@ -170,11 +173,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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
* you may not use this file except in compliance with the Elastic License.
*/

interface AmountAndUnit {
amount: string;
export interface AmountAndUnit {
amount: number;
unit: string;
}

export function amountAndUnitToObject(value: string): AmountAndUnit {
// matches any postive and negative number and its unit.
const [, amount = '', unit = ''] = value.match(/(^-?\d+)?(\w+)?/) || [];
return { amount, unit };
return { amount: parseInt(amount, 10), unit };
}

export function amountAndUnitToString({ amount, unit }: AmountAndUnit) {
cauemarcondes marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,70 @@
* 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', () => {
describe('it should not accept', () => {
[
undefined,
null,
'',
0,
'foo',
true,
false,
'100',
'mb',
'0kb',
'5gb',
'6tb'
].map(input => {
it(`${JSON.stringify(input)}`, () => {
expect(isRight(bytesRt.decode(input))).toBe(false);
describe('must accept any amount and unit', () => {
const bytesRt = getBytesRt({
units: ['b', 'mb', 'kb']
});
describe('it should not accept', () => {
['mb', 1, '1', '5gb', '6tb'].map(input => {
it(`${JSON.stringify(input)}`, () => {
expect(isRight(bytesRt.decode(input))).toBe(false);
});
});
});

describe('it should accept', () => {
['-1b', '0mb', '1b', '2kb', '3mb', '1000mb'].map(input => {
it(`${JSON.stringify(input)}`, () => {
expect(isRight(bytesRt.decode(input))).toBe(true);
});
});
});
});
describe('must be at least 0b', () => {
const bytesRt = getBytesRt({
min: '0b',
units: ['b', 'mb', 'kb']
});

describe('it should not accept', () => {
['mb', '-1kb', '5gb', '6tb'].map(input => {
it(`${JSON.stringify(input)}`, () => {
expect(isRight(bytesRt.decode(input))).toBe(false);
});
});
});

describe('it should accept', () => {
['1b', '2kb', '3mb'].map(input => {
it(`${JSON.stringify(input)}`, () => {
expect(isRight(bytesRt.decode(input))).toBe(true);
describe('it should accept', () => {
['1b', '2kb', '3mb'].map(input => {
it(`${JSON.stringify(input)}`, () => {
expect(isRight(bytesRt.decode(input))).toBe(true);
});
});
});
});
describe('must be between 500b and 1mb', () => {
const bytesRt = getBytesRt({
min: '500b',
max: '1kb',
units: ['b', 'mb', 'kb']
});
describe('it should not accept', () => {
['mb', '-1b', '1b', '499b', '1025b', '2kb', '1mb'].map(input => {
it(`${JSON.stringify(input)}`, () => {
expect(isRight(bytesRt.decode(input))).toBe(false);
});
});
});
describe('it should accept', () => {
['500b', '1024b', '1kb'].map(input => {
it(`${JSON.stringify(input)}`, () => {
expect(isRight(bytesRt.decode(input))).toBe(true);
});
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,74 @@

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;

return isValid
? t.success(inputAsString)
: t.failure(
input,
context,
`Must have numeric amount and a valid unit (${BYTE_UNITS})`
);
});
},
t.identity
);
function toBytes(amount: number, unit: string) {
switch (unit) {
case 'kb':
return amount * 2 ** 10;
case 'mb':
return amount * 2 ** 20;
case 'b':
default:
return amount;
}
}

export function getBytesRt({
min,
max,
units
}: {
min?: string;
max?: string;
units: string[];
cauemarcondes marked this conversation as resolved.
Show resolved Hide resolved
}) {
const { amount: minAmount, unit: minUnit } = min
? amountAndUnitToObject(min)
: { amount: -Infinity, unit: 'b' };

const { amount: maxAmount, unit: maxUnit } = max
? amountAndUnitToObject(max)
: { amount: Infinity, unit: 'mb' };

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(minAmount, maxAmount)
}
});

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 isValidUnit = units.includes(unit);

const inputAsBytes = toBytes(amount, unit);
const minAsBytes = toBytes(minAmount, minUnit);
const maxAsBytes = toBytes(maxAmount, maxUnit);
cauemarcondes marked this conversation as resolved.
Show resolved Hide resolved

const isValidAmount =
inputAsBytes >= minAsBytes && inputAsBytes <= maxAsBytes;

return isValidUnit && isValidAmount
? t.success(inputAsString)
: t.failure(input, context, message);
});
},
t.identity
);
}
Loading