Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[fix][cli] Fix expiration of tokens created with "pulsar tokens create" #22815

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

entvex
Copy link
Contributor

@entvex entvex commented May 31, 2024

Fixes #22811

Motivation

This fix addresses a regression caused by changes in this pull request. It’s crucial for users who rely on bin/pulsar tokens create for creating JWT tokens.

The versions affected 3.2.0 3.2.1 3.2.2 and 3.2.3

Modifications

In this fix, I have changed it back to use seconds and added additional tests to prevent similar issues in the future.

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 31, 2024
@entvex entvex force-pushed the Fix-Token-Create branch from ddc4fc9 to e69a8ae Compare June 2, 2024 18:20
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

Good work!

Please use a TestNG parameterized test for the test cases since they are essentially the same test case with different parameters.
Here's one example how it can be achieved with TestNG:

@Test(dataProvider = "ackReceiptEnabledAndSubscriptionTypes")
public void testMaxUnAckMessagesLowerThanPermits(boolean ackReceiptEnabled, SubscriptionType subType)

@DataProvider(name = "ackReceiptEnabledAndSubscriptionTypes")
public Object[][] ackReceiptEnabledAndSubscriptionTypes() {
return new Object[][] {
{true, SubscriptionType.Shared},
{true, SubscriptionType.Key_Shared},
{false, SubscriptionType.Shared},
{false, SubscriptionType.Key_Shared},
};
}

@entvex entvex self-assigned this Jun 3, 2024
@entvex entvex force-pushed the Fix-Token-Create branch from 7be80e3 to d07883a Compare June 3, 2024 10:11
@entvex
Copy link
Contributor Author

entvex commented Jun 3, 2024

Thanks, @lhotari 😄
I have changed the tests as you requested.

@entvex entvex requested a review from lhotari June 3, 2024 10:12
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM. Good work @entvex

@lhotari lhotari changed the title [fix] Fixes Create is mishandling time units (specifically, treating seconds as milliseconds). [fix][cli] Fix expiration of tokens created with "pulsar tokens create" Jun 3, 2024
@lhotari lhotari added this to the 3.4.0 milestone Jun 3, 2024
@lhotari lhotari added the type/bug The PR fixed a bug or issue reported a bug label Jun 3, 2024
@lhotari lhotari merged commit 245c3e8 into apache:master Jun 3, 2024
52 of 54 checks passed
lhotari pushed a commit that referenced this pull request Jun 4, 2024
…e" (#22815)

Co-authored-by: David Jensen <djn@danskecommodities.com>
(cherry picked from commit 245c3e8)
@lhotari
Copy link
Member

lhotari commented Jun 4, 2024

backported to branch-3.2 with commit f1c4547

Technoboy- pushed a commit that referenced this pull request Jun 6, 2024
…e" (#22815)

Co-authored-by: David Jensen <djn@danskecommodities.com>
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
…e" (apache#22815)

Co-authored-by: David Jensen <djn@danskecommodities.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
cherry-picked/branch-3.2 cherry-picked/branch-3.3 doc-not-needed Your PR changes do not impact docs release/3.2.4 release/3.3.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.

[Bug] [cli] Pulsar Tokens Create is mishandling time units (specifically, treating seconds as milliseconds)
3 participants