Skip to content

KAFKA-4363: Documentation for sasl.jaas.config property#2316

Closed
rajinisivaram wants to merge 2 commits intoapache:trunkfrom
rajinisivaram:KAFKA-4363
Closed

KAFKA-4363: Documentation for sasl.jaas.config property#2316
rajinisivaram wants to merge 2 commits intoapache:trunkfrom
rajinisivaram:KAFKA-4363

Conversation

@rajinisivaram
Copy link
Copy Markdown
Contributor

No description provided.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 5, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/530/
Test PASSed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 5, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/531/
Test FAILed (JDK 8 and Scala 2.12).

Comment thread docs/security.html Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a good reason to use this instead of the new way? I wonder if we should stick to the new way when describing client config. And have a single place where we describe the static JAAS file way.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ijuma Thank you for the review. There is no real reason to use the static JAAS config file instead of the property for client configuration. But, for the documentation, JAAS config file format is more readable since it is multi-line. And it matches the equivalent broker configuration. I was hoping that specifying the property after the JAAS config file makes it obvious that the property is just a squashed version of the JAAS config file format. Does that make sense?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I understand the reasoning, but I think we need to do a better job at guiding users towards what we consider the better option (and mention the advantage of not being restricted to one authentication user for all clients). I still think we could do what I suggested and then have a link to the static configuration with a blurb stating that it's similar to how it's done for the broker.

What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ijuma Thank you for the review. I have updated the doc.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Looks good by looking at the diff. Will check how it looks rendered and hopefully merge soon.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 5, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/532/
Test PASSed (JDK 8 and Scala 2.11).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 16, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.12/913/
Test PASSed (JDK 8 and Scala 2.12).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 16, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk7-scala2.10/913/
Test FAILed (JDK 7 and Scala 2.10).

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 16, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/kafka-pr-jdk8-scala2.11/915/
Test FAILed (JDK 8 and Scala 2.11).

@asfgit asfgit closed this in b4d8668 Jan 17, 2017
@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 17, 2017

@rajinisivaram, I had a closer look at the SASL documentation and I'm wondering if there's any benefit to the general SASL instructions section for both broker and client. It seems like the mechanism-specific sections include most/all of the steps as well (but are more specific and hence more helpful). The mechanism-specific sections tend to refer back to the general section, but it seems a bit confusing.

I merged this PR to trunk and 0.10.2, but I think it would be great to restructure the SASL docs so that people just read about the mechanisms that they are interested in and the multi-mechanism section if they need to support multiple ones.

Thoughts?

@rajinisivaram
Copy link
Copy Markdown
Contributor Author

@ijuma Thank you merging this. I need to rebase the SCRAM documentation, so I can try to reduce duplication at the same time.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Jan 17, 2017

Perfect!

asfgit pushed a commit that referenced this pull request Jan 17, 2017
Author: Rajini Sivaram <rajinisivaram@googlemail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>

Closes #2316 from rajinisivaram/KAFKA-4363
soenkeliebau pushed a commit to soenkeliebau/kafka that referenced this pull request Feb 7, 2017
Author: Rajini Sivaram <rajinisivaram@googlemail.com>

Reviewers: Ismael Juma <ismael@juma.me.uk>

Closes apache#2316 from rajinisivaram/KAFKA-4363
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