Skip to content

[DOCS] Standardize docs for url setting #41117

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.

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

Merged
merged 2 commits into from
Apr 24, 2019
Merged

[DOCS] Standardize docs for url setting #41117

merged 2 commits into from
Apr 24, 2019

Conversation

jrodewig
Copy link
Contributor

  • Updates url Active Directory realm setting to indicate you can provide multiple URLs
  • Standardizes wording for url setting in LDAP and Active Directory realms

Resolves #32830

@jrodewig jrodewig added >docs General docs changes :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) labels Apr 11, 2019
@jrodewig jrodewig requested review from tvernum and lcawl April 11, 2019 13:31
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

To provide a single URL, use the `ldap` protocol: `ldap://server1:636`. To
provide multiple URLs, use the `ldaps` protocol with an array syntax:
`["ldaps://server1:636", "ldaps://server2:636" ]`. You can't mix the `ldap` and
`ldaps` URL protocols.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect, though it's a logical interpretation of the old text (hence the need to update it)

Ignoring, for a moment, what we want the docs to say, the rules are:

  • you can specific ldap or ldaps protocols for a single URL.
  • you can specific ldap or ldaps protocols when providing multiple URLs, but you must be consistent across all URLs. That is, they must all be ldap or must all be ldaps.
  • if providing multiple URLs you can either:
    1. use YAML array syntax url: ["ldaps://server1:636", "ldaps://server2:636" ]; or
    2. use a comma separated string: url: "ldaps://server1:636,ldaps://server2:636"

I struggle to find the sweet spot between documenting every possible option, and overwhelming the reader with too much information, vs just documenting a common set of option and have users get confused about why a certain behaviour is not documented.

Which is to say, I'm not sure whether it would be better to document all of the rules that I've written above, or simplify to it.

To provide a single URL, use the `ldap` protocol: `ldap://server1:389`. To
provide multiple URLs, use the `ldaps` protocol with an array syntax:
`["ldaps://server1:389", "ldaps://server2:389" ]`. You can't mix the `ldap` and
`ldaps` URL protocols.
Copy link
Contributor

Choose a reason for hiding this comment

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

As per above - whatever we decide to do there should be included here.

@jrodewig
Copy link
Contributor Author

jrodewig commented Apr 12, 2019

Thanks for the feedback and corrections @tvernum.

I revised the definitions with ef9d7d7. To help balance simplicity v. documenting everything, I tried to create an information hierarchy using paragraphs.

All important and common information should be captured in the first paragraph. More advanced information and options are presented further on.

Please let me know if this doesn't accurately reflect the rules you outlined. Also let me know if you feel this should be simplified more.

To provide multiple URLs, use a YAML array (`["ldap://server1:636", "ldap://server2:636"]`)
or comma-separated string (`"ldap://server1:636, ldap://server2:636"`).
+
While both are supported, you can't mix the `ldap` and `ldaps` protocols.
Copy link
Contributor

Choose a reason for hiding this comment

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

The above is correct.

I do wonder whether the last sentence is too colloquial for non-native speakers, but I'll defer to the the docs team on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @tvernum. @debadair @lcawl let me know what you feel about the sentence @tvernum is referencing here:

While both are supported, you can't mix the ldapandldaps protocols.

I think we're within style bounds here, but I'd appreciate any feedback. Thanks as always!

@jrodewig jrodewig requested a review from debadair April 15, 2019 19:59
@jrodewig jrodewig merged commit 02ef53c into elastic:master Apr 24, 2019
@jrodewig jrodewig deleted the document-active-dir-url-as-list branch April 24, 2019 16:18
jrodewig added a commit that referenced this pull request Apr 24, 2019
jrodewig added a commit that referenced this pull request Apr 24, 2019
jrodewig added a commit that referenced this pull request Apr 24, 2019
jrodewig added a commit that referenced this pull request Apr 24, 2019
jrodewig added a commit that referenced this pull request Apr 24, 2019
akhil10x5 pushed a commit to akhil10x5/elasticsearch that referenced this pull request May 2, 2019
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v7.0.1 v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document that AD url is a list
4 participants