Skip to content

Conversation

@mattisonchao
Copy link
Member

Motivation

Support JSON token file format authentication for Pulsar Admin.
E.g:

{"file": "./mnt/token"}

Modifications

  • Add token file format for json.

Verifying this change

  • Make sure that the change passes the CI checks.

Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

Is it necessary? This could be done from the application side.

It looks unnecessarily complicated because now File has two supported format <token> or {"file": "<token>"}.

The existing NewAuthenticationTokenFromFile has poor design that should be deprecated.

@mattisonchao
Copy link
Member Author

Hi, @BewareMyPower

We should use JSON format as a primary format to configure the token with different key-value pairs. That will be more flexible.

E,g:

  • {"token": ""}
  • {"file": ""}

But this repo has two components.

  • client
  • admin

The JSON format only works for a client, but not for the admin.

@BewareMyPower
Copy link
Contributor

It makes sense to use the same way to configure JWT authentication for admin and client.

But from the perspective of designing the API from the very beginning:

  1. One format is enough. Pure token string is enough, JSON is scalable but it's over-designed. At least the server side only processes the token string.
  2. Reading from file (or any other external data source like DB or even a topic) is unnecessary. It should be done from the application side.

@BewareMyPower BewareMyPower merged commit f9363eb into master Jun 12, 2025
7 checks passed
@BewareMyPower BewareMyPower deleted the fix.admin.tokenfile branch June 12, 2025 03:01
@RobertIndie RobertIndie added this to the v0.16.0 milestone Jul 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants