Adding checks to EngineConfig and fixing tests

Signed-off-by: Prabhakar Sithanandam <backslasht@users.noreply.github.com>
sarthakaggarwal97 · Sep 1, 2023 · 54de39b · 54de39b
1 parent 32b4b10
commit 54de39b
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 13 deletions.
diff --git a/...custom-codecs/src/test/java/org/opensearch/index/codec/customcodecs/CustomCodecTests.java b/...custom-codecs/src/test/java/org/opensearch/index/codec/customcodecs/CustomCodecTests.java
@@ -178,7 +178,7 @@ private void assertStoredFieldsCompressionEquals(Lucene95CustomCodec.Mode expect
     private CodecService createCodecService(boolean isMapperServiceNull) throws IOException {
         Settings nodeSettings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()).build();
         if (isMapperServiceNull) {
-            return new CodecService(null, IndexSettingsModule.newIndexSettings("_na", nodeSettings), LogManager.getLogger("test"));
+            return new CustomCodecService(null, IndexSettingsModule.newIndexSettings("_na", nodeSettings), LogManager.getLogger("test"));
         }
         return buildCodecService(nodeSettings);
     }

diff --git a/server/src/internalClusterTest/java/org/opensearch/index/codec/ZstdNotEnabledIT.java b/server/src/internalClusterTest/java/org/opensearch/index/codec/ZstdNotEnabledIT.java
@@ -0,0 +1,38 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.index.codec;
+
+import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.common.settings.Settings;
+import org.opensearch.test.OpenSearchIntegTestCase;
+
+@OpenSearchIntegTestCase.ClusterScope(scope = OpenSearchIntegTestCase.Scope.TEST)
+public class ZstdNotEnabledIT extends OpenSearchIntegTestCase  {
+
+    public void testZStdCodecsWithoutPluginInstalled() {
+
+        internalCluster().startNode();
+        final String index = "test-index";
+
+        // creating index with zstd or zstd_no_dict should fail if custom-codecs plugin is not installed
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> createIndex(
+                index,
+                Settings.builder()
+                    .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
+                    .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
+                    .put("index.codec", randomFrom("zstd", "zstd_no_dict"))
+                    .build()
+            )
+        );
+    }
+
+
+}
diff --git a/server/src/main/java/org/opensearch/index/engine/EngineConfig.java b/server/src/main/java/org/opensearch/index/engine/EngineConfig.java
@@ -135,13 +135,11 @@ public Supplier<RetentionLeases> retentionLeasesSupplier() {
             case "zlib":
             case "lucene_default":
                 return s;
-            case "zstd":
-            case "zstd_no_dict":
-                if (Codec.availableCodecs().contains("Lucene95CustomCodec")) {
-                    return s;
-                }
             default:
-                if (("zstd".equals(s) || "zstd_no_dict".equals(s)) && Codec.availableCodecs().contains("Lucene95CustomCodec")) {
+                // Though the external visible codec name is zstd or zstd_no_dict, internally it is registered as Lucene95CustomCodec
+                // Hence this check is required, Lucene95CustomCodec will not be part of availableCodecs if the custom-codecs plugin
+                // is not installed
+                if (("zstd".equals(s) || "zstd_no_dict".equals(s)) && Codec.availableCodecs().contains("Lucene95CustomCodec")){
                     return s;
                 }
                 if (Codec.availableCodecs().contains(s) == false) { // we don't error message the not officially supported ones
@@ -193,6 +191,9 @@ private static void doValidateCodecSettings(final String codec) {
             case "lz4":
                 break;
             default:
+                // Though the external visible codec name is zstd or zstd_no_dict, internally it is registered as Lucene95CustomCodec
+                // Hence this check is required, Lucene95CustomCodec will not be part of availableCodecs if the custom-codecs plugin
+                // is not installed
                 if (("zstd".equals(codec) || "zstd_no_dict".equals(codec)) && Codec.availableCodecs().contains("Lucene95CustomCodec")) {
                     return;
                 }
@@ -243,6 +244,7 @@ private EngineConfig(Builder builder) {
         this.codecService = builder.codecService;
         this.eventListener = builder.eventListener;
         codecName = builder.indexSettings.getValue(INDEX_CODEC_SETTING);
+
         // We need to make the indexing buffer for this shard at least as large
         // as the amount of memory that is available for all engines on the
         // local node so that decisions to flush segments to disk are made by

diff --git a/server/src/main/resources/org/opensearch/bootstrap/security.policy b/server/src/main/resources/org/opensearch/bootstrap/security.policy
@@ -79,12 +79,6 @@ grant codeBase "${codebase.jna}" {
   permission java.lang.RuntimePermission "accessDeclaredMembers";
 };
 
-
-// ZSTD compression
-grant codeBase "${codebase.zstd-jni}" {
-  permission java.lang.RuntimePermission "loadLibrary.*";
-};
-
 //// Everything else:
 
 grant {