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 s3 server-side encryption and decryption with customer provided key #7088

Merged
merged 4 commits into from
Dec 3, 2020
Merged

Add s3 server-side encryption and decryption with customer provided key #7088

merged 4 commits into from
Dec 3, 2020

Conversation

liortamari
Copy link
Contributor

I would like to add support for AWS S3 backup/restore using server-side encryption and decryption with customer provided key.

@deepthi
Copy link
Member

deepthi commented Nov 30, 2020

Welcome @liortamari! Can you fix the DCO? Instructions here: https://github.com/vitessio/vitess/pull/7088/checks?check_run_id=1475698865

Signed-off-by: liortamary <lior.tamary@houzz.com>
@liortamari
Copy link
Contributor Author

liortamari commented Dec 1, 2020

@deepthi Thank you.
I updated the DCO.
I am also working on adding some unit tests.

liortamary added 2 commits December 1, 2020 22:20
Signed-off-by: liortamary <lior.tamary@houzz.com>
Signed-off-by: liortamary <lior.tamary@houzz.com>
@deepthi
Copy link
Member

deepthi commented Dec 1, 2020

@deepthi Thank you.
I updated the DCO.
I am also working on adding some unit tests.

That's great! We'll ask some people to review this who know better than I do.

Copy link
Contributor

@aquarapid aquarapid left a comment

Choose a reason for hiding this comment

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

Could we change this to avoid adding a new tablet option, and be more consistent, by just making sse_c a new option value for -s3_backup_server_side_encryption? Could be something like:

-s3_backup_server_side_encryption sse_c:/path/to/key/file

Rest seems fine.

@askdba askdba added Status: Fix in Progress Type: Enhancement Logical improvement (somewhere between a bug and feature) labels Dec 2, 2020
@liortamari
Copy link
Contributor Author

@aquarapid Thank you.
I applied this change, indeed it looks more consistent now.

…lled sse_c instead of different flag

Signed-off-by: liortamary <lior.tamary@houzz.com>
@deepthi deepthi requested a review from ajm188 December 2, 2020 19:54
Copy link
Contributor

@ajm188 ajm188 left a comment

Choose a reason for hiding this comment

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

This is purely opt-in, so existing users shouldn't notice, and the new feature looks correct to me. +1

@deepthi deepthi dismissed aquarapid’s stale review December 3, 2020 02:12

Requested changes have been made.

@deepthi deepthi merged commit 6454b7d into vitessio:master Dec 3, 2020
@askdba askdba added this to the v9.0 milestone Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants