From 0e8f2dedc24ac9dc2daa440f5254515573d3fce9 Mon Sep 17 00:00:00 2001 From: Cedric Date: Thu, 1 Aug 2024 21:43:51 +0100 Subject: [PATCH] [fix] Check if DefaultConfig is nil before merging (#13988) --- core/services/workflows/engine.go | 4 ++ core/services/workflows/engine_test.go | 53 +++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/core/services/workflows/engine.go b/core/services/workflows/engine.go index a5fb9a25ea7..c85d4a03b24 100644 --- a/core/services/workflows/engine.go +++ b/core/services/workflows/engine.go @@ -713,6 +713,10 @@ func (e *Engine) configForStep(ctx context.Context, executionID string, step *st return step.config, nil } + if capConfig.DefaultConfig == nil { + return step.config, nil + } + // Merge the configs for now; note that this means that a workflow can override // all of the config set by the capability. This is probably not desirable in // the long-term, but we don't know much about those use cases so stick to a simpler diff --git a/core/services/workflows/engine_test.go b/core/services/workflows/engine_test.go index b8d5a9591ed..3af87284131 100644 --- a/core/services/workflows/engine_test.go +++ b/core/services/workflows/engine_test.go @@ -123,7 +123,7 @@ func (t testConfigProvider) ConfigForCapability(ctx context.Context, capabilityI return t.configForCapability(ctx, capabilityID, donID) } - return capabilities.CapabilityConfiguration{DefaultConfig: values.EmptyMap()}, nil + return capabilities.CapabilityConfiguration{}, nil } // newTestEngine creates a new engine with some test defaults. @@ -1063,3 +1063,54 @@ func TestEngine_MergesWorkflowConfigAndCRConfig(t *testing.T) { assert.Equal(t, m.(map[string]any)["deltaStage"], "1s") assert.Equal(t, m.(map[string]any)["schedule"], "allAtOnce") } + +func TestEngine_HandlesNilConfigOnchain(t *testing.T) { + ctx := testutils.Context(t) + reg := coreCap.NewRegistry(logger.TestLogger(t)) + + trigger, _ := mockTrigger(t) + + require.NoError(t, reg.Add(ctx, trigger)) + require.NoError(t, reg.Add(ctx, mockConsensus())) + writeID := "write_polygon-testnet-mumbai@1.0.0" + + gotConfig := values.EmptyMap() + target := newMockCapability( + // Create a remote capability so we don't use the local transmission protocol. + capabilities.MustNewRemoteCapabilityInfo( + writeID, + capabilities.CapabilityTypeTarget, + "a write capability targeting polygon testnet", + &capabilities.DON{ID: 1}, + ), + func(req capabilities.CapabilityRequest) (capabilities.CapabilityResponse, error) { + gotConfig = req.Config + + return capabilities.CapabilityResponse{ + Value: req.Inputs, + }, nil + }, + ) + require.NoError(t, reg.Add(ctx, target)) + + eng, testHooks := newTestEngine( + t, + reg, + simpleWorkflow, + ) + reg.SetLocalRegistry(testConfigProvider{}) + + servicetest.Run(t, eng) + + eid := getExecutionId(t, eng, testHooks) + + state, err := eng.executionStates.Get(ctx, eid) + require.NoError(t, err) + + assert.Equal(t, state.Status, store.StatusCompleted) + + m, err := values.Unwrap(gotConfig) + require.NoError(t, err) + // The write target config contains three keys + assert.Len(t, m.(map[string]any), 3) +}