Skip to content

Remove escape hatch permitting incompatible builds #76513

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.

Sign up for GitHub

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 9 commits into from
Sep 3, 2021
Merged

Remove escape hatch permitting incompatible builds #76513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b39c3c2
Adding deprecation check for es.unsafely_permit_handshake_from_incomp…
masseyke Aug 13, 2021
a29a791
suppressing build error
masseyke Aug 13, 2021
2877d28
feedback from code review
masseyke Aug 26, 2021
0635ad5
fixing unit tests
masseyke Aug 26, 2021
298dc8f
avoiding disabling unit test security manager
masseyke Sep 1, 2021
05dc579
Merge branch '7.x' into feature/unsafe-handshake-deprecation
elasticmachine Sep 1, 2021
e05b172
merging 7.x
masseyke Sep 1, 2021
a001298
merging 7.x
masseyke Sep 2, 2021
3f7ef45
moving the System.getProperty() call to a function
masseyke Sep 2, 2021
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: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/transport/TransportService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public class TransportService extends AbstractLifecycleComponent

private static final Logger logger = LogManager.getLogger(TransportService.class);

private static final String PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY = "es.unsafely_permit_handshake_from_incompatible_builds";
public static final String PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY = "es.unsafely_permit_handshake_from_incompatible_builds";
private static final boolean PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS;

static {
Expand Down
5 changes: 5 additions & 0 deletions ...ugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.XPackSettings;

import java.util.Arrays;
Expand Down Expand Up @@ -97,6 +98,10 @@ private DeprecationChecks() {
NodeDeprecationChecks::checkImplicitlyDisabledSecurityOnBasicAndTrial,
NodeDeprecationChecks::checkSearchRemoteSettings,
NodeDeprecationChecks::checkMonitoringExporterPassword,
NodeDeprecationChecks::checkClusterRoutingAllocationIncludeRelocationsSetting,
(settings, pluginsAndModules, clusterState, licenseState) ->
NodeDeprecationChecks.checkNoPermitHandshakeFromIncompatibleBuilds(settings, pluginsAndModules, clusterState,
licenseState, () -> System.getProperty(TransportService.PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY)),
NodeDeprecationChecks::checkTransportClientProfilesFilterSetting,
NodeDeprecationChecks::checkDelayClusterStateRecoverySettings,
NodeDeprecationChecks::checkFixedAutoQueueSizeThreadpool,
Expand Down
25 changes: 25 additions & 0 deletions .../deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.threadpool.FixedExecutorBuilder;
import org.elasticsearch.transport.RemoteClusterService;
import org.elasticsearch.transport.SniffConnectionStrategy;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.core.XPackSettings;
import org.elasticsearch.xpack.core.security.SecurityField;
import org.elasticsearch.xpack.core.security.authc.RealmConfig;
Expand All @@ -51,6 +52,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.Supplier;
import java.util.stream.Collectors;

import static org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_INCLUDE_RELOCATIONS_SETTING;
Expand Down Expand Up @@ -666,6 +668,29 @@ static DeprecationIssue checkClusterRoutingAllocationIncludeRelocationsSetting(f
);
}

static DeprecationIssue checkNoPermitHandshakeFromIncompatibleBuilds(final Settings settings,
Copy link
Member

Choose a reason for hiding this comment

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

I was thinking that we pass in the System::getProperty method reference or () -> System.getProperty("......") to this function, and call the function to obtain the setting value

final PluginsAndModules pluginsAndModules,
final ClusterState clusterState,
final XPackLicenseState licenseState,
Supplier<String> permitsHandshakesFromIncompatibleBuildsSupplier) {
if (permitsHandshakesFromIncompatibleBuildsSupplier.get() != null) {
final String message = String.format(
Locale.ROOT,
"the [%s] system property is deprecated and will be removed in the next major release",
TransportService.PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY
);
final String details = String.format(
Locale.ROOT,
"allowing handshakes from incompatibile builds is deprecated and will be removed in the next major release; the [%s] " +
"system property must be removed",
TransportService.PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY
);
String url = "https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_transport_changes";
return new DeprecationIssue(DeprecationIssue.Level.CRITICAL, message, url, details, false, null);
}
return null;
}

static DeprecationIssue checkTransportClientProfilesFilterSetting(
final Settings settings,
final PluginsAndModules pluginsAndModules,
Expand Down
22 changes: 22 additions & 0 deletions ...ecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.core.Set;
import org.elasticsearch.core.SuppressForbidden;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.NodeEnvironment;
import org.elasticsearch.gateway.GatewayService;
Expand Down Expand Up @@ -962,6 +963,27 @@ public void testImplicitlyConfiguredSecurityOnGoldPlus() {
assertThat(issues, empty());
}

@SuppressForbidden(reason = "sets and unsets es.unsafely_permit_handshake_from_incompatible_builds")
public void testCheckNoPermitHandshakeFromIncompatibleBuilds() {
final DeprecationIssue expectedNullIssue =
NodeDeprecationChecks.checkNoPermitHandshakeFromIncompatibleBuilds(Settings.EMPTY,
null,
ClusterState.EMPTY_STATE,
new XPackLicenseState(Settings.EMPTY, () -> 0),
() -> null);
assertEquals(null, expectedNullIssue);
final DeprecationIssue issue =
NodeDeprecationChecks.checkNoPermitHandshakeFromIncompatibleBuilds(Settings.EMPTY,
null,
ClusterState.EMPTY_STATE,
new XPackLicenseState(Settings.EMPTY, () -> 0),
() -> randomAlphaOfLengthBetween(1, 10));
assertNotNull(issue.getDetails());
assertThat(issue.getDetails(), containsString("system property must be removed"));
assertThat(issue.getUrl(),
equalTo("https://www.elastic.co/guide/en/elasticsearch/reference/master/migrating-8.0.html#breaking_80_transport_changes"));
}

public void testCheckTransportClientProfilesFilterSetting() {
final int numProfiles = randomIntBetween(1, 3);
final String[] profileNames = new String[numProfiles];
Expand Down