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.

Sign up for GitHub

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

Jump to bottom

KAFKA-5462: Add configuration to build custom SSL principal name (KIP-371) #5684

Closed
wants to merge 2 commits into from
Closed

KAFKA-5462: Add configuration to build custom SSL principal name (KIP-371) #5684

wants to merge 2 commits into from

Conversation

omkreddy
Copy link
Contributor

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@omkreddy
Copy link
Contributor Author

@rajinisivaram Please take a look, whenever you get a chance.

@rajinisivaram
Copy link
Contributor

@omkreddy Sorry, I haven't had time to review this yet. @harshach Will you be able to review this PR?

@omkreddy
Copy link
Contributor Author

@rajinisivaram no worries. anyhow KIP vote is not yet passed. Vote pending for one more binding vote. we can merge after 2.1 rush.

@omkreddy
Copy link
Contributor Author

omkreddy commented Oct 4, 2018

@junrao @lindong28 Appreciate your review and vote for this simple KIP-371 : https://cwiki.apache.org/confluence/display/KAFKA/KIP-371%3A+Add+a+configuration+to+build+custom+SSL+principal+name)

@omkreddy
Copy link
Contributor Author

@harshach @rajinisivaram KIP-371 vote has passed now. Pls take a look, when you get a chance.

harshach
harshach reviewed Oct 19, 2018
View reviewed changes
clients/src/main/java/org/apache/kafka/common/security/ssl/SSLPrincipalMapper.java Outdated
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class SSLPrincipalMapper {
Copy link

Choose a reason for hiding this comment

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

Nit: we are using camecased "Ssl" instead of "SSL" everywhere else, can we make this consistent.

@harshach
Copy link

Overall LGTM. Minor nit. After address I am +1.

@omkreddy
Copy link
Contributor Author

retest this please

@omkreddy
Copy link
Contributor Author

@harshach Thanks for the review. Updated the PR.

@omkreddy
Copy link
Contributor Author

java 11 failures are not related

@harshach
Copy link

+1

@omkreddy
Copy link
Contributor Author

@rajinisivaram I would like to merge this to trunk. let me know if any concerns.

@rajinisivaram
Copy link
Contributor

@omkreddy I haven't had a chance to review, but since @harshach has already reviewed, feel free to merge.

@omkreddy omkreddy closed this in 32e1da5 Oct 25, 2018
@omkreddy
Copy link
Contributor Author

Merged to trunk.

@omkreddy omkreddy deleted the KAFKA-5462-SSL-Name branch October 25, 2018 11:47
@omkreddy omkreddy mentioned this pull request Oct 25, 2018
Merged
@ryannatesmith ryannatesmith mentioned this pull request Jan 15, 2019
Merged
3 tasks
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
@omkreddy @pengxiaolong
KAFKA-5462: Add configuration to build custom SSL principal name (KIP…
780ba63 
…-371)

Author: Manikumar Reddy <manikumar.reddy@gmail.com>

Reviewers: Sriharsha Chintalapani <sriharsha@apache.org>

Closes apache#5684 from omkreddy/KAFKA-5462-SSL-Name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harshach harshach harshach left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@omkreddy @rajinisivaram @harshach