Change dryrun param in create workflow to validation (#308)

* Change dryrun param in create workflow to validation Signed-off-by: Jackie Han <[email protected]> * update javadoc Signed-off-by: Jackie Han <[email protected]> * change boolean validation parameter to a string parameter Signed-off-by: Jackie Han <[email protected]> * update workflow request defualt valdiation value Signed-off-by: Jackie Han <[email protected]> --------- Signed-off-by: Jackie Han <[email protected]>
opensearch-project · Dec 24, 2023 · 113b008 · 113b008
1 parent 4efbc95
commit 113b008
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 35 deletions.
diff --git a/src/main/java/org/opensearch/flowframework/common/CommonValue.java b/src/main/java/org/opensearch/flowframework/common/CommonValue.java
@@ -63,8 +63,8 @@ private CommonValue() {}
     public static final String WORKFLOW_URI = FLOW_FRAMEWORK_BASE_URI + "/workflow";
     /** Field name for workflow Id, the document Id of the indexed use case template */
     public static final String WORKFLOW_ID = "workflow_id";
-    /** Field name for dry run, the flag to indicate if validation is necessary */
-    public static final String DRY_RUN = "dryrun";
+    /** Field name for template validation, the flag to indicate if validation is necessary */
+    public static final String VALIDATION = "validation";
     /** The field name for provision workflow within a use case template*/
     public static final String PROVISION_WORKFLOW = "provision";
 

diff --git a/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java b/src/main/java/org/opensearch/flowframework/rest/RestCreateWorkflowAction.java
@@ -32,8 +32,8 @@
 import java.util.Locale;
 
 import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken;
-import static org.opensearch.flowframework.common.CommonValue.DRY_RUN;
 import static org.opensearch.flowframework.common.CommonValue.PROVISION_WORKFLOW;
+import static org.opensearch.flowframework.common.CommonValue.VALIDATION;
 import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_ID;
 import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_URI;
 import static org.opensearch.flowframework.common.FlowFrameworkSettings.FLOW_FRAMEWORK_ENABLED;
@@ -94,10 +94,17 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
             XContentParser parser = request.contentParser();
             ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser);
             Template template = Template.parse(parser);
-            boolean dryRun = request.paramAsBoolean(DRY_RUN, false);
+            String[] validation = request.paramAsStringArray(VALIDATION, new String[] { "all" });
             boolean provision = request.paramAsBoolean(PROVISION_WORKFLOW, false);
 
-            WorkflowRequest workflowRequest = new WorkflowRequest(workflowId, template, dryRun, provision, requestTimeout, maxWorkflows);
+            WorkflowRequest workflowRequest = new WorkflowRequest(
+                workflowId,
+                template,
+                validation,
+                provision,
+                requestTimeout,
+                maxWorkflows
+            );
 
             return channel -> client.execute(CreateWorkflowAction.INSTANCE, workflowRequest, ActionListener.wrap(response -> {
                 XContentBuilder builder = response.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS);

diff --git a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
@@ -37,6 +37,7 @@
 import org.opensearch.tasks.Task;
 import org.opensearch.transport.TransportService;
 
+import java.util.Arrays;
 import java.util.List;
 
 import static org.opensearch.flowframework.common.CommonValue.PROVISIONING_PROGRESS_FIELD;
@@ -95,7 +96,8 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener<Work
             user
         );
 
-        if (request.isDryRun()) {
+        String[] validateAll = { "all" };
+        if (Arrays.equals(request.getValidation(), validateAll)) {
             try {
                 validateWorkflows(templateWithUser);
             } catch (Exception e) {

diff --git a/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java b/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java
@@ -36,7 +36,7 @@ public class WorkflowRequest extends ActionRequest {
     /**
      * Validation flag
      */
-    private boolean dryRun;
+    private String[] validation;
 
     /**
      * Provision flag
@@ -54,16 +54,16 @@ public class WorkflowRequest extends ActionRequest {
     private Integer maxWorkflows;
 
     /**
-     * Instantiates a new WorkflowRequest, defaults dry run to false and set requestTimeout and maxWorkflows to null
+     * Instantiates a new WorkflowRequest, set validation to false and set requestTimeout and maxWorkflows to null
      * @param workflowId the documentId of the workflow
      * @param template the use case template which describes the workflow
      */
     public WorkflowRequest(@Nullable String workflowId, @Nullable Template template) {
-        this(workflowId, template, false, false, null, null);
+        this(workflowId, template, new String[] { "all" }, false, null, null);
     }
 
     /**
-     * Instantiates a new WorkflowRequest and defaults dry run to false
+     * Instantiates a new WorkflowRequest and set validation to false
      * @param workflowId the documentId of the workflow
      * @param template the use case template which describes the workflow
      * @param requestTimeout timeout of the request
@@ -75,29 +75,29 @@ public WorkflowRequest(
         @Nullable TimeValue requestTimeout,
         @Nullable Integer maxWorkflows
     ) {
-        this(workflowId, template, false, false, requestTimeout, maxWorkflows);
+        this(workflowId, template, new String[] { "all" }, false, requestTimeout, maxWorkflows);
     }
 
     /**
      * Instantiates a new WorkflowRequest
      * @param workflowId the documentId of the workflow
      * @param template the use case template which describes the workflow
-     * @param dryRun flag to indicate if validation is necessary
+     * @param validation flag to indicate if validation is necessary
      * @param provision flag to indicate if provision is necessary
      * @param requestTimeout timeout of the request
      * @param maxWorkflows max number of workflows
      */
     public WorkflowRequest(
         @Nullable String workflowId,
         @Nullable Template template,
-        boolean dryRun,
+        String[] validation,
         boolean provision,
         @Nullable TimeValue requestTimeout,
         @Nullable Integer maxWorkflows
     ) {
         this.workflowId = workflowId;
         this.template = template;
-        this.dryRun = dryRun;
+        this.validation = validation;
         this.provision = provision;
         this.requestTimeout = requestTimeout;
         this.maxWorkflows = maxWorkflows;
@@ -113,7 +113,7 @@ public WorkflowRequest(StreamInput in) throws IOException {
         this.workflowId = in.readOptionalString();
         String templateJson = in.readOptionalString();
         this.template = templateJson == null ? null : Template.parse(templateJson);
-        this.dryRun = in.readBoolean();
+        this.validation = in.readStringArray();
         this.provision = in.readBoolean();
         this.requestTimeout = in.readOptionalTimeValue();
         this.maxWorkflows = in.readOptionalInt();
@@ -138,11 +138,11 @@ public Template getTemplate() {
     }
 
     /**
-     * Gets the dry run validation flag
-     * @return the dry run boolean
+     * Gets the validation flag
+     * @return the validation boolean
      */
-    public boolean isDryRun() {
-        return this.dryRun;
+    public String[] getValidation() {
+        return this.validation;
     }
 
     /**
@@ -174,7 +174,7 @@ public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         out.writeOptionalString(workflowId);
         out.writeOptionalString(template == null ? null : template.toJson());
-        out.writeBoolean(dryRun);
+        out.writeStringArray(validation);
         out.writeBoolean(provision);
         out.writeOptionalTimeValue(requestTimeout);
         out.writeOptionalInt(maxWorkflows);

diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java
@@ -324,13 +324,13 @@ protected Response createWorkflowWithProvision(Template template) throws Excepti
     }
 
     /**
-     * Helper method to invoke the Create Workflow Rest Action with dry run validation
+     * Helper method to invoke the Create Workflow Rest Action with validation
      * @param template the template to create
      * @throws Exception if the request fails
      * @return a rest response
      */
-    protected Response createWorkflowDryRun(Template template) throws Exception {
-        return TestHelpers.makeRequest(client(), "POST", WORKFLOW_URI + "?dryrun=true", ImmutableMap.of(), template.toJson(), null);
+    protected Response createWorkflowValidation(Template template) throws Exception {
+        return TestHelpers.makeRequest(client(), "POST", WORKFLOW_URI, ImmutableMap.of(), template.toJson(), null);
     }
 
     /**

diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkRestApiIT.java
@@ -154,7 +154,7 @@ public void testCreateAndProvisionRemoteModelWorkflow() throws Exception {
             .build();
 
         // Hit dry run
-        ResponseException exception = expectThrows(ResponseException.class, () -> createWorkflowDryRun(cyclicalTemplate));
+        ResponseException exception = expectThrows(ResponseException.class, () -> createWorkflowValidation(cyclicalTemplate));
         assertTrue(exception.getMessage().contains("Cycle detected: [workflow_step_2->workflow_step_1, workflow_step_1->workflow_step_2]"));
 
         // Hit Create Workflow API with original template

diff --git a/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java
@@ -138,16 +138,16 @@ public void setUp() throws Exception {
         );
     }
 
-    public void testDryRunValidation_withoutProvision_Success() {
+    public void testValidation_withoutProvision_Success() {
         Template validTemplate = generateValidTemplate();
 
         @SuppressWarnings("unchecked")
         ActionListener<WorkflowResponse> listener = mock(ActionListener.class);
-        WorkflowRequest createNewWorkflow = new WorkflowRequest(null, validTemplate, true, false, null, null);
+        WorkflowRequest createNewWorkflow = new WorkflowRequest(null, validTemplate, new String[] { "all" }, false, null, null);
         createWorkflowTransportAction.doExecute(mock(Task.class), createNewWorkflow, listener);
     }
 
-    public void testDryRunValidation_Failed() throws Exception {
+    public void testValidation_Failed() throws Exception {
 
         WorkflowNode createConnector = new WorkflowNode(
             "workflow_step_1",
@@ -203,7 +203,7 @@ public void testDryRunValidation_Failed() throws Exception {
         ActionListener<WorkflowResponse> listener = mock(ActionListener.class);
         // Stub validation failure
         doThrow(Exception.class).when(workflowProcessSorter).validate(any());
-        WorkflowRequest createNewWorkflow = new WorkflowRequest(null, cyclicalTemplate, true, false, null, null);
+        WorkflowRequest createNewWorkflow = new WorkflowRequest(null, cyclicalTemplate, new String[] { "all" }, false, null, null);
 
         createWorkflowTransportAction.doExecute(mock(Task.class), createNewWorkflow, listener);
         verify(listener, times(1)).onFailure(any());
@@ -215,7 +215,7 @@ public void testMaxWorkflow() {
         WorkflowRequest workflowRequest = new WorkflowRequest(
             null,
             template,
-            false,
+            new String[] { "off" },
             false,
             WORKFLOW_REQUEST_TIMEOUT.get(settings),
             MAX_WORKFLOWS.get(settings)
@@ -252,7 +252,7 @@ public void testFailedToCreateNewWorkflow() {
         WorkflowRequest workflowRequest = new WorkflowRequest(
             null,
             template,
-            false,
+            new String[] { "off" },
             false,
             WORKFLOW_REQUEST_TIMEOUT.get(settings),
             MAX_WORKFLOWS.get(settings)
@@ -290,7 +290,7 @@ public void testCreateNewWorkflow() {
         WorkflowRequest workflowRequest = new WorkflowRequest(
             null,
             template,
-            false,
+            new String[] { "off" },
             false,
             WORKFLOW_REQUEST_TIMEOUT.get(settings),
             MAX_WORKFLOWS.get(settings)
@@ -374,7 +374,7 @@ public void testUpdateWorkflow() {
         assertEquals("1", responseCaptor.getValue().getWorkflowId());
     }
 
-    public void testCreateWorkflow_withDryRun_withProvision_Success() throws Exception {
+    public void testCreateWorkflow_withValidation_withProvision_Success() throws Exception {
 
         Template validTemplate = generateValidTemplate();
 
@@ -385,7 +385,7 @@ public void testCreateWorkflow_withDryRun_withProvision_Success() throws Excepti
         WorkflowRequest workflowRequest = new WorkflowRequest(
             null,
             validTemplate,
-            true,
+            new String[] { "all" },
             true,
             WORKFLOW_REQUEST_TIMEOUT.get(settings),
             MAX_WORKFLOWS.get(settings)
@@ -435,7 +435,7 @@ public void testCreateWorkflow_withDryRun_withProvision_Success() throws Excepti
         assertEquals("1", workflowResponseCaptor.getValue().getWorkflowId());
     }
 
-    public void testCreateWorkflow_withDryRun_withProvision_FailedProvisioning() throws Exception {
+    public void testCreateWorkflow_withValidation_withProvision_FailedProvisioning() throws Exception {
 
         Template validTemplate = generateValidTemplate();
 
@@ -445,7 +445,7 @@ public void testCreateWorkflow_withDryRun_withProvision_FailedProvisioning() thr
         WorkflowRequest workflowRequest = new WorkflowRequest(
             null,
             validTemplate,
-            true,
+            new String[] { "all" },
             true,
             WORKFLOW_REQUEST_TIMEOUT.get(settings),
             MAX_WORKFLOWS.get(settings)