Add denylist for workflow step types users are not allowed to use

Signed-off-by: Daniel Widdis <[email protected]>
opensearch-project · Jul 12, 2024 · c9afaae · c9afaae
1 parent bb16f43
commit c9afaae
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 23 deletions.
diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java
@@ -131,13 +131,7 @@ public Collection<Object> createComponents(
             flowFrameworkSettings,
             client
         );
-        WorkflowProcessSorter workflowProcessSorter = new WorkflowProcessSorter(
-            workflowStepFactory,
-            threadPool,
-            clusterService,
-            client,
-            flowFrameworkSettings
-        );
+        WorkflowProcessSorter workflowProcessSorter = new WorkflowProcessSorter(workflowStepFactory, threadPool, flowFrameworkSettings);
 
         return List.of(workflowStepFactory, workflowProcessSorter, encryptorUtils, flowFrameworkIndicesHandler, flowFrameworkSettings);
     }

diff --git a/src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java b/src/main/java/org/opensearch/flowframework/workflow/WorkflowProcessSorter.java
@@ -10,8 +10,6 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.opensearch.client.Client;
-import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.flowframework.common.FlowFrameworkSettings;
@@ -54,11 +52,16 @@ public class WorkflowProcessSorter {
 
     private static final Logger logger = LogManager.getLogger(WorkflowProcessSorter.class);
 
+    /** Workflow step types which may not be used in a template */
+    public static final Set<String> WORKFLOW_STEP_DENYLIST = Set.of(
+        DeleteIndexStep.NAME,
+        DeleteIngestPipelineStep.NAME,
+        DeleteSearchPipelineStep.NAME
+    );
+
     private WorkflowStepFactory workflowStepFactory;
     private ThreadPool threadPool;
     private Integer maxWorkflowSteps;
-    private ClusterService clusterService;
-    private Client client;
 
     /**
      * Instantiate this class.
@@ -72,15 +75,11 @@ public class WorkflowProcessSorter {
     public WorkflowProcessSorter(
         WorkflowStepFactory workflowStepFactory,
         ThreadPool threadPool,
-        ClusterService clusterService,
-        Client client,
         FlowFrameworkSettings flowFrameworkSettings
     ) {
         this.workflowStepFactory = workflowStepFactory;
         this.threadPool = threadPool;
         this.maxWorkflowSteps = flowFrameworkSettings.getMaxWorkflowSteps();
-        this.clusterService = clusterService;
-        this.client = client;
     }
 
     /**
@@ -105,6 +104,15 @@ public List<ProcessNode> sortProcessNodes(Workflow workflow, String workflowId,
                 RestStatus.BAD_REQUEST
             );
         }
+        // Disallow some steps
+        for (WorkflowNode node : workflow.nodes()) {
+            if (WORKFLOW_STEP_DENYLIST.contains(node.type())) {
+                throw new FlowFrameworkException(
+                    "The step type [" + node.type() + "] for node [" + node.id() + "] can not be used in a workflow.",
+                    RestStatus.FORBIDDEN
+                );
+            }
+        }
         List<WorkflowNode> sortedNodes = topologicalSort(workflow.nodes(), workflow.edges());
 
         List<ProcessNode> nodes = new ArrayList<>();

diff --git a/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java b/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java
@@ -118,14 +118,8 @@ public static void setup() throws IOException {
                 FLOW_FRAMEWORK_THREAD_POOL_PREFIX + DEPROVISION_WORKFLOW_THREAD_POOL
             )
         );
-        WorkflowStepFactory factory = new WorkflowStepFactory(
-            testThreadPool,
-            mlClient,
-            flowFrameworkIndicesHandler,
-            flowFrameworkSettings,
-            client
-        );
-        workflowProcessSorter = new WorkflowProcessSorter(factory, testThreadPool, clusterService, client, flowFrameworkSettings);
+        workflowStepFactory = new WorkflowStepFactory(testThreadPool, mlClient, flowFrameworkIndicesHandler, flowFrameworkSettings, client);
+        workflowProcessSorter = new WorkflowProcessSorter(workflowStepFactory, testThreadPool, flowFrameworkSettings);
     }
 
     @AfterClass
@@ -413,6 +407,25 @@ public void testFailedGraphValidation() throws IOException {
         assertEquals(RestStatus.BAD_REQUEST, ex.getRestStatus());
     }
 
+    public void testFailedDenyListValidation() throws IOException {
+
+        // Create Delete index workflow node
+        WorkflowNode deleteIndex = new WorkflowNode(
+            "workflow_step_1",
+            DeleteIndexStep.NAME,
+            Collections.emptyMap(),
+            Map.of("index_name", "undeletable")
+        );
+        Workflow workflow = new Workflow(Collections.emptyMap(), List.of(deleteIndex), Collections.emptyList());
+
+        FlowFrameworkException ex = expectThrows(
+            FlowFrameworkException.class,
+            () -> workflowProcessSorter.sortProcessNodes(workflow, "123", Collections.emptyMap())
+        );
+        assertEquals("The step type [delete_index] for node [workflow_step_1] can not be used in a workflow.", ex.getMessage());
+        assertEquals(RestStatus.FORBIDDEN, ex.getRestStatus());
+    }
+
     public void testSuccessfulInstalledPluginValidation() throws Exception {
 
         WorkflowNode createConnector = new WorkflowNode(