search

confluentinc / kafka-connect-jdbc

Kafka Connect connector for JDBC-compatible databases
Other
1.01k stars 954 forks source link

Adds config to conditionally mask SQL statements #1297

Closed b-goyal closed 1 year ago

b-goyal commented 1 year ago

Problem

Reverts changes done in CCDB-5222 Find more details on why this is being done [here] (https://confluent.slack.com/archives/C04JV6U2HU2)

Solution

Does this solution apply anywhere else?
If yes, where?

Test Strategy

Testing done:

Release Plan

[CCDB-5222]: https://confluentinc.atlassian.net/browse/CCDB-5222?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

ManasjyotiSharma commented 1 year ago

@mukkachaitanya @b-goyal I see that the PR is already merged, nonetheless leaving following comments:

(1) Default for TRIM_SENSITIVE_LOG_ENABLED_DEFAULT is set to "false". In other words we are saying we won't trim sensitive info by default unless the low level config is overridden to true via LD flag. Is this understanding correct? If so then would we not be opening up the flood gates again for simialr/original sev-1 issue of logging sensitive info? I guess we should rather do it the opposite i.e. by default set it to true i.e. trim sensitive data and only on a need basis to help debug specific issues for specific LCC; for a short window via LD overrides should disable it.

(2) Are we not following the log redactor path as discussed here?

b-goyal commented 1 year ago

Hey @ManasjyotiSharma, sorry for the confusion here. On cloud the plan is to keep it true by default. Had this discussion with @mukkachaitanya offline - Yeah. In connector default the redaction to false, and on cloud we can have it true by default until we figure out a balance. 1) For cloud, this will be true 2) @mukkachaitanya had this concern and we added config for toggling instead of reverting the PR.