[flytepropeller][compiler] Error Handling when Type is not found (fly…

…teorg#5612) * FlytePropeller Compiler Avoid Crash when Type not found Signed-off-by: Future-Outlier <[email protected]> * Update pingsu's error message advices Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: pingsutw <[email protected]> * fix lint Signed-off-by: Future-Outlier <[email protected]> * Trigger CI Signed-off-by: Future-Outlier <[email protected]> * Trigger CI Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: pingsutw <[email protected]> Signed-off-by: Vladyslav Libov <[email protected]>
vlibov · Aug 16, 2024 · f6c0be0 · f6c0be0
1 parent c8cc940
commit f6c0be0
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 5 deletions.
diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go
@@ -13,6 +13,10 @@ import (
 	"github.com/flyteorg/flyte/flytestdlib/utils"
 )
 
+const (
+	foo = "foo"
+)
+
 var lpApplicationConfig = testutils.GetApplicationConfigWithDefaultDomains()
 
 func getWorkflowInterface() *core.TypedInterface {
@@ -344,7 +348,7 @@ func TestValidateSchedule_KickoffTimeArgPointsAtWrongType(t *testing.T) {
 	request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * * *")
 	inputMap := &core.ParameterMap{
 		Parameters: map[string]*core.Parameter{
-			"foo": {
+			foo: {
 				Var: &core.Variable{
 					Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: core.SimpleType_STRING}},
 				},
@@ -354,7 +358,7 @@ func TestValidateSchedule_KickoffTimeArgPointsAtWrongType(t *testing.T) {
 			},
 		},
 	}
-	request.Spec.EntityMetadata.Schedule.KickoffTimeInputArg = "foo"
+	request.Spec.EntityMetadata.Schedule.KickoffTimeInputArg = foo
 
 	err := validateSchedule(request, inputMap)
 	assert.NotNil(t, err)
@@ -364,7 +368,7 @@ func TestValidateSchedule_NoRequired(t *testing.T) {
 	request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * * *")
 	inputMap := &core.ParameterMap{
 		Parameters: map[string]*core.Parameter{
-			"foo": {
+			foo: {
 				Var: &core.Variable{
 					Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: core.SimpleType_STRING}},
 				},
@@ -383,7 +387,7 @@ func TestValidateSchedule_KickoffTimeBound(t *testing.T) {
 	request := testutils.GetLaunchPlanRequestWithDeprecatedCronSchedule("* * * * * *")
 	inputMap := &core.ParameterMap{
 		Parameters: map[string]*core.Parameter{
-			"foo": {
+			foo: {
 				Var: &core.Variable{
 					Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: core.SimpleType_DATETIME}},
 				},
@@ -393,7 +397,7 @@ func TestValidateSchedule_KickoffTimeBound(t *testing.T) {
 			},
 		},
 	}
-	request.Spec.EntityMetadata.Schedule.KickoffTimeInputArg = "foo"
+	request.Spec.EntityMetadata.Schedule.KickoffTimeInputArg = foo
 
 	err := validateSchedule(request, inputMap)
 	assert.Nil(t, err)

diff --git a/flyteadmin/pkg/manager/impl/validation/validation.go b/flyteadmin/pkg/manager/impl/validation/validation.go
@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"fmt"
 	"net/url"
 	"strconv"
 	"strings"
@@ -282,11 +283,27 @@ func validateParameterMap(inputMap *core.ParameterMap, fieldName string) error {
 			defaultValue := defaultInput.GetDefault()
 			if defaultValue != nil {
 				inputType := validators.LiteralTypeForLiteral(defaultValue)
+
+				if inputType == nil {
+					return errors.NewFlyteAdminErrorf(codes.InvalidArgument,
+						fmt.Sprintf(
+							"Flyte encountered an issue while determining\n"+
+								"the type of the default value for Parameter '%s' in '%s'.\n"+
+								"Registered type: [%s].\n"+
+								"Flyte needs to support the latest FlyteIDL to support this type.\n"+
+								"Suggested solution: Please update all of your Flyte images to the latest version and "+
+								"try again.",
+							name, fieldName, defaultInput.GetVar().GetType().String(),
+						),
+					)
+				}
+
 				if !validators.AreTypesCastable(inputType, defaultInput.GetVar().GetType()) {
 					return errors.NewFlyteAdminErrorf(codes.InvalidArgument,
 						"Type mismatch for Parameter %s in %s has type %s, expected %s", name, fieldName,
 						defaultInput.GetVar().GetType().String(), inputType.String())
 				}
+
 				if defaultInput.GetVar().GetType().GetSimple() == core.SimpleType_DATETIME {
 					// Make datetime specific validations
 					return ValidateDatetime(defaultValue)

diff --git a/flyteadmin/pkg/manager/impl/validation/validation_test.go b/flyteadmin/pkg/manager/impl/validation/validation_test.go
@@ -320,6 +320,44 @@ func TestValidateParameterMap(t *testing.T) {
 		err := validateParameterMap(&exampleMap, "some text")
 		assert.NoError(t, err)
 	})
+	t.Run("invalid because inputType is nil", func(t *testing.T) {
+		// Create a literal that will cause LiteralTypeForLiteral to return nil.
+		// For example, a scalar with no value.
+		unsupportedLiteral := &core.Literal{
+			Value: &core.Literal_Scalar{
+				Scalar: &core.Scalar{},
+			},
+		}
+
+		name := "foo"
+		fieldName := "test_field_name"
+		exampleMap := core.ParameterMap{
+			Parameters: map[string]*core.Parameter{
+				name: {
+					Var: &core.Variable{
+						// 1000 means an unsupported type
+						Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: 1000}},
+					},
+					Behavior: &core.Parameter_Default{
+						Default: unsupportedLiteral,
+					},
+				},
+			},
+		}
+		err := validateParameterMap(&exampleMap, fieldName)
+		assert.Error(t, err)
+		fmt.Println(err.Error())
+		expectedErrMsg := fmt.Sprintf(
+			"Flyte encountered an issue while determining\n"+
+				"the type of the default value for Parameter '%s' in '%s'.\n"+
+				"Registered type: [%s].\n"+
+				"Flyte needs to support the latest FlyteIDL to support this type.\n"+
+				"Suggested solution: Please update all of your Flyte images to the latest version and "+
+				"try again.",
+			name, fieldName, exampleMap.Parameters[name].GetVar().GetType().String(),
+		)
+		assert.Equal(t, expectedErrMsg, err.Error())
+	})
 }
 
 func TestValidateToken(t *testing.T) {