[AMORO-2897] Mask the sensitive info for the Terminal output logs#2899
[AMORO-2897] Mask the sensitive info for the Terminal output logs#2899zhoujinsong merged 12 commits intoapache:masterfrom
Conversation
| return COMMON_SECRET_FORMAT.equals(data); | ||
| } | ||
|
|
||
| public static boolean containsSensitiveVal(String input) { |
There was a problem hiding this comment.
We can define SENSITIVE_CONF_KEYWORDS as a list, then use contains method.
| public class DesensitizationUtil { | ||
| public static String COMMON_SECRET_FORMAT = "********"; | ||
|
|
||
| private static final String[] SENSITIVE_CONF_KEYWORDS = {"secret", "token", "password"}; |
There was a problem hiding this comment.
I am not sure if we should desensitize all properties containing the keywords above.
Or we may better define specific properties we should desensitize on and open terminal configuration to allow user add more?
There was a problem hiding this comment.
Good idea, i will try to do that.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2899 +/- ##
============================================
- Coverage 32.76% 27.47% -5.29%
+ Complexity 3821 2483 -1338
============================================
Files 558 360 -198
Lines 46295 37213 -9082
Branches 6175 5432 -743
============================================
- Hits 15170 10226 -4944
+ Misses 29931 26027 -3904
+ Partials 1194 960 -234
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
zhoujinsong
left a comment
zhoujinsong
left a comment
There was a problem hiding this comment.
@tcodehuber I left some small suggestions. PTAL.
| private final TerminalSessionFactory factory; | ||
| private final Configurations sessionConfiguration; | ||
|
|
||
| private final List sensitiveConfKeys; |
There was a problem hiding this comment.
| private final List sensitiveConfKeys; | |
| private final List<String> sensitiveConfKeys; |
|
|
||
| SparkSession context = null; | ||
| Configurations conf; | ||
| List sensitiveConfKeys; |
There was a problem hiding this comment.
| List sensitiveConfKeys; | |
| List<String> sensitiveConfKeys; |
Done. |
zhoujinsong
left a comment
zhoujinsong
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
4 participants
Why are the changes needed?
Close #2897.
Brief change log
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation