From 43316a5eee513e7bfbb3a1cb6f070ba26619b39c Mon Sep 17 00:00:00 2001 From: Saravanakumar Murugesan <99649913+sarmuru2@users.noreply.github.com> Date: Sat, 25 Nov 2023 11:04:32 +0530 Subject: [PATCH] ISICO-15108: javascript validation done on updating w/d (#3805) * ISICO-15108: javascript validation done on updating w/d * ISICO-14902: NPE while checking EvaluatorType is fixed * ISICO-14902: unused variables removed * ISICO-15108: javascript validation added in the constraint violation part * ISICO-15108: getExpression() is used for switch javascript code --- .../WorkflowTaskTypeConstraint.java | 43 ++++++++++++ .../service/MetadataServiceTest.java | 69 +++++++++++++++++-- 2 files changed, 105 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/com/netflix/conductor/validations/WorkflowTaskTypeConstraint.java b/core/src/main/java/com/netflix/conductor/validations/WorkflowTaskTypeConstraint.java index 888dd7a49e..99d0709853 100644 --- a/core/src/main/java/com/netflix/conductor/validations/WorkflowTaskTypeConstraint.java +++ b/core/src/main/java/com/netflix/conductor/validations/WorkflowTaskTypeConstraint.java @@ -18,8 +18,10 @@ import java.lang.annotation.Target; import java.text.ParseException; import java.time.format.DateTimeParseException; +import java.util.Map; import java.util.Optional; +import javax.script.ScriptException; import javax.validation.Constraint; import javax.validation.ConstraintValidator; import javax.validation.ConstraintValidatorContext; @@ -30,6 +32,7 @@ import com.netflix.conductor.common.metadata.tasks.TaskDef; import com.netflix.conductor.common.metadata.tasks.TaskType; import com.netflix.conductor.common.metadata.workflow.WorkflowTask; +import com.netflix.conductor.core.events.ScriptEvaluator; import com.netflix.conductor.core.utils.DateTimeUtils; import static com.netflix.conductor.core.execution.tasks.Terminate.getTerminationStatusParameter; @@ -166,9 +169,34 @@ private boolean isDecisionTaskValid( context.buildConstraintViolationWithTemplate(message).addConstraintViolation(); valid = false; } + + if (workflowTask.getCaseExpression() != null) { + try { + validateScriptExpression( + workflowTask.getCaseExpression(), workflowTask.getInputParameters()); + } catch (Exception ee) { + String message = + String.format( + ee.getMessage() + ", taskType: DECISION taskName %s", + workflowTask.getName()); + context.buildConstraintViolationWithTemplate(message).addConstraintViolation(); + valid = false; + } + } + return valid; } + private void validateScriptExpression( + String expression, Map inputParameters) { + try { + Object returnValue = ScriptEvaluator.eval(expression, inputParameters); + } catch (ScriptException e) { + throw new IllegalArgumentException( + String.format("Expression is not well formatted: %s", e.getMessage())); + } + } + private boolean isSwitchTaskValid( WorkflowTask workflowTask, ConstraintValidatorContext context) { boolean valid = true; @@ -209,6 +237,21 @@ private boolean isSwitchTaskValid( context.buildConstraintViolationWithTemplate(message).addConstraintViolation(); valid = false; } + + if ("javascript".equals(workflowTask.getEvaluatorType()) + && workflowTask.getExpression() != null) { + try { + validateScriptExpression( + workflowTask.getExpression(), workflowTask.getInputParameters()); + } catch (Exception ee) { + String message = + String.format( + ee.getMessage() + ", taskType: SWITCH taskName %s", + workflowTask.getName()); + context.buildConstraintViolationWithTemplate(message).addConstraintViolation(); + valid = false; + } + } return valid; } diff --git a/core/src/test/java/com/netflix/conductor/service/MetadataServiceTest.java b/core/src/test/java/com/netflix/conductor/service/MetadataServiceTest.java index 8ea351ac78..87fd9054e5 100644 --- a/core/src/test/java/com/netflix/conductor/service/MetadataServiceTest.java +++ b/core/src/test/java/com/netflix/conductor/service/MetadataServiceTest.java @@ -12,13 +12,7 @@ */ package com.netflix.conductor.service; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Date; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import javax.validation.ConstraintViolationException; @@ -35,6 +29,7 @@ import com.netflix.conductor.common.metadata.workflow.WorkflowDef; import com.netflix.conductor.common.metadata.workflow.WorkflowDefSummary; import com.netflix.conductor.common.metadata.workflow.WorkflowTask; +import com.netflix.conductor.common.model.BulkResponse; import com.netflix.conductor.core.config.ConductorProperties; import com.netflix.conductor.core.exception.NotFoundException; import com.netflix.conductor.dao.EventHandlerDAO; @@ -282,6 +277,66 @@ public void testUpdateWorkflowDef() { verify(metadataDAO, times(1)).updateWorkflowDef(workflowDef); } + @Test(expected = ConstraintViolationException.class) + public void testUpdateWorkflowDefWithCaseExpression() { + WorkflowDef workflowDef = new WorkflowDef(); + workflowDef.setName("somename"); + workflowDef.setOwnerEmail("sample@test.com"); + List tasks = new ArrayList<>(); + WorkflowTask workflowTask = new WorkflowTask(); + workflowTask.setTaskReferenceName("hello"); + workflowTask.setName("hello"); + workflowTask.setType("DECISION"); + + WorkflowTask caseTask = new WorkflowTask(); + caseTask.setTaskReferenceName("casetrue"); + caseTask.setName("casetrue"); + + List caseTaskList = new ArrayList<>(); + caseTaskList.add(caseTask); + + Map> decisionCases = new HashMap(); + decisionCases.put("true", caseTaskList); + + workflowTask.setDecisionCases(decisionCases); + workflowTask.setCaseExpression("1 >0abcd"); + tasks.add(workflowTask); + workflowDef.setTasks(tasks); + when(metadataDAO.getTaskDef(any())).thenReturn(new TaskDef()); + BulkResponse bulkResponse = + metadataService.updateWorkflowDef(Collections.singletonList(workflowDef)); + } + + @Test(expected = ConstraintViolationException.class) + public void testUpdateWorkflowDefWithJavscriptEvaluator() { + WorkflowDef workflowDef = new WorkflowDef(); + workflowDef.setName("somename"); + workflowDef.setOwnerEmail("sample@test.com"); + List tasks = new ArrayList<>(); + WorkflowTask workflowTask = new WorkflowTask(); + workflowTask.setTaskReferenceName("hello"); + workflowTask.setName("hello"); + workflowTask.setType("SWITCH"); + workflowTask.setEvaluatorType("javascript"); + workflowTask.setExpression("1>abcd"); + WorkflowTask caseTask = new WorkflowTask(); + caseTask.setTaskReferenceName("casetrue"); + caseTask.setName("casetrue"); + + List caseTaskList = new ArrayList<>(); + caseTaskList.add(caseTask); + + Map> decisionCases = new HashMap(); + decisionCases.put("true", caseTaskList); + + workflowTask.setDecisionCases(decisionCases); + tasks.add(workflowTask); + workflowDef.setTasks(tasks); + when(metadataDAO.getTaskDef(any())).thenReturn(new TaskDef()); + BulkResponse bulkResponse = + metadataService.updateWorkflowDef(Collections.singletonList(workflowDef)); + } + @Test(expected = ConstraintViolationException.class) public void testRegisterWorkflowDefNoName() { try {