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

Add volume and image encryption feature #1151

Merged
merged 8 commits into from
Sep 24, 2024
Merged

Conversation

a110605
Copy link
Collaborator

@a110605 a110605 commented Sep 13, 2024

Summary

  • Add Encrypted, Decrypted radio buttons and source image dropdown In image create page,
  • In storage class create page, add volume encryption radio button and secret namespace, secret name select dropdowns
  • Show lock icon if image / volume is encrypted.
  • Hide "export image" action if volume is encrypted
  • Show encryption and secret in image detail page.

PR Checklist

  • Is this a multi-tenancy feature/bug?
    • Yes, the relevant RBAC changes are at:
  • Do we need to backport changes to the old Rancher UI, such as RKE1?
    • Yes, the relevant PR is at:
  • Are backend engineers aware of UI changes?

Related Issue #
harvester/harvester#6493

Screenshot/Video

Create Storage class with Secret
create_sc.webm

Encrypted & decrypt Image
image_en:decrypt.webm

Encrypt volume
volume_encypt.webm

@Yu-Jack
Copy link
Collaborator

Yu-Jack commented Sep 13, 2024

A thought just popped into my head. Do we need to display locker icon in the VM page if some VMs use encrypted image or volumes? It might be a useful information for users? cc @bk201

Copy link
Collaborator

@Yu-Jack Yu-Jack left a comment

Choose a reason for hiding this comment

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

Just one question, should we clear the secret name when we select different secret namespace each time?

demo-01.mov

@a110605
Copy link
Collaborator Author

a110605 commented Sep 13, 2024

Just one question, should we clear the secret name when we select different secret namespace each time?

demo-01.mov

Good finding. It should clean the secret when chaning namespace.

@a110605
Copy link
Collaborator Author

a110605 commented Sep 16, 2024

A thought just popped into my head. Do we need to display locker icon in the VM page if some VMs use encrypted image or volumes? It might be a useful information for users? cc @bk201

Per discussion with @Yu-Jack , add lock icon in VM list page and show the tooltip with

  • image encrypted
  • volume encrypted
  • image and volume encrypted

See in vm-lock.webm

@a110605 a110605 force-pushed the issue_6493 branch 2 times, most recently from 117cb41 to b5ce7d8 Compare September 16, 2024 06:00
Copy link
Collaborator

@Yu-Jack Yu-Jack left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Collaborator

@Yu-Jack Yu-Jack left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@a110605 a110605 force-pushed the issue_6493 branch 2 times, most recently from 4f9bab7 to 593f957 Compare September 21, 2024 04:01
@a110605
Copy link
Collaborator Author

a110605 commented Sep 21, 2024

Show encrytion field in vm detail volume tab and render lock in

  • green icon : All volumes are encrypted.
  • yellow icon : Partial volumes are encrypted.

@jillian-maroket, could you give advice for above 2 messages ? thanks.

vm_lock_icon.webm

Copy link
Collaborator

@torchiaf torchiaf left a comment

Choose a reason for hiding this comment

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

Code looks good, just a few minor CRs and question below (Maybe something we can add in the next release as improvements).

@Yu-Jack @a110605 I have a few questions:

  • Should we add 'Encrypt' / 'Decrypt' actions in the image list ?
  • Would it make sense to have a reference to the image that is being decrypted / encrypted, in the new image?

@Yu-Jack
Copy link
Collaborator

Yu-Jack commented Sep 23, 2024

Code looks good, just a few minor CRs and question below (Maybe something we can add in the next release as improvements).

I have a few questions:

  • Should we add 'Encrypt' / 'Decrypt' actions in the image list ?
  • Would it make sense to have a reference to the image that is being decrypted / encrypted, in the new image?

I think these are "nice to have" improvements at this moment. Maybe we could put them into Backlog to next release?

Copy link
Collaborator

@Yu-Jack Yu-Jack left a comment

Choose a reason for hiding this comment

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

LGTM. BTW, there is a failed build about check-plugins-build.

Copy link
Member

@bk201 bk201 left a comment

Choose a reason for hiding this comment

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

presentation lgtm, thanks!

@a110605
Copy link
Collaborator Author

a110605 commented Sep 23, 2024

LGTM. BTW, there is a failed build about check-plugins-build.

Wait for fixed PR #1160 merge first.

Copy link

@jillian-maroket jillian-maroket left a comment

Choose a reason for hiding this comment

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

I changed the "partial" tooltip.

pkg/harvester/l10n/en-us.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@torchiaf torchiaf left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Feature New feature for up coming release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants