Skip to content

Update LDAP configuration docs#13245

Merged
techdocsmith merged 9 commits intoapache:masterfrom
writer-jill:ldap
Nov 29, 2022
Merged

Update LDAP configuration docs#13245
techdocsmith merged 9 commits intoapache:masterfrom
writer-jill:ldap

Conversation

@writer-jill
Copy link
Copy Markdown
Contributor

@writer-jill writer-jill commented Oct 20, 2022

Update the LDAP configuration instructions and add instructions for LDAPS.
Remove duplicated information on other pages, instead link back to the main LDAP page.
Add description of LDAPS properties to the security overview doc and improve general formatting in this doc to remove unwieldy tables. Note that the aim of this PR is not to improve or modify the information in the security overview doc as a whole.
LDAP/LDAPS config instructions have been tested and confirmed by @tijoparacka

This PR has:

Comment thread docs/development/extensions-core/druid-basic-security.md Outdated
Copy link
Copy Markdown
Contributor

@tijoparacka tijoparacka left a comment

Choose a reason for hiding this comment

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

Providing some review comments

Comment thread docs/development/extensions-core/druid-basic-security.md Outdated
Comment thread docs/development/extensions-core/druid-basic-security.md Outdated
Comment thread docs/development/extensions-core/druid-basic-security.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Copy link
Copy Markdown
Contributor

@techdocsmith techdocsmith left a comment

Choose a reason for hiding this comment

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

Great work @writer-jill . And thanks for the review @tijoparacka ! This topic is getting close. I've added a few additional comments, mostly minor. I think we may want to get some clarity around the certificate requirements for LDAPs.

Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
@writer-jill writer-jill marked this pull request as ready for review November 2, 2022 17:37
@tijoparacka
Copy link
Copy Markdown
Contributor

LGTM

@writer-jill writer-jill requested review from tijoparacka and removed request for techdocsmith November 10, 2022 11:19
Copy link
Copy Markdown
Contributor

@tijoparacka tijoparacka left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@techdocsmith techdocsmith left a comment

Choose a reason for hiding this comment

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

Suggested some minor changes, otherwise LGTM

Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
Comment thread docs/operations/auth-ldap.md Outdated
writer-jill and others added 4 commits November 21, 2022 11:45
Co-authored-by: Charles Smith <techdocsmith@gmail.com>
Co-authored-by: Charles Smith <techdocsmith@gmail.com>
Co-authored-by: Charles Smith <techdocsmith@gmail.com>
@writer-jill writer-jill mentioned this pull request Nov 29, 2022
@techdocsmith techdocsmith merged commit 5c520e0 into apache:master Nov 29, 2022
@techdocsmith techdocsmith deleted the ldap branch November 29, 2022 17:26
@clintropolis clintropolis added this to the 26.0 milestone Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants