template: preserve quoted numeric strings in DeepCopyWithTemplate

[holger-waschke]

This fixes an issue where a quoted numeric string produced by DeepCopyWithTemplate could end up being serialized as an integer instead of a string.

One concrete case is Jira custom fields that expect a payload like:

{"id":"15129"}

Before this change, the rendered field could become:

{"customfield_11209":{"id":15129}}

In that form, id was sent as an integer, which Jira rejected.

With this change, quoted numeric values are preserved as strings, so the generated payload keeps the expected type.

A regression test was added to cover this case.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

DeepCopyWithTemplate now preserves rendered whitespace-only strings and returns parsed YAML strings unchanged. Tests cover quoted numeric strings at the top level and inside nested maps.

Changes

Template processing

Layer / File(s)	Summary
Rendered value processing template/template.go	DeepCopyWithTemplate handles parsed strings and whitespace-only rendered strings before normalizing structured YAML values. Parse errors still return the rendered string.
Quoted string validation template/template_test.go	Tests verify that quoted numeric template results become unquoted strings at the top level and inside nested maps.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• prometheus/alertmanager#5304: Both changes modify DeepCopyWithTemplate and its tests for YAML parsing and rendered scalar values.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the bug, user impact, and regression test, but omits the required checklist and release-notes section.	Complete the required checklist and add the release-notes block with an appropriate BUGFIX entry.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the template area and the primary change to preserve quoted numeric strings.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

template/template_test.go (1)

696-703: Add one nested payload regression case for end-to-end parity.

Current case validates scalar behavior well, but a nested object case (Jira-style field map) would better lock the original regression path.

🧪 Suggested test case extension

@@
 		{
 			title: "quoted numeric string stays string",
 			input: "hello",
 			fn: TemplateFunc(func(string) (string, error) {
 				return "\"123\"", nil
 			}),
 			want: "123",
 		},
+		{
+			title: "quoted numeric string stays string in nested map",
+			input: map[string]any{
+				"customfield_11209": map[string]any{
+					"id": "TOKEN",
+				},
+			},
+			fn: TemplateFunc(func(s string) (string, error) {
+				if s == "TOKEN" {
+					return "\"15129\"", nil
+				}
+				return s, nil
+			}),
+			want: map[string]any{
+				"customfield_11209": map[string]any{
+					"id": "15129",
+				},
+			},
+		},

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@template/template_test.go` around lines 696 - 703, Add a new table test case
to cover the nested-object regression: create a test row (e.g., title "quoted
numeric string in nested payload stays string") alongside the existing case in
template_test.go that uses TemplateFunc to return a nested payload like a
Jira-style fields map where the value is a quoted numeric string (for example a
map {"fields": {"customfield_12345": "\"123\""}}), and assert the processed
output preserves the string value "123" (not numeric). Reference the existing
test harness and TemplateFunc usage so the new case mirrors the scalar case but
with the nested object shape to lock the regression.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@template/template_test.go`:
- Around line 696-703: Add a new table test case to cover the nested-object
regression: create a test row (e.g., title "quoted numeric string in nested
payload stays string") alongside the existing case in template_test.go that uses
TemplateFunc to return a nested payload like a Jira-style fields map where the
value is a quoted numeric string (for example a map {"fields":
{"customfield_12345": "\"123\""}}), and assert the processed output preserves
the string value "123" (not numeric). Reference the existing test harness and
TemplateFunc usage so the new case mirrors the scalar case but with the nested
object shape to lock the regression.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 14633b63-013a-4316-bf02-0988e8a40f0a

📥 Commits

Reviewing files that changed from the base of the PR and between 35cbf8d and 3a36ca5.

📒 Files selected for processing (2)

• template/template.go
• template/template_test.go

[TheMeier]

Thank you @holger-waschke. LGTM
The addtional checks from proposed from rabbitai sound sensible to me, what do you think?

[TheMeier]

Loosely related to #5012

[holger-waschke]

Thank you @holger-waschke. LGTM The addtional checks from proposed from rabbitai sound sensible to me, what do you think?

Looks reasonable to me, I added the unit test to the template_test.go

[themavik]

test review body 12345 unique

[themavik]

Using a string type assert after yaml.Unmarshal fixes quoted numeric template output regressing to float recursion in DeepCopyWithTemplate. nit: TrimSpace-only early return means quoted strings with only whitespace still recurse—odd but probably harmless.

[holger-waschke]

Updated branch with main, resolving merge conflicts from #5304. Ready to be merged.

[siavashs]

I think this PR fixes a narrow issue, but we need more test cases here since this affects 3 integration: Jira, PagerDuty and Webhook:

• a Jira test which uses toJson on a numeric string and expects and output like {"id":"1234"} in the body
• regression test for ordinary string like issue # 1234
• test for existing numeric values being rendered as integers
• test for a JSON object with numeric strings stays as is

[SoloJacobs]

@holger-waschke There is a ci failure now.

[holger-waschke]

@holger-waschke There is a ci failure now.

Whoopsie, all green now.