You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2021/12/17 21:04:35 UTC

[kudu-CR] [encryption] KUDU-3331 Encrypt file system

Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17974 )

Change subject: [encryption] KUDU-3331 Encrypt file system
......................................................................


Patch Set 9: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/17974/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17974/7//COMMIT_MSG@133
PS7, Line 133: I also wanted to run dense_node-itest with -num_seconds=240 and
> It seems the results are all over the place even with rowset compaction dis
Yeah, I agree. It doesn't seem too useful to include.

Perhaps we can consider a similar test though, perhaps one that copies data from an encrypted server onto a non-encrypted server, and then bootstraps from the non-encrypted server (or vice versa). Though I'm fine leaving that for a later patch


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
nit: Is this actually meant to determine whether the file is sensitive or not?


http://gerrit.cloudera.org:8080/#/c/17974/9/src/kudu/util/pb_util-test.cc
File src/kudu/util/pb_util-test.cc:

PS9: 
Do you think it's worth adding a test that checks that we can't read a sensitive file when encryption is disabled? Or that we can't read a sensitive unencrypted file (e.g. written when encryption was disabled) when encryption is enabled?

I suppose the error scenarios may change when you add the headers, but it's worth thinking about early what it looks like when we're looking at mismatched encryption statuses



-- 
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: 9
Gerrit-Owner: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 17 Dec 2021 21:04:35 +0000
Gerrit-HasComments: Yes