cloudposse / terraform-datadog-platform

Terraform module to configure and provision Datadog monitors, custom RBAC roles with permissions, Datadog synthetic tests, Datadog child organizations, and other Datadog resources from a YAML configuration, complete with automated tests.

https://cloudposse.com/accelerate

Apache License 2.0

104 stars 55 forks source link

Fix: add missing inputs to datadog_monitor resource, update datadog_monitors var to use map(any) #86

Closed mldevpants closed 9 months ago

mldevpants commented 9 months ago

what

The main resource for "datadog_monitor" was missing several inputs, even the examples include those inputs but they all set to "null" when applied or plan.

main.tf to include the missing inputs with lookup function
datadog_monitors variable was updated to use map(any) with objects in description
no README reference was found to update
examples have all these inputs, no need to add

why

Priority and others are essential part of the monitors to be used in some cases

references

85

mldevpants commented 9 months ago

the change should should be back compatible since the var that was updated can include any map, while the resource uses lookup with default to null for the new inputs, if few of the original inputs that don't use lookup are missing the terraform will through an error either way. Meaning it can be a hotifx version update only

mldevpants commented 9 months ago

additionally, the code was tested with datadog provider versions 3.35 and 3.34

Nuru commented 9 months ago

@mldevpants Thank you for this contribution.

Sorry for the timing: We will have an updated version of the Datadog monitor support out very soon now. We are taking a different approach moving forward, so I am going to close this PR as not compatible with our upcoming changes.

FYI, map(any) does not work, because although any can be anything, it has to be something. Meaning that even though the values of a map(any) can be of any type, they have to all be of the same type. That is not a restriction we want to apply, so we need to stick with a type of any.

mldevpants commented 9 months ago

Thanks @Nuru for letting me know. Good point for map(any), didn't think of it yesterday, I was helping out with for some team. Do you have an ETA for the new one? Hmmm, I was creating an internal module and saw this one; probably I'll stick to finish internal then.

Nuru commented 8 months ago

@mldevpants most inputs were added in #87 which has been released as v1.4.0. The remaining few left out are in #89 and should be released very soon.