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.

Sign up for GitHub

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

Jump to bottom

feat(lep): backing image clone and encryption support #8740

Merged
merged 1 commit into from
Jun 14, 2024
Merged

feat(lep): backing image clone and encryption support #8740

merged 1 commit into from
Jun 14, 2024

Conversation

ChanYiLin
Copy link
Contributor

ref: #7051

  • Add LEP for backing image encryption support

@ChanYiLin ChanYiLin self-assigned this Jun 12, 2024
@ChanYiLin ChanYiLin requested a review from a team as a code owner June 12, 2024 10:41
@ChanYiLin ChanYiLin force-pushed the LH7051_backingimage_encryption_support branch from fd27c9c to ef4ff50 Compare June 12, 2024 10:42
shuo-wu
shuo-wu reviewed Jun 13, 2024
View reviewed changes
Copy link
Contributor

@shuo-wu shuo-wu left a comment

Choose a reason for hiding this comment

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

In general LGTM

enhancements/20240612-backing-image-encryption-and-clone-supprt.md Show resolved Hide resolved
@ChanYiLin ChanYiLin force-pushed the LH7051_backingimage_encryption_support branch from ef4ff50 to 2c52088 Compare June 13, 2024 09:12
derekbit
derekbit reviewed Jun 13, 2024
View reviewed changes
Copy link
Member

@derekbit derekbit left a comment

Choose a reason for hiding this comment

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

In general, LGTM.

enhancements/20240612-backing-image-encryption-and-clone-supprt.md Outdated Show resolved Hide resolved
enhancements/20240612-backing-image-encryption-and-clone-supprt.md Show resolved Hide resolved
enhancements/20240612-backing-image-encryption-and-clone-supprt.md Show resolved Hide resolved
enhancements/20240612-backing-image-encryption-and-clone-supprt.md Show resolved Hide resolved
enhancements/20240612-backing-image-encryption-and-clone-supprt.md Show resolved Hide resolved
enhancements/20240612-backing-image-encryption-and-clone-supprt.md Show resolved Hide resolved
enhancements/20240612-backing-image-encryption-and-clone-supprt.md Show resolved Hide resolved
enhancements/20240612-backing-image-encryption-and-clone-supprt.md Show resolved Hide resolved
@ChanYiLin
feat(lep): backing image clone and encryption support
1b50ae4 
ref: longhorn/longhorn 7051

Signed-off-by: Jack Lin <jack.lin@suse.com>
@ChanYiLin ChanYiLin force-pushed the LH7051_backingimage_encryption_support branch from 2c52088 to 1b50ae4 Compare June 14, 2024 07:49
@derekbit
Copy link
Member

Will the progress of the encryption or decryption be displayed?

This was referenced Jun 14, 2024
Merged
Merged
Closed
@ChanYiLin
Copy link
Contributor Author

ChanYiLin commented Jun 14, 2024 •
edited
Loading

Will the progress of the encryption or decryption be displayed?

Hi @derekbit Yes, it is using the same mechanism as upload and download to copy the data from the sourceReader to targetWriter 4MB at a time.
The progress will be updated.

The PR is ready
You can check the implementation of this function IdleTimeoutCopy()

@ChanYiLin
Copy link
Contributor Author

derekbit
derekbit approved these changes Jun 14, 2024
View reviewed changes
Copy link
Member

@derekbit derekbit left a comment

Choose a reason for hiding this comment

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

LGTM.

TODO:

  1. Cross-disk and cross-node backing image encryption and decryption
  2. Collect and display disk space usage for the backing images
  3. Check the disk usage before encrypting or decryption a backing image
  4. Re-encryption of an encrypted backing image

cc @ChanYiLin @shuo-wu @innobead

@innobead innobead merged commit 9bdd9cc into longhorn:master Jun 14, 2024
4 checks passed
@ChanYiLin ChanYiLin mentioned this pull request Jun 20, 2024
Merged
Yu-Jack
Yu-Jack reviewed Jul 12, 2024
View reviewed changes
enhancements/20240612-backing-image-encryption-and-clone-supprt.md
Comment on lines +190 to +191
backingImage: "parrot-cloned-encrypted"
backingImageDataSourceType: "clone"
Copy link

@Yu-Jack Yu-Jack Jul 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi ChanYiLin, I have a question.

If I decided to use same secret, do I need to create a new storage class when I try to encrypt different backingimages each time? Or I could just use one storage class to encrypt all?

The reason I asked that is I saw this example storage class has the backingimage and backingImageDataSourceType. Thank you.

Just a misunderstanding, each backingimage has its own SC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

discussed with @Yu-Jack
each backing image will have its own storageclass

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

@Yu-Jack Yu-Jack Yu-Jack left review comments

@derekbit derekbit derekbit approved these changes

@innobead innobead innobead approved these changes

@james-munson james-munson Awaiting requested review from james-munson

@shuo-wu shuo-wu Awaiting requested review from shuo-wu

Assignees

@ChanYiLin ChanYiLin

Labels
None yet
Projects
Longhorn Sprint
Status: Closed
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ChanYiLin @derekbit @innobead @Yu-Jack @shuo-wu @james-munson