Skip to content

(fix): YAML parsing of JSON number to int#21690

Open
justinkaseman wants to merge 1 commit intodevelopfrom
fix/job-proposal-yaml-number
Open

(fix): YAML parsing of JSON number to int#21690
justinkaseman wants to merge 1 commit intodevelopfrom
fix/job-proposal-yaml-number

Conversation

@justinkaseman
Copy link
Contributor

Requires

Supports

@justinkaseman justinkaseman requested review from a team as code owners March 25, 2026 01:32
Copilot AI review requested due to automatic review settings March 25, 2026 01:32
@github-actions
Copy link
Contributor

👋 justinkaseman, thanks for creating this pull request!

To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team.

Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks!

@github-actions
Copy link
Contributor

✅ No conflicts with other open PRs targeting develop

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Risk Rating: MEDIUM (changes behavior of a shared input decoding helper used across CRE job spec handling)

This PR fixes CRE job input unmarshalling when YAML-originated integers travel through a JSON pipeline with json.Decoder.UseNumber(), causing numeric values to arrive as json.Number and fail decoding into int fields.

Changes:

  • Sanitize JobSpecInput by recursively converting json.Number values into Go numeric primitives before YAML marshal/unmarshal.
  • Add regression tests covering json.Numberint decoding (including nested structs/slices) and a native-int happy path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
deployment/cre/jobs/types/job_spec.go Converts json.Number values prior to YAML marshal/unmarshal so numeric YAML inputs decode into typed numeric fields correctly.
deployment/cre/jobs/types/job_spec_test.go Adds regression tests to ensure UnmarshalTo handles json.Number for int fields and still works for native int values.

Areas requiring scrupulous human review:

  • convertValue conversion semantics (int vs float vs string fallback) to ensure it matches expectations for all job templates/inputs that can flow through the YAML→JSON→env→JSON pipeline.

Suggested reviewers (per .github/CODEOWNERS for /deployment/cre):

  • @smartcontractkit/keystone
  • @smartcontractkit/operations-platform

@trunk-io
Copy link

trunk-io bot commented Mar 25, 2026

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@cl-sonarqube-production
Copy link


func (j JobSpecInput) UnmarshalTo(target any) error {
bytes, err := yaml.Marshal(j)
sanitized := convertJSONNumbers(map[string]any(j))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is going to work... at this stage we already lost int types in the nested map[string]any array

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants