[v15] Fix opsgenie plugin to use correct annotation to determine whether or not to create alerts

api/types/constants.go

@@ -725,10 +725,10 @@ const (
 	// DiscoveryAppIgnore specifies if a Kubernetes service should be ignored by discovery service.
 	DiscoveryAppIgnore = TeleportNamespace + "/ignore"
 
-	// ReqAnnotationSchedulesLabel is the request annotation key at which schedules are stored for access plugins.
-	ReqAnnotationSchedulesLabel = "/schedules"
-	// ReqAnnotationNotifyServicesLabel is the request annotation key at which notify services are stored for access plugins.
-	ReqAnnotationNotifyServicesLabel = "/notify-services"
+	// ReqAnnotationApproveSchedulesLabel is the request annotation key at which schedules are stored for access plugins.
+	ReqAnnotationApproveSchedulesLabel = "/schedules"
+	// ReqAnnotationNotifySchedulesLabel is the request annotation key at which notify schedules are stored for access plugins.
+	ReqAnnotationNotifySchedulesLabel = "/notify-services"
 
 	// CloudAWS identifies that a resource was discovered in AWS.
 	CloudAWS = "AWS"

docs/pages/access-controls/access-request-plugins/opsgenie.mdx

@@ -60,8 +60,6 @@ To create a user first navigate to Management -> Access -> Roles
 
 Then select 'Create New Role' and create the requester role.
 
-![Add user one](../../../img/enterprise/plugins/opsgenie/add-requester-role.png)
-
 ```
 kind: role
 version: v5
@@ -75,10 +73,13 @@ spec:
         - approve: 1
           deny: 1
       annotations:
-        teleport.dev/schedules: ['teleport-access-request-notifications']
+        teleport.dev/notify-services: ['teleport-access-request-notifications']
+        teleport.dev/schedules: ['teleport-access-alert-schedules']
 ```
 
-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 created for.
+The `teleport.dev/schedules` annotation specifies the schedules the alert will check, and auto approve the
+Access Request if the requesting user is on-call.
 
 ### Create a user who will request access
 
@@ -121,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
 

integrations/access/accessrequest/app.go

@@ -352,16 +352,23 @@ 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 {
-			for _, recipient := range recipients {
-				rec, err := a.bot.FetchRecipient(ctx, recipient)
-				if err != nil {
-					log.Warning(err)
-				}
-				recipientSet.Add(*rec)
+		// When both notify-services and approve-schedules are present, each is used for their own intended purpose.
+		recipients := make([]string, 0)
+		if approveSchedules, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationApproveSchedulesLabel]; ok {
+			recipients = approveSchedules
+		}
+		if notifySchedules, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationNotifySchedulesLabel]; ok {
+			recipients = notifySchedules
+		}
+		for _, recipient := range recipients {
+			rec, err := a.bot.FetchRecipient(ctx, recipient)
+			if err != nil {
+				log.Warningf("Failed to fetch Opsgenie recipient: %v", err)
+				continue
 			}
-			return recipientSet.ToSlice()
+			recipientSet.Add(*rec)
 		}
+		return recipientSet.ToSlice()
 	}
 
 	validEmailSuggReviewers := []string{}

integrations/access/accessrequest/app_test.go

@@ -0,0 +1,118 @@
+/*
+ * Teleport
+ * Copyright (C) 2024  Gravitational, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package accessrequest
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/api/types/wrappers"
+	"github.com/gravitational/teleport/integrations/access/common"
+)
+
+func TestOpsGenieGetMessageRecipients(t *testing.T) {
+	a := App{pluginType: types.PluginTypeOpsgenie, bot: testBot{}}
+	ctx := context.Background()
+	tests := []struct {
+		name               string
+		annotations        map[string][]string
+		expectedRecipients []common.Recipient
+	}{
+		{
+			name:               "no annotation",
+			annotations:        map[string][]string{},
+			expectedRecipients: []common.Recipient{},
+		},
+		{
+			name: "just notify-schedules",
+			annotations: map[string][]string{
+				types.TeleportNamespace + types.ReqAnnotationNotifySchedulesLabel: {"foo", "bar"},
+			},
+			expectedRecipients: []common.Recipient{
+				{
+					Name: "foo",
+					ID:   "foo",
+				},
+				{
+					Name: "bar",
+					ID:   "bar",
+				},
+			},
+		},
+		{
+			name: "just approval-schedules",
+			annotations: map[string][]string{
+				types.TeleportNamespace + types.ReqAnnotationApproveSchedulesLabel: {"foo", "bar"},
+			},
+			expectedRecipients: []common.Recipient{
+				{
+					Name: "foo",
+					ID:   "foo",
+				},
+				{
+					Name: "bar",
+					ID:   "bar",
+				},
+			},
+		},
+		{
+			name: "both notify and approval schedules",
+			annotations: map[string][]string{
+				types.TeleportNamespace + types.ReqAnnotationNotifySchedulesLabel:  {"foo", "bar"},
+				types.TeleportNamespace + types.ReqAnnotationApproveSchedulesLabel: {"baz", "hello"},
+			},
+			expectedRecipients: []common.Recipient{
+				{
+					Name: "foo",
+					ID:   "foo",
+				},
+				{
+					Name: "bar",
+					ID:   "bar",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := &types.AccessRequestV3{
+				Spec: types.AccessRequestSpecV3{
+					SystemAnnotations: wrappers.Traits(tt.annotations),
+				},
+			}
+			recipients := a.getMessageRecipients(ctx, req)
+			require.Equal(t, tt.expectedRecipients, recipients)
+		})
+	}
+
+}
+
+type testBot struct {
+	MessagingBot
+}
+
+func (testBot) FetchRecipient(ctx context.Context, recipient string) (*common.Recipient, error) {
+	return &common.Recipient{
+		Name: recipient,
+		ID:   recipient,
+	}, nil
+}

integrations/access/opsgenie/app.go

@@ -31,7 +31,6 @@ import (
 	tp "github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/types"
-	"github.com/gravitational/teleport/integrations/access/common"
 	"github.com/gravitational/teleport/integrations/access/common/teleport"
 	"github.com/gravitational/teleport/integrations/lib"
 	"github.com/gravitational/teleport/integrations/lib/backoff"
@@ -45,9 +44,9 @@ const (
 	// minServerVersion is the minimal teleport version the plugin supports.
 	minServerVersion = "6.1.0"
 	// initTimeout is used to bound execution time of health check and teleport version check.
-	initTimeout = time.Second * 10
+	initTimeout = time.Second * 30
 	// handlerTimeout is used to bound the execution time of watcher event handler.
-	handlerTimeout = time.Second * 5
+	handlerTimeout = time.Second * 30
 	// modifyPluginDataBackoffBase is an initial (minimum) backoff value.
 	modifyPluginDataBackoffBase = time.Millisecond
 	// modifyPluginDataBackoffMax is a backoff threshold
@@ -141,10 +140,9 @@ func (a *App) init(ctx context.Context) error {
 	defer cancel()
 
 	var err error
-	if a.teleport == nil {
-		if a.teleport, err = common.GetTeleportClient(ctx, a.conf.Teleport); err != nil {
-			return trace.Wrap(err)
-		}
+	a.teleport, err = a.conf.GetTeleportClient(ctx)
+	if err != nil {
+		return trace.Wrap(err, "getting teleport client")
 	}
 
 	if _, err = a.checkTeleportVersion(ctx); err != nil {
@@ -155,6 +153,13 @@ func (a *App) init(ctx context.Context) error {
 	if err != nil {
 		return trace.Wrap(err)
 	}
+
+	log := logger.Get(ctx)
+	log.Debug("Starting API health check...")
+	if err = a.opsgenie.CheckHealth(ctx); err != nil {
+		return trace.Wrap(err, "API health check failed")
+	}
+	log.Debug("API health check finished ok")
 	return nil
 }
 
@@ -269,15 +274,15 @@ func (a *App) onDeletedRequest(ctx context.Context, reqID string) error {
 }
 
 func (a *App) getNotifyServiceNames(req types.AccessRequest) ([]string, error) {
-	services, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationNotifyServicesLabel]
+	services, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationNotifySchedulesLabel]
 	if !ok {
 		return nil, trace.NotFound("notify services not specified")
 	}
 	return services, nil
 }
 
 func (a *App) getOnCallServiceNames(req types.AccessRequest) ([]string, error) {
-	services, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationSchedulesLabel]
+	services, ok := req.GetSystemAnnotations()[types.TeleportNamespace+types.ReqAnnotationApproveSchedulesLabel]
 	if !ok {
 		return nil, trace.NotFound("on-call schedules not specified")
 	}
@@ -294,11 +299,16 @@ func (a *App) tryNotifyService(ctx context.Context, req types.AccessRequest) (bo
 	}
 
 	reqID := req.GetName()
+	annotations := types.Labels{}
+	for k, v := range req.GetSystemAnnotations() {
+		annotations[k] = v
+	}
 	reqData := RequestData{
-		User:          req.GetUser(),
-		Roles:         req.GetRoles(),
-		Created:       req.GetCreationTime(),
-		RequestReason: req.GetRequestReason(),
+		User:              req.GetUser(),
+		Roles:             req.GetRoles(),
+		Created:           req.GetCreationTime(),
+		RequestReason:     req.GetRequestReason(),
+		SystemAnnotations: annotations,
 	}
 
 	// Create plugin data if it didn't exist before.
@@ -429,7 +439,7 @@ func (a *App) tryApproveRequest(ctx context.Context, req types.AccessRequest) er
 		if _, err := a.teleport.SubmitAccessReview(ctx, types.AccessReviewSubmission{
 			RequestID: req.GetName(),
 			Review: types.AccessReview{
-				Author:        tp.SystemAccessApproverUserName,
+				Author:        a.conf.TeleportUserName,
 				ProposedState: types.RequestState_APPROVED,
 				Reason: fmt.Sprintf("Access requested by user %s who is on call on service(s) %s",
 					tp.SystemAccessApproverUserName,

integrations/access/opsgenie/bot.go

@@ -60,13 +60,17 @@ 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{}
-	for _, recipient := range recipients {
-		schedules = append(schedules, recipient.Name)
+func (b *Bot) BroadcastAccessRequestMessage(ctx context.Context, recipientSchedules []common.Recipient, reqID string, reqData pd.AccessRequestData) (data accessrequest.SentMessages, err error) {
+	notificationSchedules := make([]string, 0, len(recipientSchedules))
+	for _, notifySchedule := range recipientSchedules {
+		notificationSchedules = append(notificationSchedules, notifySchedule.Name)
 	}
-	if len(recipients) == 0 {
-		schedules = append(schedules, b.client.DefaultSchedules...)
+	autoApprovalSchedules := []string{}
+	if annotationAutoApprovalSchedules, ok := reqData.SystemAnnotations[types.TeleportNamespace+types.ReqAnnotationApproveSchedulesLabel]; ok {
+		autoApprovalSchedules = annotationAutoApprovalSchedules
+	}
+	if len(autoApprovalSchedules) == 0 {
+		autoApprovalSchedules = append(autoApprovalSchedules, b.client.DefaultSchedules...)
 	}
 	opsgenieReqData := RequestData{
 		User:          reqData.User,
@@ -79,7 +83,8 @@ func (b *Bot) BroadcastAccessRequestMessage(ctx context.Context, recipients []co
 			Reason: reqData.ResolutionReason,
 		},
 		SystemAnnotations: types.Labels{
-			types.TeleportNamespace + types.ReqAnnotationSchedulesLabel: schedules,
+			types.TeleportNamespace + types.ReqAnnotationApproveSchedulesLabel: autoApprovalSchedules,
+			types.TeleportNamespace + types.ReqAnnotationNotifySchedulesLabel:  notificationSchedules,
 		},
 	}
 	opsgenieData, err := b.client.CreateAlert(ctx, reqID, opsgenieReqData)