Skip to content

Commit

Permalink
[Fleet] Improve handling of frozen variables (#127493)
Browse files Browse the repository at this point in the history
  • Loading branch information
nchaulet authored Mar 10, 2022
1 parent ce0ccfa commit 62c2082
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 18 deletions.
3 changes: 3 additions & 0 deletions x-pack/plugins/fleet/common/openapi/bundled.json
Original file line number Diff line number Diff line change
Expand Up @@ -4247,6 +4247,9 @@
},
"description": {
"type": "string"
},
"force": {
"type": "boolean"
}
},
"required": [
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/fleet/common/openapi/bundled.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2676,6 +2676,8 @@ components:
type: string
description:
type: string
force:
type: boolean
required:
- name
- namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ properties:
type: string
description:
type: string
force:
type: boolean
required:
- name
- namespace
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/fleet/server/routes/package_policy/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export const updatePackagePolicyHandler: RequestHandler<
throw Boom.notFound('Package policy not found');
}

const body = { ...request.body };
const { force, ...body } = request.body;
// removed fields not recognized by schema
const packagePolicyInputs = packagePolicy.inputs.map((input) => {
const newInput = {
Expand Down Expand Up @@ -180,7 +180,7 @@ export const updatePackagePolicyHandler: RequestHandler<
esClient,
request.params.packagePolicyId,
newData,
{ user },
{ user, force },
packagePolicy.package?.version
);
return response.ok({
Expand Down
135 changes: 128 additions & 7 deletions x-pack/plugins/fleet/server/services/package_policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ describe('Package policy service', () => {
).rejects.toThrow('Saved object [abc/123] conflict');
});

it('should only update input vars that are not frozen', async () => {
it('should throw if the user try to update input vars that are frozen', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
const mockPackagePolicy = createPackagePolicyMock();
const mockInputs = [
Expand Down Expand Up @@ -743,22 +743,143 @@ describe('Package policy service', () => {
);
const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser;

const result = await packagePolicyService.update(
const res = packagePolicyService.update(
savedObjectsClient,
elasticsearchClient,
'the-package-policy-id',
{ ...mockPackagePolicy, inputs: inputsUpdate }
);

await expect(res).rejects.toThrow('cat is a frozen variable and cannot be modified');
});

it('should allow to update input vars that are frozen with the force flag', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
const mockPackagePolicy = createPackagePolicyMock();
const mockInputs = [
{
config: {},
enabled: true,
keep_enabled: true,
type: 'endpoint',
vars: {
dog: {
type: 'text',
value: 'dalmatian',
},
cat: {
type: 'text',
value: 'siamese',
frozen: true,
},
},
streams: [
{
data_stream: {
type: 'birds',
dataset: 'migratory.patterns',
},
enabled: false,
id: `endpoint-migratory.patterns-${mockPackagePolicy.id}`,
vars: {
paths: {
value: ['north', 'south'],
type: 'text',
frozen: true,
},
period: {
value: '6mo',
type: 'text',
},
},
},
],
},
];
const inputsUpdate = [
{
config: {},
enabled: false,
type: 'endpoint',
vars: {
dog: {
type: 'text',
value: 'labrador',
},
cat: {
type: 'text',
value: 'tabby',
},
},
streams: [
{
data_stream: {
type: 'birds',
dataset: 'migratory.patterns',
},
enabled: false,
id: `endpoint-migratory.patterns-${mockPackagePolicy.id}`,
vars: {
paths: {
value: ['east', 'west'],
type: 'text',
},
period: {
value: '12mo',
type: 'text',
},
},
},
],
},
];
const attributes = {
...mockPackagePolicy,
inputs: mockInputs,
};

savedObjectsClient.get.mockResolvedValue({
id: 'test',
type: 'abcd',
references: [],
version: 'test',
attributes,
});

savedObjectsClient.update.mockImplementation(
async (
type: string,
id: string,
attrs: any
): Promise<SavedObjectsUpdateResponse<PackagePolicySOAttributes>> => {
savedObjectsClient.get.mockResolvedValue({
id: 'test',
type: 'abcd',
references: [],
version: 'test',
attributes: attrs,
});
return attrs;
}
);
const elasticsearchClient = elasticsearchServiceMock.createClusterClient().asInternalUser;

const result = await packagePolicyService.update(
savedObjectsClient,
elasticsearchClient,
'the-package-policy-id',
{ ...mockPackagePolicy, inputs: inputsUpdate },
{ force: true }
);

const [modifiedInput] = result.inputs;
expect(modifiedInput.enabled).toEqual(true);
expect(modifiedInput.vars!.dog.value).toEqual('labrador');
expect(modifiedInput.vars!.cat.value).toEqual('siamese');
expect(modifiedInput.vars!.cat.value).toEqual('tabby');
const [modifiedStream] = modifiedInput.streams;
expect(modifiedStream.vars!.paths.value).toEqual(expect.arrayContaining(['north', 'south']));
expect(modifiedStream.vars!.paths.value).toEqual(expect.arrayContaining(['east', 'west']));
expect(modifiedStream.vars!.period.value).toEqual('12mo');
});

it('should add new input vars when updating', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
const mockPackagePolicy = createPackagePolicyMock();
Expand Down Expand Up @@ -810,7 +931,7 @@ describe('Package policy service', () => {
},
cat: {
type: 'text',
value: 'tabby',
value: 'siamese',
},
},
streams: [
Expand All @@ -823,7 +944,7 @@ describe('Package policy service', () => {
id: `endpoint-migratory.patterns-${mockPackagePolicy.id}`,
vars: {
paths: {
value: ['east', 'west'],
value: ['north', 'south'],
type: 'text',
},
period: {
Expand Down
31 changes: 22 additions & 9 deletions x-pack/plugins/fleet/server/services/package_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { omit, partition } from 'lodash';
import { omit, partition, isEqual } from 'lodash';
import { i18n } from '@kbn/i18n';
import semverLt from 'semver/functions/lt';
import { getFlattenedObject } from '@kbn/std';
Expand Down Expand Up @@ -358,7 +358,7 @@ class PackagePolicyService implements PackagePolicyServiceInterface {
esClient: ElasticsearchClient,
id: string,
packagePolicyUpdate: UpdatePackagePolicy,
options?: { user?: AuthenticatedUser },
options?: { user?: AuthenticatedUser; force?: boolean },
currentVersion?: string
): Promise<PackagePolicy> {
const packagePolicy = { ...packagePolicyUpdate, name: packagePolicyUpdate.name.trim() };
Expand Down Expand Up @@ -386,7 +386,7 @@ class PackagePolicyService implements PackagePolicyServiceInterface {
assignStreamIdToInput(oldPackagePolicy.id, input)
);

inputs = enforceFrozenInputs(oldPackagePolicy.inputs, inputs);
inputs = enforceFrozenInputs(oldPackagePolicy.inputs, inputs, options?.force);
let elasticsearch: PackagePolicy['elasticsearch'];
if (packagePolicy.package?.name) {
const pkgInfo = await getPackageInfo({
Expand Down Expand Up @@ -1119,21 +1119,25 @@ async function _compilePackageStream(
return { ...stream };
}

function enforceFrozenInputs(oldInputs: PackagePolicyInput[], newInputs: PackagePolicyInput[]) {
function enforceFrozenInputs(
oldInputs: PackagePolicyInput[],
newInputs: PackagePolicyInput[],
force = false
) {
const resultInputs = [...newInputs];

for (const input of resultInputs) {
const oldInput = oldInputs.find((i) => i.type === input.type);
if (oldInput?.keep_enabled) input.enabled = oldInput.enabled;
if (input.vars && oldInput?.vars) {
input.vars = _enforceFrozenVars(oldInput.vars, input.vars);
input.vars = _enforceFrozenVars(oldInput.vars, input.vars, force);
}
if (input.streams && oldInput?.streams) {
for (const stream of input.streams) {
const oldStream = oldInput.streams.find((s) => s.id === stream.id);
if (oldStream?.keep_enabled) stream.enabled = oldStream.enabled;
if (stream.vars && oldStream?.vars) {
stream.vars = _enforceFrozenVars(oldStream.vars, stream.vars);
stream.vars = _enforceFrozenVars(oldStream.vars, stream.vars, force);
}
}
}
Expand All @@ -1144,12 +1148,21 @@ function enforceFrozenInputs(oldInputs: PackagePolicyInput[], newInputs: Package

function _enforceFrozenVars(
oldVars: Record<string, PackagePolicyConfigRecordEntry>,
newVars: Record<string, PackagePolicyConfigRecordEntry>
newVars: Record<string, PackagePolicyConfigRecordEntry>,
force = false
) {
const resultVars: Record<string, PackagePolicyConfigRecordEntry> = {};
for (const [key, val] of Object.entries(newVars)) {
if (oldVars[key]?.frozen) {
resultVars[key] = oldVars[key];
if (force) {
resultVars[key] = val;
} else if (!isEqual(oldVars[key].value, val.value) || oldVars[key].type !== val.type) {
throw new PackagePolicyValidationError(
`${key} is a frozen variable and cannot be modified`
);
} else {
resultVars[key] = oldVars[key];
}
} else {
resultVars[key] = val;
}
Expand Down Expand Up @@ -1206,7 +1219,7 @@ export interface PackagePolicyServiceInterface {
esClient: ElasticsearchClient,
id: string,
packagePolicyUpdate: UpdatePackagePolicy,
options?: { user?: AuthenticatedUser },
options?: { user?: AuthenticatedUser; force?: boolean },
currentVersion?: string
): Promise<PackagePolicy>;

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/server/types/models/package_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export const UpdatePackagePolicyRequestBodySchema = schema.object({
)
),
version: schema.maybe(schema.string()),
force: schema.maybe(schema.boolean()),
});

export const UpdatePackagePolicySchema = schema.object({
Expand Down

0 comments on commit 62c2082

Please sign in to comment.