Compiler validates bound value for enum (flyteorg#277)

The PR ensures that a bound value for Enums is valid at registration / compile time Signed-off-by: Ketan Umare <[email protected]>
eapolinario · Jun 14, 2021 · d1aacdd · d1aacdd
1 parent dc71485
commit d1aacdd
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 0 deletions.
diff --git a/pkg/compiler/errors/compiler_errors.go b/pkg/compiler/errors/compiler_errors.go
@@ -78,6 +78,9 @@ const (
 
 	// A workflow is missing any nodes to execute
 	NoNodesFound ErrorCode = "NoNodesFound"
+
+	// Given value is not a legal Enum value (or not part of the defined set of enum values)
+	IllegalEnumValue ErrorCode = "IllegalEnumValue"
 )
 
 func NewBranchNodeNotSpecified(branchNodeID string) *CompileError {
@@ -193,6 +196,14 @@ func NewMismatchingBindingsErr(nodeID, sinkParam, expectedType, receivedType str
 	)
 }
 
+func NewIllegalEnumValueError(nodeID, sinkParam, receivedVal string, expectedVals []string) *CompileError {
+	return newError(
+		IllegalEnumValue,
+		fmt.Sprintf("Input [%v] on node [%v] is an Enum and expects value to be one of [%v].  Received [%v]", sinkParam, nodeID, expectedVals, receivedVal),
+		nodeID,
+	)
+}
+
 func NewMismatchingInterfacesErr(nodeID1, nodeID2 string) *CompileError {
 	return newError(
 		MismatchingInterfaces,

diff --git a/pkg/compiler/validators/bindings.go b/pkg/compiler/validators/bindings.go
@@ -91,6 +91,21 @@ func validateBinding(w c.WorkflowBuilder, nodeID c.NodeID, nodeParam string, bin
 			errs.Collect(errors.NewMismatchingTypesErr(nodeID, nodeParam, literalType.String(), expectedType.String()))
 		}
 
+		if expectedType.GetEnumType() != nil {
+			v := binding.GetScalar().GetPrimitive().GetStringValue()
+			// Let us assert that the bound value is a correct enum Value
+			found := false
+			for _, ev := range expectedType.GetEnumType().Values {
+				if ev == v {
+					found = true
+					break
+				}
+			}
+			if !found {
+				errs.Collect(errors.NewIllegalEnumValueError(nodeID, nodeParam, v, expectedType.GetEnumType().Values))
+			}
+		}
+
 		return literalType, []c.NodeID{}, !errs.HasErrors()
 	default:
 		errs.Collect(errors.NewUnrecognizedValueErr(nodeID, reflect.TypeOf(binding.GetValue()).String()))

diff --git a/pkg/compiler/validators/bindings_test.go b/pkg/compiler/validators/bindings_test.go
@@ -105,6 +105,70 @@ func TestValidateBindings(t *testing.T) {
 		}
 	})
 
+	t.Run("Enum legal string", func(t *testing.T) {
+		wf := &mocks.WorkflowBuilder{}
+		n := &mocks.NodeBuilder{}
+		n.OnGetId().Return("node1")
+
+		bindings := []*core.Binding{
+			{
+				Var:     "x",
+				Binding: LiteralToBinding(coreutils.MustMakeLiteral("x")),
+			},
+		}
+
+		vars := &core.VariableMap{
+			Variables: map[string]*core.Variable{
+				"x": {
+					Type: &core.LiteralType{Type: &core.LiteralType_EnumType{
+						EnumType: &core.EnumType{
+							Values: []string{"x", "y", "z"},
+						},
+					}},
+				},
+			},
+		}
+
+		compileErrors := compilerErrors.NewCompileErrors()
+		_, ok := ValidateBindings(wf, n, bindings, vars, true, c.EdgeDirectionBidirectional, compileErrors)
+		assert.True(t, ok)
+		if compileErrors.HasErrors() {
+			assert.NoError(t, compileErrors)
+		}
+	})
+
+	t.Run("Enum illegal string", func(t *testing.T) {
+		wf := &mocks.WorkflowBuilder{}
+		n := &mocks.NodeBuilder{}
+		n.OnGetId().Return("node1")
+
+		bindings := []*core.Binding{
+			{
+				Var:     "m",
+				Binding: LiteralToBinding(coreutils.MustMakeLiteral("x")),
+			},
+		}
+
+		vars := &core.VariableMap{
+			Variables: map[string]*core.Variable{
+				"x": {
+					Type: &core.LiteralType{Type: &core.LiteralType_EnumType{
+						EnumType: &core.EnumType{
+							Values: []string{"x", "y", "z"},
+						},
+					}},
+				},
+			},
+		}
+
+		compileErrors := compilerErrors.NewCompileErrors()
+		_, ok := ValidateBindings(wf, n, bindings, vars, true, c.EdgeDirectionBidirectional, compileErrors)
+		assert.False(t, ok)
+		if !compileErrors.HasErrors() {
+			assert.Error(t, compileErrors)
+		}
+	})
+
 	t.Run("Maps", func(t *testing.T) {
 		wf := &mocks.WorkflowBuilder{}
 		n := &mocks.NodeBuilder{}