[Backport 2.x] Adding reprovision integration tests (#834) (#839)

Adding reprovision integration tests (#834) * Adding reprovision integration tests Signed-off-by: Joshua Palis <[email protected]> * spotless Signed-off-by: Joshua Palis <[email protected]> * Adding deprovision/delete to reprovision integration tests Signed-off-by: Joshua Palis <[email protected]> * Adding deprovision/delete to reprovision failure tests Signed-off-by: Joshua Palis <[email protected]> * Using remote models rather than local models to reduce flakiness Signed-off-by: Joshua Palis <[email protected]> * Fixing forbiddenApis check Signed-off-by: Joshua Palis <[email protected]> * Fixing forbiddenAPI check, addressing PR comments Signed-off-by: Joshua Palis <[email protected]> * Fixing forbiddenAPIs main Signed-off-by: Joshua Palis <[email protected]> * increasing getResource timeout Signed-off-by: Joshua Palis <[email protected]> * Addressing PR comments Signed-off-by: Joshua Palis <[email protected]> * Fixing multi-node integration tests Signed-off-by: Joshua Palis <[email protected]> * fixing multi-node integration tests Signed-off-by: Joshua Palis <[email protected]> * Fixing syntax error Signed-off-by: Joshua Palis <[email protected]> * Blocking reprovision requests with substitution params Signed-off-by: Joshua Palis <[email protected]> * Fixes update settings request issue for multi-node Signed-off-by: Joshua Palis <[email protected]> * Increasing test coverage Signed-off-by: Joshua Palis <[email protected]> * Adding return to javadoc Signed-off-by: Joshua Palis <[email protected]> * Adding test coverage Signed-off-by: Joshua Palis <[email protected]> * Increasing test coverage Signed-off-by: Joshua Palis <[email protected]> * Increasing test coverage Signed-off-by: Joshua Palis <[email protected]> --------- Signed-off-by: Joshua Palis <[email protected]> (cherry picked from commit 562d476)
opensearch-project · Aug 16, 2024 · 4cb470a · 4cb470a
1 parent 543b9ce
commit 4cb470a
Show file tree

Hide file tree

Showing 15 changed files with 894 additions and 89 deletions.
diff --git a/src/main/java/org/opensearch/flowframework/model/Template.java b/src/main/java/org/opensearch/flowframework/model/Template.java
@@ -8,12 +8,12 @@
  */
 package org.opensearch.flowframework.model;
 
-import org.apache.logging.log4j.util.Strings;
 import org.opensearch.Version;
 import org.opensearch.common.xcontent.LoggingDeprecationHandler;
 import org.opensearch.common.xcontent.json.JsonXContent;
 import org.opensearch.common.xcontent.yaml.YamlXContent;
 import org.opensearch.commons.authuser.User;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.core.xcontent.ToXContentObject;
@@ -372,10 +372,10 @@ public static Template updateExistingTemplate(Template existingTemplate, Templat
         if (templateWithNewFields.name() != null) {
             builder.name(templateWithNewFields.name());
         }
-        if (!Strings.isBlank(templateWithNewFields.description())) {
+        if (Strings.hasText(templateWithNewFields.description())) {
             builder.description(templateWithNewFields.description());
         }
-        if (!Strings.isBlank(templateWithNewFields.useCase())) {
+        if (Strings.hasText(templateWithNewFields.useCase())) {
             builder.useCase(templateWithNewFields.useCase());
         }
         if (templateWithNewFields.templateVersion() != null) {

diff --git a/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java b/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
@@ -138,6 +138,13 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
             );
             return processError(ffe, params, request);
         }
+        if (reprovision && !params.isEmpty()) {
+            FlowFrameworkException ffe = new FlowFrameworkException(
+                "Only the parameters " + request.consumedParams() + " are permitted unless the provision parameter is set to true.",
+                RestStatus.BAD_REQUEST
+            );
+            return processError(ffe, params, request);
+        }
         try {
             Template template;
             Map<String, String> useCaseDefaultsMap = Collections.emptyMap();

diff --git a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java
@@ -533,4 +533,21 @@ public static void flattenSettings(String prefix, Map<String, Object> settings,
             }
         }
     }
+
+    /**
+     * Ensures index is prepended to flattened setting keys
+     * @param originalSettings the original settings map
+     * @return new map with keys prepended with index
+     */
+    public static Map<String, Object> prependIndexToSettings(Map<String, Object> originalSettings) {
+        Map<String, Object> newSettings = new HashMap<>();
+        originalSettings.entrySet().stream().forEach(x -> {
+            if (!x.getKey().startsWith("index.")) {
+                newSettings.put("index." + x.getKey(), x.getValue());
+            } else {
+                newSettings.put(x.getKey(), x.getValue());
+            }
+        });
+        return newSettings;
+    }
 }
diff --git a/src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java b/src/main/java/org/opensearch/flowframework/workflow/UpdateIndexStep.java
@@ -113,7 +113,10 @@ public PlainActionFuture<WorkflowData> execute(
                     if (updatedSettings.containsKey("index")) {
                         ParseUtils.flattenSettings("", updatedSettings, flattenedSettings);
                     } else {
-                        flattenedSettings.putAll(updatedSettings);
+                        // Create index setting configuration can be a mix of flattened or expanded settings
+                        // prepend index. to ensure successful setting comparison
+
+                        flattenedSettings.putAll(ParseUtils.prependIndexToSettings(updatedSettings));
                     }
 
                     Map<String, Object> filteredSettings = new HashMap<>();
@@ -133,35 +136,39 @@ public PlainActionFuture<WorkflowData> execute(
                                 filteredSettings.put(e.getKey(), e.getValue());
                             }
                         }
+
+                        // Create and send the update settings request
+                        updateSettingsRequest.settings(filteredSettings);
+                        if (updateSettingsRequest.settings().size() == 0) {
+                            String errorMessage = "Failed to update index settings for index "
+                                + indexName
+                                + ", no settings have been updated";
+                            updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, RestStatus.BAD_REQUEST));
+                        } else {
+                            client.admin().indices().updateSettings(updateSettingsRequest, ActionListener.wrap(acknowledgedResponse -> {
+                                String resourceName = getResourceByWorkflowStep(getName());
+                                logger.info("Updated index settings for index {}", indexName);
+                                updateIndexFuture.onResponse(
+                                    new WorkflowData(Map.of(resourceName, indexName), currentNodeInputs.getWorkflowId(), currentNodeId)
+                                );
+
+                            }, ex -> {
+                                Exception e = getSafeException(ex);
+                                String errorMessage = (e == null
+                                    ? "Failed to update the index settings for index " + indexName
+                                    : e.getMessage());
+                                logger.error(errorMessage, e);
+                                updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
+                            }));
+                        }
                     }, ex -> {
                         Exception e = getSafeException(ex);
                         String errorMessage = (e == null ? "Failed to retrieve the index settings for index " + indexName : e.getMessage());
                         logger.error(errorMessage, e);
                         updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
                     }));
-
-                    updateSettingsRequest.settings(filteredSettings);
                 }
             }
-
-            if (updateSettingsRequest.settings().size() == 0) {
-                String errorMessage = "Failed to update index settings for index " + indexName + ", no settings have been updated";
-                throw new FlowFrameworkException(errorMessage, RestStatus.BAD_REQUEST);
-            } else {
-                client.admin().indices().updateSettings(updateSettingsRequest, ActionListener.wrap(acknowledgedResponse -> {
-                    String resourceName = getResourceByWorkflowStep(getName());
-                    logger.info("Updated index settings for index {}", indexName);
-                    updateIndexFuture.onResponse(
-                        new WorkflowData(Map.of(resourceName, indexName), currentNodeInputs.getWorkflowId(), currentNodeId)
-                    );
-
-                }, ex -> {
-                    Exception e = getSafeException(ex);
-                    String errorMessage = (e == null ? "Failed to update the index settings for index " + indexName : e.getMessage());
-                    logger.error(errorMessage, e);
-                    updateIndexFuture.onFailure(new WorkflowStepException(errorMessage, ExceptionsHelper.status(e)));
-                }));
-            }
         } catch (Exception e) {
             updateIndexFuture.onFailure(new WorkflowStepException(e.getMessage(), ExceptionsHelper.status(e)));
         }

diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java
@@ -406,6 +406,25 @@ protected Response createWorkflowValidation(RestClient client, Template template
         return TestHelpers.makeRequest(client, "POST", WORKFLOW_URI, Collections.emptyMap(), template.toJson(), null);
     }
 
+    /**
+     * Helper method to invoke the Reprovision Workflow API
+     * @param client the rest client
+     * @param workflowId the document id
+     * @param templateFields the template to reprovision
+     * @throws Exception if the request fails
+     * @return a rest response
+     */
+    protected Response reprovisionWorkflow(RestClient client, String workflowId, Template template) throws Exception {
+        return TestHelpers.makeRequest(
+            client,
+            "PUT",
+            String.format(Locale.ROOT, "%s/%s?reprovision=true", WORKFLOW_URI, workflowId),
+            Collections.emptyMap(),
+            template.toJson(),
+            null
+        );
+    }
+
     /**
      * Helper method to invoke the Update Workflow API
      * @param client the rest client

diff --git a/src/test/java/org/opensearch/flowframework/TestHelpers.java b/src/test/java/org/opensearch/flowframework/TestHelpers.java
@@ -11,7 +11,6 @@
 import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.nio.entity.NStringEntity;
-import org.apache.logging.log4j.util.Strings;
 import org.opensearch.client.Request;
 import org.opensearch.client.RequestOptions;
 import org.opensearch.client.Response;
@@ -24,6 +23,7 @@
 import org.opensearch.common.xcontent.XContentHelper;
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.commons.authuser.User;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.core.xcontent.ToXContent;
@@ -74,7 +74,7 @@ public static Response makeRequest(
         String jsonEntity,
         List<Header> headers
     ) throws IOException {
-        HttpEntity httpEntity = Strings.isBlank(jsonEntity) ? null : new NStringEntity(jsonEntity, APPLICATION_JSON);
+        HttpEntity httpEntity = !Strings.hasText(jsonEntity) ? null : new NStringEntity(jsonEntity, APPLICATION_JSON);
         return makeRequest(client, method, endpoint, params, httpEntity, headers);
     }