feat(xo-web/VM): display accurate Secure Boot status #7751
Conversation
Pizzosaure
force-pushed
the
secureBootStatus
branch
from
95ed982
to
8542b5c
Compare
| </td> | ||
| </tr> | ||
| )} | ||
| {vm.boot.firmware === 'uefi' && semver.satisfies(host?.version, '>=8.3.0') && ( |
There was a problem hiding this comment.
At the moment the warning "This feature is only available for UEFI VMs" is always displayed, this should be fixed when this PR will be merged, thanks to the fix in #7767
| const SECUREBOOT_STATUS_MESSAGES = { | ||
| disabled: _('secureBootStatusNotEnforced'), | ||
| first_boot: _('secureBootStatusPending'), | ||
| ready: _('secureBootStatus'), |
There was a problem hiding this comment.
This displays just "Secure Boot status". I think that it's a mistake and that a secureBootStatusReady message is missing.
There was a problem hiding this comment.
Most things work well, but I found a few issues, and there's also something important from the specs that is missing : "This should be displayed both in the general VM view, because it's an important piece of information, and also below the Secure Boot switch in the Advanced view of the VM, updated whenever someone changes the switch's state." (by "general VM view", I meant the VM's "General" tab).
| setup_mode: _('secureBootWantedButDisabled'), | ||
| certs_incomplete: _('secureBootWantedButCertificatesMissing'), |
There was a problem hiding this comment.
I initially thought all message identifiers were to start with secureBootStatus.
There was a problem hiding this comment.
Some identifiers are already pretty long and I wanted to keep a certain consistency, but reading this again I see I failed at that in some of them... Maybe change this one to just secureBootNoDbx or change them all to start with securebootStatus
| > | ||
| {_('propagateCertificates')} | ||
| </ActionButton> | ||
| {isDisabled && ( |
There was a problem hiding this comment.
If I read it well, this condition is not enough. It should not display the warning about UEFI being required in the case when the reason why it's disabled is because poolGuestSecurebootReadiness === 'not_ready'.
As currently implemented, when the pool is not ready for Secure Boot, both warnings are displayed. We should only see one.
| }) | ||
| } | ||
|
|
||
| await vmSetUefiMode(vm, 'user') |
There was a problem hiding this comment.
This fails with method not found: vm.setUefiMode.
In addition to this, the error popup has a "show logs" button, but when clicked it doesn't show any log that is related to the error.
| </td> | ||
| </tr> | ||
| )} | ||
| {vm.boot.firmware === 'uefi' && ( |
There was a problem hiding this comment.
I think it was not very clear in the specs, but the new method that allows to propagate certificates is only available starting with XCP-ng 8.3 (and possibly XenServer 8, but they don't really need it).
Pizzosaure
force-pushed
the
secureBootStatus
branch
from
1f1ea57
to
30a628f
Compare
Pizzosaure
requested review from
pdonias and
MathieuRA
and removed request for
pdonias
packages/xo-server/src/api/pool.mjs
Outdated
| } | ||
|
|
||
| getGuestSecureBootReadiness.params = { | ||
| pool: { |
| @@ -36,6 +36,7 @@ import { | |||
| import { CpuSparkLines, MemorySparkLines, NetworkSparkLines, XvdSparkLines } from 'xo-sparklines' | |||
| import { injectState, provideState } from 'reaclette' | |||
| import { find } from 'lodash' | |||
| import { subscribeSecurebootReadiness } from '../../common/xo' | |||
There was a problem hiding this comment.
move this import into the xo import. Line 15
Co-authored-by: Pierre Donias <pierre.donias@gmail.com>
Co-authored-by: Mathieu <70369997+MathieuRA@users.noreply.github.com>
Description
Fixes #7495
Checklist
Fixes #007,See xoa-support#42,See https://...)Introduced byCHANGELOG.unreleased.md