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

Remove heuristics that enable security on trial licenses #38075

Merged
merged 5 commits into from
Feb 1, 2019

Conversation

tvernum
Copy link
Contributor

@tvernum tvernum commented Jan 31, 2019

In v6.3 trial licenses were changed to default to security
disabled, and we added some heuristics to detect when security should
be automatically be enabled if xpack.security.enabled was not set.

This change removes those heuristics, and requires that security be
explicitly enabled (via the xpack.security.enabled setting) for
trial licenses.

Relates: #38009

In 6.3 trial licenses were changed to default to security
disabled, and ee added some heuristics to detect when security should
be automatically be enabled if `xpack.security.enabled` was not set.

This change removes those heuristics, and requires that security be
explicitly enabled (via the `xpack.security.enabled` setting) for
trial licenses.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -271,25 +270,21 @@ private static boolean isBasic(OperationMode mode) {
private final boolean isSecurityExplicitlyEnabled;

private Status status = new Status(OperationMode.TRIAL, true);
private boolean isSecurityEnabledByTrialVersion;

public XPackLicenseState(Settings settings) {
this.listeners = new CopyOnWriteArrayList<>();
this.isSecurityEnabled = XPackSettings.SECURITY_ENABLED.get(settings);
// 6.0+ requires TLS for production licenses, so if TLS is enabled and security is enabled
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this comment

Copy link
Member

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

LGTM


In prior versions, a trial license would automatically enable security if either

* `xpack.security.transport.enabled` was `true`; _or_
Copy link
Member

Choose a reason for hiding this comment

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

is a semicolon syntactically needed here? or is it an asciidoc thing ? ( didn't find any references).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed, no.

Semicolons within lists, and semicolons before conjunctions are both acceptable, but a little bit outdated.
I only use them when (like this case) you have a multi-line list (bullet points, or numbers) and a conjunction that relates to the overall list. In this case the ; makes clear that the or is not part of the first point (it would be grammatically incorrect if it were, but may still confuse readers) but applies to the list itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't find any great references off hand, but here's an OK one: https://www.onlinegrammar.com.au/punctuation-in-lists/

Copy link
Member

Choose a reason for hiding this comment

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

aha! They don't teach you that stuff at English school :) (or they did and I have forgotten) - Thanks for the clarification

@tvernum tvernum merged commit 6fcbd07 into elastic:master Feb 1, 2019
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Feb 4, 2019
In 6.x security is implicitly enabled on a trial license if transport
SSL is enabled, or the trial is from pre-6.3.

This is no longer true on 7.0, so this behaviour is now deprecated.

Relates: elastic#38009, elastic#38075
tvernum added a commit that referenced this pull request Feb 5, 2019
In 6.x security is implicitly enabled on a trial license if transport
SSL is enabled, or the trial is from pre-6.3.

This is no longer true on 7.0, so this behaviour is now deprecated.

Relates: #38009, #38075
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants