Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alerting: Workaround for bad handling of empty strings in contact points API #656

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

alexweav
Copy link
Contributor

The Terraform default is to use empty-strings for blank or missing fields. This is consistent with a lot of Go-based clients, but Terraform is very aggressive with this. In fact, in many cases the provider API has no way of telling whether a field was missing from HCL entirely, or present with an empty string: hashicorp/terraform-plugin-sdk#741

This is normally fine, assuming that the underlying API is well behaved. Most of Grafana's API handle this well, but the settings field in contact points do not handle this consistently. Empty strings circumvent the API validation entirely. See this issue in Grafana: grafana/grafana#55139

This issue in Grafana will take quite some time to fix. This PR implements a much faster workaround in the provider. This allows us to get a workaround released to users faster, while we fix the main bug in the Grafana API. This also means the provider will continue to work with Grafana versions affected by the above issue, which is nice for compatibility.

We will now treat this settings blob as if it's a struct with all fields being omitempty. Empty settings values will simply not be sent to the server at all.

@alexweav alexweav requested a review from a team as a code owner September 14, 2022 18:44
Copy link
Member

@JohnnyQQQQ JohnnyQQQQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@julienduchesne julienduchesne merged commit ca46f0d into master Sep 14, 2022
@julienduchesne julienduchesne deleted the alexweav/omitempty-workaround branch September 14, 2022 20:29
@marinnedea
Copy link

While it’s merged – it doesn’t look to be released yet and available in the main terraform registry.
Can you let us know when it would be released to the registry?

@julienduchesne
Copy link
Member

While it’s merged – it doesn’t look to be released yet and available in the main terraform registry. Can you let us know when it would be released to the registry?

It's released in v1.28.2

@chrisdetsicas
Copy link

@alexweav Thanks for this - I can confirm it works as intended for us now - notification policy provisioned by Terraform can trigger a pagerduty contact point provisioned by Terraform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants