Ensure Alerting Rules Not Generated for Empty Alerting Fields on TF State File #939
Conversation
… for instances where the Alerting field on the Terraform state file is not specified by the user
|
In order to lower resource usage and have a faster runtime, PRs will not run Cloud tests automatically. To do so, a Grafana Labs employee must promote the Drone build. For maintainers, it's better to run only the Cloud tests you need, rather than all of them. You can do so by setting the following parameter when promoting:
|
There was a problem hiding this comment.
I "approve" this - I'll ask one of the provider maintainers to also give this a look today; since it's my first review of this codebase I want to have an expert review my review.
- if the Alerting Block within the Terraform State file is populated, then the appropriate recording/alerting/labels are generated
- if the Alerting Block within the Terraform State file is non-existent, then no alerting rules are generated
- if the Alerting Block within the Terraform State file is an empty block ({}) then the default alerting rules are generated
👍
| if len(alerting) > 0 { | ||
| alert := alerting[0].(map[string]interface{}) | ||
| tfalerting = packAlerting(alert) | ||
| alerting, alertingOK := d.GetOk("alerting") |
There was a problem hiding this comment.
It's more idiomatic to collapse this into the conditional - you don't need to worry about alerting, and alertingOK "contaminating" the scope outside of the block in that case:
if alerting, ok := d.GetOk("alerting"); ok {
...
}
// alerting is no longer declared here, compile-time error:
alerting = nil
| tfalerting = packAlerting(alert) | ||
| } | ||
|
|
||
| slo := gapi.Slo{ |
There was a problem hiding this comment.
Instead of an early return here, consider just modifying the Slo object and having a single return for a no- error execution. I think it reduces the mental burden of modifying it later:
slo := gapi.Slo{
UUID: d.Id(),
Name: tfname,
Objectives: tfobjective,
Query: tfquery,
Alerting: nil,
Labels: tflabels,
}
if alerting, ok := d.GetOk("alerting"); ok {
...
tfAlerting := packAlerting(alert)
slo.Alerting = &tfAlerting
}
return slo, nil
There was a problem hiding this comment.
LGTM but if you can address @joeblubaugh comments it will look much better 😬
| if len(alerting) > 0 { | ||
| alert := alerting[0].(map[string]interface{}) | ||
| tfalerting = packAlerting(alert) | ||
| alerting, alertingOK := d.GetOk("alerting") |
| tfalerting = packAlerting(alert) | ||
| } | ||
|
|
||
| slo := gapi.Slo{ |
Updates Terraform Provider to ensure Alerting rules are not generated for instances where the Alerting field on the Terraform state file is not specified by the user
This PR addresses the three scenarios on the state of the Alerting Field within a Terraform State file:
{}) then the default alerting rules are generated