Reject null customs at build time (#41782)

Today you can add a null `Custom` to the cluster state or its metadata, but
attempting to publish such a cluster state will fail. Unfortunately, the
publication-time failure gives very little information about the source of the
problem. This change causes the failure to manifest earlier and adds
information about which `Custom` was null in order to simplify the
investigation.

Relates #41090.

server/src/main/java/org/elasticsearch/cluster/ClusterState.java

@@ -66,8 +66,10 @@
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.stream.StreamSupport;
 
 import static org.elasticsearch.cluster.coordination.Coordinator.ZEN1_BWC_TERM;
 
@@ -736,7 +738,7 @@ public Builder minimumMasterNodesOnPublishingMaster(int minimumMasterNodesOnPubl
         }
 
         public Builder putCustom(String type, Custom custom) {
-            customs.put(type, custom);
+            customs.put(type, Objects.requireNonNull(custom, type));
             return this;
         }
 
@@ -746,6 +748,7 @@ public Builder removeCustom(String type) {
         }
 
         public Builder customs(ImmutableOpenMap<String, Custom> customs) {
+            StreamSupport.stream(customs.spliterator(), false).forEach(cursor -> Objects.requireNonNull(cursor.value, cursor.key));
             this.customs.putAll(customs);
             return this;
         }

server/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java

@@ -74,11 +74,13 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import java.util.function.Function;
 import java.util.function.Predicate;
+import java.util.stream.StreamSupport;
 
 import static org.elasticsearch.common.settings.Settings.readSettingsFromStream;
 import static org.elasticsearch.common.settings.Settings.writeSettingsToStream;
@@ -1069,7 +1071,7 @@ public Custom getCustom(String type) {
         }
 
         public Builder putCustom(String type, Custom custom) {
-            customs.put(type, custom);
+            customs.put(type, Objects.requireNonNull(custom, type));
             return this;
         }
 
@@ -1079,6 +1081,7 @@ public Builder removeCustom(String type) {
         }
 
         public Builder customs(ImmutableOpenMap<String, Custom> customs) {
+            StreamSupport.stream(customs.spliterator(), false).forEach(cursor -> Objects.requireNonNull(cursor.value, cursor.key));
             this.customs.putAll(customs);
             return this;
         }

server/src/test/java/org/elasticsearch/cluster/ClusterStateTests.java

@@ -21,11 +21,13 @@
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
+import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.ESTestCase;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 
 public class ClusterStateTests extends ESTestCase {
@@ -56,4 +58,19 @@ public void testSupersedes() {
         // state from the same master compare by version
         assertThat(withMaster1a.supersedes(withMaster1b), equalTo(withMaster1a.version() > withMaster1b.version()));
     }
+
+    public void testBuilderRejectsNullCustom() {
+        final ClusterState.Builder builder = ClusterState.builder(ClusterName.DEFAULT);
+        final String key = randomAlphaOfLength(10);
+        assertThat(expectThrows(NullPointerException.class, () -> builder.putCustom(key, null)).getMessage(), containsString(key));
+    }
+
+    public void testBuilderRejectsNullInCustoms() {
+        final ClusterState.Builder builder = ClusterState.builder(ClusterName.DEFAULT);
+        final String key = randomAlphaOfLength(10);
+        final ImmutableOpenMap.Builder<String, ClusterState.Custom> mapBuilder = ImmutableOpenMap.builder();
+        mapBuilder.put(key, null);
+        final ImmutableOpenMap<String, ClusterState.Custom> map = mapBuilder.build();
+        assertThat(expectThrows(NullPointerException.class, () -> builder.customs(map)).getMessage(), containsString(key));
+    }
 }

server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataTests.java

@@ -50,6 +50,7 @@
 import java.util.Map;
 import java.util.Set;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.startsWith;
@@ -891,4 +892,19 @@ public void testTransientSettingsOverridePersistentSettings() {
             .transientSettings(Settings.builder().put(setting.getKey(), "transient-value").build()).build();
         assertThat(setting.get(metaData.settings()), equalTo("transient-value"));
     }
+
+    public void testBuilderRejectsNullCustom() {
+        final MetaData.Builder builder = MetaData.builder();
+        final String key = randomAlphaOfLength(10);
+        assertThat(expectThrows(NullPointerException.class, () -> builder.putCustom(key, null)).getMessage(), containsString(key));
+    }
+
+    public void testBuilderRejectsNullInCustoms() {
+        final MetaData.Builder builder = MetaData.builder();
+        final String key = randomAlphaOfLength(10);
+        final ImmutableOpenMap.Builder<String, MetaData.Custom> mapBuilder = ImmutableOpenMap.builder();
+        mapBuilder.put(key, null);
+        final ImmutableOpenMap<String, MetaData.Custom> map = mapBuilder.build();
+        assertThat(expectThrows(NullPointerException.class, () -> builder.customs(map)).getMessage(), containsString(key));
+    }
 }