Ignore unknown fields if overriding node metadata (#44689)

The `elasticsearch-node override-version` command fails if it cannot read the
existing node metadata file. However, it reads this file strictly and fails if
there are any unknown fields, which means it will not be useful if we add
another field in future.

This commit adds leniency to this command, allowing it to ignore any unknown
fields and proceed with the downgrade. A downgrade is already unsafe, and the
user is already copiously warned about this, so being lenient in this case does
not make things much worse.

Author: DaveCTurner

server/src/main/java/org/elasticsearch/env/NodeMetaData.java

@@ -37,8 +37,8 @@
  */
 public final class NodeMetaData {
 
-    private static final String NODE_ID_KEY = "node_id";
-    private static final String NODE_VERSION_KEY = "node_version";
+    static final String NODE_ID_KEY = "node_id";
+    static final String NODE_VERSION_KEY = "node_version";
 
     private final String nodeId;
 
@@ -71,13 +71,6 @@ public String toString() {
             '}';
     }
 
-    private static ObjectParser<Builder, Void> PARSER = new ObjectParser<>("node_meta_data", Builder::new);
-
-    static {
-        PARSER.declareString(Builder::setNodeId, new ParseField(NODE_ID_KEY));
-        PARSER.declareInt(Builder::setNodeVersionId, new ParseField(NODE_VERSION_KEY));
-    }
-
     public String nodeId() {
         return nodeId;
     }
@@ -130,7 +123,20 @@ public NodeMetaData build() {
         }
     }
 
-    public static final MetaDataStateFormat<NodeMetaData> FORMAT = new MetaDataStateFormat<NodeMetaData>("node-") {
+    static class NodeMetaDataStateFormat extends MetaDataStateFormat<NodeMetaData> {
+
+        private ObjectParser<Builder, Void> objectParser;
+
+        /**
+         * @param ignoreUnknownFields whether to ignore unknown fields or not. Normally we are strict about this, but
+         *                            {@link OverrideNodeVersionCommand} is lenient.
+         */
+        NodeMetaDataStateFormat(boolean ignoreUnknownFields) {
+            super("node-");
+            objectParser = new ObjectParser<>("node_meta_data", ignoreUnknownFields, Builder::new);
+            objectParser.declareString(Builder::setNodeId, new ParseField(NODE_ID_KEY));
+            objectParser.declareInt(Builder::setNodeVersionId, new ParseField(NODE_VERSION_KEY));
+        }
 
         @Override
         protected XContentBuilder newXContentBuilder(XContentType type, OutputStream stream) throws IOException {
@@ -146,8 +152,10 @@ public void toXContent(XContentBuilder builder, NodeMetaData nodeMetaData) throw
         }
 
         @Override
-        public NodeMetaData fromXContent(XContentParser parser) {
-            return PARSER.apply(parser, null).build();
+        public NodeMetaData fromXContent(XContentParser parser) throws IOException {
+            return objectParser.apply(parser, null).build();
         }
-    };
+    }
+
+    public static final MetaDataStateFormat<NodeMetaData> FORMAT = new NodeMetaDataStateFormat(false);
 }

server/src/main/java/org/elasticsearch/env/OverrideNodeVersionCommand.java

@@ -73,7 +73,8 @@ public OverrideNodeVersionCommand() {
     @Override
     protected void processNodePaths(Terminal terminal, Path[] dataPaths, Environment env) throws IOException {
         final Path[] nodePaths = Arrays.stream(toNodePaths(dataPaths)).map(p -> p.path).toArray(Path[]::new);
-        final NodeMetaData nodeMetaData = NodeMetaData.FORMAT.loadLatestState(logger, namedXContentRegistry, nodePaths);
+        final NodeMetaData nodeMetaData
+            = new NodeMetaData.NodeMetaDataStateFormat(true).loadLatestState(logger, namedXContentRegistry, nodePaths);
         if (nodeMetaData == null) {
             throw new ElasticsearchException(NO_METADATA_MESSAGE);
         }

server/src/test/java/org/elasticsearch/env/OverrideNodeVersionCommandTests.java

@@ -22,16 +22,22 @@
 import org.elasticsearch.Version;
 import org.elasticsearch.cli.MockTerminal;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.gateway.MetaDataStateFormat;
 import org.elasticsearch.gateway.WriteStateException;
 import org.elasticsearch.test.ESTestCase;
 import org.junit.Before;
 
 import java.io.IOException;
 import java.nio.file.Path;
 
+import static org.elasticsearch.env.NodeMetaData.NODE_ID_KEY;
+import static org.elasticsearch.env.NodeMetaData.NODE_VERSION_KEY;
 import static org.hamcrest.Matchers.allOf;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasToString;
 
 public class OverrideNodeVersionCommandTests extends ESTestCase {
 
@@ -152,4 +158,55 @@ public void testOverwritesIfTooNew() throws Exception {
         assertThat(nodeMetaData.nodeId(), equalTo(nodeId));
         assertThat(nodeMetaData.nodeVersion(), equalTo(Version.CURRENT));
     }
+
+    public void testLenientlyIgnoresExtraFields() throws Exception {
+        final String nodeId = randomAlphaOfLength(10);
+        final Version nodeVersion = NodeMetaDataTests.tooNewVersion();
+        FutureNodeMetaData.FORMAT.writeAndCleanup(new FutureNodeMetaData(nodeId, nodeVersion, randomLong()), nodePaths);
+        assertThat(expectThrows(ElasticsearchException.class,
+            () -> NodeMetaData.FORMAT.loadLatestState(logger, xContentRegistry(), nodePaths)),
+            hasToString(containsString("unknown field [future_field]")));
+
+        final MockTerminal mockTerminal = new MockTerminal();
+        mockTerminal.addTextInput(randomFrom("y", "Y"));
+        new OverrideNodeVersionCommand().processNodePaths(mockTerminal, nodePaths, environment);
+        assertThat(mockTerminal.getOutput(), allOf(
+            containsString("data loss"),
+            containsString("You should not use this tool"),
+            containsString(Version.CURRENT.toString()),
+            containsString(nodeVersion.toString()),
+            containsString(OverrideNodeVersionCommand.SUCCESS_MESSAGE)));
+        expectThrows(IllegalStateException.class, () -> mockTerminal.readText(""));
+
+        final NodeMetaData nodeMetaData = NodeMetaData.FORMAT.loadLatestState(logger, xContentRegistry(), nodePaths);
+        assertThat(nodeMetaData.nodeId(), equalTo(nodeId));
+        assertThat(nodeMetaData.nodeVersion(), equalTo(Version.CURRENT));
+    }
+
+    private static class FutureNodeMetaData {
+        private final String nodeId;
+        private final Version nodeVersion;
+        private final long futureValue;
+
+        FutureNodeMetaData(String nodeId, Version nodeVersion, long futureValue) {
+            this.nodeId = nodeId;
+            this.nodeVersion = nodeVersion;
+            this.futureValue = futureValue;
+        }
+
+        static final MetaDataStateFormat<FutureNodeMetaData> FORMAT
+            = new MetaDataStateFormat<FutureNodeMetaData>(NodeMetaData.FORMAT.getPrefix()) {
+            @Override
+            public void toXContent(XContentBuilder builder, FutureNodeMetaData state) throws IOException {
+                builder.field(NODE_ID_KEY, state.nodeId);
+                builder.field(NODE_VERSION_KEY, state.nodeVersion.id);
+                builder.field("future_field", state.futureValue);
+            }
+
+            @Override
+            public FutureNodeMetaData fromXContent(XContentParser parser) {
+                throw new AssertionError("shouldn't be loading a FutureNodeMetaData");
+            }
+        };
+    }
 }