fix: handle null values properly in trigger payload construction #3738
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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 argoproj#3719 Signed-off-by: puretension <[email protected]>
puretension
force-pushed
the
fix/null-values-invalid-json-3719
branch
from
eafb47f to
466a8bd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes the issue where null values in trigger payloads create invalid JSON. The getValueByKey function now
properly handles gjson.Null types by returning the raw "null" string instead of an empty string.
Motivation
Fixes #3719 - When a trigger defines a payload with a source value that is null, the JSON passed as the
trigger payload becomes invalid (e.g., {"nullable_key": ,} instead of {"nullable_key": null}). This
breaks JSON parsing in downstream systems.
Describe how you validated your changes
• Added comprehensive unit tests for null value handling in TestGetValueByKey_NullValue
• Added integration test TestConstructPayload_NullValue to verify end-to-end null value preservation
• All existing tests continue to pass
• Verified that the generated JSON is valid and contains proper null values
Additional Notes
The fix specifically handles the gjson.Null type by returning res.Raw (which contains "null") instead of
res.String() (which returns empty string for null values). This ensures JSON validity while preserving
the semantic meaning of null values.