Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/20050 )
Change subject: KUDU-3448 Add support for encrypting existing keys ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/20050/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20050/5//COMMIT_MSG@24 PS5, Line 24: they > nit: the Done http://gerrit.cloudera.org:8080/#/c/20050/5//COMMIT_MSG@27 PS5, Line 27: has been deleted > Consider introducing a dedicated method there, instead of re-using AddCertA Done http://gerrit.cloudera.org:8080/#/c/20050/5/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/20050/5/src/kudu/master/catalog_manager.cc@1351 PS5, Line 1351: RETURN_NOT_OK_PREPEND(sys_catalog_->AddCertAuthorityEntry(info), : "IPKI certificate couldn't be written to syscatalog"); > Did you consider doing this not in LoadCertAuthorityInfo(), but in the oute I'm not sure I agree, we're not generating a new CA in this case, just changing the way it's stored, while using the same key as before. Also, LoadCertAuthorityInfo() method is called in two separate places, so we'd need to handle this in two different places instead of in a single place. http://gerrit.cloudera.org:8080/#/c/20050/5/src/kudu/master/sys_catalog.cc File src/kudu/master/sys_catalog.cc: http://gerrit.cloudera.org:8080/#/c/20050/5/src/kudu/master/sys_catalog.cc@927 PS5, Line 927: enc.Add(RowOperationsPB::UPSERT, row); > Maybe, introduce a separate dedicated method UpdateCertAuthorityEntry() or Done -- To view, visit http://gerrit.cloudera.org:8080/20050 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ide6ec4fb86325897f2b011aee9643d276044279d Gerrit-Change-Number: 20050 Gerrit-PatchSet: 5 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Fri, 16 Jun 2023 07:29:18 +0000 Gerrit-HasComments: Yes
