Browse Source

Remove escape hatch permitting incompatible builds (#65753)

Today in `7.x` there is a deprecated system property that bypasses the
check that prevents nodes of incompatible builds from communicating.
This commit removes the system property in `master` so that the check is
always enforced.

Relates #65601, #65249
David Turner 4 years ago
parent
commit
ff5cb90cc9

+ 18 - 0
docs/reference/migration/migrate_8_0/transport.asciidoc

@@ -27,3 +27,21 @@ on startup.
 ====
 
 // end::notable-breaking-changes[]
+
+.The `es.unsafely_permit_handshake_from_incompatible_builds` system property has been removed.
+[%collapsible]
+====
+*Details* +
+{es} has a check that verifies that communicating pairs of nodes of the same
+version are running exactly the same build and therefore using the same wire
+format as each other. In previous versions this check can be bypassed by
+setting the system property
+`es.unsafely_permit_handshake_from_incompatible_builds` to `true`. The use of
+this system property is now forbidden.
+
+*Impact* +
+Discontinue use of the `es.unsafely_permit_handshake_from_incompatible_builds`
+system property, and ensure that all nodes of the same version are running
+exactly the same build. Setting this system property will result in an error
+on startup.
+====

+ 12 - 34
server/src/main/java/org/elasticsearch/transport/TransportService.java

@@ -35,7 +35,6 @@ import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.lease.Releasable;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.ClusterSettings;
@@ -75,22 +74,6 @@ 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";
-    private static final boolean PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS;
-
-    static {
-        final String value = System.getProperty(PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY);
-        if (value == null) {
-            PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS = false;
-        } else if (Boolean.parseBoolean(value)) {
-            PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS = true;
-        } else {
-            throw new IllegalArgumentException("invalid value [" + value + "] for system property ["
-                    + PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY + "]");
-        }
-    }
-
-
     public static final String DIRECT_RESPONSE_PROFILE = ".direct";
     public static final String HANDSHAKE_ACTION_NAME = "internal:transport/handshake";
 
@@ -202,13 +185,6 @@ public class TransportService extends AbstractLifecycleComponent
             HandshakeRequest::new,
             (request, channel, task) -> channel.sendResponse(
                 new HandshakeResponse(localNode.getVersion(), Build.CURRENT.hash(), localNode, clusterName)));
-
-        if (PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS) {
-            logger.warn("transport handshakes from incompatible builds are unsafely permitted on this node; remove system property [" +
-                    PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY + "] to resolve this warning");
-            DeprecationLogger.getLogger(TransportService.class).deprecate("permit_handshake_from_incompatible_builds",
-                "system property [" + PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY + "] is deprecated and should be removed");
-        }
     }
 
     public RemoteClusterService getRemoteClusterService() {
@@ -528,16 +504,9 @@ public class TransportService extends AbstractLifecycleComponent
             }
 
             if (isIncompatibleBuild(version, buildHash)) {
-                if (PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS) {
-                    logger.warn("remote node [{}] is build [{}] of version [{}] but this node is build [{}] of version [{}] " +
-                                    "which may not be compatible; remove system property [{}] to resolve this warning",
-                            discoveryNode, buildHash, version, Build.CURRENT.hash(), Version.CURRENT,
-                            PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY);
-                } else {
-                    throw new IllegalArgumentException("remote node [" + discoveryNode + "] is build [" + buildHash +
-                            "] of version [" + version + "] but this node is build [" + Build.CURRENT.hash() +
-                            "] of version [" + Version.CURRENT + "] which has an incompatible wire format");
-                }
+                throw new IllegalArgumentException("remote node [" + discoveryNode + "] is build [" + buildHash +
+                        "] of version [" + version + "] but this node is build [" + Build.CURRENT.hash() +
+                        "] of version [" + Version.CURRENT + "] which has an incompatible wire format");
             }
 
             clusterName = new ClusterName(in);
@@ -1370,4 +1339,13 @@ public class TransportService extends AbstractLifecycleComponent
         }
     }
 
+    static {
+        // Ensure that this property, introduced and immediately deprecated in 7.11, is not used in 8.x
+        final String PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY = "es.unsafely_permit_handshake_from_incompatible_builds";
+        if (System.getProperty(PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY) != null) {
+            throw new IllegalArgumentException("system property [" + PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY + "] must not be set");
+        }
+        assert Version.CURRENT.major == Version.V_7_0_0.major + 1; // we can remove this whole block in v9
+    }
+
 }