Validate method parameters only after version 2_17_0

.github/workflows/CI.yml

@@ -30,8 +30,7 @@ on:
       - 'jni/**'
       - 'micro-benchmarks/**'
       - '.github/workflows/CI.yml'
-env:
-  ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true
+
 
 jobs:
   Get-CI-Image-Tag:
@@ -52,11 +51,13 @@ jobs:
       # this image tag is subject to change as more dependencies and updates will arrive over time
       image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }}
       # need to switch to root so that github actions can install runner binary on container without permission issues.
-      options: --user root
+      options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }}
 
     steps:
+      - name: Run start commands
+        run: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-command }}
       - name: Checkout k-NN
-        uses: actions/checkout@v1
+        uses: actions/checkout@v4
 
       # Setup git user so that patches for native libraries can be applied and committed
       - name: Setup git user
@@ -65,8 +66,9 @@ jobs:
           su `id -un 1000` -c 'git config --global user.email "github-actions[bot]@users.noreply.github.com"'
 
       - name: Setup Java ${{ matrix.java }}
-        uses: actions/setup-java@v1
+        uses: actions/setup-java@v4
         with:
+          distribution: "corretto"
           java-version: ${{ matrix.java }}
 
       - name: Run build
@@ -84,7 +86,7 @@ jobs:
 
 
       - name: Upload Coverage Report
-        uses: codecov/codecov-action@v1
+        uses: codecov/codecov-action@v4
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
 
@@ -99,7 +101,7 @@ jobs:
 
     steps:
       - name: Checkout k-NN
-        uses: actions/checkout@v1
+        uses: actions/checkout@v4
 
       # Setup git user so that patches for native libraries can be applied and committed
       - name: Setup git user

.github/workflows/test_security.yml

@@ -28,8 +28,6 @@ on:
       - 'gradle/**'
       - 'jni/**'
       - '.github/workflows/test_security.yml'
-env:
-  ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true
 
 jobs:
   Get-CI-Image-Tag:
@@ -50,21 +48,24 @@ jobs:
       # this image tag is subject to change as more dependencies and updates will arrive over time
       image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }}
       # need to switch to root so that github actions can install runner binary on container without permission issues.
-      options: --user root
+      options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }}
 
     steps:
+      - name: Run start commands
+        run: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-command }}
       - name: Checkout k-NN
-        uses: actions/checkout@v1
+        uses: actions/checkout@v4
       # Setup git user so that patches for native libraries can be applied and committed
       - name: Setup git user
         run: |
           su `id -un 1000` -c 'git config --global user.name "github-actions[bot]"'
           su `id -un 1000` -c 'git config --global user.email "github-actions[bot]@users.noreply.github.com"'
 
       - name: Setup Java ${{ matrix.java }}
-        uses: actions/setup-java@v1
+        uses: actions/setup-java@v4
         with:
           java-version: ${{ matrix.java }}
+          distribution: "corretto"
 
       - name: Run build
         # switching the user, as OpenSearch cluster can only be started as root/Administrator on linux-deb/linux-rpm/windows-zip.

src/main/java/org/opensearch/knn/index/mapper/KNNVectorFieldMapper.java

@@ -182,6 +182,8 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
         @Setter
         @Getter
         private OriginalMappingParameters originalParameters;
+        @Setter
+        private boolean isKnnIndex;
 
         public Builder(
             String name,
@@ -195,6 +197,7 @@ public Builder(
             this.indexCreatedVersion = indexCreatedVersion;
             this.knnMethodConfigContext = knnMethodConfigContext;
             this.originalParameters = originalParameters;
+            this.isKnnIndex = true;
         }
 
         @Override
@@ -250,7 +253,7 @@ public KNNVectorFieldMapper build(BuilderContext context) {
                 );
             }
 
-            if (originalParameters.getResolvedKnnMethodContext() == null) {
+            if (originalParameters.getResolvedKnnMethodContext() == null || isKnnIndex == false) {
                 return FlatVectorFieldMapper.createFieldMapper(
                     buildFullName(context),
                     name,
@@ -362,10 +365,14 @@ public Mapper.Builder<?> parse(String name, Map<String, Object> node, ParserCont
                     String.format(Locale.ROOT, "Method and model can not be both specified in the mapping: %s", name)
                 );
             }
-
             // Check for flat configuration
             if (isKNNDisabled(parserContext.getSettings())) {
-                validateFromFlat(builder);
+                builder.setKnnIndex(false);
+                if (parserContext.indexVersionCreated().onOrAfter(Version.V_2_17_0)) {
+                    // on and after 2_17_0 we validate to makes sure that mapping doesn't contain parameters that are
+                    // specific to approximate knn search algorithms
+                    validateFromFlat(builder);
+                }
             } else if (builder.modelId.get() != null) {
                 validateFromModel(builder);
             } else {

src/test/java/org/opensearch/knn/index/OpenSearchIT.java

@@ -483,4 +483,27 @@ public void testIndexingVectorValidation_updateVectorWithNull() throws Exception
         assertArrayEquals(vectorForDocumentOne, vectorRestoreInitialValue);
     }
 
+    public void testCreateNonKNNIndex_withKNNModelID() throws Exception {
+        Settings settings = Settings.builder().put(createKNNDefaultScriptScoreSettings()).build();
+        ResponseException ex = expectThrows(
+            ResponseException.class,
+            () -> createKnnIndex(INDEX_NAME, settings, createKnnIndexMapping(FIELD_NAME, "random-model-id"))
+        );
+        String expMessage = "Cannot set modelId or method parameters when index.knn setting is false";
+        assertThat(EntityUtils.toString(ex.getResponse().getEntity()), containsString(expMessage));
+    }
+
+    public void testCreateNonKNNIndex_withKNNMethodParams() throws Exception {
+        Settings settings = Settings.builder().put(createKNNDefaultScriptScoreSettings()).build();
+        ResponseException ex = expectThrows(
+            ResponseException.class,
+            () -> createKnnIndex(
+                INDEX_NAME,
+                settings,
+                createKnnIndexMapping(FIELD_NAME, 2, "hnsw", KNNEngine.FAISS.getName(), SpaceType.DEFAULT.getValue(), false)
+            )
+        );
+        String expMessage = "Cannot set modelId or method parameters when index.knn setting is false";
+        assertThat(EntityUtils.toString(ex.getResponse().getEntity()), containsString(expMessage));
+    }
 }

src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java

@@ -400,6 +400,22 @@ protected String createKnnIndexMapping(final String fieldName, final Integer dim
             .toString();
     }
 
+    /**
+     * Utility to create a Knn Index Mapping with model id
+     */
+    protected String createKnnIndexMapping(String fieldName, String modelId) throws IOException {
+        return XContentFactory.jsonBuilder()
+            .startObject()
+            .startObject("properties")
+            .startObject(fieldName)
+            .field("type", "knn_vector")
+            .field("model_id", modelId)
+            .endObject()
+            .endObject()
+            .endObject()
+            .toString();
+    }
+
     /**
      * Utility to create a Knn Index Mapping with specific algorithm and engine
      */