From 66ff1528893813763b1202763b3850ae27e86f48 Mon Sep 17 00:00:00 2001 From: Future-Outlier Date: Sun, 29 Sep 2024 15:30:12 +0800 Subject: [PATCH] [flytepropeller][flyteadmin] Compiler unknown literal type error handling (#5651) * [propeller][admin] Compiler literal type error handling Signed-off-by: Future-Outlier * support all err msg Signed-off-by: Future-Outlier * add execution_validator tests Signed-off-by: Future-Outlier * add launch_plan_validator tests Signed-off-by: Future-Outlier * add tests for signal_validator Signed-off-by: Future-Outlier * change var Signed-off-by: Future-Outlier * add tests for compiler_errors Signed-off-by: Future-Outlier * support nodes/array/handler.go Signed-off-by: Future-Outlier * fix imports Signed-off-by: Future-Outlier * add tests for k8s/inputs and catalog/datacatalog/transformer Signed-off-by: Future-Outlier * kevin's change Signed-off-by: Kevin Su * update all pingsu's interface Signed-off-by: Future-Outlier * Trigger Build Signed-off-by: Future-Outlier * update pingsu's advice Signed-off-by: Future-Outlier Co-authored-by: pingsutw * nit Signed-off-by: Future-Outlier * use InvalidLiteralTypeErr instead of NewIDLTypeNotFoundErr Signed-off-by: Future-Outlier * nit Signed-off-by: Kevin Su --------- Signed-off-by: Future-Outlier Signed-off-by: Kevin Su Co-authored-by: Kevin Su --- flyteadmin/pkg/errors/errors.go | 5 ++ .../impl/validation/execution_validator.go | 4 ++ .../validation/execution_validator_test.go | 38 +++++++++++ .../impl/validation/launch_plan_validator.go | 4 ++ .../validation/launch_plan_validator_test.go | 44 +++++++++++++ .../impl/validation/signal_validator.go | 4 ++ .../impl/validation/signal_validator_test.go | 48 ++++++++++++++ .../pkg/manager/impl/validation/validation.go | 17 +---- .../impl/validation/validation_test.go | 11 +--- .../compiler/errors/compiler_error_test.go | 3 +- .../pkg/compiler/errors/compiler_errors.go | 11 ++++ .../pkg/compiler/transformers/k8s/inputs.go | 7 +++ .../compiler/transformers/k8s/inputs_test.go | 54 ++++++++++++++++ .../pkg/compiler/validators/utils.go | 17 +++++ .../pkg/controller/nodes/array/handler.go | 9 ++- .../controller/nodes/array/handler_test.go | 63 +++++++++++++++++++ .../nodes/catalog/datacatalog/transformer.go | 4 ++ .../catalog/datacatalog/transformer_test.go | 40 ++++++++++++ .../pkg/controller/nodes/errors/codes.go | 1 + 19 files changed, 358 insertions(+), 26 deletions(-) diff --git a/flyteadmin/pkg/errors/errors.go b/flyteadmin/pkg/errors/errors.go index 0fd9542ba7..5fc48b0b67 100644 --- a/flyteadmin/pkg/errors/errors.go +++ b/flyteadmin/pkg/errors/errors.go @@ -213,3 +213,8 @@ func NewInactiveProjectError(ctx context.Context, id string) FlyteAdminError { } return statusErr } + +func NewInvalidLiteralTypeError(name string, err error) FlyteAdminError { + return NewFlyteAdminErrorf(codes.InvalidArgument, + fmt.Sprintf("Failed to validate literal type for [%s] with err: %s", name, err)) +} diff --git a/flyteadmin/pkg/manager/impl/validation/execution_validator.go b/flyteadmin/pkg/manager/impl/validation/execution_validator.go index f7b385b8a8..639f29073f 100644 --- a/flyteadmin/pkg/manager/impl/validation/execution_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/execution_validator.go @@ -107,6 +107,10 @@ func CheckAndFetchInputsForExecution( default: inputType = validators.LiteralTypeForLiteral(executionInputMap[name]) } + err := validators.ValidateLiteralType(inputType) + if err != nil { + return nil, errors.NewInvalidLiteralTypeError(name, err) + } if !validators.AreTypesCastable(inputType, expectedInput.GetVar().GetType()) { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid %s input wrong type. Expected %s, but got %s", name, expectedInput.GetVar().GetType(), inputType) } diff --git a/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go b/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go index 7e5f991788..943e5006e7 100644 --- a/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/execution_validator_test.go @@ -17,6 +17,8 @@ import ( var execConfig = testutils.GetApplicationConfigWithDefaultDomains() +const failedToValidateLiteralType = "Failed to validate literal type" + func TestValidateExecEmptyProject(t *testing.T) { request := testutils.GetExecutionRequest() request.Project = "" @@ -209,6 +211,42 @@ func TestValidateExecEmptyInputs(t *testing.T) { assert.EqualValues(t, expectedMap, actualInputs) } +func TestValidateExecUnknownIDLInputs(t *testing.T) { + unsupportedLiteral := &core.Literal{ + Value: &core.Literal_Scalar{ + Scalar: &core.Scalar{}, + }, + } + defaultInputs := &core.ParameterMap{ + Parameters: map[string]*core.Parameter{ + "foo": { + Var: &core.Variable{ + // 1000 means an unsupported type + Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: 1000}}, + }, + Behavior: &core.Parameter_Default{ + Default: unsupportedLiteral, + }, + }, + }, + } + userInputs := &core.LiteralMap{ + Literals: map[string]*core.Literal{ + "foo": unsupportedLiteral, // This will lead to a nil inputType + }, + } + + _, err := CheckAndFetchInputsForExecution( + userInputs, + nil, + defaultInputs, + ) + assert.NotNil(t, err) + + // Expected error message + assert.Contains(t, err.Error(), failedToValidateLiteralType) +} + func TestValidExecutionId(t *testing.T) { err := CheckValidExecutionID("abcde123", "a") assert.Nil(t, err) diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go index 0a5e87fe99..2a49b4da87 100644 --- a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator.go @@ -143,6 +143,10 @@ func checkAndFetchExpectedInputForLaunchPlan( return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "unexpected fixed_input %s", name) } inputType := validators.LiteralTypeForLiteral(fixedInput) + err := validators.ValidateLiteralType(inputType) + if err != nil { + return nil, errors.NewInvalidLiteralTypeError(name, err) + } if !validators.AreTypesCastable(inputType, value.GetType()) { return nil, errors.NewFlyteAdminErrorf(codes.InvalidArgument, "invalid fixed_input wrong type %s, expected %v, got %v instead", name, value.GetType(), inputType) diff --git a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go index 3bad5110c5..ab2832eeeb 100644 --- a/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/launch_plan_validator_test.go @@ -231,6 +231,50 @@ func TestGetLpExpectedInvalidFixedInput(t *testing.T) { assert.Nil(t, actualMap) } +func TestGetLpExpectedInvalidFixedInputWithUnknownIDL(t *testing.T) { + unsupportedLiteral := &core.Literal{ + Value: &core.Literal_Scalar{ + Scalar: &core.Scalar{}, + }, + } + workflowVariableMap := &core.VariableMap{ + Variables: map[string]*core.Variable{ + "foo": { + Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: 1000}}, + }, + }, + } + defaultInputs := &core.ParameterMap{ + Parameters: map[string]*core.Parameter{ + "foo": { + Var: &core.Variable{ + // 1000 means an unsupported type + Type: &core.LiteralType{Type: &core.LiteralType_Simple{Simple: 1000}}, + }, + Behavior: &core.Parameter_Default{ + Default: unsupportedLiteral, + }, + }, + }, + } + fixedInputs := &core.LiteralMap{ + Literals: map[string]*core.Literal{ + "foo": unsupportedLiteral, // This will lead to a nil inputType + }, + } + + _, err := checkAndFetchExpectedInputForLaunchPlan( + workflowVariableMap, + fixedInputs, + defaultInputs, + ) + + assert.NotNil(t, err) + + // Expected error message + assert.Contains(t, err.Error(), failedToValidateLiteralType) +} + func TestGetLpExpectedNoFixedInput(t *testing.T) { request := testutils.GetLaunchPlanRequest() actualMap, err := checkAndFetchExpectedInputForLaunchPlan( diff --git a/flyteadmin/pkg/manager/impl/validation/signal_validator.go b/flyteadmin/pkg/manager/impl/validation/signal_validator.go index c5e49401b0..af1d4425aa 100644 --- a/flyteadmin/pkg/manager/impl/validation/signal_validator.go +++ b/flyteadmin/pkg/manager/impl/validation/signal_validator.go @@ -76,6 +76,10 @@ func ValidateSignalSetRequest(ctx context.Context, db repositoryInterfaces.Repos if err != nil { return err } + err = propellervalidators.ValidateLiteralType(valueType) + if err != nil { + return errors.NewInvalidLiteralTypeError("", err) + } if !propellervalidators.AreTypesCastable(lookupSignal.Type, valueType) { return errors.NewFlyteAdminErrorf(codes.InvalidArgument, "requested signal value [%v] is not castable to existing signal type [%v]", diff --git a/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go b/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go index aa978f80d6..c78c2c366b 100644 --- a/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go +++ b/flyteadmin/pkg/manager/impl/validation/signal_validator_test.go @@ -283,4 +283,52 @@ func TestValidateSignalUpdateRequest(t *testing.T) { utils.AssertEqualWithSanitizedRegex(t, "requested signal value [scalar:{ primitive:{ boolean:false } } ] is not castable to existing signal type [[8 1]]", ValidateSignalSetRequest(ctx, repo, request).Error()) }) + + t.Run("UnknownIDLType", func(t *testing.T) { + ctx := context.TODO() + + // Define an unsupported literal type with a simple type of 1000 + unsupportedLiteralType := &core.LiteralType{ + Type: &core.LiteralType_Simple{ + Simple: 1000, // Using 1000 as an unsupported type + }, + } + unsupportedLiteralTypeBytes, _ := proto.Marshal(unsupportedLiteralType) + + // Mock the repository to return a signal with this unsupported type + repo := repositoryMocks.NewMockRepository() + repo.SignalRepo().(*repositoryMocks.SignalRepoInterface). + OnGetMatch(mock.Anything, mock.Anything).Return( + models.Signal{ + Type: unsupportedLiteralTypeBytes, // Set the unsupported type + }, + nil, + ) + + // Set up the unsupported literal that will trigger the nil valueType condition + unsupportedLiteral := &core.Literal{ + Value: &core.Literal_Scalar{ + Scalar: &core.Scalar{}, + }, + } + + request := admin.SignalSetRequest{ + Id: &core.SignalIdentifier{ + ExecutionId: &core.WorkflowExecutionIdentifier{ + Project: "project", + Domain: "domain", + Name: "name", + }, + SignalId: "signal", + }, + Value: unsupportedLiteral, // This will lead to valueType being nil + } + + // Invoke the function and check for the expected error + err := ValidateSignalSetRequest(ctx, repo, &request) + assert.NotNil(t, err) + + // Expected error message + assert.Contains(t, err.Error(), failedToValidateLiteralType) + }) } diff --git a/flyteadmin/pkg/manager/impl/validation/validation.go b/flyteadmin/pkg/manager/impl/validation/validation.go index 894eaee435..de2927495c 100644 --- a/flyteadmin/pkg/manager/impl/validation/validation.go +++ b/flyteadmin/pkg/manager/impl/validation/validation.go @@ -1,7 +1,6 @@ package validation import ( - "fmt" "net/url" "strconv" "strings" @@ -283,19 +282,9 @@ 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(), - ), - ) + err := validators.ValidateLiteralType(inputType) + if err != nil { + return errors.NewInvalidLiteralTypeError(name, err) } if !validators.AreTypesCastable(inputType, defaultInput.GetVar().GetType()) { diff --git a/flyteadmin/pkg/manager/impl/validation/validation_test.go b/flyteadmin/pkg/manager/impl/validation/validation_test.go index 0cc20cfd1b..265868789e 100644 --- a/flyteadmin/pkg/manager/impl/validation/validation_test.go +++ b/flyteadmin/pkg/manager/impl/validation/validation_test.go @@ -347,16 +347,7 @@ func TestValidateParameterMap(t *testing.T) { 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()) + assert.Contains(t, err.Error(), failedToValidateLiteralType) }) } diff --git a/flytepropeller/pkg/compiler/errors/compiler_error_test.go b/flytepropeller/pkg/compiler/errors/compiler_error_test.go index 5a993aeff2..8394ed5bb7 100644 --- a/flytepropeller/pkg/compiler/errors/compiler_error_test.go +++ b/flytepropeller/pkg/compiler/errors/compiler_error_test.go @@ -33,6 +33,7 @@ func TestErrorCodes(t *testing.T) { UnrecognizedValue: NewUnrecognizedValueErr("", ""), WorkflowBuildError: NewWorkflowBuildError(errors.New("")), NoNodesFound: NewNoNodesFoundErr(""), + InvalidLiteralTypeError: NewInvalidLiteralTypeErr("", "", errors.New("")), } for key, value := range testCases { @@ -48,6 +49,6 @@ func TestIncludeSource(t *testing.T) { SetConfig(Config{IncludeSource: true}) e = NewCycleDetectedInWorkflowErr("", "") - assert.Equal(t, e.source, "compiler_error_test.go:50") + assert.Equal(t, e.source, "compiler_error_test.go:51") SetConfig(Config{}) } diff --git a/flytepropeller/pkg/compiler/errors/compiler_errors.go b/flytepropeller/pkg/compiler/errors/compiler_errors.go index 9b762f72ca..b2e3796edd 100755 --- a/flytepropeller/pkg/compiler/errors/compiler_errors.go +++ b/flytepropeller/pkg/compiler/errors/compiler_errors.go @@ -96,6 +96,9 @@ const ( // Field not found in the dataclass FieldNotFoundError ErrorCode = "FieldNotFound" + + // IDL not found when variable binding + InvalidLiteralTypeError ErrorCode = "InvalidLiteralType" ) func NewBranchNodeNotSpecified(branchNodeID string) *CompileError { @@ -218,6 +221,14 @@ func NewMismatchingVariablesErr(nodeID, fromVar, fromType, toVar, toType string) ) } +func NewInvalidLiteralTypeErr(nodeID, inputVar string, err error) *CompileError { + return newError( + InvalidLiteralTypeError, + fmt.Sprintf("Failed to validate literal type for [%s] with err: %s", inputVar, err), + nodeID, + ) +} + func NewMismatchingBindingsErr(nodeID, sinkParam, expectedType, receivedType string) *CompileError { return newError( MismatchingBindings, diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go index 0976df669b..21250bd28d 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs.go @@ -42,6 +42,13 @@ func validateInputs(nodeID common.NodeID, iface *core.TypedInterface, inputs cor default: inputType = validators.LiteralTypeForLiteral(inputVal) } + + err := validators.ValidateLiteralType(inputType) + if err != nil { + errs.Collect(errors.NewInvalidLiteralTypeErr(nodeID, inputVar, err)) + continue + } + if !validators.AreTypesCastable(inputType, v.Type) { errs.Collect(errors.NewMismatchingTypesErr(nodeID, inputVar, v.Type.String(), inputType.String())) continue diff --git a/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go b/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go index 01d667c35d..d77aafec49 100644 --- a/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go +++ b/flytepropeller/pkg/compiler/transformers/k8s/inputs_test.go @@ -1 +1,55 @@ package k8s + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core" + "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/common" + "github.com/flyteorg/flyte/flytepropeller/pkg/compiler/errors" +) + +func TestValidateInputs_InvalidLiteralType(t *testing.T) { + nodeID := common.NodeID("test-node") + + iface := &core.TypedInterface{ + Inputs: &core.VariableMap{ + Variables: map[string]*core.Variable{ + "input1": { + Type: &core.LiteralType{ + Type: &core.LiteralType_Simple{ + Simple: 1000, + }, + }, + }, + }, + }, + } + + inputs := core.LiteralMap{ + Literals: map[string]*core.Literal{ + "input1": nil, // Set this to nil to trigger the nil case + }, + } + + errs := errors.NewCompileErrors() + ok := validateInputs(nodeID, iface, inputs, errs) + + assert.False(t, ok) + assert.True(t, errs.HasErrors()) + + idlNotFound := false + var errMsg string + for _, err := range errs.Errors().List() { + if err.Code() == "InvalidLiteralType" { + idlNotFound = true + errMsg = err.Error() + break + } + } + assert.True(t, idlNotFound, "Expected InvalidLiteralType error was not found in errors") + + expectedContainedErrorMsg := "Failed to validate literal type" + assert.Contains(t, errMsg, expectedContainedErrorMsg) +} diff --git a/flytepropeller/pkg/compiler/validators/utils.go b/flytepropeller/pkg/compiler/validators/utils.go index fb4ba04548..fb05e32bb3 100644 --- a/flytepropeller/pkg/compiler/validators/utils.go +++ b/flytepropeller/pkg/compiler/validators/utils.go @@ -249,6 +249,23 @@ func literalTypeForLiterals(literals []*core.Literal) *core.LiteralType { return buildMultipleTypeUnion(innerType) } +// ValidateLiteralType check if the literal type is valid, return error if the literal is invalid. +func ValidateLiteralType(lt *core.LiteralType) error { + if lt == nil { + err := fmt.Errorf("got unknown literal type: [%v].\n"+ + "Suggested solution: Please update all your Flyte deployment images to the latest version and try again", lt) + return err + } + if lt.GetCollectionType() != nil { + return ValidateLiteralType(lt.GetCollectionType()) + } + if lt.GetMapValueType() != nil { + return ValidateLiteralType(lt.GetMapValueType()) + } + + return nil +} + // LiteralTypeForLiteral gets LiteralType for literal, nil if the value of literal is unknown, or type collection/map of // type None if the literal is a non-homogeneous type. func LiteralTypeForLiteral(l *core.Literal) *core.LiteralType { diff --git a/flytepropeller/pkg/controller/nodes/array/handler.go b/flytepropeller/pkg/controller/nodes/array/handler.go index 5e9f910e14..6859bf46f0 100644 --- a/flytepropeller/pkg/controller/nodes/array/handler.go +++ b/flytepropeller/pkg/controller/nodes/array/handler.go @@ -191,8 +191,15 @@ func (a *arrayNodeHandler) Handle(ctx context.Context, nCtx interfaces.NodeExecu } size := -1 - for _, variable := range literalMap.Literals { + for key, variable := range literalMap.Literals { literalType := validators.LiteralTypeForLiteral(variable) + err := validators.ValidateLiteralType(literalType) + if err != nil { + errMsg := fmt.Sprintf("Failed to validate literal type for [%s] with err: %s", key, err) + return handler.DoTransition(handler.TransitionTypeEphemeral, + handler.PhaseInfoFailure(idlcore.ExecutionError_USER, errors.IDLNotFoundErr, errMsg, nil), + ), nil + } switch literalType.Type.(type) { case *idlcore.LiteralType_CollectionType: collectionLength := len(variable.GetCollection().Literals) diff --git a/flytepropeller/pkg/controller/nodes/array/handler_test.go b/flytepropeller/pkg/controller/nodes/array/handler_test.go index cb2f2898a6..08eea22e09 100644 --- a/flytepropeller/pkg/controller/nodes/array/handler_test.go +++ b/flytepropeller/pkg/controller/nodes/array/handler_test.go @@ -20,6 +20,7 @@ import ( execmocks "github.com/flyteorg/flyte/flytepropeller/pkg/controller/executors/mocks" "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes" "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/catalog" + "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/errors" gatemocks "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/gate/mocks" "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/handler" "github.com/flyteorg/flyte/flytepropeller/pkg/controller/nodes/interfaces" @@ -943,6 +944,68 @@ func TestHandleArrayNodePhaseExecuting(t *testing.T) { } } +func TestHandle_InvalidLiteralType(t *testing.T) { + ctx := context.Background() + scope := promutils.NewTestScope() + dataStore, err := storage.NewDataStore(&storage.Config{ + Type: storage.TypeMemory, + }, scope) + assert.NoError(t, err) + nodeHandler := &mocks.NodeHandler{} + + // Initialize ArrayNodeHandler + arrayNodeHandler, err := createArrayNodeHandler(ctx, t, nodeHandler, dataStore, scope) + assert.NoError(t, err) + + // Test cases + tests := []struct { + name string + inputLiteral *idlcore.Literal + expectedTransitionType handler.TransitionType + expectedPhase handler.EPhase + expectedErrorCode string + expectedContainedErrorMsg string + }{ + { + name: "InvalidLiteralType", + inputLiteral: &idlcore.Literal{ + Value: &idlcore.Literal_Scalar{ + Scalar: &idlcore.Scalar{}, + }, + }, + expectedTransitionType: handler.TransitionTypeEphemeral, + expectedPhase: handler.EPhaseFailed, + expectedErrorCode: errors.IDLNotFoundErr, + expectedContainedErrorMsg: "Failed to validate literal type", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + // Create NodeExecutionContext + literalMap := &idlcore.LiteralMap{ + Literals: map[string]*idlcore.Literal{ + "invalidInput": test.inputLiteral, + }, + } + arrayNodeState := &handler.ArrayNodeState{ + Phase: v1alpha1.ArrayNodePhaseNone, + } + nCtx := createNodeExecutionContext(dataStore, newBufferedEventRecorder(), nil, literalMap, &arrayNodeSpec, arrayNodeState, 0, workflowMaxParallelism) + + // Evaluate node + transition, err := arrayNodeHandler.Handle(ctx, nCtx) + assert.NoError(t, err) + + // Validate results + assert.Equal(t, test.expectedTransitionType, transition.Type()) + assert.Equal(t, test.expectedPhase, transition.Info().GetPhase()) + assert.Equal(t, test.expectedErrorCode, transition.Info().GetErr().Code) + assert.Contains(t, transition.Info().GetErr().Message, test.expectedContainedErrorMsg) + }) + } +} + func TestHandleArrayNodePhaseExecutingSubNodeFailures(t *testing.T) { ctx := context.Background() diff --git a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go index 5c0ac0c30b..ba94bdadec 100644 --- a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go +++ b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer.go @@ -54,6 +54,10 @@ func GenerateTaskOutputsFromArtifact(id core.Identifier, taskInterface core.Type expectedVarType := outputVariables[artifactData.Name].GetType() inputType := validators.LiteralTypeForLiteral(artifactData.Value) + err := validators.ValidateLiteralType(inputType) + if err != nil { + return nil, fmt.Errorf("failed to validate literal type for %s with err: %s", artifactData.Name, err) + } if !validators.AreTypesCastable(inputType, expectedVarType) { return nil, fmt.Errorf("unexpected artifactData: [%v] type: [%v] does not match any task output type: [%v]", artifactData.Name, inputType, expectedVarType) } diff --git a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go index 1c6b9e2e1b..92e4c82926 100644 --- a/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go +++ b/flytepropeller/pkg/controller/nodes/catalog/datacatalog/transformer_test.go @@ -331,3 +331,43 @@ func TestDatasetIDToIdentifier(t *testing.T) { assert.Equal(t, "d", id.Domain) assert.Equal(t, "v", id.Version) } + +func TestGenerateTaskOutputsFromArtifact_IDLNotFound(t *testing.T) { + taskID := core.Identifier{ + ResourceType: core.ResourceType_TASK, + Project: "project", + Domain: "domain", + Name: "name", + Version: "version", + } + + taskInterface := core.TypedInterface{ + Outputs: &core.VariableMap{ + Variables: map[string]*core.Variable{ + "output1": { + Type: &core.LiteralType{ + Type: &core.LiteralType_Simple{ + Simple: 1000, + }, + }, + }, + }, + }, + } + + artifact := &datacatalog.Artifact{ + Id: "artifact_id", + Data: []*datacatalog.ArtifactData{ + { + Name: "output1", + Value: &core.Literal{}, // This will cause LiteralTypeForLiteral to return nil + }, + }, + } + + _, err := GenerateTaskOutputsFromArtifact(taskID, taskInterface, artifact) + + expectedContainedErrorMsg := "failed to validate literal type" + assert.Error(t, err) + assert.Contains(t, err.Error(), expectedContainedErrorMsg) +} diff --git a/flytepropeller/pkg/controller/nodes/errors/codes.go b/flytepropeller/pkg/controller/nodes/errors/codes.go index 53d3bbc8d7..dafccdc5c0 100644 --- a/flytepropeller/pkg/controller/nodes/errors/codes.go +++ b/flytepropeller/pkg/controller/nodes/errors/codes.go @@ -27,4 +27,5 @@ const ( CatalogCallFailed ErrorCode = "CatalogCallFailed" InvalidArrayLength ErrorCode = "InvalidArrayLength" PromiseAttributeResolveError ErrorCode = "PromiseAttributeResolveError" + IDLNotFoundErr ErrorCode = "IDLNotFoundErr" )