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.

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

[DOCS] Add impact sections to security 8.0 breaking changes #56439

Merged
merged 4 commits into from
May 26, 2020
Merged

[DOCS] Add impact sections to security 8.0 breaking changes #56439

merged 4 commits into from
May 26, 2020

Conversation

jrodewig
Copy link
Contributor

@jrodewig jrodewig commented May 8, 2020

Adds impact sections to the 8.0 breaking changes for security.

Relates to #55629 and #53229

@tvernum This is intended largely as a formatting update. However, I want to ensure I didn't unintentionally lose any context or introduce errors here. Can you review at your convenience?

@jrodewig jrodewig added >docs General docs changes >non-issue :Security/Security Security issues without another label :Docs v8.0.0 labels May 8, 2020
@jrodewig jrodewig requested a review from tvernum May 8, 2020 15:46
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (:Docs)

@elasticmachine elasticmachine added the Team:Docs Meta label for docs team label May 8, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Security)

@elasticmachine elasticmachine added the Team:Security Meta label for security team label May 8, 2020
@jrodewig jrodewig removed the :Docs label May 20, 2020
If using `xpack.security.transport.ssl` settings, ensure
`xpack.security.transport.ssl.enabled` is set as `true`. Not specifying
`xpack.security.transport.ssl.enabled` as `true` in `elasticsearch.yml` will
result in an error on startup.
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't quite right. You are fee to set xpack.security.transport.ssl.enabled to true or false, you just can't leave it unspecified if you have set other any xpack.security.transport.ssl.* settings.

This setting defaults to false if unspecified. The 7.x (and earlier) behaviour is that nodes that had a xpack.security.transport.ssl setting (e.g. xpack.security.transport.ssl.certificate) but did not explicitly set ssl.enabled, would not have SSL enabled.
That was confusing because on first inspection it would look like SSL was configured, but since it was not explicitly enabled, it didn't actually do anything.

For people in that situation, the no impact upgrade would be to explicitly set ssl.enabled to false to acknowledge that even though they've configured SSL they don't want it to be on.

The "right fix" is for them to work out why they have ssl.* settings in their yml but don't have SSL enabled. Is that intentional?

  • If so, remove the ssl.* settings or set ssl.enabled to false.
  • Otherwise, maybe they want to turn on ssl (ssl.enabled: true), but that's a bigger change & we can't really walk them through it here (but we could point to the relevant docs).

I don't really know how to explain that well in the breaking changes doc. How much detail do we want to go to?
But I don't think we can recommend people just set it to true because that would cause a noticeable change to their cluster - it would turn on SSL when it was previously disabled - and they need to at least think through how they want to coordinate that change across their nodes (for transport.ssl) or with clients (for http.ssl)

`xpack.security.http.ssl.enabled` is set as `true`. Not specifying
`xpack.security.transport.ssl.enabled` as `true` in `elasticsearch.yml` will
result in an error on startup.

Copy link
Contributor

Choose a reason for hiding this comment

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

The comment above for transport.ssl.enabled applies here too.

docs/reference/migration/migrate_8_0/security.asciidoc Outdated Show resolved Hide resolved
@jrodewig
Copy link
Contributor Author

Thanks for the corrections and explanation @tvernum. I've updated the changes to address.

@jrodewig jrodewig requested a review from tvernum May 21, 2020 13:10
Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks.

@jrodewig jrodewig merged commit b79629d into elastic:master May 26, 2020
@jrodewig jrodewig deleted the docs__add-impact-sections-sec-breaking-changes branch May 26, 2020 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes >non-issue :Security/Security Security issues without another label Team:Docs Meta label for docs team Team:Security Meta label for security team v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants