Validate monitor in pre-flight #639
Conversation
Using CustomizeDiff, we can call the validation API to provide more validation for monitor creation. This allows returning an error during the `plan` phase, instead of waiting for `apply`. Closes #414
therve
changed the title
therve
force-pushed
the
therve/validate-monitor
branch
from
e7e72be to
94b9b03
Compare
therve
force-pushed
the
therve/validate-monitor
branch
from
94b9b03 to
e6ad977
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -342,6 +349,22 @@ func resourceDatadogMonitorExists(d *schema.ResourceData, meta interface{}) (b b | |||
| return true, nil | |||
| } | |||
|
|
|||
| func resourceDatadogMonitorCustomizeDiff(diff *schema.ResourceDiff, meta interface{}) error { | |||
| if _, ok := diff.GetOk("query"); !ok { | |||
There was a problem hiding this comment.
Does this mean we only validate the monitor if the query changed?
Will we have access to all the other non changed fields like thresholds?
(Can we debug log something here about not validating)
There was a problem hiding this comment.
No it means we only validate if we have a value, in particular when the query depends on another resource that's not resolved.
There was a problem hiding this comment.
Thanks. My main concern was that this diff object wouldn't contain the entire monitor object and only include what changed. But we discussed offline and that shouldn't be the case.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| datadogClientV1 := providerConf.DatadogClientV1 | ||
| authV1 := providerConf.AuthV1 | ||
| if _, _, err := datadogClientV1.MonitorsApi.ValidateMonitor(authV1).Body(*m).Execute(); err != nil { | ||
| return translateClientError(err, "error validating monitor") |
There was a problem hiding this comment.
If the error is a http400, we probably want to return a more specific error message with the invalid parts of the monitor indicated. The reasons be returned in the errors key of the response.
There was a problem hiding this comment.
I think that's what translateClientError is supposed to do.
| @@ -195,11 +202,15 @@ func resourceDatadogMonitor() *schema.Resource { | |||
| Type: schema.TypeBool, | |||
| Optional: true, | |||
| }, | |||
| "validate": { | |||
There was a problem hiding this comment.
What happens if the only change is to flip this from default true to false? If there aren't any changes to send to the API, I'm not sure if the endpoint accepts PUT /monitor/<id> {}, would be good to verify.
There was a problem hiding this comment.
You mean if the monitor object is already created, and we change the flag? I'll test it.
There was a problem hiding this comment.
Good catch, removed it from the diff explicitly.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Awesome 🎉 . Just wondering @therve , are you planning to cut a release soon so we can leverage plan-time validation? Looking forward trying this out! 🙏 |
Using CustomizeDiff, we can call the validation API to provide more validation for monitor creation. This allows returning an error during the
planphase, instead of waiting forapply.Closes #414