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

Deprecate security implicitly disabled on trial/basic #72339

Merged
merged 16 commits into from
Jul 15, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/reference/migration/index.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ For more information about {minor-version},
see the <<release-highlights>> and <<es-release-notes>>.
For information about how to upgrade your cluster, see <<setup-upgrade>>.

* <<breaking-changes-7.14,Migrating to 7.14>>
* <<breaking-changes-7.13,Migrating to 7.13>>
* <<breaking-changes-7.12,Migrating to 7.12>>
* <<breaking-changes-7.11,Migrating to 7.11>>
Expand All @@ -45,6 +46,7 @@ For information about how to upgrade your cluster, see <<setup-upgrade>>.

--

include::migrate_7_14.asciidoc[]
include::migrate_7_13.asciidoc[]
include::migrate_7_12.asciidoc[]
include::migrate_7_11.asciidoc[]
Expand Down
46 changes: 46 additions & 0 deletions docs/reference/migration/migrate_7_14.asciidoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
[[migrating-7.14]]
== Migrating to 7.14
++++
<titleabbrev>7.14</titleabbrev>
++++

This section discusses the changes that you need to be aware of when migrating
your application to {es} 7.14.

See also <<release-highlights>> and <<es-release-notes>>.

// * <<breaking_714_blah_changes>>
// * <<breaking_714_blah_changes>>

//NOTE: The notable-breaking-changes tagged regions are re-used in the
//Installation and Upgrade Guide

//tag::notable-breaking-changes[]

[discrete]
[[breaking-changes-7.14]]
=== Breaking changes

The following changes in {es} 7.14 might affect your applications
and prevent them from operating normally.
Before upgrading to 7.14, review these changes and take the described steps
to mitigate the impact.

NOTE: Breaking changes introduced in minor versions are
normally limited to security and bug fixes.
Significant changes in behavior are deprecated in a minor release and
the old behavior is supported until the next major release.
To find out if you are using any deprecated functionality,
enable <<deprecation-logging, deprecation logging>>.

[discrete]
[[breaking_714_security_changes]]
==== Security deprecations

[[implicitly-disabled-security]]
Currently, security features are disabled when operating on a basic or trial
license when `xpack.security.enabled` has not been explicitly set to `true`.

This behavior is now deprecated. In version 8.0.0, security features will be
enabled by default for all licenses, unless explicitly disabled (by setting
`xpack.security.enabled` to `false`).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Currently, security features are disabled when operating on a basic or trial
license when `xpack.security.enabled` has not been explicitly set to `true`.
This behavior is now deprecated. In version 8.0.0, security features will be
enabled by default for all licenses, unless explicitly disabled (by setting
`xpack.security.enabled` to `false`).
.Security features on basic or trial license aren't disabled by default
[%collapsible]
====
*Details* +
Currently, security features are disabled when operating on a basic or trial
license when `xpack.security.enabled` has not been explicitly set to `true`.
This behavior is now deprecated. In version 8.0.0, security features will be
enabled by default for all licenses, unless explicitly disabled (by setting
`xpack.security.enabled` to `false`).
====

Copy link
Contributor

Choose a reason for hiding this comment

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

This collapsible format isn't required, but I think it's nice to separate the text from the main header, especially when linking to this specific section. See the Mapping changes section in 7.13 for an example of how this formatting renders.

Copy link
Member Author

Choose a reason for hiding this comment

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

I like this but the title might confuse folks as something we're doing in 7.14( if they don't click to expand the collapsible ). How does "The default behavior of disabling security on basic and trial licenses is deprecated" sound as a title instead @lockewritesdocs ?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 works for me @jkakavas

Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.deprecation.DeprecationInfoAction;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
Expand All @@ -19,7 +20,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -42,13 +42,14 @@ private DeprecationChecks() {
));


static final List<BiFunction<Settings, PluginsAndModules, DeprecationIssue>> NODE_SETTINGS_CHECKS;
static final List<TriFunction<Settings, PluginsAndModules, XPackLicenseState, DeprecationIssue>> NODE_SETTINGS_CHECKS;

static {
final Stream<BiFunction<Settings, PluginsAndModules, DeprecationIssue>> legacyRoleSettings = DiscoveryNode.getPossibleRoles()
final Stream<TriFunction<Settings, PluginsAndModules, XPackLicenseState, DeprecationIssue>> legacyRoleSettings =
DiscoveryNode.getPossibleRoles()
.stream()
.filter(r -> r.legacySetting() != null)
.map(r -> (s, p) -> NodeDeprecationChecks.checkLegacyRoleSettings(r.legacySetting(), s, p));
.map(r -> (s, p, t) -> NodeDeprecationChecks.checkLegacyRoleSettings(r.legacySetting(), s, p));
NODE_SETTINGS_CHECKS = Stream.concat(
legacyRoleSettings,
Stream.of(
Expand All @@ -58,33 +59,34 @@ private DeprecationChecks() {
NodeDeprecationChecks::checkMissingRealmOrders,
NodeDeprecationChecks::checkUniqueRealmOrders,
NodeDeprecationChecks::checkImplicitlyDisabledBasicRealms,
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkThreadPoolListenerQueueSize(settings),
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkThreadPoolListenerSize(settings),
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkThreadPoolListenerQueueSize(settings),
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkThreadPoolListenerSize(settings),
NodeDeprecationChecks::checkClusterRemoteConnectSetting,
NodeDeprecationChecks::checkNodeLocalStorageSetting,
NodeDeprecationChecks::checkGeneralScriptSizeSetting,
NodeDeprecationChecks::checkGeneralScriptExpireSetting,
NodeDeprecationChecks::checkGeneralScriptCompileSettings,
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
XPackSettings.ENRICH_ENABLED_SETTING),
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
XPackSettings.FLATTENED_ENABLED),
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
XPackSettings.INDEX_LIFECYCLE_ENABLED),
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
XPackSettings.MONITORING_ENABLED),
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
XPackSettings.ROLLUP_ENABLED),
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
XPackSettings.SNAPSHOT_LIFECYCLE_ENABLED),
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
XPackSettings.SQL_ENABLED),
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
XPackSettings.TRANSFORM_ENABLED),
(settings, pluginsAndModules) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
(settings, pluginsAndModules, state) -> NodeDeprecationChecks.checkNodeBasicLicenseFeatureEnabledSetting(settings,
XPackSettings.VECTORS_ENABLED),
NodeDeprecationChecks::checkMultipleDataPaths,
NodeDeprecationChecks::checkDataPathsList
NodeDeprecationChecks::checkDataPathsList,
NodeDeprecationChecks::checkImplicitlyDisabledSecurityOnBasicAndTrial
)
).collect(Collectors.toList());
}
Expand All @@ -105,10 +107,15 @@ private DeprecationChecks() {
*
* @param checks The functional checks to execute using the mapper function
* @param mapper The function that executes the lambda check with the appropriate arguments
* @param <T> The signature of the check (BiFunction, Function, including the appropriate arguments)
* @param <T> The signature of the check (TriFunction, BiFunction, Function, including the appropriate arguments)
* @return The list of {@link DeprecationIssue} that were found in the cluster
*/
static <T> List<DeprecationIssue> filterChecks(List<T> checks, Function<T, DeprecationIssue> mapper) {
return checks.stream().map(mapper).filter(Objects::nonNull).collect(Collectors.toList());
}

@FunctionalInterface
public interface TriFunction<F, S, T, R> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this interface is local to the class, I think it should have a more specific, meaningful name like NodeDeprecationCheck

R apply(F first, S second, T third);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

package org.elasticsearch.xpack.deprecation;

import org.elasticsearch.Version;
import org.elasticsearch.action.admin.cluster.node.info.PluginsAndModules;
import org.elasticsearch.bootstrap.JavaVersion;
import org.elasticsearch.cluster.node.DiscoveryNode;
Expand All @@ -18,11 +19,14 @@
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.env.Environment;
import org.elasticsearch.license.License;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.node.Node;
import org.elasticsearch.node.NodeRoleSettings;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.threadpool.FixedExecutorBuilder;
import org.elasticsearch.transport.RemoteClusterService;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
import org.elasticsearch.xpack.core.security.authc.RealmSettings;
Expand All @@ -40,7 +44,8 @@

class NodeDeprecationChecks {

static DeprecationIssue checkPidfile(final Settings settings, final PluginsAndModules pluginsAndModules) {
static DeprecationIssue checkPidfile(final Settings settings, final PluginsAndModules pluginsAndModules,
XPackLicenseState licenseState) {
return checkDeprecatedSetting(
settings,
pluginsAndModules,
Expand All @@ -49,7 +54,8 @@ static DeprecationIssue checkPidfile(final Settings settings, final PluginsAndMo
"https://www.elastic.co/guide/en/elasticsearch/reference/7.4/breaking-changes-7.4.html#deprecate-pidfile");
}

static DeprecationIssue checkProcessors(final Settings settings , final PluginsAndModules pluginsAndModules) {
static DeprecationIssue checkProcessors(final Settings settings , final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
return checkDeprecatedSetting(
settings,
pluginsAndModules,
Expand All @@ -58,7 +64,8 @@ static DeprecationIssue checkProcessors(final Settings settings , final PluginsA
"https://www.elastic.co/guide/en/elasticsearch/reference/7.4/breaking-changes-7.4.html#deprecate-processors");
}

static DeprecationIssue checkMissingRealmOrders(final Settings settings, final PluginsAndModules pluginsAndModules) {
static DeprecationIssue checkMissingRealmOrders(final Settings settings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
final Set<String> orderNotConfiguredRealms = RealmSettings.getRealmSettings(settings).entrySet()
.stream()
.filter(e -> false == e.getValue().hasValue(RealmSettings.ORDER_SETTING_KEY))
Expand All @@ -82,7 +89,8 @@ static DeprecationIssue checkMissingRealmOrders(final Settings settings, final P
);
}

static DeprecationIssue checkUniqueRealmOrders(final Settings settings, final PluginsAndModules pluginsAndModules) {
static DeprecationIssue checkUniqueRealmOrders(final Settings settings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
final Map<String, List<String>> orderToRealmSettings =
RealmSettings.getRealmSettings(settings).entrySet()
.stream()
Expand Down Expand Up @@ -115,7 +123,28 @@ static DeprecationIssue checkUniqueRealmOrders(final Settings settings, final Pl
);
}

static DeprecationIssue checkImplicitlyDisabledBasicRealms(final Settings settings, final PluginsAndModules pluginsAndModules) {
static DeprecationIssue checkImplicitlyDisabledSecurityOnBasicAndTrial(final Settings settings,
final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
if ( settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) == false
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to do:

Suggested change
if ( settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) == false
if ( XPackSettings.SECURITY_ENABLED.exists(settings) == false

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really. The former ends up doing a get() on a Map and checks for null, whereas the latter will do a contains on the Set of settings keys. I think this equivalent performance wise but I do agree that your suggestion is less verbose/cleaner

&& (licenseState.getOperationMode().equals(License.OperationMode.BASIC)
|| licenseState.getOperationMode().equals(License.OperationMode.TRIAL))) {
String details = "The behavior where the value of [xpack.security.enabled] setting is false for " +
licenseState.getOperationMode() + " licenses is deprecated and will be changed in a future version." +
"See https://www.elastic.co/guide/en/elasticsearch/reference/" + Version.CURRENT.major + "." +
Version.CURRENT.minor + "/security-minimal-setup.html to enable security, or explicitly disable security by " +
"setting [xpack.security.enabled] to false in elasticsearch.yml";
return new DeprecationIssue(
DeprecationIssue.Level.WARNING,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we decide WARNING is preferable to CRITICAL?

On the one hand, people who don't resolve this will have a terrible time during (or immediately after) upgrade.
On the other hand, for most people it will be just as easy to fix at/after upgrade as it is before upgrade (change the setting).

Copy link
Member Author

Choose a reason for hiding this comment

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

I can be definitely swayed. The way I read WARNING / CRITICAL is that the "Failures will occur unless" meant to convey that i.e. the node will fail to start or the cluster won't form etc. So I went with WARNING.

It will probably matter a lot how this is surfaced in the upgrade assistant though and if warning is something users are allowed to click through while upgrading, we should make this CRITICAL. Thank for raising this

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect we can revisit this during the final 8.0 upgrade preparations and make sure it's presenting well to the user.

"Security is enabled by default for all licenses in the next major version.",
"https://www.elastic.co/guide/en/elasticsearch/reference/7.14/deprecated-7.14.html#implicitly-disabled-security",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"https://www.elastic.co/guide/en/elasticsearch/reference/7.14/deprecated-7.14.html#implicitly-disabled-security",
"https://www.elastic.co/guide/en/elasticsearch/reference/7.14/migrating-7.14.html#implicitly-disabled-security",

Copy link
Contributor

Choose a reason for hiding this comment

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

details);
}
return null;
}

static DeprecationIssue checkImplicitlyDisabledBasicRealms(final Settings settings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
final Map<RealmConfig.RealmIdentifier, Settings> realmSettings = RealmSettings.getRealmSettings(settings);
if (realmSettings.isEmpty()) {
return null;
Expand Down Expand Up @@ -185,7 +214,8 @@ private static DeprecationIssue checkThreadPoolListenerSetting(final String name
"https://www.elastic.co/guide/en/elasticsearch/reference/7.x/breaking-changes-7.7.html#deprecate-listener-thread-pool");
}

public static DeprecationIssue checkClusterRemoteConnectSetting(final Settings settings, final PluginsAndModules pluginsAndModules) {
public static DeprecationIssue checkClusterRemoteConnectSetting(final Settings settings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
return checkDeprecatedSetting(
settings,
pluginsAndModules,
Expand All @@ -199,7 +229,8 @@ public static DeprecationIssue checkClusterRemoteConnectSetting(final Settings s
);
}

public static DeprecationIssue checkNodeLocalStorageSetting(final Settings settings, final PluginsAndModules pluginsAndModules) {
public static DeprecationIssue checkNodeLocalStorageSetting(final Settings settings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
return checkRemovedSetting(
settings,
Node.NODE_LOCAL_STORAGE_SETTING,
Expand All @@ -215,7 +246,8 @@ public static DeprecationIssue checkNodeBasicLicenseFeatureEnabledSetting(final
);
}

public static DeprecationIssue checkGeneralScriptSizeSetting(final Settings settings, final PluginsAndModules pluginsAndModules) {
public static DeprecationIssue checkGeneralScriptSizeSetting(final Settings settings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
return checkDeprecatedSetting(
settings,
pluginsAndModules,
Expand All @@ -226,7 +258,8 @@ public static DeprecationIssue checkGeneralScriptSizeSetting(final Settings sett
);
}

public static DeprecationIssue checkGeneralScriptExpireSetting(final Settings settings, final PluginsAndModules pluginsAndModules) {
public static DeprecationIssue checkGeneralScriptExpireSetting(final Settings settings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
return checkDeprecatedSetting(
settings,
pluginsAndModules,
Expand All @@ -237,7 +270,8 @@ public static DeprecationIssue checkGeneralScriptExpireSetting(final Settings se
);
}

public static DeprecationIssue checkGeneralScriptCompileSettings(final Settings settings, final PluginsAndModules pluginsAndModules) {
public static DeprecationIssue checkGeneralScriptCompileSettings(final Settings settings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
return checkDeprecatedSetting(
settings,
pluginsAndModules,
Expand Down Expand Up @@ -365,7 +399,7 @@ static DeprecationIssue checkRemovedSetting(final Settings settings, final Setti
return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, message, url, details);
}

static DeprecationIssue javaVersionCheck(Settings nodeSettings, PluginsAndModules plugins) {
static DeprecationIssue javaVersionCheck(Settings nodeSettings, PluginsAndModules plugins, final XPackLicenseState licenseState) {
final JavaVersion javaVersion = JavaVersion.current();

if (javaVersion.compareTo(JavaVersion.parse("11")) < 0) {
Expand All @@ -379,7 +413,8 @@ static DeprecationIssue javaVersionCheck(Settings nodeSettings, PluginsAndModule
return null;
}

static DeprecationIssue checkMultipleDataPaths(Settings nodeSettings, PluginsAndModules plugins) {
static DeprecationIssue checkMultipleDataPaths(final Settings nodeSettings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
List<String> dataPaths = Environment.PATH_DATA_SETTING.get(nodeSettings);
if (dataPaths.size() > 1) {
return new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
Expand All @@ -390,7 +425,8 @@ static DeprecationIssue checkMultipleDataPaths(Settings nodeSettings, PluginsAnd
return null;
}

static DeprecationIssue checkDataPathsList(Settings nodeSettings, PluginsAndModules plugins) {
static DeprecationIssue checkDataPathsList(final Settings nodeSettings, final PluginsAndModules pluginsAndModules,
final XPackLicenseState licenseState) {
if (Environment.dataPathUsesList(nodeSettings)) {
return new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
"[path.data] in a list is deprecated, use a string value",
Expand Down
Loading