-
Notifications
You must be signed in to change notification settings - Fork 239
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
add grafana_oncall_user_notification_rule
resource
#1653
add grafana_oncall_user_notification_rule
resource
#1653
Conversation
In order to lower resource usage and have a faster runtime, PRs will not run Cloud tests automatically. |
This is going to be an annoying comment but we're trying to get rid of the old TF framework (which is what this PR is using). New resources look like this: https://github.com/grafana/terraform-provider-grafana/blob/main/internal/resources/cloud/resource_cloud_org_member.go |
thanks for pointing that out @julienduchesne, will take a look 👍 |
…of github.com:grafana/terraform-provider-grafana into jorlando/add-oncall-user-notification-policy-resource
# What this PR does Related to #4410 The changes in this PR are a prerequisite to grafana/terraform-provider-grafana#1653. See the conversation [here](https://raintank-corp.slack.com/archives/C04JCU51NF8/p1719806995902499?thread_ts=1719520920.744319&cid=C04JCU51NF8) for more context on why we decided to move away from always creating default personal notification rules for users. ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the failing linter + an import test, this looks great! Thanks for redoing everything in the new plugin framework. Much appreciated.
internal/resources/oncall/resource_user_notification_rule_test.go
Outdated
Show resolved
Hide resolved
var ( | ||
resourceUserNotificationRuleName = "grafana_oncall_user_notification_rule" | ||
|
||
userNotificationRuleTypeOptions = []string{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should MS Teams be in this list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in bdc35ec (I'll also update our public API docs to make a mention that this is available as well (only in Grafana Cloud))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…of github.com:grafana/terraform-provider-grafana into jorlando/add-oncall-user-notification-policy-resource
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Related to the Terraform portion of grafana/oncall#4410
Closes grafana/oncall#1193
Once a new provider version is released, I will open a PR in https://github.com/grafana/crossplane-provider-grafana to generate the related Crossplane resources
Tested on my local stack all of the following cases and all work as expected:
grafana_oncall_user_notification_rule
, varioustype
s andposition
suser_id
orimportant
value for a resource, should delete the old resource and create a new one