util: explicitly set the Luks2 header size #5394

iPraveenParihar · 2025-06-24T15:06:35Z

Describe what this PR does

util: explicitly set the Luks2 header size

The LUKS2 header size is variable and it can be adjusted
on creation by using the --luks2-metadata-size and
--luks2-keyslots-size options. By default, the header size is
16MiB.

This commit explicitly uses the options to define the default
header size.

Ref 10.10: https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/FAQ.md

Checklist:

Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
Reviewed the developer guide on Submitting a Pull Request
Pending release notes updated with breaking and/or notable changes for the next major release.
Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
Integration tests have been added, if necessary.

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

/retest ci/centos/<job-name>: retest the <job-name> after unrelated
failure (please report the failure too!)

internal/util/cryptsetup/cryptsetup.go

iPraveenParihar · 2025-06-25T06:54:45Z

/test ci/centos/mini-e2e/k8s-1.33

iPraveenParihar · 2025-06-25T08:59:41Z

passed e2e logs

Rakshith-R · 2025-06-25T09:24:10Z

internal/rbd/encryption.go

+			return 0, fmt.Errorf("failed to get %s metadata on image %s: %w", luks2HeaderSizeKey, ri, err)
+		}
+
+		return 0, nil


This should be default size ?

As the name implies, getLuksHeaderSizeMetadata() is intended to return the LUKS2 header size only if it's explicitly set in the image metadata. If the metadata key is missing, it returns 0. It's the responsibility of the caller to interpret this and decide whether to fall back to the default LUKS2 header size (cryptsetup.DefaultLuks2HeaderSize, 16 MiB) or take another action.

Then, you need to adjust your comment appropriately And in that case, the default LUKS2 header // size is used, which is 16 MiB.

I think the default value needs to be returned in this function itself to avoid confusion.

😅 yea, that needs to be removed to avoid confusion.

Rakshith-R · 2025-06-25T09:25:06Z

internal/util/cryptsetup/cryptsetup.go

+	// on creation by using the `--luks2-metadata-size` and
+	// `--luks2-keyslots-size` options. By default, the header size is
+	// 16MiB.
+	DefaultLuks2HeaderSize = 16 * helpers.MiB


Add a separate comment for this one, explaining older images do have this set in image metadata.

addressed PTAL.

Rakshith-R

Thanks

nixpanic

Just a small improvement to prevent potential bugs when calling the new getLuksHeaderSizeMetadata() function.

internal/rbd/encryption.go

Pull request has been modified.

nixpanic · 2025-06-25T15:11:48Z

@Mergifyio rebase

The LUKS2 header size is variable and it can be adjusted on creation by using the `--luks2-metadata-size` and `--luks2-keyslots-size` options. By default, the header size is 16MiB. This commit explicitly uses the options to define the default header size. Ref 10.10: https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/FAQ.md Signed-off-by: Praveen M <[email protected]>

Signed-off-by: Praveen M <[email protected]>

mergify · 2025-06-25T15:12:04Z

rebase

✅ Branch has been successfully rebased

nixpanic · 2025-06-25T15:12:13Z

@Mergifyio queue

mergify · 2025-06-25T15:12:17Z

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at d0003aa

ceph-csi-bot · 2025-06-25T15:12:31Z

/test ci/centos/k8s-e2e-external-storage/1.31

ceph-csi-bot · 2025-06-25T15:12:31Z

/test ci/centos/upgrade-tests-cephfs

ceph-csi-bot · 2025-06-25T15:12:31Z

/test ci/centos/mini-e2e-helm/k8s-1.31

ceph-csi-bot · 2025-06-25T15:12:32Z

/test ci/centos/upgrade-tests-rbd

ceph-csi-bot · 2025-06-25T15:12:32Z

/test ci/centos/mini-e2e/k8s-1.31

ceph-csi-bot · 2025-06-25T15:12:32Z

/test ci/centos/k8s-e2e-external-storage/1.32

ceph-csi-bot · 2025-06-25T15:12:32Z

/test ci/centos/k8s-e2e-external-storage/1.33

ceph-csi-bot · 2025-06-25T15:12:33Z

/test ci/centos/mini-e2e-helm/k8s-1.32

ceph-csi-bot · 2025-06-25T15:12:33Z

/test ci/centos/mini-e2e-helm/k8s-1.33

ceph-csi-bot · 2025-06-25T15:12:33Z

/test ci/centos/mini-e2e/k8s-1.32

ceph-csi-bot · 2025-06-25T15:12:34Z

/test ci/centos/mini-e2e/k8s-1.33

mergify · 2025-06-25T17:09:36Z

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated.

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

Madhu-1 · 2025-06-26T08:21:22Z

internal/rbd/encryption.go

 	oldMetadataDEK = ".rbd.csi.ceph.com/dek"

+	// luks2 header size metadata key.
+	luks2HeaderSizeKey = "rbd.csi.ceph.com/luks2HeaderSize"


Do we want this metadata key to be present on all the clones/snapshots and also across cluster when mirroring is enabled?

nixpanic reviewed Jun 24, 2025

View reviewed changes

internal/util/cryptsetup/cryptsetup.go Show resolved Hide resolved

iPraveenParihar force-pushed the encryption/luks2-explicit-header-size branch from bbb07c4 to 22ce1d4 Compare June 25, 2025 06:52

iPraveenParihar force-pushed the encryption/luks2-explicit-header-size branch from 22ce1d4 to ffee6f6 Compare June 25, 2025 08:59

iPraveenParihar marked this pull request as ready for review June 25, 2025 08:59

iPraveenParihar force-pushed the encryption/luks2-explicit-header-size branch from ffee6f6 to a035b30 Compare June 25, 2025 09:07

iPraveenParihar requested review from Rakshith-R and nixpanic June 25, 2025 09:11

Rakshith-R reviewed Jun 25, 2025

View reviewed changes

iPraveenParihar force-pushed the encryption/luks2-explicit-header-size branch 2 times, most recently from 4cf929f to 2172248 Compare June 25, 2025 10:00

iPraveenParihar requested a review from Rakshith-R June 25, 2025 10:16

iPraveenParihar force-pushed the encryption/luks2-explicit-header-size branch 2 times, most recently from 47889b3 to 709d14b Compare June 25, 2025 11:50

Rakshith-R previously approved these changes Jun 25, 2025

View reviewed changes

Rakshith-R requested a review from a team June 25, 2025 12:16

nixpanic requested changes Jun 25, 2025

View reviewed changes

internal/rbd/encryption.go Show resolved Hide resolved

iPraveenParihar force-pushed the encryption/luks2-explicit-header-size branch from 709d14b to b4e6f86 Compare June 25, 2025 13:26

iPraveenParihar requested a review from nixpanic June 25, 2025 13:33

Rakshith-R approved these changes Jun 25, 2025

View reviewed changes

Rakshith-R requested a review from a team June 25, 2025 13:36

nixpanic approved these changes Jun 25, 2025

View reviewed changes

iPraveenParihar added 2 commits June 25, 2025 15:12

e2e: validate luks2 header size metadata

6cfe178

Signed-off-by: Praveen M <[email protected]>

nixpanic force-pushed the encryption/luks2-explicit-header-size branch from b4e6f86 to 6cfe178 Compare June 25, 2025 15:12

mergify bot added the ok-to-test Label to trigger E2E tests label Jun 25, 2025

ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jun 25, 2025

mergify bot merged commit d0003aa into ceph:devel Jun 25, 2025
36 of 37 checks passed

Madhu-1 reviewed Jun 26, 2025

View reviewed changes

Uh oh!

util: explicitly set the Luks2 header size #5394

util: explicitly set the Luks2 header size #5394

Uh oh!

Conversation

iPraveenParihar commented Jun 24, 2025

Describe what this PR does

Uh oh!

Uh oh!

iPraveenParihar commented Jun 25, 2025

Uh oh!

iPraveenParihar commented Jun 25, 2025

Uh oh!

Rakshith-R Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

iPraveenParihar Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Rakshith-R Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Rakshith-R Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iPraveenParihar Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Rakshith-R Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

iPraveenParihar Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Rakshith-R left a comment

Choose a reason for hiding this comment

Uh oh!

nixpanic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nixpanic commented Jun 25, 2025

Uh oh!

mergify bot commented Jun 25, 2025

✅ Branch has been successfully rebased

Uh oh!

nixpanic commented Jun 25, 2025

Uh oh!

mergify bot commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ The pull request has been merged automatically

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

ceph-csi-bot commented Jun 25, 2025

Uh oh!

Uh oh!

mergify bot commented Jun 25, 2025

Uh oh!

Madhu-1 Jun 26, 2025

Choose a reason for hiding this comment

Rakshith-R Jun 25, 2025 •

edited

Loading

mergify bot commented Jun 25, 2025 •

edited

Loading