Hello Dan Burkert, Adar Dembo,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5562

to look at the new patch set (#2).

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
---
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
77 files changed, 611 insertions(+), 523 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/5562/2
-- 
To view, visit http://gerrit.cloudera.org:8080/5562
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2c5d1355bdfdbf2232aae8c0d809cc044790de28
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <t...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Kudu Jenkins

Reply via email to