Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17974 )
Change subject: [encryption] KUDU-3331 Encrypt file system ...................................................................... Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/17974/9/src/kudu/tools/tool_action_pbc.cc File src/kudu/tools/tool_action_pbc.cc: http://gerrit.cloudera.org:8080/#/c/17974/9/src/kudu/tools/tool_action_pbc.cc@106 PS9, Line 106: IsFileEncrypted > I wouldn't mind leaving it like this, so long as it's clear what's what. I You're right, temp files would cause a problem with this approach. I guess it's better to open the file to see if it's encrypted after all. In this case, this should stay IsFileEncrypted() as we can't tell if a file is sensitive based on the header, only if it's encrypted. I'll change this behavior in the encryption header patch. http://gerrit.cloudera.org:8080/#/c/17974/9/src/kudu/util/pb_util-test.cc File src/kudu/util/pb_util-test.cc: PS9: > Missed this? I wanted to figure out the expected behavior of the IsFileEncrypted before addressing it. With the header-based decision making the second case wouldn't be true, we would be able to read sensitive unencrypted files with encryption enabled. The first case would still be true, but I'm not sure there needs to be a test for that specifically, TestEncryption in env-test already has a similar scenario. Or do you think it's worth adding a tooling-specific test for it? If yes, I can add it in the encryption header patch. -- To view, visit http://gerrit.cloudera.org:8080/17974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I909d0c4af0c1fca0d14c99a6627842dbe2ed7524 Gerrit-Change-Number: 17974 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 07 Jan 2022 09:38:37 +0000 Gerrit-HasComments: Yes
