From 371346332f47541c88f3f78465399fde414ac383 Mon Sep 17 00:00:00 2001 From: EdwardDowling Date: Thu, 8 Feb 2024 16:26:18 +0000 Subject: [PATCH 1/4] Fix bug in opsgenie plugin mixing up notify-services and schedules --- integrations/access/accessrequest/app.go | 2 +- integrations/access/opsgenie/bot.go | 13 +++++++++---- integrations/access/opsgenie/client.go | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/integrations/access/accessrequest/app.go b/integrations/access/accessrequest/app.go index dc8ac30727908..2f0344de75643 100644 --- a/integrations/access/accessrequest/app.go +++ b/integrations/access/accessrequest/app.go @@ -350,7 +350,7 @@ func (a *App) getMessageRecipients(ctx context.Context, req types.AccessRequest) recipientSet.Add(common.Recipient{}) return recipientSet.ToSlice() case types.PluginTypeOpsgenie: - if recipients, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationSchedulesLabel]; ok { + if recipients, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationNotifyServicesLabel]; ok { for _, recipient := range recipients { rec, err := a.bot.FetchRecipient(ctx, recipient) if err != nil { diff --git a/integrations/access/opsgenie/bot.go b/integrations/access/opsgenie/bot.go index a5101fe68248f..68729a7b01324 100644 --- a/integrations/access/opsgenie/bot.go +++ b/integrations/access/opsgenie/bot.go @@ -59,11 +59,15 @@ func (b Bot) SendReviewReminders(ctx context.Context, recipients []common.Recipi // BroadcastAccessRequestMessage creates an alert for the provided recipients (schedules) func (b *Bot) BroadcastAccessRequestMessage(ctx context.Context, recipients []common.Recipient, reqID string, reqData pd.AccessRequestData) (data accessrequest.SentMessages, err error) { - schedules := []string{} + rawRecipients := []string{} for _, recipient := range recipients { - schedules = append(schedules, recipient.Name) + rawRecipients = append(rawRecipients, recipient.Name) + } + schedules := []string{} + if annSchedules, ok := reqData.SystemAnnotations[types.TeleportNamespace+types.ReqAnnotationSchedulesLabel]; ok { + schedules = annSchedules } - if len(recipients) == 0 { + if len(schedules) == 0 { schedules = append(schedules, b.client.DefaultSchedules...) } opsgenieReqData := RequestData{ @@ -77,7 +81,8 @@ func (b *Bot) BroadcastAccessRequestMessage(ctx context.Context, recipients []co Reason: reqData.ResolutionReason, }, SystemAnnotations: types.Labels{ - types.TeleportNamespace + types.ReqAnnotationSchedulesLabel: schedules, + types.TeleportNamespace + types.ReqAnnotationSchedulesLabel: schedules, + types.TeleportNamespace + types.ReqAnnotationNotifyServicesLabel: rawRecipients, }, } opsgenieData, err := b.client.CreateAlert(ctx, reqID, opsgenieReqData) diff --git a/integrations/access/opsgenie/client.go b/integrations/access/opsgenie/client.go index 44eefb5d4c3eb..c6b5270026d30 100644 --- a/integrations/access/opsgenie/client.go +++ b/integrations/access/opsgenie/client.go @@ -158,7 +158,7 @@ func (og Client) CreateAlert(ctx context.Context, reqID string, reqData RequestD func (og Client) getResponders(reqData RequestData) []Responder { schedules := og.DefaultSchedules - if reqSchedules, ok := reqData.SystemAnnotations[types.TeleportNamespace+types.ReqAnnotationSchedulesLabel]; ok { + if reqSchedules, ok := reqData.SystemAnnotations[types.TeleportNamespace+types.ReqAnnotationNotifyServicesLabel]; ok { schedules = reqSchedules } responders := make([]Responder, 0, len(schedules)) From cc875f2f921da18560f8ea27eb127e41e87560ff Mon Sep 17 00:00:00 2001 From: EdwardDowling Date: Thu, 8 Feb 2024 16:35:31 +0000 Subject: [PATCH 2/4] Update docs for opsgenie plugin to use correct schedules --- .../access-controls/access-request-plugins/opsgenie.mdx | 6 +++--- integrations/access/opsgenie/client.go | 2 +- rfd/0109-opsgenie-plugin.md | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/pages/access-controls/access-request-plugins/opsgenie.mdx b/docs/pages/access-controls/access-request-plugins/opsgenie.mdx index 94c9620608c0d..8a059a2003206 100644 --- a/docs/pages/access-controls/access-request-plugins/opsgenie.mdx +++ b/docs/pages/access-controls/access-request-plugins/opsgenie.mdx @@ -75,10 +75,10 @@ spec: - approve: 1 deny: 1 annotations: - teleport.dev/schedules: ['teleport-access-request-notifications'] + teleport.dev/notify-services: ['teleport-access-request-notifications'] ``` -The `teleport.dev/schedules` annotation specifies the schedule the alert will be be created for. +The `teleport.dev/notify-services` annotation specifies the schedules the alert will be be created for. ### Create a user who will request access @@ -122,7 +122,7 @@ As the Teleport user `myuser`, create an Access Request for the `editor` role: In Opsgenie, you will see a new alert containing information about the Access Request in either the default schedule specified when enrolling the plugin, -or in the schedules specified by `teleport.dev/schedules` annotation in the requester's role. +or in the schedules specified by `teleport.dev/notify-services` annotation in the requester's role. ### Resolve the request diff --git a/integrations/access/opsgenie/client.go b/integrations/access/opsgenie/client.go index c6b5270026d30..fa82c69a07fde 100644 --- a/integrations/access/opsgenie/client.go +++ b/integrations/access/opsgenie/client.go @@ -234,7 +234,7 @@ func (og Client) GetOnCall(ctx context.Context, scheduleName string) (Responders "flat": "true", }). SetResult(&result). - Post("v2/schedules/{scheduleName}/on-calls") + Get("v2/schedules/{scheduleName}/on-calls") if err != nil { return RespondersResult{}, trace.Wrap(err) } diff --git a/rfd/0109-opsgenie-plugin.md b/rfd/0109-opsgenie-plugin.md index 936aa060904cf..38ceb5de45fb1 100644 --- a/rfd/0109-opsgenie-plugin.md +++ b/rfd/0109-opsgenie-plugin.md @@ -108,8 +108,8 @@ spec: request: roles: [someOtherRole] annotations: - opsgenie_notify_services: ["service1", "service2"] # These are the Opsgenie services alerts will be created under - opsgenie_oncall_schedules: ["service1", "service2"] # These are the Opsgenie schedules checked during auto approval + teleport.dev/notify-services: ["service1", "service2"] # These are the Opsgenie schedules alerts will be created under + teleport.dev/schedules: ["service1", "service2"] # These are the Opsgenie schedules checked during auto approval ``` ## Implementation details From dcd7e27e4725526256c644999ec7a7483324b05c Mon Sep 17 00:00:00 2001 From: EdwardDowling Date: Fri, 9 Feb 2024 14:16:59 +0000 Subject: [PATCH 3/4] Remove accidental change to opsgenie client --- integrations/access/opsgenie/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/access/opsgenie/client.go b/integrations/access/opsgenie/client.go index fa82c69a07fde..c6b5270026d30 100644 --- a/integrations/access/opsgenie/client.go +++ b/integrations/access/opsgenie/client.go @@ -234,7 +234,7 @@ func (og Client) GetOnCall(ctx context.Context, scheduleName string) (Responders "flat": "true", }). SetResult(&result). - Get("v2/schedules/{scheduleName}/on-calls") + Post("v2/schedules/{scheduleName}/on-calls") if err != nil { return RespondersResult{}, trace.Wrap(err) } From 49435777d606985469b03016d1eefac53abaedf2 Mon Sep 17 00:00:00 2001 From: EdwardDowling Date: Mon, 12 Feb 2024 17:21:40 +0000 Subject: [PATCH 4/4] Update opsgenie client tests --- integrations/access/opsgenie/client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/access/opsgenie/client_test.go b/integrations/access/opsgenie/client_test.go index 99dda669cf95e..3dba4dbc16bff 100644 --- a/integrations/access/opsgenie/client_test.go +++ b/integrations/access/opsgenie/client_test.go @@ -54,7 +54,7 @@ func TestCreateAlert(t *testing.T) { Roles: []string{"role1", "role2"}, RequestReason: "someReason", SystemAnnotations: types.Labels{ - types.TeleportNamespace + types.ReqAnnotationSchedulesLabel: {"responder@teleport.com"}, + types.TeleportNamespace + types.ReqAnnotationNotifyServicesLabel: {"responder@teleport.com"}, }, }) assert.NoError(t, err)