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.

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

[Issue 8338][Function Worker] Splitting the authentication logic of function worker and client #8824

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

nodece
Copy link
Member

@nodece nodece commented Dec 4, 2020

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

Fixes #8338

Motivation

In some scenarios, users use their own function-worker to connect to an existing pulsar cluster. Their own function-worker and pulsar cluster have different authentication methods, In the following code, when both function-worker and client have enabled the authentication and authorization services, the authentication and authorization can take effect. A better way is to separate them. function-worker can enable and disable the authentication service, and the broker-client can also enable and disable the authentication service according to the configuration.

Modifications

Add a configuration called brokerClientAuthenticationEnabled in the configuration file, which is disabled by default. It is used to control whether the broker-client of function-worker enable or disable the authentication.

Verifying this change

  • Make sure that the change passes the CI checks.

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
@sijie sijie modified the milestones: 2.7.0, 2.8.0 Dec 4, 2020
@sijie sijie added area/function doc-required Your PR changes impact docs and you will update later. type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages labels Dec 4, 2020
@wolfstudy
Copy link
Member

@nodece @sijie There is a bit of confusion here. If we want to add the prefix of brokerClient to distinguish the different operations between client and broker_client, do we also need to add the prefix of brokerClient to the following options?

# Enforce authorization on accessing functions api
authorizationEnabled: false
# Set of authentication provider name list, which is a list of class names
authenticationProviders:
# Authorization provider fully qualified class-name
authorizationProvider: org.apache.pulsar.broker.authorization.PulsarAuthorizationProvider

Copy link
Member

@wolfstudy wolfstudy left a comment

Choose a reason for hiding this comment

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

The change LGTM+1, just a little confusion, please check.

@sijie
Copy link
Member

sijie commented Dec 4, 2020

@wolfstudy you are right.

Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

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

@nodece Can you address @wolfstudy 's comments?

@nodece
Copy link
Member Author

nodece commented Dec 7, 2020

@tuteng
Copy link
Member

tuteng commented Dec 7, 2020

@nodece You are right.

@nodece @sijie There is a bit of confusion here. If we want to add the prefix of brokerClient to distinguish the different operations between client and broker_client, do we also need to add the prefix of brokerClient to the following options?

# Enforce authorization on accessing functions api
authorizationEnabled: false
# Set of authentication provider name list, which is a list of class names
authenticationProviders:
# Authorization provider fully qualified class-name
authorizationProvider: org.apache.pulsar.broker.authorization.PulsarAuthorizationProvider

This is the authentication and authorization plugin configuration for function-worker, brokerClientAuthenticationPlugin, brokerClientAuthenticationParameters already exist https://github.com/apache/pulsar/blob/master/pulsar-functions/runtime/src/main/java/org/apache/pulsar/functions/worker/WorkerConfig.java#L280 to distinguish it from function-worker @wolfstudy

@sijie
Copy link
Member

sijie commented Dec 7, 2020

@nodece @tuteng thank you for clarifications!

@nodece great contribution!

@sijie sijie merged commit 3464f46 into apache:master Dec 7, 2020
RobertIndie pushed a commit to RobertIndie/pulsar that referenced this pull request Dec 8, 2020
…che#8824)

Fixes apache#8338 

### Motivation

>In some scenarios, users use their own function-worker to connect to an existing pulsar cluster. Their own function-worker and pulsar cluster have different authentication methods, In the following code, when both function-worker and client have enabled the authentication and authorization services, the authentication and authorization can take effect. A better way is to separate them. function-worker can enable and disable the authentication service, and the broker-client can also enable and disable the authentication service according to the configuration.

### Modifications

Add a configuration called `brokerClientAuthenticationEnabled` in the configuration file, which is disabled by default. It is used to control whether the broker-client of function-worker enable or disable the authentication.
codelipenghui pushed a commit that referenced this pull request Jan 14, 2021
Fixes #8338 

### Motivation

>In some scenarios, users use their own function-worker to connect to an existing pulsar cluster. Their own function-worker and pulsar cluster have different authentication methods, In the following code, when both function-worker and client have enabled the authentication and authorization services, the authentication and authorization can take effect. A better way is to separate them. function-worker can enable and disable the authentication service, and the broker-client can also enable and disable the authentication service according to the configuration.

### Modifications

Add a configuration called `brokerClientAuthenticationEnabled` in the configuration file, which is disabled by default. It is used to control whether the broker-client of function-worker enable or disable the authentication.

(cherry picked from commit 3464f46)
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Jan 14, 2021
freeznet pushed a commit to streamnative/pulsar-archived that referenced this pull request Jan 14, 2021
…che#8824)

Fixes apache#8338 

### Motivation

>In some scenarios, users use their own function-worker to connect to an existing pulsar cluster. Their own function-worker and pulsar cluster have different authentication methods, In the following code, when both function-worker and client have enabled the authentication and authorization services, the authentication and authorization can take effect. A better way is to separate them. function-worker can enable and disable the authentication service, and the broker-client can also enable and disable the authentication service according to the configuration.

### Modifications

Add a configuration called `brokerClientAuthenticationEnabled` in the configuration file, which is disabled by default. It is used to control whether the broker-client of function-worker enable or disable the authentication.

(cherry picked from commit 3464f46)
@Jennifer88huang-zz Jennifer88huang-zz removed the doc-required Your PR changes impact docs and you will update later. label Mar 5, 2021
@Jennifer88huang-zz
Copy link
Contributor

Confirmed with Penghui, the right logic is to allow users to enable/disable authentication separately. This PR fixes the logic and add description in the code snippet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/function cherry-picked/branch-2.7 Archived: 2.7 is end of life release/2.7.1 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Splitting the authentication logic of function worker and client
6 participants