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

[Functions-worker] Fix broker and functions-worker authentication compatibility #9190

Merged
merged 2 commits into from
Jan 14, 2021
Merged

[Functions-worker] Fix broker and functions-worker authentication compatibility #9190

merged 2 commits into from
Jan 14, 2021

Conversation

nodece
Copy link
Member

@nodece nodece commented Jan 12, 2021

Signed-off-by: Zixuan Liu nodeces@gmail.com

Motivation

The brokerClientAuthenticationEnabled is boxed type in WebConfig.java for solve broker and functions-worker authentication compatibility, so brokerClientAuthenticationEnabled should not be in conf/functions-worker.yml.

Modifications

  • commented out brokerClientAuthenticationEnabled in conf/functions-worker.yml

@nodece
Fix broker and functions-worker authentication compatibility
51ee92d 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
zymap
zymap reviewed Jan 12, 2021
View reviewed changes
Copy link
Member

@zymap zymap left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a unit test for this? You can use the PulsarService.initializeWorkerConfigFromBrokerConfig to load the worker config to check the configuration is set right or not to avoid someone change the isBrokerClientAuthenticationEnabled.

@nodece
Copy link
Member Author

nodece commented Jan 12, 2021

@zymap I made a tests for this.

@sijie
Copy link
Member

sijie commented Jan 12, 2021

@nodece Thank you for your fix!

zymap
zymap approved these changes Jan 13, 2021
View reviewed changes
Copy link
Member

@zymap zymap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the test!

@sijie sijie added area/function area/security type/bug The PR fixed a bug or issue reported a bug release/2.7.1 labels Jan 13, 2021
@sijie sijie added this to the 2.8.0 milestone Jan 13, 2021
@sijie
Copy link
Member

sijie commented Jan 13, 2021

@nodece Can you fix the license header issue?

@nodece
Add tests to check authentication compatibility
b1c1223 
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@nodece nodece force-pushed the broker_and_functions_works_auth branch from 4e72817 to b1c1223 Compare January 13, 2021 05:57
@nodece
Copy link
Member Author

nodece commented Jan 13, 2021

@sijie @zymap thanks for your review! license header has been fixed.

@zymap
Copy link
Member

zymap commented Jan 14, 2021

/pulsarbot run-failure-checks

@freeznet
Copy link
Contributor

/pulsarbot run-failure-checks

eolivelli
eolivelli approved these changes Jan 14, 2021
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@freeznet
Copy link
Contributor

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit 56ae93e into apache:master Jan 14, 2021
codelipenghui pushed a commit that referenced this pull request Jan 15, 2021
@nodece @codelipenghui
[Functions-worker] Fix broker and functions-worker authentication com…
8b7c81e 
…patibility (#9190)

* Fix broker and functions-worker authentication compatibility

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* Add tests to check authentication compatibility

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 56ae93e)
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Jan 15, 2021
freeznet pushed a commit to streamnative/pulsar-archived that referenced this pull request Jan 15, 2021
@nodece @freeznet
[Functions-worker] Fix broker and functions-worker authentication com…
24437d9 
…patibility (apache#9190)

* Fix broker and functions-worker authentication compatibility

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* Add tests to check authentication compatibility

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 56ae93e)
merlimat pushed a commit to merlimat/pulsar that referenced this pull request Apr 6, 2021
@nodece @merlimat
[Functions-worker] Fix broker and functions-worker authentication com…
8cc8d59 
…patibility (apache#9190)

* Fix broker and functions-worker authentication compatibility

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* Add tests to check authentication compatibility

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sijie sijie sijie approved these changes

@jiazhai jiazhai jiazhai approved these changes

@eolivelli eolivelli eolivelli approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@zymap zymap zymap approved these changes

Assignees

@nodece nodece

Labels
area/function area/security cherry-picked/branch-2.7 Archived: 2.7 is end of life release/2.7.1 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Milestone
2.8.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@nodece @sijie @zymap @freeznet @jiazhai @eolivelli @codelipenghui