-
Notifications
You must be signed in to change notification settings - Fork 28.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-27869][CORE] Redact sensitive information in System Properties from UI #24733
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me. Does it happen to match any JVM properties, or really will only affect user-supplied secret, token, password keys?
Test build #4787 has finished for PR 24733 at commit
|
Test build #4789 has finished for PR 24733 at commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
… from UI Currently system properties are not redacted. This PR fixes that, so that any credentials passed as System properties are redacted as well. Manual test. Run the following and see the UI. ``` bin/spark-shell --conf 'spark.driver.extraJavaOptions=-DMYSECRET=app' ``` Closes #24733 from aaruna/27869. Authored-by: Aaruna <aaruna.godthi@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit bfa7f11) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
… from UI Currently system properties are not redacted. This PR fixes that, so that any credentials passed as System properties are redacted as well. Manual test. Run the following and see the UI. ``` bin/spark-shell --conf 'spark.driver.extraJavaOptions=-DMYSECRET=app' ``` Closes #24733 from aaruna/27869. Authored-by: Aaruna <aaruna.godthi@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit bfa7f11) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
@dongjoon-hyun @srowen Thank you for reviewing and merging this.
This PR will only redact the System Properties wherein the property key/value matches the |
@aaruna Yes, of course. My question was: does that regex match any standard JVM system properties? I don't think so, and maybe OK to redact anyway, but I was trying to understand whether it would ever affect anything but user-supplied system properties. |
… from UI Currently system properties are not redacted. This PR fixes that, so that any credentials passed as System properties are redacted as well. Manual test. Run the following and see the UI. ``` bin/spark-shell --conf 'spark.driver.extraJavaOptions=-DMYSECRET=app' ``` Closes apache#24733 from aaruna/27869. Authored-by: Aaruna <aaruna.godthi@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit bfa7f11) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
… from UI Currently system properties are not redacted. This PR fixes that, so that any credentials passed as System properties are redacted as well. Manual test. Run the following and see the UI. ``` bin/spark-shell --conf 'spark.driver.extraJavaOptions=-DMYSECRET=app' ``` Closes apache#24733 from aaruna/27869. Authored-by: Aaruna <aaruna.godthi@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit bfa7f11) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
… from UI Currently system properties are not redacted. This PR fixes that, so that any credentials passed as System properties are redacted as well. Manual test. Run the following and see the UI. ``` bin/spark-shell --conf 'spark.driver.extraJavaOptions=-DMYSECRET=app' ``` Closes apache#24733 from aaruna/27869. Authored-by: Aaruna <aaruna.godthi@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit bfa7f11) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
What changes were proposed in this pull request?
Currently system properties are not redacted. This PR fixes that, so that any credentials passed as System properties are redacted as well.
How was this patch tested?
Manual test. Run the following and see the UI.