API to validate a workflow definition

Netflix · Jul 19, 2022 · e841f73 · e841f73
1 parent ed638cd
commit e841f73
Show file tree

Hide file tree

Showing 9 changed files with 114 additions and 10 deletions.
diff --git a/client/src/main/java/com/netflix/conductor/client/http/MetadataClient.java b/client/src/main/java/com/netflix/conductor/client/http/MetadataClient.java
@@ -39,15 +39,25 @@ public MetadataClient(
     // Workflow Metadata Operations
 
     /**
-     * Register a workflow definition with the server
+     * Register a workflow definition with the server.
      *
      * @param workflowDef the workflow definition
      */
     public void registerWorkflowDef(WorkflowDef workflowDef) {
-        Validate.notNull(workflowDef, "Worfklow definition cannot be null");
+        Validate.notNull(workflowDef, "Workflow definition cannot be null");
         post("metadata/workflow", workflowDef);
     }
 
+    /**
+     * Validates a workflow definition with the server.
+     *
+     * @param workflowDef the workflow definition
+     */
+    public void validateWorkflowDef(WorkflowDef workflowDef) {
+        Validate.notNull(workflowDef, "Workflow definition cannot be null");
+        post("metadata/workflow/validate", workflowDef);
+    }
+
     /**
      * Updates a list of existing workflow definitions
      *

diff --git a/common/src/main/java/com/netflix/conductor/common/metadata/workflow/WorkflowDef.java b/common/src/main/java/com/netflix/conductor/common/metadata/workflow/WorkflowDef.java
@@ -76,7 +76,7 @@ public enum TimeoutPolicy {
     @Max(value = 2, message = "workflowDef schemaVersion: {value} is only supported")
     private int schemaVersion = 2;
 
-    // By default a workflow is restartable
+    // By default, a workflow is restartable
     @ProtoField(id = 9)
     private boolean restartable = true;
 

diff --git a/core/src/main/java/com/netflix/conductor/service/MetadataService.java b/core/src/main/java/com/netflix/conductor/service/MetadataService.java
@@ -103,6 +103,16 @@ Optional<WorkflowDef> getLatestWorkflow(
     void registerWorkflowDef(
             @NotNull(message = "WorkflowDef cannot be null") @Valid WorkflowDef workflowDef);
 
+    /**
+     * Validates a {@link WorkflowDef}.
+     *
+     * @param workflowDef The {@link WorkflowDef} object.
+     */
+    default void validateWorkflowDef(
+            @NotNull(message = "WorkflowDef cannot be null") @Valid WorkflowDef workflowDef) {
+        // do nothing, WorkflowDef is annotated with @Valid and calling this method will validate it
+    }
+
     /**
      * @param name Name of the workflow definition to be removed
      * @param version Version of the workflow definition to be removed

diff --git a/core/src/main/java/com/netflix/conductor/service/MetadataServiceImpl.java b/core/src/main/java/com/netflix/conductor/service/MetadataServiceImpl.java
@@ -64,6 +64,11 @@ public void registerTaskDef(List<TaskDef> taskDefinitions) {
         }
     }
 
+    @Override
+    public void validateWorkflowDef(WorkflowDef workflowDef) {
+        // do nothing, WorkflowDef is annotated with @Valid and calling this method will validate it
+    }
+
     /**
      * @param taskDefinition Task Definition to be updated
      */
@@ -153,13 +158,6 @@ public List<WorkflowDef> getWorkflowDefs() {
     }
 
     public void registerWorkflowDef(WorkflowDef workflowDef) {
-        if (workflowDef.getName().contains(":")) {
-            throw new IllegalArgumentException(
-                    "Workflow name cannot contain the following set of characters: ':'");
-        }
-        if (workflowDef.getSchemaVersion() < 1 || workflowDef.getSchemaVersion() > 2) {
-            workflowDef.setSchemaVersion(2);
-        }
         workflowDef.setCreateTime(System.currentTimeMillis());
         metadataDAO.createWorkflowDef(workflowDef);
     }

diff --git a/core/src/test/java/com/netflix/conductor/service/MetadataServiceTest.java b/core/src/test/java/com/netflix/conductor/service/MetadataServiceTest.java
@@ -298,6 +298,22 @@ public void testRegisterWorkflowDefNoName() {
         fail("metadataService.registerWorkflowDef did not throw ConstraintViolationException !");
     }
 
+    @Test(expected = ConstraintViolationException.class)
+    public void testValidateWorkflowDefNoName() {
+        try {
+            WorkflowDef workflowDef = new WorkflowDef();
+            metadataService.validateWorkflowDef(workflowDef);
+        } catch (ConstraintViolationException ex) {
+            assertEquals(3, ex.getConstraintViolations().size());
+            Set<String> messages = getConstraintViolationMessages(ex.getConstraintViolations());
+            assertTrue(messages.contains("WorkflowDef name cannot be null or empty"));
+            assertTrue(messages.contains("WorkflowTask list cannot be empty"));
+            assertTrue(messages.contains("ownerEmail cannot be empty"));
+            throw ex;
+        }
+        fail("metadataService.validateWorkflowDef did not throw ConstraintViolationException !");
+    }
+
     @Test(expected = ConstraintViolationException.class)
     public void testRegisterWorkflowDefInvalidName() {
         try {
@@ -318,6 +334,26 @@ public void testRegisterWorkflowDefInvalidName() {
         fail("metadataService.registerWorkflowDef did not throw ConstraintViolationException !");
     }
 
+    @Test(expected = ConstraintViolationException.class)
+    public void testValidateWorkflowDefInvalidName() {
+        try {
+            WorkflowDef workflowDef = new WorkflowDef();
+            workflowDef.setName("invalid:name");
+            workflowDef.setOwnerEmail("inavlid-email");
+            metadataService.validateWorkflowDef(workflowDef);
+        } catch (ConstraintViolationException ex) {
+            assertEquals(3, ex.getConstraintViolations().size());
+            Set<String> messages = getConstraintViolationMessages(ex.getConstraintViolations());
+            assertTrue(messages.contains("WorkflowTask list cannot be empty"));
+            assertTrue(
+                    messages.contains(
+                            "Workflow name cannot contain the following set of characters: ':'"));
+            assertTrue(messages.contains("ownerEmail should be valid email address"));
+            throw ex;
+        }
+        fail("metadataService.validateWorkflowDef did not throw ConstraintViolationException !");
+    }
+
     @Test
     public void testRegisterWorkflowDef() {
         WorkflowDef workflowDef = new WorkflowDef();
@@ -336,6 +372,24 @@ public void testRegisterWorkflowDef() {
         assertEquals(2, workflowDef.getSchemaVersion());
     }
 
+    @Test
+    public void testValidateWorkflowDef() {
+        WorkflowDef workflowDef = new WorkflowDef();
+        workflowDef.setName("somename");
+        workflowDef.setSchemaVersion(2);
+        workflowDef.setOwnerEmail("[email protected]");
+        List<WorkflowTask> tasks = new ArrayList<>();
+        WorkflowTask workflowTask = new WorkflowTask();
+        workflowTask.setTaskReferenceName("hello");
+        workflowTask.setName("hello");
+        tasks.add(workflowTask);
+        workflowDef.setTasks(tasks);
+        when(metadataDAO.getTaskDef(any())).thenReturn(new TaskDef());
+        metadataService.validateWorkflowDef(workflowDef);
+        verify(metadataDAO, times(1)).createWorkflowDef(workflowDef);
+        assertEquals(2, workflowDef.getSchemaVersion());
+    }
+
     @Test(expected = ConstraintViolationException.class)
     public void testUnregisterWorkflowDefNoName() {
         try {

diff --git a/grpc-server/src/main/java/com/netflix/conductor/grpc/server/service/MetadataServiceImpl.java b/grpc-server/src/main/java/com/netflix/conductor/grpc/server/service/MetadataServiceImpl.java
@@ -55,6 +55,16 @@ public void createWorkflow(
         response.onCompleted();
     }
 
+    @Override
+    public void validateWorkflow(
+            MetadataServicePb.ValidateWorkflowRequest req,
+            StreamObserver<MetadataServicePb.ValidateWorkflowResponse> response) {
+        WorkflowDef workflow = PROTO_MAPPER.fromProto(req.getWorkflow());
+        service.validateWorkflowDef(workflow);
+        response.onNext(MetadataServicePb.ValidateWorkflowResponse.getDefaultInstance());
+        response.onCompleted();
+    }
+
     @Override
     public void updateWorkflows(
             MetadataServicePb.UpdateWorkflowsRequest req,

diff --git a/grpc/src/main/proto/grpc/metadata_service.proto b/grpc/src/main/proto/grpc/metadata_service.proto
@@ -12,6 +12,9 @@ service MetadataService {
     // POST /workflow
     rpc CreateWorkflow(CreateWorkflowRequest) returns (CreateWorkflowResponse);
 
+    // POST /workflow/validate
+    rpc ValidateWorkflow(ValidateWorkflowRequest) returns (ValidateWorkflowResponse);
+
     // PUT /workflow
     rpc UpdateWorkflows(UpdateWorkflowsRequest) returns (UpdateWorkflowsResponse);
 
@@ -37,6 +40,12 @@ message CreateWorkflowRequest {
 
 message CreateWorkflowResponse {}
 
+message ValidateWorkflowRequest {
+    conductor.proto.WorkflowDef workflow = 1;
+}
+
+message ValidateWorkflowResponse {}
+
 message UpdateWorkflowsRequest {
     repeated conductor.proto.WorkflowDef defs = 1;
 }

diff --git a/rest/src/main/java/com/netflix/conductor/rest/controllers/MetadataResource.java b/rest/src/main/java/com/netflix/conductor/rest/controllers/MetadataResource.java
@@ -50,6 +50,12 @@ public void create(@RequestBody WorkflowDef workflowDef) {
         metadataService.registerWorkflowDef(workflowDef);
     }
 
+    @PostMapping("/workflow/validate")
+    @Operation(summary = "Validates a new workflow definition")
+    public void validate(@RequestBody WorkflowDef workflowDef) {
+        metadataService.validateWorkflowDef(workflowDef);
+    }
+
     @PutMapping("/workflow")
     @Operation(summary = "Create or update workflow definition")
     public void update(@RequestBody List<WorkflowDef> workflowDefs) {

diff --git a/rest/src/test/java/com/netflix/conductor/rest/controllers/MetadataResourceTest.java b/rest/src/test/java/com/netflix/conductor/rest/controllers/MetadataResourceTest.java
@@ -50,6 +50,13 @@ public void testCreateWorkflow() {
         verify(mockMetadataService, times(1)).registerWorkflowDef(any(WorkflowDef.class));
     }
 
+    @Test
+    public void testValidateWorkflow() {
+        WorkflowDef workflowDef = new WorkflowDef();
+        metadataResource.validate(workflowDef);
+        verify(mockMetadataService, times(1)).validateWorkflowDef(any(WorkflowDef.class));
+    }
+
     @Test
     public void testUpdateWorkflow() {
         WorkflowDef workflowDef = new WorkflowDef();