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

[STORM-2466] The example of jaas.conf in jaas_kerberos.conf should give more details #2110

Merged
merged 1 commit into from Jun 12, 2017
Merged

[STORM-2466] The example of jaas.conf in jaas_kerberos.conf should give more details #2110

merged 1 commit into from Jun 12, 2017

Conversation

liu-zhaokun
Copy link
Contributor

https://issues.apache.org/jira/browse/STORM-2466
This documentation lacks several section such as "Client" and "Server".The Client section is used by processes wanting to talk to ZooKeeper and really only needs to be included with nimbus and the supervisors.The Server section is used by the ZooKeeper servers. "jaas_kerberos.conf" should give users a good lead,so it should should give more details as SECURITY.md .Also,Having unused sections in the jaas is not a problem.So I think The example of jaas.conf in jaas_kerberos.conf should be the same as the PR.

@liu-zhaokun
Copy link
Contributor Author

Hello, @harshach
I have made some changes for STORM-2466 as your advice.But there is some misuse, so I create a new PR.Could you help me review it?Thanks.

@liu-zhaokun
Copy link
Contributor Author

@vesense
Could you help me review this PR?

@liu-zhaokun
Copy link
Contributor Author

@HeartSaVioR
Could you help me review this PR?

@liu-zhaokun
Copy link
Contributor Author

Hello,@harshach
Did you notice this PR?

@liu-zhaokun
Copy link
Contributor Author

@kishorvpatil
Could you help me merge this PR?

@liu-zhaokun
Copy link
Contributor Author

@harshach
Could you review this PR?

1 similar comment
@liu-zhaokun
Copy link
Contributor Author

@harshach
Could you review this PR?

@liu-zhaokun
Copy link
Contributor Author

@HeartSaVioR
Could you review this PR?

@liu-zhaokun
Copy link
Contributor Author

@vesense
Could you review this PR?

1 similar comment
@liu-zhaokun
Copy link
Contributor Author

@vesense
Could you review this PR?

@liu-zhaokun
Copy link
Contributor Author

@HeartSaVioR
Could you review this PR?

@liu-zhaokun
Copy link
Contributor Author

@harshach
Could you review this PR?

@liu-zhaokun
Copy link
Contributor Author

@kishorvpatil
Could you help me merge this PR?

1 similar comment
@liu-zhaokun
Copy link
Contributor Author

@kishorvpatil
Could you help me merge this PR?

@liu-zhaokun
Copy link
Contributor Author

@harshach
Could you review this PR?

@liu-zhaokun
Copy link
Contributor Author

@HeartSaVioR
Could you review this PR?

@liu-zhaokun
Copy link
Contributor Author

@kishorvpatil @vesense @HeartSaVioR @harshach ,can you take a look at this PR? It's too long to almost forget
The issue is still “IN PROGRESS”
I just want to finish it properly
thx

@HeartSaVioR
Copy link
Contributor

HeartSaVioR commented Jun 9, 2017 •
edited

@liu-zhaokun Sorry to visit too late. Btw I'm not expert on this and also not sure what $<variable> came from. Do users need to replace it manually, or they're reserved variables?

@liu-zhaokun
Copy link
Contributor Author

@HeartSaVioR
Hello,long time no see!Anyway,thanks for your reply.
The $ here should be replace with their keytab and principal by users manually.

@liu-zhaokun
Copy link
Contributor Author

@HeartSaVioR
kishorvpatil approved these changes 28 days ago,could you help me merge this PR as soon as possible?

@HeartSaVioR
Copy link
Contributor

@liu-zhaokun
Then it would be better to document that to make intention clearer. Could you please add explanation
above the example?

@liu-zhaokun
Copy link
Contributor Author

@HeartSaVioR
The explanation has existed in docs\SECURITY.md,so I didn't add it in these files.

@liu-zhaokun
Copy link
Contributor Author

@HeartSaVioR
Do you agree with me?Could you help me merge it?

@HeartSaVioR
Copy link
Contributor

@liu-zhaokun Ah OK. Missed that. +1

@asfgit asfgit merged commit a67a3d2 into apache:master Jun 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kishorvpatil kishorvpatil kishorvpatil approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@liu-zhaokun @HeartSaVioR @kishorvpatil @asfgit