V2 Wizard: Fix OpenSCAP step

OpenSCAP profile info is now correctly loaded into the state and
verified to be in the final request with tests.

I modified the fixtures for the OpenSCAP profiles. My changes ensure we
have a Venn-diagram like overlap when changing from one profile to
another where one package is the same, one package is removed, and one
package is added. The same is true for the services and partitions. For
kernel args it is not so important as that is just a string (as opposed
to an array), so it is enough to be different.

I was able to eliminate a useEffect by replacing it with a lazy query
trigger function. Setting the second arg `preferCacheValue` to `true`
means that a request is only made if cached data is not available.

I modified the Redux reducers a bit to add some additional safety.

`changeFileSystemPartitionMode` is now responsible for initializing the
partitions field in the state by adding the root partition – previously
this was done in the components themselves by dispatching
`addPartition`. This reducer can always be safely dispatched – if the
mode is ‘manual’, dispatching it with a payload of ‘manual’ will not
result in any changes to the state.

`addPackage` is also safer now. When a package is added, the list of
packages is checked. If there is a package with an identical name, the
new package overwrites the previous package. This is useful because the
description may be different for the same package – for instance, when
adding an OpenSCAP package, we use a custom description (‘this package
required by OpenSCAP’).
This commit is contained in:
lucasgarfield 2024-04-22 16:55:57 +02:00 committed by Lucas Garfield
parent f9fcbf8cdf
commit a1bdcaa54f
10 changed files with 374 additions and 102 deletions

View file

@ -1,20 +1,23 @@
import React from 'react';
import { FormGroup, Label, Radio } from '@patternfly/react-core';
import { v4 as uuidv4 } from 'uuid';
import { UNIT_GIB } from '../../../../constants';
import { useAppDispatch, useAppSelector } from '../../../../store/hooks';
import {
changeFileSystemConfiguration,
changeFileSystemPartitionMode,
selectFileSystemPartitionMode,
selectProfile,
} from '../../../../store/wizardSlice';
const FileSystemPartition = () => {
const id = uuidv4();
const dispatch = useAppDispatch();
const fileSystemPartitionMode = useAppSelector(selectFileSystemPartitionMode);
const hasOscapProfile = useAppSelector(selectProfile);
if (hasOscapProfile) {
return undefined;
}
return (
<FormGroup>
<Radio
@ -33,7 +36,6 @@ const FileSystemPartition = () => {
isChecked={fileSystemPartitionMode === 'automatic'}
onChange={() => {
dispatch(changeFileSystemPartitionMode('automatic'));
dispatch(changeFileSystemConfiguration([]));
}}
/>
<Radio
@ -45,16 +47,6 @@ const FileSystemPartition = () => {
isChecked={fileSystemPartitionMode === 'manual'}
onChange={() => {
dispatch(changeFileSystemPartitionMode('manual'));
dispatch(
changeFileSystemConfiguration([
{
id: id,
mountpoint: '/',
min_size: (10 * UNIT_GIB).toString(),
unit: 'GiB',
},
])
);
}}
/>
</FormGroup>

View file

@ -1,4 +1,4 @@
import React, { useState, useEffect } from 'react';
import React, { useState } from 'react';
import {
Alert,
@ -14,30 +14,38 @@ import {
SelectVariant,
} from '@patternfly/react-core/deprecated';
import { HelpIcon } from '@patternfly/react-icons';
import { v4 as uuidv4 } from 'uuid';
import OscapProfileInformation from './OscapProfileInformation';
import { useAppDispatch, useAppSelector } from '../../../../store/hooks';
import {
useAppDispatch,
useAppSelector,
useServerStore,
} from '../../../../store/hooks';
import {
DistributionProfileItem,
Filesystem,
useGetOscapCustomizationsQuery,
useGetOscapProfilesQuery,
useLazyGetOscapCustomizationsQuery,
} from '../../../../store/imageBuilderApi';
import {
changeOscapProfile,
selectDistribution,
selectProfile,
clearOscapPackages,
addPackage,
selectPackages,
addPartition,
changeFileSystemPartitionMode,
removePackage,
clearPartitions,
} from '../../../../store/wizardSlice';
import { Partition } from '../FileSystem/FileSystemConfiguration';
const ProfileSelector = () => {
const oscapProfile = useAppSelector(selectProfile);
const oscapData = useServerStore();
const release = useAppSelector(selectDistribution);
const packages = useAppSelector(selectPackages);
const dispatch = useAppDispatch();
const [isOpen, setIsOpen] = useState(false);
const {
@ -50,38 +58,9 @@ const ProfileSelector = () => {
distribution: release,
});
const { data: oscapData } = useGetOscapCustomizationsQuery(
{
distribution: release,
// @ts-ignore if oscapProfile is undefined the query is going to get skipped, so it's safe here to ignore the linter here
profile: oscapProfile,
},
{
skip: !oscapProfile,
}
);
const profileName = oscapProfile ? oscapData?.openscap?.profile_name : 'None';
const profileName = oscapProfile ? oscapData.profileName : 'None';
useEffect(() => {
dispatch(clearOscapPackages());
for (const pkg in oscapData?.packages) {
if (
packages
.map((pkg) => pkg.name)
.includes(oscapData?.packages[Number(pkg)])
) {
dispatch(removePackage(oscapData?.packages[Number(pkg)]));
}
dispatch(
addPackage({
name: oscapData?.packages[Number(pkg)],
summary: 'Required by chosen OpenSCAP profile',
repository: 'distro',
isRequiredByOpenScap: true,
})
);
}
}, [oscapData?.packages, dispatch]);
const [trigger] = useLazyGetOscapCustomizationsQuery();
const handleToggle = () => {
if (!isOpen) {
@ -92,14 +71,79 @@ const ProfileSelector = () => {
const handleClear = () => {
dispatch(changeOscapProfile(undefined));
dispatch(clearOscapPackages());
clearOscapPackages(oscapData.packages || []);
dispatch(changeFileSystemPartitionMode('automatic'));
};
const handlePackages = (
oldOscapPackages: string[],
newOscapPackages: string[]
) => {
clearOscapPackages(oldOscapPackages);
for (const pkg of newOscapPackages) {
dispatch(
addPackage({
name: pkg,
summary: 'Required by chosen OpenSCAP profile',
repository: 'distro',
})
);
}
};
const clearOscapPackages = (oscapPackages: string[]) => {
for (const pkg of oscapPackages) {
dispatch(removePackage(pkg));
}
};
const handlePartitions = (oscapPartitions: Filesystem[]) => {
dispatch(clearPartitions());
const newPartitions = oscapPartitions.map((filesystem) => {
const partition: Partition = {
mountpoint: filesystem.mountpoint,
min_size: filesystem.min_size.toString(),
unit: 'GiB',
id: uuidv4(),
};
return partition;
});
if (newPartitions) {
dispatch(changeFileSystemPartitionMode('manual'));
for (const partition of newPartitions) {
dispatch(addPartition(partition));
}
}
};
const handleSelect = (
_event: React.MouseEvent<Element, MouseEvent>,
selection: OScapSelectOptionValueType
) => {
dispatch(changeOscapProfile(selection.id));
if (selection.id === undefined) {
// handle user has selected 'None' case
handleClear();
} else {
const oldOscapPackages = oscapData.packages || [];
trigger(
{
distribution: release,
profile: selection.id,
},
true // preferCacheValue
)
.unwrap()
.then((response) => {
const oscapPartitions = response.filesystem || [];
const newOscapPackages = response.packages || [];
handlePartitions(oscapPartitions);
handlePackages(oldOscapPackages, newOscapPackages);
dispatch(changeOscapProfile(selection.id));
});
}
setIsOpen(false);
};

View file

@ -67,7 +67,6 @@ export type IBPackageWithRepositoryInfo = {
name: Package['name'];
summary: Package['summary'];
repository: PackageRepository;
isRequiredByOpenScap: boolean;
};
const Packages = () => {
@ -386,7 +385,6 @@ const Packages = () => {
transformedDistroData = dataDistroPackages.data.map((values) => ({
...values,
repository: 'distro',
isRequiredByOpenScap: false,
}));
}
@ -395,7 +393,6 @@ const Packages = () => {
name: values.package_name!,
summary: values.summary!,
repository: 'custom',
isRequiredByOpenScap: false,
}));
}
@ -413,7 +410,6 @@ const Packages = () => {
name: values.package_name!,
summary: values.summary!,
repository: 'recommended',
isRequiredByOpenScap: false,
}));
combinedPackageData = combinedPackageData.concat(

View file

@ -54,8 +54,8 @@ import {
import { GcpAccountType } from '../steps/TargetEnvironment/Gcp';
type ServerStore = {
kernel?: { append?: string };
services?: { enabled?: string[]; disabled?: string[] };
kernel?: { append?: string }; // TODO use API types
services?: { enabled?: string[]; disabled?: string[]; masked?: string[] }; // TODO use API types
};
/**
@ -172,7 +172,6 @@ export const mapRequestToState = (request: BlueprintResponse): wizardState => {
name: pkg,
summary: '',
repository: '',
isRequiredByOpenScap: false,
})) || [],
stepValidations: {},
};
@ -305,11 +304,13 @@ const getCustomizations = (
const getServices = (serverStore: ServerStore): Services | undefined => {
const enabledServices = serverStore.services?.enabled;
const disabledServices = serverStore.services?.disabled;
const maskedServices = serverStore.services?.masked;
if (enabledServices || disabledServices) {
if (enabledServices || disabledServices || maskedServices) {
return {
enabled: enabledServices,
disabled: disabledServices,
masked: maskedServices,
};
}
return undefined;

View file

@ -27,7 +27,11 @@ export const useOscapData = () => {
services: {
enabled: data?.services?.enabled,
disabled: data?.services?.disabled,
masked: data?.services?.masked,
},
packages: data?.packages,
filesystem: data?.filesystem,
profileName: data?.openscap?.profile_name,
};
};

View file

@ -1,4 +1,5 @@
import { PayloadAction, createSlice } from '@reduxjs/toolkit';
import { v4 as uuidv4 } from 'uuid';
import { ApiRepositoryResponseRead } from './contentSourcesApi';
import {
@ -24,7 +25,7 @@ import {
GcpShareMethod,
} from '../Components/CreateImageWizardV2/steps/TargetEnvironment/Gcp';
import { V1ListSourceResponseItem } from '../Components/CreateImageWizardV2/types';
import { RHEL_9, X86_64 } from '../constants';
import { RHEL_9, UNIT_GIB, X86_64 } from '../constants';
import { RootState } from '.';
@ -389,14 +390,47 @@ export const wizardSlice = createSlice({
setIsNextButtonTouched: (state, action: PayloadAction<boolean>) => {
state.fileSystem.isNextButtonTouched = action.payload;
},
changeFileSystemPartitionMode: (
state,
action: PayloadAction<FileSystemPartitionMode>
) => {
state.fileSystem.mode = action.payload;
const currentMode = state.fileSystem.mode;
// Only trigger if mode is being *changed*
if (currentMode !== action.payload) {
state.fileSystem.mode = action.payload;
switch (action.payload) {
case 'automatic':
state.fileSystem.partitions = [];
break;
case 'manual':
state.fileSystem.partitions = [
{
id: uuidv4(),
mountpoint: '/',
min_size: (10 * UNIT_GIB).toString(),
unit: 'GiB',
},
];
}
}
},
clearPartitions: (state) => {
const currentMode = state.fileSystem.mode;
if (currentMode === 'manual') {
state.fileSystem.partitions = [
{
id: uuidv4(),
mountpoint: '/',
min_size: (10 * UNIT_GIB).toString(),
unit: 'GiB',
},
];
}
},
addPartition: (state, action: PayloadAction<Partition>) => {
// Duplicate partitions are allowed temporarily, the wizard is responsible for final validation
state.fileSystem.partitions.push(action.payload);
},
removePartition: (state, action: PayloadAction<Partition['id']>) => {
@ -407,6 +441,17 @@ export const wizardSlice = createSlice({
1
);
},
removePartitionByMountpoint: (
state,
action: PayloadAction<Partition['mountpoint']>
) => {
state.fileSystem.partitions.splice(
state.fileSystem.partitions.findIndex(
(partition) => partition.mountpoint === action.payload
),
1
);
},
changePartitionOrder: (state, action: PayloadAction<string[]>) => {
state.fileSystem.partitions = state.fileSystem.partitions.sort(
(a, b) => action.payload.indexOf(a.id) - action.payload.indexOf(b.id)
@ -479,7 +524,15 @@ export const wizardSlice = createSlice({
);
},
addPackage: (state, action: PayloadAction<IBPackageWithRepositoryInfo>) => {
state.packages.push(action.payload);
const existingPackageIndex = state.packages.findIndex(
(pkg) => pkg.name === action.payload.name
);
if (existingPackageIndex !== -1) {
state.packages[existingPackageIndex] = action.payload;
} else {
state.packages.push(action.payload);
}
},
removePackage: (
state,
@ -490,11 +543,6 @@ export const wizardSlice = createSlice({
1
);
},
clearOscapPackages: (state) => {
state.packages = state.packages.filter(
(pkg) => pkg.isRequiredByOpenScap !== true
);
},
changeBlueprintName: (state, action: PayloadAction<string>) => {
state.details.blueprintName = action.payload;
},
@ -556,8 +604,10 @@ export const {
changeFileSystemConfiguration,
setIsNextButtonTouched,
changeFileSystemPartitionMode,
clearPartitions,
addPartition,
removePartition,
removePartitionByMountpoint,
changePartitionMountpoint,
changePartitionUnit,
changePartitionMinSize,
@ -568,7 +618,6 @@ export const {
removeRecommendedRepository,
addPackage,
removePackage,
clearOscapPackages,
changeBlueprintName,
changeBlueprintDescription,
loadWizardState,

View file

@ -165,14 +165,14 @@ describe('Step Compliance', () => {
await screen.findByRole('heading', { name: /File system configuration/i });
await screen.findByText(/tmp/i);
// check that the Packages contain a nftable package
// check that the Packages contains correct packages
await clickNext();
await screen.findByRole('heading', {
name: /Additional Red Hat packages/i,
});
await screen.findByText(/nftables/i);
await screen.findByText(/libselinux/i);
await screen.findByText(/aide/i);
await screen.findByText(/neovim/i);
});
});

View file

@ -171,14 +171,14 @@ describe('Step Compliance', () => {
await clickNext(); // skip Repositories
// check that the Packages contain a nftable package
// check that the Packages contains correct packages
await clickNext();
await screen.findByRole('heading', {
name: /Additional packages/i,
});
await user.click(await screen.findByText(/Selected/));
await screen.findByText(/nftables/i);
await screen.findByText(/libselinux/i);
await screen.findByText(/aide/i);
await screen.findByText(/neovim/i);
});
});
//

View file

@ -0,0 +1,194 @@
import { screen } from '@testing-library/react';
import { userEvent } from '@testing-library/user-event';
import { CREATE_BLUEPRINT } from '../../../../../constants';
import { CreateBlueprintRequest } from '../../../../../store/imageBuilderApi';
import { clickNext } from '../../../../testUtils';
import {
blueprintRequest,
clickRegisterLater,
enterBlueprintName,
interceptBlueprintRequest,
render,
} from '../../wizardTestUtils';
jest.mock('@redhat-cloud-services/frontend-components/useChrome', () => ({
useChrome: () => ({
auth: {
getUser: () => {
return {
identity: {
internal: {
org_id: 5,
},
},
};
},
},
isBeta: () => true,
isProd: () => true,
getEnvironment: () => 'prod',
}),
}));
const goToOscapStep = async () => {
const bareMetalCheckBox = await screen.findByRole('checkbox', {
name: /bare metal installer checkbox/i,
});
await userEvent.click(bareMetalCheckBox);
await clickNext(); // Registration
await clickRegisterLater();
await clickNext(); // OpenSCAP
};
const selectProfile = async () => {
await userEvent.click(
await screen.findByRole('textbox', {
name: /select a profile/i,
})
);
await userEvent.click(
await screen.findByText(
/cis red hat enterprise linux 8 benchmark for level 1 - workstation/i
)
);
};
const selectDifferentProfile = async () => {
await userEvent.click(
await screen.findByRole('textbox', {
name: /select a profile/i,
})
);
await userEvent.click(
await screen.findByText(
/cis red hat enterprise linux 8 benchmark for level 2 - workstation/i
)
);
};
const selectNone = async () => {
await userEvent.click(
await screen.findByRole('textbox', {
name: /select a profile/i,
})
);
await userEvent.click(await screen.findByText(/none/i));
};
const goToReviewStep = async () => {
await clickNext(); // File system configuration
await clickNext(); // Custom repositories
await clickNext(); // Additional packages
await clickNext(); // Details
await enterBlueprintName();
await clickNext(); // Review
};
const expectedOpenscapCisL1 = {
profile_id: 'xccdf_org.ssgproject.content_profile_cis_workstation_l1',
};
const expectedPackagesCisL1 = ['aide', 'neovim'];
const expectedServicesCisL1 = {
enabled: ['crond', 'neovim-service'],
masked: ['nfs-server', 'emacs-service'],
};
const expectedKernelCisL1 = {
append: 'audit_backlog_limit=8192 audit=1',
};
const expectedFilesystemCisL1 = [
{ min_size: 10737418240, mountpoint: '/' },
{ min_size: 1073741824, mountpoint: '/tmp' },
{ min_size: 1073741824, mountpoint: '/home' },
];
const expectedOpenscapCisL2 = {
profile_id: 'xccdf_org.ssgproject.content_profile_cis_workstation_l2',
};
const expectedPackagesCisL2 = ['aide', 'emacs'];
const expectedServicesCisL2 = {
enabled: ['crond', 'emacs-service'],
masked: ['nfs-server', 'neovim-service'],
};
const expectedKernelCisL2 = {
append: 'audit_backlog_limit=8192 audit=2',
};
const expectedFilesystemCisL2 = [
{ min_size: 10737418240, mountpoint: '/' },
{ min_size: 1073741824, mountpoint: '/tmp' },
{ min_size: 1073741824, mountpoint: '/app' },
];
describe('oscap', () => {
test('add a profile', async () => {
await render();
await goToOscapStep();
await selectProfile();
await goToReviewStep();
const receivedRequest = await interceptBlueprintRequest(CREATE_BLUEPRINT);
const expectedRequest: CreateBlueprintRequest = {
...blueprintRequest,
customizations: {
packages: expectedPackagesCisL1,
openscap: expectedOpenscapCisL1,
services: expectedServicesCisL1,
kernel: expectedKernelCisL1,
filesystem: expectedFilesystemCisL1,
},
};
expect(receivedRequest).toEqual(expectedRequest);
});
test('remove a profile', async () => {
await render();
await goToOscapStep();
await selectProfile();
await selectNone();
await goToReviewStep();
const receivedRequest = await interceptBlueprintRequest(CREATE_BLUEPRINT);
const expectedRequest: CreateBlueprintRequest = {
...blueprintRequest,
};
expect(receivedRequest).toEqual(expectedRequest);
});
test('change profile', async () => {
await render();
await goToOscapStep();
await selectProfile();
await selectDifferentProfile();
await goToReviewStep();
const receivedRequest = await interceptBlueprintRequest(CREATE_BLUEPRINT);
const expectedRequest: CreateBlueprintRequest = {
...blueprintRequest,
customizations: {
packages: expectedPackagesCisL2,
openscap: expectedOpenscapCisL2,
services: expectedServicesCisL2,
kernel: expectedKernelCisL2,
filesystem: expectedFilesystemCisL2,
},
};
expect(receivedRequest).toEqual(expectedRequest);
});
});

View file

@ -16,7 +16,10 @@ export const oscapCustomizations = (
): GetOscapCustomizationsApiResponse => {
if (profile === 'xccdf_org.ssgproject.content_profile_cis_workstation_l1') {
return {
filesystem: [{ min_size: 1073741824, mountpoint: '/tmp' }],
filesystem: [
{ min_size: 1073741824, mountpoint: '/tmp' },
{ min_size: 1073741824, mountpoint: '/home' },
],
openscap: {
profile_id: 'xccdf_org.ssgproject.content_profile_cis_workstation_l1',
profile_name:
@ -24,26 +27,22 @@ export const oscapCustomizations = (
profile_description:
'This is a mocked profile description. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean posuere velit enim, tincidunt porttitor nisl elementum eu.',
},
packages: [
'aide',
'sudo',
'rsyslog',
'firewalld',
'nftables',
'libselinux',
],
packages: ['aide', 'neovim'],
kernel: {
append: 'audit_backlog_limit=8192 audit=1',
},
services: {
masked: ['nfs-server'],
enabled: ['crond'],
masked: ['nfs-server', 'emacs-service'],
enabled: ['crond', 'neovim-service'],
},
};
}
if (profile === 'xccdf_org.ssgproject.content_profile_cis_workstation_l2') {
return {
filesystem: [{ min_size: 1073741824, mountpoint: '/tmp' }],
filesystem: [
{ min_size: 1073741824, mountpoint: '/tmp' },
{ min_size: 1073741824, mountpoint: '/app' },
],
openscap: {
profile_id: 'content_profile_cis_workstation_l2',
profile_name:
@ -51,20 +50,13 @@ export const oscapCustomizations = (
profile_description:
'This is a mocked profile description. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aenean posuere velit enim, tincidunt porttitor nisl elementum eu.',
},
packages: [
'aide',
'sudo',
'rsyslog',
'firewalld',
'nftables',
'libselinux',
],
packages: ['aide', 'emacs'],
kernel: {
append: 'audit_backlog_limit=8192 audit=1',
append: 'audit_backlog_limit=8192 audit=2',
},
services: {
disabled: ['nfs-server', 'nftables'],
enabled: ['crond', 'firewalld'],
masked: ['nfs-server', 'neovim-service'],
enabled: ['crond', 'emacs-service'],
},
};
}
@ -88,7 +80,7 @@ export const oscapCustomizations = (
append: 'audit_backlog_limit=8192 audit=1',
},
services: {
disabled: ['nfs-server', 'rpcbind', 'autofs', 'nftables'],
masked: ['nfs-server', 'rpcbind', 'autofs', 'nftables'],
enabled: ['crond', 'firewalld', 'systemd-journald', 'rsyslog', 'auditd'],
},
};