[BUG] Bug fix for checksum validation for mapping metadata (#15885)

* Bug fix for checksum validation for mapping metadata Signed-off-by: Himshikha Gupta <[email protected]> (cherry picked from commit f8515c7) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Sep 10, 2024 · db26695 · db26695
1 parent 6729220
commit db26695
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 3 deletions.
diff --git a/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/IndexMetadata.java
@@ -1299,7 +1299,7 @@ public void writeVerifiableTo(BufferedChecksumStreamOutput out) throws IOExcepti
         out.writeByte(state.id());
         writeSettingsToStream(settings, out);
         out.writeVLongArray(primaryTerms);
-        out.writeMapValues(mappings, (stream, val) -> val.writeTo(stream));
+        out.writeMapValues(mappings, (stream, val) -> val.writeVerifiableTo((BufferedChecksumStreamOutput) stream));
         out.writeMapValues(aliases, (stream, val) -> val.writeTo(stream));
         out.writeMap(customData, StreamOutput::writeString, (stream, val) -> val.writeTo(stream));
         out.writeMap(
@@ -1314,6 +1314,44 @@ public void writeVerifiableTo(BufferedChecksumStreamOutput out) throws IOExcepti
         }
     }
 
+    @Override
+    public String toString() {
+        return new StringBuilder().append("IndexMetadata{routingNumShards=")
+            .append(routingNumShards)
+            .append(", index=")
+            .append(index)
+            .append(", version=")
+            .append(version)
+            .append(", state=")
+            .append(state)
+            .append(", settingsVersion=")
+            .append(settingsVersion)
+            .append(", mappingVersion=")
+            .append(mappingVersion)
+            .append(", aliasesVersion=")
+            .append(aliasesVersion)
+            .append(", primaryTerms=")
+            .append(Arrays.toString(primaryTerms))
+            .append(", aliases=")
+            .append(aliases)
+            .append(", settings=")
+            .append(settings)
+            .append(", mappings=")
+            .append(mappings)
+            .append(", customData=")
+            .append(customData)
+            .append(", inSyncAllocationIds=")
+            .append(inSyncAllocationIds)
+            .append(", rolloverInfos=")
+            .append(rolloverInfos)
+            .append(", isSystem=")
+            .append(isSystem)
+            .append(", context=")
+            .append(context)
+            .append("}")
+            .toString();
+    }
+
     public boolean isSystem() {
         return isSystem;
     }

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MappingMetadata.java b/server/src/main/java/org/opensearch/cluster/metadata/MappingMetadata.java
@@ -41,8 +41,10 @@
 import org.opensearch.common.xcontent.XContentFactory;
 import org.opensearch.common.xcontent.XContentHelper;
 import org.opensearch.core.common.bytes.BytesReference;
+import org.opensearch.core.common.io.stream.BufferedChecksumStreamOutput;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
+import org.opensearch.core.common.io.stream.VerifiableWriteable;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.index.mapper.DocumentMapper;
 import org.opensearch.index.mapper.MapperService;
@@ -61,7 +63,7 @@
  * @opensearch.api
  */
 @PublicApi(since = "1.0.0")
-public class MappingMetadata extends AbstractDiffable<MappingMetadata> {
+public class MappingMetadata extends AbstractDiffable<MappingMetadata> implements VerifiableWriteable {
     public static final MappingMetadata EMPTY_MAPPINGS = new MappingMetadata(MapperService.SINGLE_MAPPING_NAME, Collections.emptyMap());
 
     private final String type;
@@ -168,6 +170,13 @@ public void writeTo(StreamOutput out) throws IOException {
         }
     }
 
+    @Override
+    public void writeVerifiableTo(BufferedChecksumStreamOutput out) throws IOException {
+        out.writeString(type());
+        source().writeVerifiableTo(out);
+        out.writeBoolean(routingRequired);
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;

diff --git a/server/src/main/java/org/opensearch/common/compress/CompressedXContent.java b/server/src/main/java/org/opensearch/common/compress/CompressedXContent.java
@@ -38,6 +38,7 @@
 import org.opensearch.common.xcontent.XContentFactory;
 import org.opensearch.core.common.bytes.BytesArray;
 import org.opensearch.core.common.bytes.BytesReference;
+import org.opensearch.core.common.io.stream.BufferedChecksumStreamOutput;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.compress.Compressor;
@@ -169,6 +170,10 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeByteArray(bytes);
     }
 
+    public void writeVerifiableTo(BufferedChecksumStreamOutput out) throws IOException {
+        out.writeInt(crc32);
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;

diff --git a/server/src/test/java/org/opensearch/cluster/metadata/IndexMetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/IndexMetadataTests.java
@@ -199,7 +199,30 @@ public void testWriteVerifiableTo() throws IOException {
             ),
             randomNonNegativeLong()
         );
-
+        String mappings = "    {\n"
+            + "        \"_doc\": {\n"
+            + "            \"properties\": {\n"
+            + "                \"actiongroups\": {\n"
+            + "                    \"type\": \"text\",\n"
+            + "                    \"fields\": {\n"
+            + "                        \"keyword\": {\n"
+            + "                            \"type\": \"keyword\",\n"
+            + "                            \"ignore_above\": 256\n"
+            + "                        }\n"
+            + "                    }\n"
+            + "                },\n"
+            + "                \"allowlist\": {\n"
+            + "                    \"type\": \"text\",\n"
+            + "                    \"fields\": {\n"
+            + "                        \"keyword\": {\n"
+            + "                            \"type\": \"keyword\",\n"
+            + "                            \"ignore_above\": 256\n"
+            + "                        }\n"
+            + "                    }\n"
+            + "                }\n"
+            + "            }\n"
+            + "        }\n"
+            + "    }";
         IndexMetadata metadata1 = IndexMetadata.builder("foo")
             .settings(
                 Settings.builder()
@@ -220,11 +243,13 @@ public void testWriteVerifiableTo() throws IOException {
             .putRolloverInfo(info1)
             .putRolloverInfo(info2)
             .putInSyncAllocationIds(0, Set.of("1", "2", "3"))
+            .putMapping(mappings)
             .build();
 
         BytesStreamOutput out = new BytesStreamOutput();
         BufferedChecksumStreamOutput checksumOut = new BufferedChecksumStreamOutput(out);
         metadata1.writeVerifiableTo(checksumOut);
+        assertNotNull(metadata1.toString());
 
         IndexMetadata metadata2 = IndexMetadata.builder(metadata1.getIndex().getName())
             .settings(
@@ -246,6 +271,7 @@ public void testWriteVerifiableTo() throws IOException {
             .putRolloverInfo(info2)
             .putRolloverInfo(info1)
             .putInSyncAllocationIds(0, Set.of("3", "1", "2"))
+            .putMapping(mappings)
             .build();
 
         BytesStreamOutput out2 = new BytesStreamOutput();