From ee23968f0509fc8dc7ffb79b79341b2d4b3b199f Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 23 Jul 2019 08:43:09 +0100 Subject: [PATCH] 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. --- .../org/elasticsearch/env/NodeMetaData.java | 34 ++++++----- .../env/OverrideNodeVersionCommand.java | 3 +- .../env/OverrideNodeVersionCommandTests.java | 57 +++++++++++++++++++ 3 files changed, 80 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/env/NodeMetaData.java b/server/src/main/java/org/elasticsearch/env/NodeMetaData.java index f9deba8f6c3..ab621ec939d 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeMetaData.java +++ b/server/src/main/java/org/elasticsearch/env/NodeMetaData.java @@ -37,8 +37,8 @@ import java.util.Objects; */ 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 final class NodeMetaData { '}'; } - private static ObjectParser 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 final class NodeMetaData { } } - public static final MetaDataStateFormat FORMAT = new MetaDataStateFormat("node-") { + static class NodeMetaDataStateFormat extends MetaDataStateFormat { + + private ObjectParser 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 final class NodeMetaData { } @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 FORMAT = new NodeMetaDataStateFormat(false); } diff --git a/server/src/main/java/org/elasticsearch/env/OverrideNodeVersionCommand.java b/server/src/main/java/org/elasticsearch/env/OverrideNodeVersionCommand.java index a46e185a253..34c7e9599e0 100644 --- a/server/src/main/java/org/elasticsearch/env/OverrideNodeVersionCommand.java +++ b/server/src/main/java/org/elasticsearch/env/OverrideNodeVersionCommand.java @@ -73,7 +73,8 @@ public class OverrideNodeVersionCommand extends ElasticsearchNodeCommand { @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); } diff --git a/server/src/test/java/org/elasticsearch/env/OverrideNodeVersionCommandTests.java b/server/src/test/java/org/elasticsearch/env/OverrideNodeVersionCommandTests.java index 704617c7b5e..d8b86d3d51e 100644 --- a/server/src/test/java/org/elasticsearch/env/OverrideNodeVersionCommandTests.java +++ b/server/src/test/java/org/elasticsearch/env/OverrideNodeVersionCommandTests.java @@ -22,6 +22,9 @@ import org.elasticsearch.ElasticsearchException; 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; @@ -29,9 +32,12 @@ 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 class OverrideNodeVersionCommandTests extends ESTestCase { 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 FORMAT + = new MetaDataStateFormat(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"); + } + }; + } }