Allow scoping Datadog incident notifications - #69324
Conversation
The Datadog plugin creates an incident for every Access Request, because role_to_recipients and fallback_recipient are both mandatory and a catch-all recipient therefore always resolves. Deployments that only use the plugin for on-call auto-approval of a few production roles have no way to narrow that down, so every request, including ones a human reviews by hand, pages the fallback recipient. Make the catch-all optional and let the requesting role decide: - role_to_recipients and its "*" entry are no longer required, matching the Opsgenie, PagerDuty, Jira and Mattermost plugins. - fallback_recipient is no longer required on the plugin resource. - The teleport.dev/notify-services annotation now routes Datadog notifications, as it already does for Opsgenie. When present it is authoritative for that request; when absent, role_to_recipients and the suggested reviewers are used as before. - The Helm chart exposes datadog.roleToRecipients and only renders the "*" entry when a fallback recipient is set. Requests that match no Access Monitoring Rule, carry no annotation and have no recipient for their role now create no incident. Existing configurations are unaffected: they all define a catch-all recipient, which keeps resolving for every request. Co-authored-by: Cursor <cursoragent@cursor.com>
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6638c26526
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| {{- with .Values.datadog.fallbackRecipient }} | ||
| "*" = [{{ . | quote }}] | ||
| {{- end }} | ||
| {{- range $role, $recipients := .Values.datadog.roleToRecipients }} | ||
| {{ $role | quote }} = {{ toJson $recipients }} |
There was a problem hiding this comment.
When an existing chart user keeps datadog.fallbackRecipient set and also adds datadog.roleToRecipients["*"] (documented as equivalent to the fallback), this template renders two "*" entries in the same [role_to_recipients] TOML table. The Datadog plugin loads this ConfigMap via toml.LoadFile, so the pod will fail to start with a duplicate-key config instead of using the requested routing; suppress one source when the other defines the wildcard, or reject the combination in the schema.
Useful? React with 👍 / 👎.
1 participant
Problem
The Datadog Incident Management plugin creates an incident for every Access Request in the cluster, and no configuration narrows that down. Two mandatory settings guarantee that at least one recipient always resolves:
api/types/plugin.go—fallback_recipient must be seton the plugin resource.integrations/access/datadog/config.go—role_to_recipientsandrole_to_recipients["*"]are both required.getMessageRecipients()creates an incident whenever the resolved recipient list is non-empty, and the DatadogFetchRecipientnever returns an error, so an operator cannot empty the list by pointing the fallback at a non-existent handle either.For deployments that adopt the plugin mainly for on-call auto-approval (
teleport.dev/schedules) of a few production roles, this means every unrelated request also pages the fallback recipient. In our cluster the plugin produced 208 incidents in 11 days, while only 2 request policies actually rely on on-call auto-approval; the rest are non-production requests reviewed by a human, and Slack already notifies the reviewers.The Opsgenie plugin does not have this problem: it imposes no recipient requirement, and its notifications are routed by the
teleport.dev/notify-servicesannotation. This PR gives the Datadog plugin the same capability. Related: #65345.Changes
role_to_recipientsand its*entry are no longer required by the plugin config, matching the Opsgenie, PagerDuty, Jira and Mattermost plugins.fallback_recipientis no longer required on thePluginDatadogAccessSettingsresource.teleport.dev/notify-serviceson the requesting role now routes Datadog notifications. When the annotation is present it is authoritative for that request (only the annotated team handles or user emails are notified); when it is absent,role_to_recipientsand the suggested reviewers keep being used exactly as before.datadog.roleToRecipientsand only renders the"*"entry whendatadog.fallbackRecipientis set.A request that matches no Access Monitoring Rule, carries no annotation and has no recipient configured for its role now creates no incident.
Backwards compatibility
Existing deployments are unaffected. Every current configuration defines a catch-all recipient — the chart hardcodes
"*" = [fallbackRecipient]and the hosted plugin requires the field — so recipients keep resolving for every request. The new behaviour only applies once an operator deliberately removes the catch-all.Changelog: The Datadog Incident Management plugin can now scope incidents to specific roles:
role_to_recipients, its*entry andfallback_recipientare optional, and theteleport.dev/notify-servicesannotation routes notifications.Manual Test Plan
Test Environment
macOS, Go 1.26, in-process auth server (
MinimalAuthHelper),helm unittestfor the chart.Test Cases
go test ./integrations/access/datadog/...— full OSS suite passes, including two new cases:TestIncidentCreationForNotifyServices(annotation routes the incident to the annotated team and suppresses suggested reviewers) andTestNoIncidentWithoutRecipients(no recipients, no incident).go test ./integrations/access/accessrequest/... ./integrations/access/opsgenie/... ./integrations/access/common/...— other plugins unchanged.go test ./api/types/—PluginDatadogAccessSettingsvalidation, including the updated case for an empty fallback recipient.helm unittest examples/chart/access/datadog— existing configmap snapshot unchanged whenfallbackRecipientis set; new cases cover aroleToRecipientsmap and the omission of the"*"entry.fallback_recipient, which needs a Cloud tenant.Made with Cursor