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,88 @@
* 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';
import { PathReporter } from 'io-ts/lib/PathReporter';

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

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 return correct error message', () => {
['-1kb', '5gb', '6tb'].map(input => {
it(`${JSON.stringify(input)}`, () => {
const result = bytesRt.decode(input);
const message = PathReporter.report(result)[0];
expect(message).toEqual('Must be greater than 0b');
expect(isRight(result)).toBeFalsy();
});
});
});

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 1kb', () => {
const bytesRt = getBytesRt({
min: '500b',
max: '1kb'
});
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 return correct error message', () => {
['-1b', '1b', '499b', '1025b', '2kb', '1mb'].map(input => {
it(`${JSON.stringify(input)}`, () => {
const result = bytesRt.decode(input);
const message = PathReporter.report(result)[0];
expect(message).toEqual('Must be between 500b and 1kb');
expect(isRight(result)).toBeFalsy();
});
});
});
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 @@ -7,27 +7,50 @@
import * as t from 'io-ts';
import { either } from 'fp-ts/lib/Either';
import { amountAndUnitToObject } from '../amount_and_unit';
import { getRangeTypeMessage } from './get_range_type';

export const BYTE_UNITS = ['b', 'kb', 'mb'];
function toBytes(amount: number, unit: string) {
switch (unit) {
case 'b':
return amount;
case 'kb':
return amount * 2 ** 10;
case 'mb':
return amount * 2 ** 20;
}
}

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;
function amountAndUnitToBytes(value?: string): number | undefined {
if (value) {
const { amount, unit } = amountAndUnitToObject(value);
if (isFinite(amount) && unit) {
return toBytes(amount, unit);
}
}
}

return isValid
? t.success(inputAsString)
: t.failure(
input,
context,
`Must have numeric amount and a valid unit (${BYTE_UNITS})`
);
});
},
t.identity
);
export function getBytesRt({ min, max }: { min?: string; max?: string }) {
const minAsBytes = amountAndUnitToBytes(min) ?? -Infinity;
const maxAsBytes = amountAndUnitToBytes(max) ?? Infinity;
const message = getRangeTypeMessage(min, max);

return new t.Type<string, string, unknown>(
'bytesRt',
t.string.is,
(input, context) => {
return either.chain(t.string.validate(input, context), inputAsString => {
const inputAsBytes = amountAndUnitToBytes(inputAsString);

const isValidAmount =
typeof inputAsBytes !== 'undefined' &&
cauemarcondes marked this conversation as resolved.
Show resolved Hide resolved
inputAsBytes >= minAsBytes &&
inputAsBytes <= maxAsBytes;

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