Hello Adar Dembo, Kudu Jenkins,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5573
to review the following change.
Change subject: KUDU-1812. Replace PB DebugString calls with redactable variants
......................................................................
KUDU-1812. Replace PB DebugString calls with redactable variants
This uses the clang tool from the prior commit to replace all call sites
of Message::DebugString and Message::ShortDebugString with corresponding
SecureDebugString(msg) and SecureShortDebugString(msg) calls.
The commit was done using the tool except for the following:
- a few call sites inside macros weren't handled by the tool, which
instead inserted 'TODO(PBString)' comments. I fixed those few cases by
hand.
- re-wrapped to avoid long lines called out by ilint
- added appropriate #includes for the new calls
The only potentially controversial bit here is whether we should make
this change in the various tests. In fact, the tests that are checking
whether one PB matches another should probably not be redacting
anything. However, the tests also configure redaction to be disabled, so
the secure and regular variants should have identical output. I chose to
make the substitutions in test code as well as production code following
the reasoning that it is less cognitive load (and easier to check in
precommit) to say "thou shalt never use Message::DebugString" rather
than have different rules in the case of tests.
Change-Id: I2c5d1355bdfdbf2232aae8c0d809cc044790de28
Reviewed-on: http://gerrit.cloudera.org:8080/5562
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <[email protected]>
---
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/index_block.cc
M src/kudu/client/batcher.cc
M src/kudu/client/client-internal.cc
M src/kudu/client/meta_cache.cc
M src/kudu/common/partition.cc
M src/kudu/common/wire_protocol-test.cc
M src/kudu/common/wire_protocol.cc
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_peers.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/leader_election.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_reader.cc
M src/kudu/consensus/peer_manager.cc
M src/kudu/consensus/quorum_util.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/raft_consensus_state.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/create-table-stress-test.cc
M src/kudu/integration-tests/delete_table-test.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/master_replication-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/ts_itest-base.h
M src/kudu/integration-tests/ts_tablet_manager-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-path-handlers.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/ts_descriptor.cc
M src/kudu/master/ts_manager.cc
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc_context.cc
M src/kudu/rpc/rpc_stub-test.cc
M src/kudu/tablet/row_op.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_metadata-test.cc
M src/kudu/tablet/tablet_metadata.cc
M src/kudu/tablet/tablet_peer-test.cc
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_fs.cc
M src/kudu/tools/tool_action_local_replica.cc
M src/kudu/tools/tool_action_remote_replica.cc
M src/kudu/tserver/heartbeater.cc
M src/kudu/tserver/tablet_copy_client.cc
M src/kudu/tserver/tablet_copy_service-test.cc
M src/kudu/tserver/tablet_copy_service.cc
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_copy_source_session.cc
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager-test.cc
M src/kudu/tserver/tserver-path-handlers.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
78 files changed, 620 insertions(+), 525 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/73/5573/1
--
To view, visit http://gerrit.cloudera.org:8080/5573
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2c5d1355bdfdbf2232aae8c0d809cc044790de28
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.2.x
Gerrit-Owner: Dan Burkert <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
