Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[fix][broker] Sanitize values before logging in apply-config-from-env.py script #22044

Merged
merged 1 commit into from
Feb 9, 2024
Merged

[fix][broker] Sanitize values before logging in apply-config-from-env.py script #22044

merged 1 commit into from
Feb 9, 2024

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Feb 8, 2024

Fixes #22043

Motivation

See #22043

Modifications

  • add value sanitization logic to apply-config-from-env.py script
  • refactor the script
  • add support for passing the prefix so that there isn't a need for code duplication in the apply-config-from-env-with-prefix.py script. That can be deprecated.

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added type/bug The PR fixed a bug or issue reported a bug area/broker labels Feb 8, 2024
@lhotari lhotari added this to the 3.3.0 milestone Feb 8, 2024
@lhotari lhotari self-assigned this Feb 8, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 8, 2024
@lhotari lhotari force-pushed the lh-sanitize-logging-in-apply-config-python-script branch from 6c51c34 to 8965225 Compare February 8, 2024 18:33
@lhotari lhotari requested a review from heesung-sn February 8, 2024 18:38
@lhotari lhotari force-pushed the lh-sanitize-logging-in-apply-config-python-script branch from 8965225 to 68379b2 Compare February 8, 2024 18:43
@lhotari
Copy link
Member Author

lhotari commented Feb 8, 2024

Tested on command line in this way:

cp conf/broker.conf /tmp

brokerInterceptors=foo PULSAR_PREFIX_brokerClientAuthenticationParameters=SECRET_VALUE ./docker/pulsar/scripts/apply-config-from-env.py /tmp/broker.conf

brokerInterceptors=foo brokerClientAuthenticationParameters=SECRET_VALUE ./docker/pulsar/scripts/apply-config-from-env.py /tmp/broker.conf

# also tested the new --prefix parameter that replaces the duplicate script apply-config-from-env-with-prefix.py (used in Pulsar SQL)
FOO_brokerInterceptors=foo brokerClientAuthenticationParameters=SECRET_VALUE ./docker/pulsar/scripts/apply-config-from-env.py --prefix FOO_ /tmp/broker.conf

@lhotari lhotari merged commit 3036783 into apache:master Feb 9, 2024
48 of 49 checks passed
lhotari added a commit that referenced this pull request Feb 9, 2024
@lhotari
[fix][broker] Sanitize values before logging in apply-config-from-env…
f5922bc 
….py script (#22044)

(cherry picked from commit 3036783)
lhotari added a commit that referenced this pull request Feb 9, 2024
@lhotari
[fix][broker] Sanitize values before logging in apply-config-from-env…
a88f592 
….py script (#22044)

(cherry picked from commit 3036783)
lhotari added a commit that referenced this pull request Feb 9, 2024
@lhotari
[fix][broker] Sanitize values before logging in apply-config-from-env…
9a8761e 
….py script (#22044)

(cherry picked from commit 3036783)
lhotari added a commit that referenced this pull request Feb 9, 2024
@lhotari
[fix][broker] Sanitize values before logging in apply-config-from-env…
4d320e7 
….py script (#22044)

(cherry picked from commit 3036783)
lhotari added a commit that referenced this pull request Feb 9, 2024
@lhotari
[fix][broker] Sanitize values before logging in apply-config-from-env…
1434205 
….py script (#22044)

(cherry picked from commit 3036783)
@liangyepianzhou liangyepianzhou mentioned this pull request Feb 19, 2024
Merged
15 tasks
nodece pushed a commit to nodece/pulsar that referenced this pull request Feb 23, 2024
@lhotari @nodece
[fix][broker] Sanitize values before logging in apply-config-from-env…
167efb7 
….py script (apache#22044)

(cherry picked from commit 3036783)
mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 1, 2024
@lhotari @mukesh-ctds
[fix][broker] Sanitize values before logging in apply-config-from-env…
3bf0b57 
….py script (apache#22044)

(cherry picked from commit 3036783)
(cherry picked from commit 9a8761e)
mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 6, 2024
@lhotari @mukesh-ctds
[fix][broker] Sanitize values before logging in apply-config-from-env…
4697fb0 
….py script (apache#22044)

(cherry picked from commit 3036783)
(cherry picked from commit 9a8761e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heesung-sn heesung-sn heesung-sn approved these changes

@merlimat merlimat merlimat approved these changes

@michaeljmarshall michaeljmarshall Awaiting requested review from michaeljmarshall

Assignees

@lhotari lhotari

Labels
area/broker cherry-picked/branch-2.10 cherry-picked/branch-2.11 cherry-picked/branch-3.0 cherry-picked/branch-3.1 cherry-picked/branch-3.2 doc-not-needed Your PR changes do not impact docs ready-to-test release/2.10.6 release/2.11.4 release/3.0.3 release/3.1.3 release/3.2.1 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.

[Bug] Sensitive information such as brokerClientAuthenticationParameters gets logged in Pulsar Broker
3 participants
@lhotari @merlimat @heesung-sn