Skip to content

Allow to config no-password truststore#13424

Merged
hezhangjian merged 4 commits intoapache:masterfrom
hezhangjian:allow-truststore-without-pwd
Jan 6, 2022
Merged

Allow to config no-password truststore#13424
hezhangjian merged 4 commits intoapache:masterfrom
hezhangjian:allow-truststore-without-pwd

Conversation

@hezhangjian
Copy link
Member

Motivation

Supprot to config truststore which password is empty ""

Modifications

  • when password is null, load as ""

Verifying this change

  • add test method to verify tls success

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

  • no-need-doc

That's a little enhance, doesn't need doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 21, 2021
Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

I don't believe we're allowed to add binary files to our repo. @dave2wave and @eolivelli - can you confirm this for me?

@dave2wave
Copy link
Member

I don't believe we're allowed to add binary files to our repo.

Assuming it is not possible to create this test file from source then to me this test case is reasonable.

It would be good to have a description of how these two keystores are created. Then we can determine if it is relatively simple to build.

@merlimat
Copy link
Contributor

I don't believe we're allowed to add binary files to our repo. @dave2wave and @eolivelli - can you confirm this for me?

We do have binaries when it make sense. eg: PNG image for diagrams. I believe that if you want to test with a keystore and be able to read it, there's no other way than to include it.

Or if you want, just encode in base64.. which kind of demonstrate that bytes are bytes..

@hezhangjian
Copy link
Member Author

@codelipenghui @lhotari @merlimat @315157973 @hangc0276 PTAL, thanks
@michaeljmarshall PTAL agian, thanks

@hezhangjian
Copy link
Member Author

/pulsarbot run-failure-checks

@hezhangjian
Copy link
Member Author

ping @michaeljmarshall

Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

Thanks for confirming @dave2wave and @merlimat.

LGTM

@hezhangjian hezhangjian merged commit 53ad936 into apache:master Jan 6, 2022
@hezhangjian hezhangjian deleted the allow-truststore-without-pwd branch January 6, 2022 02:15
@BewareMyPower
Copy link
Contributor

Hi, just a tip for new committers. When you merge a PR, it's better to copy and paste the PR description like

image

image

Otherwise, the commit log would only contain all the single commits like 53ad936

See a correct commit like: 40ac793

@hezhangjian
Copy link
Member Author

Hi, just a tip for new committers. When you merge a PR, it's better to copy and paste the PR description like

image

image

Otherwise, the commit log would only contain all the single commits like 53ad936

See a correct commit like: 40ac793

Thank you, I haven't known that before, sorry for my fault.

@BewareMyPower
Copy link
Contributor

Never mind. There was a PR that adds the guideline for merging a PR: #12988. It will be included in Pulsar 2.10.0 website.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants