Skip to content

Commit

Permalink
[Fleet] Fix missing package-level vars in GCP policy editor (elastic#…
Browse files Browse the repository at this point in the history
…132068)

* Fix missing package-level vars in GCP policy editor

Display package-level variables even if they aren't defined on the
existing package policy during an upgrade, honoring default values for
all variables if they exist.

Fixes elastic#131251

* Fix not persisting default values for new variables

* Fix tests

* Address PR feedback + update tests

(cherry picked from commit 5fff510)

# Conflicts:
#	x-pack/plugins/fleet/common/services/validate_package_policy.test.ts
#	x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/step_define_package_policy.test.tsx
  • Loading branch information
kpollich committed Jul 13, 2022
1 parent 9bcd62b commit b3ae98e
Show file tree
Hide file tree
Showing 4 changed files with 312 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ describe('Fleet - validatePackagePolicy()', () => {
],
},
],
vars: {},
};

const invalidPackagePolicy: NewPackagePolicy = {
Expand Down Expand Up @@ -332,6 +333,7 @@ describe('Fleet - validatePackagePolicy()', () => {
],
},
],
vars: {},
};

const noErrorsValidationResults = {
Expand Down Expand Up @@ -370,6 +372,7 @@ describe('Fleet - validatePackagePolicy()', () => {
vars: { 'var-name': null },
},
},
vars: {},
};

it('returns no errors for valid package policy', () => {
Expand Down Expand Up @@ -416,6 +419,7 @@ describe('Fleet - validatePackagePolicy()', () => {
streams: { 'with-no-stream-vars-bar': {} },
},
},
vars: {},
});
});

Expand Down Expand Up @@ -487,6 +491,7 @@ describe('Fleet - validatePackagePolicy()', () => {
streams: { 'with-no-stream-vars-bar': {} },
},
},
vars: {},
});
});

Expand All @@ -505,6 +510,7 @@ describe('Fleet - validatePackagePolicy()', () => {
description: null,
namespace: null,
inputs: null,
vars: {},
});
expect(
validatePackagePolicy(
Expand All @@ -520,6 +526,7 @@ describe('Fleet - validatePackagePolicy()', () => {
description: null,
namespace: null,
inputs: null,
vars: {},
});
});

Expand All @@ -538,6 +545,7 @@ describe('Fleet - validatePackagePolicy()', () => {
description: null,
namespace: null,
inputs: null,
vars: {},
});
expect(
validatePackagePolicy(
Expand All @@ -553,6 +561,59 @@ describe('Fleet - validatePackagePolicy()', () => {
description: null,
namespace: null,
inputs: null,
vars: {},
});
});

it('returns no errors when required field is present but empty', () => {
expect(
validatePackagePolicy(
{
...validPackagePolicy,
inputs: [
{
type: 'foo',
policy_template: 'pkgPolicy1',
enabled: true,
vars: {
'foo-input-var-name': { value: '', type: 'text' },
'foo-input2-var-name': { value: '', type: 'text' },
'foo-input3-var-name': { value: ['test'], type: 'text' },
},
streams: [
{
data_stream: { dataset: 'foo', type: 'logs' },
enabled: true,
vars: { 'var-name': { value: 'test_yaml: value', type: 'yaml' } },
},
],
},
],
},
mockPackage,
safeLoad
)
).toEqual({
name: null,
description: null,
namespace: null,
inputs: {
foo: {
streams: {
foo: {
vars: {
'var-name': null,
},
},
},
vars: {
'foo-input-var-name': null,
'foo-input2-var-name': null,
'foo-input3-var-name': null,
},
},
},
vars: {},
});
});

Expand Down Expand Up @@ -678,6 +739,7 @@ describe('Fleet - validatePackagePolicy()', () => {
},
},
},
vars: {},
name: null,
namespace: null,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export const validatePackagePolicy = (
description: null,
namespace: null,
inputs: {},
vars: {},
};
const namespaceValidation = isValidNamespace(packagePolicy.namespace);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React from 'react';
import { act, fireEvent, waitFor } from '@testing-library/react';

import type { TestRenderer } from '../../../../../mock';
import { createFleetTestRendererMock } from '../../../../../mock';
import type { AgentPolicy, NewPackagePolicy, PackageInfo } from '../../../types';

import { useGetPackagePolicies } from '../../../hooks';

import { StepDefinePackagePolicy } from './step_define_package_policy';

jest.mock('../../../hooks', () => {
return {
...jest.requireActual('../../../hooks'),
useGetPackagePolicies: jest.fn().mockReturnValue({
data: {
items: [{ name: 'nginx-1' }, { name: 'other-policy' }],
},
isLoading: false,
}),
useFleetStatus: jest.fn().mockReturnValue({ isReady: true } as any),
sendGetStatus: jest
.fn()
.mockResolvedValue({ data: { isReady: true, missing_requirements: [] } }),
};
});

describe('StepDefinePackagePolicy', () => {
const packageInfo: PackageInfo = {
name: 'apache',
version: '1.0.0',
description: '',
format_version: '',
release: 'ga',
owner: { github: '' },
title: 'Apache',
latestVersion: '',
assets: {} as any,
status: 'not_installed',
vars: [
{
show_user: true,
name: 'Show user var',
type: 'string',
default: 'showUserVarVal',
},
{
required: true,
name: 'Required var',
type: 'bool',
},
{
name: 'Advanced var',
type: 'bool',
default: true,
},
],
};
const agentPolicy: AgentPolicy = {
id: 'agent-policy-1',
namespace: 'ns',
name: 'Agent policy 1',
is_managed: false,
status: 'active',
updated_at: '',
updated_by: '',
revision: 1,
package_policies: [],
};
let packagePolicy: NewPackagePolicy;
const mockUpdatePackagePolicy = jest.fn().mockImplementation((val: any) => {
packagePolicy = {
...val,
...packagePolicy,
};
});

const validationResults = {
name: null,
description: null,
namespace: null,
inputs: {},
vars: {},
};

let testRenderer: TestRenderer;
let renderResult: ReturnType<typeof testRenderer.render>;
const render = ({ isUpdate } = { isUpdate: false }) =>
(renderResult = testRenderer.render(
<StepDefinePackagePolicy
agentPolicy={agentPolicy}
packageInfo={packageInfo}
packagePolicy={packagePolicy}
updatePackagePolicy={mockUpdatePackagePolicy}
validationResults={validationResults}
submitAttempted={false}
isUpdate={isUpdate}
/>
));

beforeEach(() => {
packagePolicy = {
name: '',
description: 'desc',
namespace: 'default',
policy_id: '',
enabled: true,
output_id: '',
inputs: [],
};
testRenderer = createFleetTestRendererMock();
});

describe('default API response', () => {
beforeEach(() => {
render();
});

it('should set index 1 name to package policy on init if no package policies exist for this package', () => {
waitFor(() => {
expect(renderResult.getByDisplayValue('apache-1')).toBeInTheDocument();
expect(renderResult.getByDisplayValue('desc')).toBeInTheDocument();
});

expect(mockUpdatePackagePolicy.mock.calls[0]).toEqual([
{
description: 'desc',
enabled: true,
inputs: [],
name: 'apache-1',
namespace: 'default',
policy_id: 'agent-policy-1',
output_id: '',
package: {
name: 'apache',
title: 'Apache',
version: '1.0.0',
},
vars: {
'Advanced var': {
type: 'bool',
value: true,
},
'Required var': {
type: 'bool',
value: undefined,
},
'Show user var': {
type: 'string',
value: 'showUserVarVal',
},
},
},
]);
expect(mockUpdatePackagePolicy.mock.calls[1]).toEqual([
{
namespace: 'ns',
policy_id: 'agent-policy-1',
},
]);
});

it('should display vars coming from package policy', async () => {
waitFor(() => {
expect(renderResult.getByDisplayValue('showUserVarVal')).toBeInTheDocument();
expect(renderResult.getByRole('switch')).toHaveAttribute('aria-label', 'Required var');
expect(renderResult.getByText('Required var is required')).toHaveAttribute(
'class',
'euiFormErrorText'
);
});

await act(async () => {
fireEvent.click(renderResult.getByText('Advanced options').closest('button')!);
});

waitFor(() => {
expect(renderResult.getByRole('switch')).toHaveAttribute('aria-label', 'Advanced var');
});
});
});

it('should set incremented name if other package policies exist', () => {
(useGetPackagePolicies as jest.MockedFunction<any>).mockReturnValueOnce({
data: {
items: [
{ name: 'apache-1' },
{ name: 'apache-2' },
{ name: 'apache-9' },
{ name: 'apache-10' },
],
},
isLoading: false,
});

render();

waitFor(() => {
expect(renderResult.getByDisplayValue('apache-11')).toBeInTheDocument();
});
});

describe('update', () => {
describe('when package vars are introduced in a new package version', () => {
it('should display new package vars', () => {
render({ isUpdate: true });

waitFor(async () => {
expect(renderResult.getByDisplayValue('showUserVarVal')).toBeInTheDocument();
expect(renderResult.getByText('Required var')).toBeInTheDocument();

await act(async () => {
fireEvent.click(renderResult.getByText('Advanced options').closest('button')!);
});

expect(renderResult.getByText('Advanced var')).toBeInTheDocument();
});
});
});
});
});
Loading

0 comments on commit b3ae98e

Please sign in to comment.