[datadog_monitor] - Migrate datadog_monitor module parameter from locked to restricted_roles #8193
Conversation
ansibullbot
added
module
felixfontein
added
check-before-release
| description: | ||
| - Whether changes to this monitor should be restricted to the creator or admins. | ||
| type: bool | ||
| default: false |
There was a problem hiding this comment.
Please note that simply removing a module option is a breaking change and cannot be merged without a deprecation period.
Also it seems that the services can apparently be hosted on-premises, in which case this could work a lot longer than for the cloud-hosted solution.
There was a problem hiding this comment.
Thanks for the feedback. Do you have any reference to that? To my knowledge datadog.com is a cloud hosted monitoring solution with no on-premise equivalent. You can run on-premise agents on your machine, that report metrics to the datadog cloud. There you deploy monitors, which alert on the metrics or logs.
I understand that this is a breaking change, when Datadog rejects all API with the locked parameter in the future, the datadog_monitor module will fail.
There was a problem hiding this comment.
Thanks for the feedback. Do you have any reference to that? To my knowledge datadog.com is a cloud hosted monitoring solution with no on-premise equivalent.
I have no real reference, I found https://www.datadoghq.com/solutions/on-premises-monitoring/ when searching for 'datadog on premises', but that could well be just about the on-premise agents; but then there's also a module parameter called api_host.
There was a problem hiding this comment.
The api_host parameter refers to the different Datadog API endpoints. The API endpoint changes, depending which site of Datadog the user is using. For example the endpoint for site US is https://api.datadoghq.com/api/v1/hosts and for site US3 is https://api.us3.datadoghq.com/api/v1/hosts . Visible in the API docs, you can change the site on top right corner.
There was a problem hiding this comment.
Ah, that's very helpful to know. Thanks!
| @@ -396,7 +415,6 @@ def install_monitor(module): | |||
| "renotify_interval": module.params['renotify_interval'], | |||
| "escalation_message": module.params['escalation_message'], | |||
| "notify_audit": module.boolean(module.params['notify_audit']), | |||
| "locked": module.boolean(module.params['locked']), | |||
There was a problem hiding this comment.
Is it possible to obtain an API version or something that indicates whether locked is accepted or not? That would allow to change the code to work fine while locked is still supported, and stop sending it when it's no longer supported.
There was a problem hiding this comment.
Not that I am aware of it. Datadog will reject any calls to the API with the locked parameter, in the near future. I can't find official date that is announced publicly.
Though the locked parameter is deprecated in the python api client since March 2022 and the restricted_roles parameter is available since then. See the datadog owned terraform module.
Two weeks ago datadog did a brownout and rejected all API calls with the locked parameters.
There was a problem hiding this comment.
From DataDog/terraform-provider-datadog#2327 (comment) and the comments below it it seems that only locked=true is problematic, but not locked=false. If that's true indeed, it would be much less a problem, then users simply have to stop setting locked to true and instead start using the new restricted_roles parameter.
There was a problem hiding this comment.
Hey Felix, thanks for supporting this on the weekend!
Datadog says in their docs :
The locked parameter corresponding to the above mentioned locking mechanism is no longer supported.
A part of the support correspondence, though the mentioned data, does not have to be the date, when datadog drops the locked feature from the API :
Hello,
We are reaching out to you regarding the deprecation of the "locked" attribute for monitors because you are an admin user of the Safety io Datadog account.
We noticed your org still have some monitors using the locked attribute and new monitors have been created with this attribute during the last week.
If you plan to migrate these monitors (see guide here, please let us know as soon as possible by replying to this email.
If we don't receive any response by April 12, 2024 we will enforce a migration on all the monitors using “locked” to use “restricted_roles” instead. Any tools that leverage our APIs (see here to check what are the affected endpoints) will start to surface errors and very likely fail when executed if the “locked” is used (e.g. Terraform resources that use the "locked" option).
Please do not hesitate to ask any questions, we are looking forward to support you with this deprecation.
There was a problem hiding this comment.
Hmm, how about changing the module as follows then: if locked=false, do not put locked into options in install_monitor().
The main thing I'm worried about is in case:
- If an existing monitor has
locked=trueset. Will not passinglockedat all on updating will changelocked? And if it does, ischanged=truereported by the module? - If a user sets
locked=truefor a monitor which isn't locked right now, ischanged=truestill correctly reported by the module?
The second point is less important, but the first one is.
There was a problem hiding this comment.
Hey Felix, I do not understand why 'locked' would be still needed if Datadog is removing it from their API. Once they have removed it, you can not create any monitors with the locked parameter and this will cause this module to fail.
There was a problem hiding this comment.
It's needed for backwards compatibility. If you have an existing task that sets locked=false, it should keep working. If someone sets locked=true and that stops working due to the API no longer supporting it, well, that's a breaking change not caused by this collection. But simply removing this option is a breaking change on our side, and that's not acceptable.
There was a problem hiding this comment.
Thank you for the explanation that makes perfect sense.
Co-authored-by: Felix Fontein <felix@fontein.de>
felixfontein
removed
the
backport-8
felixfontein
added
the
backport-9
|
@greenflowers ping needs_info |
ansibullbot
added
the
needs_info
|
@greenflowers This pullrequest is waiting for your response. Please respond or the pullrequest will be closed. |
Fixes: #8192
SUMMARY
Migrate datadog_monitor module parameter from locked to restricted_roles. locked parameter is deprecated and will soonish lead to failures when deploying datadog monitors.
COMPONENT NAME
datadog_monitor
ADDITIONAL INFORMATION
Increase your datadog-api-client version to a version, which supports the locked_parameter