Skip to content

Commit

Permalink
Bug 2181920: Display "UEFI" for Boot mode for "efi: {}" in yaml
Browse files Browse the repository at this point in the history
This is a followup of kubevirt-ui#1236
introducing the change that "efi: { secureBoot: false }" was written to
VM's yaml when changing Boot mode field value to "UEFI", because that
was correct representation of "UEFI" boot mode (secure boot disabled).

In this commit, the following remaining issues are fixed:

- for existing VM:
-- it shows "UEFI" for Boot mode field in Details for "efi: {}" in the
yaml's bootloader section

- in Create VM wizard:
-- it shows "UEFI" for Boot mode field in Review and create
VirtualMachine screen for "efi: {}" in the yaml's bootloader section

- for existing template:
-- it shows "UEFI" for Boot mode field in Details for "efi: {}" in the
yaml's bootloader section
-- after changing Boot mode field to "UEFI", "efi: {}" occurs in the
yaml (this doesn't happen for existing VMs or when creating a VM)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2181920
  • Loading branch information
Hilda Stastna committed May 8, 2023
1 parent da1582d commit 712263c
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 135 deletions.
1 change: 0 additions & 1 deletion cypress/utils/const/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ export const adminOnlyDescribe = Cypress.env('NON_PRIV') ? xdescribe : describe;

export const TEST_NS = 'auto-test-ns';

// VM Status
export enum VM_STATUS {
Starting = 'Starting',
Running = 'Running',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import * as React from 'react';
import produce from 'immer';
import React, { ChangeEvent, FC, useMemo, useState } from 'react';

import { V1VirtualMachine, V1VirtualMachineInstance } from '@kubevirt-ui/kubevirt-api/kubevirt';
import TabModal from '@kubevirt-utils/components/TabModal/TabModal';
import { useKubevirtTranslation } from '@kubevirt-utils/hooks/useKubevirtTranslation';
import { ensurePath } from '@kubevirt-utils/utils/utils';
import { Form, FormGroup, Select, SelectOption, SelectVariant } from '@patternfly/react-core';

import { ModalPendingChangesAlert } from '../PendingChanges/ModalPendingChangesAlert/ModalPendingChangesAlert';
import { checkBootModeChanged } from '../PendingChanges/utils/helpers';

import { BootloaderLabel, BootloaderOptionValue } from './utils/constants';
import { getBootloaderFromVM } from './utils/utils';
import { bootloaderOptions } from './utils/constants';
import { BootloaderOptionValue } from './utils/types';
import { getBootloaderFromVM, updatedVMBootMode } from './utils/utils';

type FirmwareBootloaderModalProps = {
vm: V1VirtualMachine;
Expand All @@ -21,73 +20,28 @@ type FirmwareBootloaderModalProps = {
vmi?: V1VirtualMachineInstance;
};

const FirmwareBootloaderModal: React.FC<FirmwareBootloaderModalProps> = ({
const FirmwareBootloaderModal: FC<FirmwareBootloaderModalProps> = ({
vm,
isOpen,
onClose,
onSubmit,
vmi,
}) => {
const { t } = useKubevirtTranslation();
const [isDropdownOpen, setIsDropdownOpen] = React.useState(false);
const [isDropdownOpen, setIsDropdownOpen] = useState(false);
const [selectedFirmwareBootloader, setSelectedFirmwareBootloader] =
React.useState<BootloaderOptionValue>(getBootloaderFromVM(vm));
useState<BootloaderOptionValue>(getBootloaderFromVM(vm));

const bootloaderOptions: BootloaderLabel[] = [
{
value: 'bios',
title: t('BIOS'),
description: t('Use BIOS when bootloading the guest OS (Default)'),
},
{
value: 'uefi',
title: t('UEFI'),
description: t(
'Use UEFI when bootloading the guest OS. Requires SMM feature, if the SMM feature is not set, choosing this method will set it to true',
),
},
{
value: 'uefiSecure',
title: t('UEFI (secure)'),
description: t(
'Use UEFI when bootloading the guest OS. Requires SMM feature, if the SMM feature is not set, choosing this method will set it to true',
),
},
];

const handleChange = (
event: React.ChangeEvent<HTMLSelectElement>,
value: BootloaderOptionValue,
) => {
const handleChange = (event: ChangeEvent<HTMLSelectElement>, value: BootloaderOptionValue) => {
event.preventDefault();
setSelectedFirmwareBootloader(value);
setIsDropdownOpen(false);
};

const updatedVirtualMachine = React.useMemo(() => {
const updatedVM = produce<V1VirtualMachine>(vm, (vmDraft: V1VirtualMachine) => {
ensurePath(vmDraft, 'spec.template.spec.domain.firmware.bootloader');

ensurePath(vmDraft, 'spec.template.spec.domain.features.smm');
vmDraft.spec.template.spec.domain.features.smm = { enabled: true };

switch (selectedFirmwareBootloader) {
case 'uefi':
vmDraft.spec.template.spec.domain.firmware.bootloader = {
efi: { secureBoot: false },
};
break;
case 'uefiSecure':
vmDraft.spec.template.spec.domain.firmware.bootloader = {
efi: { secureBoot: true },
};
break;
default: // 'bios'
vmDraft.spec.template.spec.domain.firmware.bootloader = { bios: {} };
}
});
return updatedVM;
}, [vm, selectedFirmwareBootloader]);
const updatedVirtualMachine = useMemo(
() => updatedVMBootMode(vm, selectedFirmwareBootloader),
[vm, selectedFirmwareBootloader],
);

return (
<TabModal
Expand Down
38 changes: 36 additions & 2 deletions src/utils/components/FirmwareBootloaderModal/utils/constants.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,37 @@
export type BootloaderLabel = { value: string; title: string; description: string };
import { t } from '@kubevirt-utils/hooks/useKubevirtTranslation';

export type BootloaderOptionValue = 'bios' | 'uefi' | 'uefiSecure';
import { BootloaderLabel } from './types';

export enum BootMode {
uefi = 'uefi',
uefiSecure = 'uefiSecure',
bios = 'bios',
}

export const BootModeTitles = {
[BootMode.uefiSecure]: t('UEFI (secure)'),
[BootMode.uefi]: t('UEFI '),
[BootMode.bios]: t('BIOS'),
};

export const bootloaderOptions: BootloaderLabel[] = [
{
value: BootMode.bios,
title: BootModeTitles[BootMode.bios],
description: t('Use BIOS when bootloading the guest OS (Default)'),
},
{
value: BootMode.uefi,
title: BootModeTitles[BootMode.uefi],
description: t(
'Use UEFI when bootloading the guest OS. Requires SMM feature, if the SMM feature is not set, choosing this method will set it to true',
),
},
{
value: BootMode.uefiSecure,
title: BootModeTitles[BootMode.uefiSecure],
description: t(
'Use UEFI when bootloading the guest OS. Requires SMM feature, if the SMM feature is not set, choosing this method will set it to true',
),
},
];
60 changes: 46 additions & 14 deletions src/utils/components/FirmwareBootloaderModal/utils/utils.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,58 @@
import { TFunction } from 'i18next';
import produce from 'immer';

import { V1VirtualMachine } from '@kubevirt-ui/kubevirt-api/kubevirt';
import { ensurePath, isEmpty } from '@kubevirt-utils/utils/utils';

import { BootloaderOptionValue } from './constants';
import { BootMode, BootModeTitles } from './constants';
import { BootloaderOptionValue } from './types';

export const isObjectEmpty = (obj: object): boolean => obj && isEmpty(obj);

export const getBootloaderFromVM = (vm: V1VirtualMachine): BootloaderOptionValue => {
if (vm?.spec?.template?.spec?.domain?.firmware?.bootloader?.efi?.secureBoot) {
return 'uefiSecure';
const secureBoot = vm?.spec?.template?.spec?.domain?.firmware?.bootloader?.efi;

if (secureBoot?.secureBoot === true || isObjectEmpty(secureBoot)) {
return BootMode.uefiSecure;
}
if (vm?.spec?.template?.spec?.domain?.firmware?.bootloader?.efi) {
return `uefi`;
if (secureBoot?.secureBoot === false) {
return BootMode.uefi;
}
return 'bios';
return BootMode.bios;
};

export const getBootloaderTitleFromVM = (vm: V1VirtualMachine, t: TFunction): string => {
export const getBootloaderTitleFromVM = (vm: V1VirtualMachine): string => {
const bootloader = getBootloaderFromVM(vm);
const titles = {
uefiSecure: t('UEFI (secure)'),
uefi: t('UEFI '),
bios: t('BIOS'),
};

return titles[bootloader];
return BootModeTitles[bootloader];
};

/**
* A function to return the VirtualMachine object updated with a given boot mode
* @param {V1VirtualMachine} vm - VirtualMachine object
* @param {BootloaderOptionValue} firmwareBootloader - selected boot mode
* @returns {V1VirtualMachine} updated VirtualMachine object
*/
export const updatedVMBootMode = (
vm: V1VirtualMachine,
firmwareBootloader: BootloaderOptionValue,
) =>
produce<V1VirtualMachine>(vm as V1VirtualMachine, (vmDraft: V1VirtualMachine) => {
ensurePath(vmDraft, 'spec.template.spec.domain.firmware.bootloader');
ensurePath(vmDraft, 'spec.template.spec.domain.features.smm');
vmDraft.spec.template.spec.domain.features.smm = { enabled: true };

switch (firmwareBootloader) {
case BootMode.uefi:
vmDraft.spec.template.spec.domain.firmware.bootloader = {
efi: { secureBoot: false },
};
break;
case BootMode.uefiSecure:
vmDraft.spec.template.spec.domain.firmware.bootloader = {
efi: { secureBoot: true },
};
break;
default:
vmDraft.spec.template.spec.domain.firmware.bootloader = { bios: {} };
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ const WizardOverviewGrid: React.FC<WizardOverviewGridProps> = ({ vm, tabsData, u
/>
))
}
description={getBootloaderTitleFromVM(vm, t)}
description={getBootloaderTitleFromVM(vm)}
/>

<WizardDescriptionItem
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const BootMethod: React.FC<BootMethodProps> = ({ template }) => {
const { isTemplateEditable } = useEditTemplateAccessReview(template);
const firmwareBootloaderTitle = getBootloaderTitleFromVM(
getTemplateVirtualMachineObject(template),
t,
);
const onSubmit = React.useCallback(
(updatedTemplate: V1Template) =>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import * as React from 'react';
import React, { ChangeEvent, FC, useMemo, useState } from 'react';
import produce from 'immer';

import { V1Template } from '@kubevirt-ui/kubevirt-api/console';
import { bootloaderOptions } from '@kubevirt-utils/components/FirmwareBootloaderModal/utils/constants';
import { BootloaderOptionValue } from '@kubevirt-utils/components/FirmwareBootloaderModal/utils/types';
import {
BootloaderLabel,
BootloaderOptionValue,
} from '@kubevirt-utils/components/FirmwareBootloaderModal/utils/constants';
import { getBootloaderFromVM } from '@kubevirt-utils/components/FirmwareBootloaderModal/utils/utils';
getBootloaderFromVM,
updatedVMBootMode,
} from '@kubevirt-utils/components/FirmwareBootloaderModal/utils/utils';
import TabModal from '@kubevirt-utils/components/TabModal/TabModal';
import { useKubevirtTranslation } from '@kubevirt-utils/hooks/useKubevirtTranslation';
import { getTemplateVirtualMachineObject } from '@kubevirt-utils/resources/template';
import { ensurePath } from '@kubevirt-utils/utils/utils';
import { Form, FormGroup, Select, SelectOption, SelectVariant } from '@patternfly/react-core';

type TemplateBootloaderModalProps = {
Expand All @@ -20,74 +20,31 @@ type TemplateBootloaderModalProps = {
onSubmit: (updatedVM: V1Template) => Promise<V1Template | void>;
};

const TemplateBootloaderModal: React.FC<TemplateBootloaderModalProps> = ({
const TemplateBootloaderModal: FC<TemplateBootloaderModalProps> = ({
template,
isOpen,
onClose,
onSubmit,
}) => {
const { t } = useKubevirtTranslation();
const [isDropdownOpen, setIsDropdownOpen] = React.useState(false);
const [isDropdownOpen, setIsDropdownOpen] = useState(false);
const [selectedFirmwareBootloader, setSelectedFirmwareBootloader] =
React.useState<BootloaderOptionValue>(
getBootloaderFromVM(getTemplateVirtualMachineObject(template)),
);
useState<BootloaderOptionValue>(getBootloaderFromVM(getTemplateVirtualMachineObject(template)));

const bootloaderOptions: BootloaderLabel[] = [
{
value: 'bios',
title: t('BIOS'),
description: t('Use BIOS when bootloading the guest OS (Default)'),
},
{
value: 'uefi',
title: t('UEFI'),
description: t(
'Use UEFI when bootloading the guest OS. Requires SMM feature, if the SMM feature is not set, choosing this method will set it to true',
),
},
{
value: 'uefiSecure',
title: t('UEFI (secure)'),
description: t(
'Use UEFI when bootloading the guest OS. Requires SMM feature, if the SMM feature is not set, choosing this method will set it to true',
),
},
];

const handleChange = (
event: React.ChangeEvent<HTMLSelectElement>,
value: BootloaderOptionValue,
) => {
const handleChange = (event: ChangeEvent<HTMLSelectElement>, value: BootloaderOptionValue) => {
event.preventDefault();
setSelectedFirmwareBootloader(value);
setIsDropdownOpen(false);
};

const updatedTemplate = React.useMemo(() => {
const updatedTemplate = useMemo(() => {
return produce<V1Template>(template, (templateDraft: V1Template) => {
const vmDraft = getTemplateVirtualMachineObject(templateDraft);
ensurePath(vmDraft, 'spec.template.spec.domain.firmware.bootloader');

ensurePath(vmDraft, 'spec.template.spec.domain.features.smm');
vmDraft.spec.template.spec.domain.features.smm = { enabled: true };
const updatedVM = updatedVMBootMode(vmDraft, selectedFirmwareBootloader);

switch (selectedFirmwareBootloader) {
case 'uefi':
vmDraft.spec.template.spec.domain.firmware.bootloader = {
efi: {},
};
break;
case 'uefiSecure':
vmDraft.spec.template.spec.domain.firmware.bootloader = {
efi: { secureBoot: true },
};
break;
default: // 'bios'
vmDraft.spec.template.spec.domain.firmware.bootloader = { bios: {} };
}
vmDraft.spec.template.spec.domain = updatedVM.spec.template.spec.domain;
});
}, [template, selectedFirmwareBootloader]);
}, [selectedFirmwareBootloader, template]);

return (
<TabModal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const VirtualMachineDetailsLeftGrid: React.FC<VirtualMachineDetailsLeftGridProps
const accessReview = asAccessReview(VirtualMachineModel, vm, 'update' as K8sVerb);
const [canUpdateVM] = useAccessReview(accessReview || {});
const [guestAgentData, loadedGuestAgent] = useGuestOS(vmi);
const firmwareBootloaderTitle = getBootloaderTitleFromVM(vm, t);
const firmwareBootloaderTitle = getBootloaderTitleFromVM(vm);
const templateName = getLabel(vm, VM_TEMPLATE_ANNOTATION);
const templateNamespace = getLabel(vm, LABEL_USED_TEMPLATE_NAMESPACE);

Expand Down

0 comments on commit 712263c

Please sign in to comment.