-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
HDDS-2174. Delete GDPR Encryption Key from metadata when a Key is deleted #1519
Conversation
/label ozone |
💔 -1 overall
This message was automatically generated. |
The test failures are not related, however, I realized I missed updating the delete path for S3 / Multipart. I will push updates shortly to this PR. |
4ea3fab
to
4098abb
Compare
/retest |
4098abb
to
f145db6
Compare
/retest |
💔 -1 overall
This message was automatically generated. |
*/ | ||
public static String getDeletedKeyName(String key, long timestamp) { | ||
return key + "_" + timestamp; | ||
public static RepeatedOmKeyInfo stripGdprMetadata( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add javadoc for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking @bharatviswa504 . I have updated PR to address this and also renamed the method appropriately while I was writing the javadoc.
💔 -1 overall
This message was automatically generated. |
f145db6
to
aa197dd
Compare
/retest |
💔 -1 overall
This message was automatically generated. |
* @return Deleted key name | ||
* Prepares key info to be moved to deletedTable. | ||
* 1. It strips GDPR metadata from key info | ||
* 2. Check if an entry exists in deletedTable for given objectKey, if yes, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 2nd point should be updated, it is still mentioning about old logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM. One minor comment inline.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM, pending CI.
💔 -1 overall
This message was automatically generated. |
…eted HDDS-2174. Delete GDPR Encryption Key from metadata when a Key is deleted HDDS-2174. Delete GDPR Encryption Key from metadata when a Key is deleted addressed checkstyle violations HDDS-2174. Delete GDPR Encryption Key from metadata when a Key is deleted addressed review comments
…eted Addressed review comment to rectify javadoc
3ceeb03
to
301d0a8
Compare
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will commit this after I finish testing this on my local machine.
Thank you for the contribution. I have committed this patch to the trunk. |
Thank you @anuengineer for review/commit. |
As advised, deleted GDPR related metadata from KeyInfo before moving it to deletedTable.
Added/updated test.
P.S. The changes in KeyManagerImpl are not really needed but made them for sanity & it is no harm.