Fix template execution test failure#4
Fix template execution test failure#4abhinavshashank wants to merge 1 commit intoece9bb928fa4-amg-DO-NOT-DELETEfrom
Conversation
chriscerie
left a comment
There was a problem hiding this comment.
- Please link the failing test for context
- How is this tested?
| // resetTimeNow resets the global variable timeNow to the default value, which is time.Now | ||
| func resetTimeNow() { | ||
| timeNow = time.Now | ||
| } | ||
|
|
||
| func mockTimeNow(constTime time.Time) func() { | ||
| timeNow = func() time.Time { | ||
| return constTime | ||
| } | ||
| return resetTimeNow | ||
| } |
There was a problem hiding this comment.
Used by templates/template_data_test.go which was added in the original CVE fix PR (#2) calls mockTimeNow() but the function wasn't
defined anywhere. Found this out when aws-grafana CI ran tests.
| } | ||
| textTmpl := tmpltext.New("").Option("missingkey=zero") | ||
| textTmpl, err := textTmpl.Parse(text) | ||
| func executeTextStringWithLimit(tmpl *Template, name string, data *ExtendedData) (string, error) { |
There was a problem hiding this comment.
Why are these names changing? Is this change backported?
There was a problem hiding this comment.
These are our changes, not a direct backport from upstream.
The upstream v11.6.x fix (grafana/alerting@35f3427554ec) uses tmpl.Text() to get the base
template before applying size limits. But tmpl.Text() doesn't exist in v10's
prometheus-alertmanager (requires Go 1.22+, v10 uses Go 1.21).
So changed the fix to call tmpl.ExecuteTextString(name, data) (the existing v10 method)
and check output size through LimitedWriter afterward. S
The func rename was just to clarify that this now forces size limits.
|
Please build an image with this commit to test in preprod |
Fix template execution test failure
Fixes the cause of failure for
TestEmailNotifierIntegrationtest where executeTextString parsed template names as text instead of executing them. Email subjects came back empty because "default.title" was treated as literal text to parse, not a template name to execute.TestEmailNotifierIntegration - expected "[FIRING:1] (AlwaysFiring warning)", got empty string.
Test failure: https://github.com/aws-observability/aws-grafana/actions/runs/22005371748/job/63593865329?pr=1145