Skip to content

chore: Remove attached LUKS header from Store partition#10604

Open
frankdavid wants to merge 5 commits into
masterfrom
frankdavid/remove-attached-header
Open

chore: Remove attached LUKS header from Store partition#10604
frankdavid wants to merge 5 commits into
masterfrom
frankdavid/remove-attached-header

Conversation

@frankdavid

Copy link
Copy Markdown
Contributor

The Store partition is now formatted with a detached LUKS header only. A legacy attached header still present on a device (which was the previous approach) is wiped on the next open, so only the detached header remains going forward.

In addition, guest_disk now uses a shared metrics Registry and writes all metrics once. The PR adds the guest_disk_store_attached_luks2_header_status gauge to track the header migration state.

@frankdavid frankdavid requested a review from a team as a code owner June 29, 2026 13:31
@github-actions github-actions Bot added the chore label Jun 29, 2026
@zeropath-ai

zeropath-ai Bot commented Jun 29, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to eb8df54.

Security Overview
Detected Code Changes
Change Type Relevant files
Refactor ► rs/ic_os/guest_upgrade/client/src/lib.rs
    Remove fallback for LUKS header in client
► rs/ic_os/guest_upgrade/server/src/lib.rs
    Remove unused send_luks_header field
► rs/ic_os/guest_upgrade/server/src/orchestrator.rs
    Remove unused send_luks_header argument
► rs/ic_os/guest_upgrade/server/src/service.rs
    Remove unused send_luks_header field
► rs/ic_os/guest_upgrade/tests/src/lib.rs
    Remove unused test configuration fields
► rs/ic_os/os_tools/guest_disk/src/crypt.rs
    Add function to wipe attached LUKS header
    Add function to check for attached LUKS header
    Change metrics file parameter to registry
► rs/ic_os/os_tools/guest_disk/src/generated_key.rs
    Change metrics file parameter to registry
► rs/ic_os/os_tools/guest_disk/src/main.rs
    Use Registry for metrics instead of file path
► rs/ic_os/os_tools/guest_disk/src/metrics.rs
    Refactor metrics export to use registry
    Add metric for attached LUKS2 header status
► rs/ic_os/os_tools/guest_disk/src/sev.rs
    Wipe attached LUKS header on store partition open
    Export attached LUKS2 header status metric
    Change metrics file parameter to registry
► rs/ic_os/os_tools/guest_disk/src/tests.rs
    Update tests to use registry for metrics
    Add test for wiping attached LUKS header
    Add tests for attached LUKS2 header status metric
Enhancement ► rs/ic_os/os_tools/guest_disk/src/crypt.rs
    Allow setting permissions on detached LUKS header file

@github-actions github-actions Bot added the @node label Jun 29, 2026
.await
.with_context(|| {
format!(
"Failed to write store LUKS header to {}",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe uppercase Store or quote it, otherwise looks like a typo

false
});

if self.store_luks_header_path.exists() && attached_header_present {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we actively check that the existing header is valid? Or this is already done before somewhere?

if self.store_luks_header_path.exists() {
bail!(
"Refusing to format Store because detached LUKS header {} already exists. \
Remove the stale header first if you really want to reformat the device.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm is this error even actionable? How can a user remove it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants