Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(xo-web/VM): display accurate Secure Boot status #7751

Merged
merged 9 commits into from
Jul 29, 2024

Conversation

MelissaFrncJrg
Copy link
Contributor

@MelissaFrncJrg MelissaFrncJrg commented Jun 17, 2024

Description

Fixes #7495
secureBootStatus
propagateCertificatesButton
propagateCertificates

Checklist

  • Commit
    • Title follows commit conventions
    • Reference the relevant issue (Fixes #007, See xoa-support#42, See https://...)
    • If bug fix, add Introduced by
  • Changelog
    • If visible by XOA users, add changelog entry
    • Update "Packages to release" in CHANGELOG.unreleased.md
  • PR
    • If UI changes, add screenshots
    • If not finished or not tested, open as Draft

@MelissaFrncJrg MelissaFrncJrg marked this pull request as ready for review June 24, 2024 15:08
Copy link
Collaborator

@stormi stormi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

packages/xo-web/src/xo-app/vm/tab-advanced.js Show resolved Hide resolved
packages/xo-web/src/xo-app/vm/tab-advanced.js Outdated Show resolved Hide resolved
packages/xo-web/src/xo-app/vm/tab-advanced.js Show resolved Hide resolved
@MelissaFrncJrg MelissaFrncJrg requested review from pdonias and MathieuRA and removed request for pdonias June 27, 2024 14:25
packages/xo-web/src/xo-app/vm/tab-general.js Outdated Show resolved Hide resolved
packages/xo-web/src/common/xo/index.js Outdated Show resolved Hide resolved
@xen-orchestra/xapi/vm.mjs Outdated Show resolved Hide resolved
CHANGELOG.unreleased.md Outdated Show resolved Hide resolved
packages/xo-server/src/api/pool.mjs Outdated Show resolved Hide resolved
packages/xo-server/src/xapi-object-to-xo.mjs Outdated Show resolved Hide resolved
packages/xo-server/src/xapi-object-to-xo.mjs Outdated Show resolved Hide resolved
packages/xo-web/src/common/xo/index.js Outdated Show resolved Hide resolved
packages/xo-web/src/xo-app/vm/tab-general.js Outdated Show resolved Hide resolved
packages/xo-server/src/api/pool.mjs Outdated Show resolved Hide resolved
packages/xo-web/src/common/xo/index.js Outdated Show resolved Hide resolved
CHANGELOG.unreleased.md Outdated Show resolved Hide resolved
CHANGELOG.unreleased.md Show resolved Hide resolved
Copy link
Member

@pdonias pdonias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing to add.

This will need to be tested again end-to-end after the latest changes.
Don't forget to add xo-server in the commit message's scope.

@stormi
Copy link
Collaborator

stormi commented Jul 3, 2024

In the General tab, I still see "SecureBoot status: " on BIOS VMs, as well as on VMs running on XCP-ng 8.2. There should be nothing instead, as decided on thursday during the daily meeting.

image

packages/xo-server/src/api/vm.mjs Outdated Show resolved Hide resolved
@MathieuRA MathieuRA requested a review from julien-f July 22, 2024 08:15
@MathieuRA MathieuRA force-pushed the secureBootStatus branch 2 times, most recently from 87f661d to e87c7ff Compare July 22, 2024 13:30
@MathieuRA MathieuRA removed the request for review from julien-f July 22, 2024 14:18
@MathieuRA MathieuRA force-pushed the secureBootStatus branch 2 times, most recently from 1cfccbd to cdf28ae Compare July 24, 2024 14:07
@MathieuRA MathieuRA marked this pull request as draft July 24, 2024 15:04
@MathieuRA MathieuRA requested a review from julien-f July 25, 2024 08:16
@MathieuRA MathieuRA marked this pull request as ready for review July 25, 2024 08:16
@julien-f julien-f removed their request for review July 25, 2024 08:57
@MathieuRA MathieuRA requested a review from julien-f July 25, 2024 09:58
Copy link
Member

@julien-f julien-f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Server-side lgtm.

Copy link
Collaborator

@stormi stormi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests OK on my side.

@MathieuRA MathieuRA dismissed their stale review July 29, 2024 12:11

PR co worker

@MathieuRA MathieuRA merged commit c9699c8 into master Jul 29, 2024
1 check passed
@MathieuRA MathieuRA deleted the secureBootStatus branch July 29, 2024 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display accurate Secure Boot status and allow to fix a VM's UEFI certs
5 participants