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

Fix using partitioned topic name to get Policy #11294

Merged
merged 2 commits into from
Jul 14, 2021

Conversation

315157973
Copy link
Contributor

Motivation

In the master branch, the REST API no longer allows the topic name of the partition to be used to set the topic policy, but there are still many places where it will be used internally.

Suppose we set a Topic policy for persistent://tenant/namespace/topic
However, the policy cannot be obtained through persistent://tenant/namespace/topic-partition-0, which causes the policy to become invalid.

For example:PersistentTopic.checkSubscriptionTypesEnable

Modifications

Convert the name in SystemTopicBasedTopicPoliciesService

Verifying this change

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

I left a couple of minor comments

@315157973 315157973 added the doc-not-needed Your PR changes do not impact docs label Jul 13, 2021
@sijie sijie added component/topic-policy type/bug The PR fixed a bug or issue reported a bug labels Jul 14, 2021
@sijie sijie added this to the 2.9.0 milestone Jul 14, 2021
@sijie sijie merged commit 35d29b9 into apache:master Jul 14, 2021
codelipenghui pushed a commit that referenced this pull request Sep 2, 2021
### Motivation
In the master branch, the REST API no longer allows the topic name of the partition to be used to set the topic policy, but there are still many places where it will be used internally.

Suppose we set a Topic policy for `persistent://tenant/namespace/topic`
However, the policy cannot be obtained through `persistent://tenant/namespace/topic-partition-0`, which causes the policy to become invalid.

For example:PersistentTopic.checkSubscriptionTypesEnable

### Modifications
Convert the name in SystemTopicBasedTopicPoliciesService

(cherry picked from commit 35d29b9)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Sep 2, 2021
codelipenghui pushed a commit that referenced this pull request Sep 6, 2021
…licies (#11897)

### Motivation

There is a bug that using the partitioned topic name to get topic policies. The PR #11294 fix this issue, but it's hard to cherry-pick the PR to `branch-2.7`, so create this PR to fix the issue in `branch-2.7`.

This PR contains [PR-11294](#11294) and [PR-11863](#11863).

### Modifications

1. Fix using the partitioned topic name to get topic policies.
2. Change some warning logs to debug level for the getting topic policies operation.

### Verifying this change

The test method `TopicPoliciesTest#testBacklogQuotaWithPartitionedTopic` is used to verify getting topic policies by the partitioned topic name.
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Sep 6, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
### Motivation
In the master branch, the REST API no longer allows the topic name of the partition to be used to set the topic policy, but there are still many places where it will be used internally.

Suppose we set a Topic policy for `persistent://tenant/namespace/topic`
However, the policy cannot be obtained through `persistent://tenant/namespace/topic-partition-0`, which causes the policy to become invalid.

For example:PersistentTopic.checkSubscriptionTypesEnable

### Modifications
Convert the name in SystemTopicBasedTopicPoliciesService
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-2.7 Archived: 2.7 is end of life cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.7.4 release/2.8.1 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants