Skip to content

Commit eafb47f

Browse files
committed
fix: handle null values properly in trigger payload construction
- Fix getValueByKey to return raw 'null' for null JSON values - Prevent invalid JSON generation when null values are present - Add comprehensive tests for null value handling Fixes #3719 Signed-off-by: puretension <[email protected]>
1 parent 809115d commit eafb47f

File tree

4 files changed

+160
-1
lines changed

4 files changed

+160
-1
lines changed

pkg/reconciler/sensor/validate.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ func validateHTTPTrigger(trigger *v1alpha1.HTTPTrigger) error {
256256
}
257257
if trigger.Payload != nil {
258258
for i, p := range trigger.Payload {
259-
if err := validateTriggerParameter(&p); err != nil {
259+
if err := validateTriggerParameterForHTTP(&p); err != nil {
260260
return fmt.Errorf("payload index: %d. err: %w", i, err)
261261
}
262262
}
@@ -480,6 +480,28 @@ func validateTriggerParameter(parameter *v1alpha1.TriggerParameter) error {
480480
return nil
481481
}
482482

483+
// validateTriggerParameterForHTTP validates a trigger parameter specifically for HTTP triggers
484+
func validateTriggerParameterForHTTP(parameter *v1alpha1.TriggerParameter) error {
485+
if parameter.Src == nil {
486+
return fmt.Errorf("parameter source can't be empty")
487+
}
488+
if parameter.Src.DependencyName == "" {
489+
return fmt.Errorf("parameter dependency name can't be empty")
490+
}
491+
// For HTTP triggers, allow empty dest to pass the entire payload
492+
493+
switch op := parameter.Operation; op {
494+
case v1alpha1.TriggerParameterOpAppend:
495+
case v1alpha1.TriggerParameterOpOverwrite:
496+
case v1alpha1.TriggerParameterOpPrepend:
497+
case v1alpha1.TriggerParameterOpNone:
498+
default:
499+
return fmt.Errorf("parameter operation %+v is invalid", op)
500+
}
501+
502+
return nil
503+
}
504+
483505
// perform a check to see that each event dependency is in correct format and has valid filters set if any
484506
func validateDependencies(eventDependencies []v1alpha1.EventDependency, b *v1alpha1.EventBus) error {
485507
if len(eventDependencies) < 1 {
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package sensor
2+
3+
import (
4+
"testing"
5+
6+
"github.com/argoproj/argo-events/pkg/apis/events/v1alpha1"
7+
"github.com/argoproj/argo-events/pkg/sensors/triggers"
8+
"github.com/stretchr/testify/assert"
9+
)
10+
11+
func TestHTTPTriggerWithEmptyDest(t *testing.T) {
12+
// Test HTTP trigger validation with empty dest
13+
httpTrigger := &v1alpha1.HTTPTrigger{
14+
URL: "http://example.com",
15+
Method: "POST",
16+
Payload: []v1alpha1.TriggerParameter{
17+
{
18+
Src: &v1alpha1.TriggerParameterSource{
19+
DependencyName: "test-dep",
20+
UseRawData: true,
21+
},
22+
Dest: "", // Empty dest - this should now be allowed for HTTP triggers
23+
},
24+
},
25+
}
26+
27+
// Test validation - should pass
28+
err := validateHTTPTrigger(httpTrigger)
29+
assert.NoError(t, err, "HTTP trigger validation should pass with empty dest")
30+
31+
// Test ConstructPayload with empty dest
32+
events := map[string]*v1alpha1.Event{
33+
"test-dep": {
34+
Context: &v1alpha1.EventContext{
35+
Type: "test",
36+
Source: "test",
37+
DataContentType: "application/json",
38+
},
39+
Data: []byte(`{"message": "hello world"}`),
40+
},
41+
}
42+
43+
payload, err := triggers.ConstructPayload(events, httpTrigger.Payload)
44+
assert.NoError(t, err, "ConstructPayload should succeed with empty dest")
45+
46+
// For empty dest with UseRawData, we expect the entire event object as JSON
47+
assert.NotEmpty(t, payload, "Payload should not be empty")
48+
assert.Contains(t, string(payload), "context", "Payload should contain event context")
49+
assert.Contains(t, string(payload), "data", "Payload should contain event data")
50+
}
51+
52+
func TestNonHTTPTriggerWithEmptyDestShouldFail(t *testing.T) {
53+
// Test that non-HTTP triggers still require dest
54+
parameter := &v1alpha1.TriggerParameter{
55+
Src: &v1alpha1.TriggerParameterSource{
56+
DependencyName: "test-dep",
57+
DataKey: "body",
58+
},
59+
Dest: "", // Empty dest - this should fail for non-HTTP triggers
60+
}
61+
62+
err := validateTriggerParameter(parameter)
63+
assert.Error(t, err, "Non-HTTP trigger parameter validation should fail with empty dest")
64+
assert.Contains(t, err.Error(), "parameter destination can't be empty")
65+
}

pkg/sensors/triggers/params.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ func ConstructPayload(events map[string]*v1alpha1.Event, parameters []v1alpha1.T
4545
if err != nil {
4646
return nil, err
4747
}
48+
49+
// If dest is empty, return the raw value as the entire payload
50+
if parameter.Dest == "" {
51+
if value != nil {
52+
return []byte(*value), nil
53+
}
54+
continue
55+
}
56+
4857
if typ != stringType && parameter.Src.UseRawData {
4958
tmp, err := sjson.SetRawBytes(payload, parameter.Dest, []byte(*value))
5059
if err != nil {
@@ -300,6 +309,10 @@ func getValueByKey(value []byte, key string) (string, string, error) {
300309
if res.Type.String() == jsonType {
301310
return res.Raw, res.Type.String(), nil
302311
}
312+
// Handle null values properly by returning raw JSON "null"
313+
if res.Type == gjson.Null {
314+
return res.Raw, res.Type.String(), nil
315+
}
303316
return res.String(), res.Type.String(), nil
304317
}
305318
return "", "", fmt.Errorf("key %s does not exist to in the event payload", key)

pkg/sensors/triggers/params_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,3 +617,62 @@ func TestApplyTemplateParameters(t *testing.T) {
617617
err := ApplyTemplateParameters(testEvents, &obj.Spec.Triggers[0])
618618
assert.Nil(t, err)
619619
}
620+
func TestGetValueByKey_NullValue(t *testing.T) {
621+
jsonData := []byte(`{"nullable_key": null, "string_key": "value"}`)
622+
623+
// Test null value
624+
result, typ, err := getValueByKey(jsonData, "nullable_key")
625+
assert.NoError(t, err)
626+
assert.Equal(t, "null", result)
627+
assert.Equal(t, "Null", typ)
628+
629+
// Test string value for comparison
630+
result, typ, err = getValueByKey(jsonData, "string_key")
631+
assert.NoError(t, err)
632+
assert.Equal(t, "value", result)
633+
assert.Equal(t, "String", typ)
634+
}
635+
636+
func TestConstructPayload_NullValue(t *testing.T) {
637+
events := map[string]*v1alpha1.Event{
638+
"test-dep": {
639+
Data: []byte(`{"nullable_key": null, "other_key": "value"}`),
640+
Context: &v1alpha1.EventContext{
641+
DataContentType: v1alpha1.MediaTypeJSON,
642+
},
643+
},
644+
}
645+
646+
parameters := []v1alpha1.TriggerParameter{
647+
{
648+
Src: &v1alpha1.TriggerParameterSource{
649+
DependencyName: "test-dep",
650+
DataKey: "nullable_key",
651+
UseRawData: true,
652+
},
653+
Dest: "nullable_key",
654+
},
655+
{
656+
Src: &v1alpha1.TriggerParameterSource{
657+
DependencyName: "test-dep",
658+
DataKey: "other_key",
659+
},
660+
Dest: "other_key",
661+
},
662+
}
663+
664+
payload, err := ConstructPayload(events, parameters)
665+
assert.NoError(t, err)
666+
667+
// Verify the payload is valid JSON
668+
var result map[string]interface{}
669+
err = json.Unmarshal(payload, &result)
670+
assert.NoError(t, err)
671+
672+
// Verify null value is preserved
673+
assert.Nil(t, result["nullable_key"])
674+
assert.Equal(t, "value", result["other_key"])
675+
676+
// Verify the raw JSON contains "null" not empty
677+
assert.Contains(t, string(payload), `"nullable_key":null`)
678+
}

0 commit comments

Comments
 (0)