You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/12/04 07:14:17 UTC

[GitHub] [pulsar] michaeljmarshall opened a new pull request #13133: [Authorization] Revert new AuthorizationProvider method

michaeljmarshall opened a new pull request #13133:
URL: https://github.com/apache/pulsar/pull/13133

### Motivation

In PR https://github.com/apache/pulsar/pull/12600, we added a method to the `AuthorizationProvider` that is not used outside of implementations of the `AuthorizationProvider` itself. Since this interface is public, we should make sure that all methods are necessary. Further, since we cherry picked the commit for https://github.com/apache/pulsar/pull/12600 to `branch-2.9` and `branch-2.8`, we introduced a potentially breaking change for third party implementations of the `AuthorizationProvider` interface. While the commit itself isn't breaking, it does imply that the interface has a method that _could_ be used by the authorization service or some other part of Pulsar. Since third party implementations might not have this method implemented, they could break.

For added context, it is likely that this method was added because there are other similar methods in the interface. For example, the interface has the following methods: `allowSinkOpsAsync`, `allowSourceOpsAsync`, and `allowFunctionOpsAsync`. These methods are much older and are called by the `AuthenticationService`.

### Modifications

* Remove the `AuthorizationProvider#allowConsumeOpsAsync` method from the interface and from all implementations of the interface.

### Verifying this change

This is a trivial change. No underlying logic is changed by this commit.

### Does this pull request potentially affect one of the following parts:

This fix addresses a potentially breaking change.

### Documentation
- [x] `no-need-doc`

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] codelipenghui commented on pull request #13133: [Authorization] Revert new AuthorizationProvider method

Posted by GitBox <gi...@apache.org>.

codelipenghui commented on pull request #13133:
URL: https://github.com/apache/pulsar/pull/13133#issuecomment-986461131


   @michaeljmarshall I think it's ok to remove it in 2.9.1 since we will not introduce breaking changes to 2.9.1, just a user implemented method will not be used. So we only have a breaking change in 2.9.0. /cc @merlimat Please help check.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] codelipenghui commented on pull request #13133: [Authorization] Revert new AuthorizationProvider method

Posted by GitBox <gi...@apache.org>.

codelipenghui commented on pull request #13133:
URL: https://github.com/apache/pulsar/pull/13133#issuecomment-986343968


   > Note: the new method has not yet been published in any releases
   
   I think the 2.9.0 already contains #12600? do you need to also revert the 2.9.0 release?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] michaeljmarshall commented on pull request #13133: [Authorization] Revert new AuthorizationProvider method

Posted by GitBox <gi...@apache.org>.

michaeljmarshall commented on pull request #13133:
URL: https://github.com/apache/pulsar/pull/13133#issuecomment-986048930


   /pulsarbot run-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] michaeljmarshall commented on pull request #13133: [Authorization] Revert new AuthorizationProvider method

Posted by GitBox <gi...@apache.org>.

michaeljmarshall commented on pull request #13133:
URL: https://github.com/apache/pulsar/pull/13133#issuecomment-986060396


   /pulsarbot run-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] michaeljmarshall commented on pull request #13133: [Authorization] Revert new AuthorizationProvider method

Posted by GitBox <gi...@apache.org>.

michaeljmarshall commented on pull request #13133:
URL: https://github.com/apache/pulsar/pull/13133#issuecomment-986355731


   > > Note: the new method has not yet been published in any releases
   > 
   > I think the 2.9.0 already contains #12600? do you need to also revert the 2.9.0 release?
   
   @codelipenghui - you're right, it was included in the 2.9.0 release. I thought it had missed the release because it has the label `release/2.9.1`. I'm not sure what the right choice is for branch-2.9. The release is not good to use in production, so perhaps it is okay to remove it? If it is not okay to remove it, we could annotate the method as `deprecated`. What do you think we should do?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] michaeljmarshall commented on pull request #13133: [Authorization] Revert new AuthorizationProvider method

Posted by GitBox <gi...@apache.org>.

michaeljmarshall commented on pull request #13133:
URL: https://github.com/apache/pulsar/pull/13133#issuecomment-986069711


   /pulsarbot run-failure-checks


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar] eolivelli merged pull request #13133: [Authorization] Revert new AuthorizationProvider method

Posted by GitBox <gi...@apache.org>.

eolivelli merged pull request #13133:
URL: https://github.com/apache/pulsar/pull/13133


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org