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

[v16] Implement auto-approvals for datadog #47602

Merged
merged 4 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions integrations/access/accesslist/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ func (m *mockPluginConfig) GetPluginType() types.PluginType {
return types.PluginTypeSlack
}

func (m *mockPluginConfig) GetTeleportUser() string {
return ""
}

func TestAccessListReminders_Single(t *testing.T) {
t.Parallel()

Expand Down
51 changes: 51 additions & 0 deletions integrations/access/accessrequest/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"context"
"fmt"
"slices"
"strings"
"time"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -54,6 +55,9 @@ type App struct {
job lib.ServiceJob

accessMonitoringRules *accessmonitoring.RuleHandler
// teleportUser is the name of the Teleport user that will act as the
// access request approver.
teleportUser string
}

// NewApp will create a new access request application.
Expand Down Expand Up @@ -88,6 +92,7 @@ func (a *App) Init(baseApp *common.BaseApp) error {
PluginName: a.pluginName,
FetchRecipientCallback: a.bot.FetchRecipient,
})
a.teleportUser = baseApp.Conf.GetTeleportUser()

return nil
}
Expand Down Expand Up @@ -262,6 +267,11 @@ func (a *App) onPendingRequest(ctx context.Context, req types.AccessRequest) err
} else {
log.Warning("No channel to post")
}

// Try to approve the request if user is currently on-call.
if err := a.tryApproveRequest(ctx, reqID, req); err != nil {
log.Warningf("Failed to auto approve request: %v", err)
}
case trace.IsAlreadyExists(err):
// The messages were already sent, nothing to do, we can update the reviews
default:
Expand Down Expand Up @@ -531,3 +541,44 @@ func (a *App) getResourceNames(ctx context.Context, req types.AccessRequest) ([]
}
return resourceNames, nil
}

// tryApproveRequest attempts to automatically approve the access request if the
// user is on call for the configured service/team.
func (a *App) tryApproveRequest(ctx context.Context, reqID string, req types.AccessRequest) error {
log := logger.Get(ctx).
WithField("req_id", reqID).
WithField("user", req.GetUser())

oncallUsers, err := a.bot.FetchOncallUsers(ctx, req)
if trace.IsNotImplemented(err) {
log.Debugf("Skipping auto-approval because %q bot does not support automatic approvals.", a.pluginName)
return nil
}
if err != nil {
return trace.Wrap(err)
}

if !slices.Contains(oncallUsers, req.GetUser()) {
log.Debug("Skipping approval because user is not on-call.")
return nil
}

if _, err := a.apiClient.SubmitAccessReview(ctx, types.AccessReviewSubmission{
RequestID: reqID,
Review: types.AccessReview{
Author: a.teleportUser,
ProposedState: types.RequestState_APPROVED,
Reason: fmt.Sprintf("Access request has been automatically approved by %q plugin because user %q is on-call.", a.pluginName, req.GetUser()),
Created: time.Now(),
},
}); err != nil {
if strings.HasSuffix(err.Error(), "has already reviewed this request") {
log.Debug("Request has already been reviewed.")
return nil
}
return trace.Wrap(err)
}

log.Info("Successfully submitted a request approval.")
return nil
}
87 changes: 87 additions & 0 deletions integrations/access/accessrequest/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ package accessrequest

import (
"context"
"fmt"
"testing"
"time"

"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
Expand All @@ -39,6 +41,22 @@ func (m *mockTeleportClient) GetRole(ctx context.Context, name string) (types.Ro
return args.Get(0).(types.Role), args.Error(1)
}

func (m *mockTeleportClient) SubmitAccessReview(ctx context.Context, review types.AccessReviewSubmission) (types.AccessRequest, error) {
review.Review.Created = time.Time{}
args := m.Called(ctx, review)
return (types.AccessRequest)(nil), args.Error(1)
}

type mockMessagingBot struct {
mock.Mock
MessagingBot
}

func (m *mockMessagingBot) FetchOncallUsers(ctx context.Context, req types.AccessRequest) ([]string, error) {
args := m.Called(ctx, req)
return args.Get(0).([]string), args.Error(1)
}

func TestGetLoginsByRole(t *testing.T) {
teleportClient := &mockTeleportClient{}
teleportClient.On("GetRole", mock.Anything, "admin").Return(&types.RoleV6{
Expand Down Expand Up @@ -82,3 +100,72 @@ func TestGetLoginsByRole(t *testing.T) {
require.Equal(t, expected, loginsByRole)
teleportClient.AssertNumberOfCalls(t, "GetRole", 3)
}

func TestTryApproveRequest(t *testing.T) {
teleportClient := &mockTeleportClient{}
bot := &mockMessagingBot{}
app := App{
apiClient: teleportClient,
bot: bot,
teleportUser: "test-access-plugin",
pluginName: "test",
}
user := "[email protected]"
requestID := "request-0"

// Example with user on-call
bot.On("FetchOncallUsers", mock.Anything, &types.AccessRequestV3{
Spec: types.AccessRequestSpecV3{
User: user,
SystemAnnotations: map[string][]string{
"example-auto-approvals": {"team-includes-requester"},
},
},
}).Return([]string{user}, (error)(nil))

// Example with user not on-call
bot.On("FetchOncallUsers", mock.Anything, &types.AccessRequestV3{
Spec: types.AccessRequestSpecV3{
User: user,
SystemAnnotations: map[string][]string{
"example-auto-approvals": {"team-not-includes-requester"},
},
},
}).Return([]string{"[email protected]"}, (error)(nil))

// Successful review
teleportClient.On("SubmitAccessReview", mock.Anything, types.AccessReviewSubmission{
RequestID: requestID,
Review: types.AccessReview{
Author: app.teleportUser,
ProposedState: types.RequestState_APPROVED,
Reason: fmt.Sprintf("Access request has been automatically approved by %q plugin because user %q is on-call.", app.pluginName, user),
},
}).Return((types.AccessRequest)(nil), (error)(nil))

ctx := context.Background()

// Test user is on-call
require.NoError(t, app.tryApproveRequest(ctx, requestID, &types.AccessRequestV3{
Spec: types.AccessRequestSpecV3{
User: user,
SystemAnnotations: map[string][]string{
"example-auto-approvals": {"team-includes-requester"},
},
},
}))
bot.AssertNumberOfCalls(t, "FetchOncallUsers", 1)
teleportClient.AssertNumberOfCalls(t, "SubmitAccessReview", 1)

// Test user is not on-call
require.NoError(t, app.tryApproveRequest(ctx, requestID, &types.AccessRequestV3{
Spec: types.AccessRequestSpecV3{
User: user,
SystemAnnotations: map[string][]string{
"example-auto-approvals": {"team-not-includes-requester"},
},
},
}))
bot.AssertNumberOfCalls(t, "FetchOncallUsers", 2)
teleportClient.AssertNumberOfCalls(t, "SubmitAccessReview", 1)
}
2 changes: 2 additions & 0 deletions integrations/access/accessrequest/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,6 @@ type MessagingBot interface {
UpdateMessages(ctx context.Context, reqID string, data pd.AccessRequestData, messageData SentMessages, reviews []types.AccessReview) error
// NotifyUser notifies the user if their access request status has changed
NotifyUser(ctx context.Context, reqID string, ard pd.AccessRequestData) error
// FetchOncallUsers fetches on-call users filtered by the provided annotations
FetchOncallUsers(ctx context.Context, req types.AccessRequest) ([]string, error)
}
14 changes: 7 additions & 7 deletions integrations/access/common/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ import (
"github.com/gravitational/teleport/api/types"
)

// GetServiceNamesFromAnnotations reads systems annotations from an access
// requests and returns the services to notify/use for approval.
// GetNamesFromAnnotations reads system annotations from an access request and
// returns the services/teams to notify/use for approval.
// The list is sorted and duplicates are removed.
func GetServiceNamesFromAnnotations(req types.AccessRequest, annotationKey string) ([]string, error) {
serviceNames, ok := req.GetSystemAnnotations()[annotationKey]
func GetNamesFromAnnotations(req types.AccessRequest, annotationKey string) ([]string, error) {
names, ok := req.GetSystemAnnotations()[annotationKey]
if !ok {
return nil, trace.NotFound("request annotation %s is missing", annotationKey)
}
if len(serviceNames) == 0 {
if len(names) == 0 {
return nil, trace.BadParameter("request annotation %s is present but empty", annotationKey)
}
slices.Sort(serviceNames)
return slices.Compact(serviceNames), nil
slices.Sort(names)
return slices.Compact(names), nil
}
4 changes: 2 additions & 2 deletions integrations/access/common/annotations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"github.com/gravitational/teleport/api/types"
)

func TestGetServiceNamesFromAnnotations(t *testing.T) {
func TestGetNamesFromAnnotations(t *testing.T) {
testAnnotationKey := "test-key"
tests := []struct {
name string
Expand Down Expand Up @@ -75,7 +75,7 @@ func TestGetServiceNamesFromAnnotations(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
request := &types.AccessRequestV3{Spec: types.AccessRequestSpecV3{SystemAnnotations: tt.annotations}}
got, err := GetServiceNamesFromAnnotations(request, testAnnotationKey)
got, err := GetNamesFromAnnotations(request, testAnnotationKey)
tt.assertErr(t, err)
require.ElementsMatch(t, tt.want, got)
})
Expand Down
12 changes: 12 additions & 0 deletions integrations/access/common/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,19 @@ type PluginConfiguration interface {
GetRecipients() RawRecipientsMap
NewBot(clusterName string, webProxyAddr string) (MessagingBot, error)
GetPluginType() types.PluginType
// GetTeleportUser returns the name of the teleport user that acts as the
// access request approver.
GetTeleportUser() string
}

type BaseConfig struct {
Teleport lib.TeleportConfig `toml:"teleport"`
Recipients RawRecipientsMap `toml:"role_to_recipients"`
Log logger.Config `toml:"log"`
PluginType types.PluginType
// TeleportUser is the name of the teleport user that acts as the
// access request approver.
TeleportUser string
}

func (c BaseConfig) GetRecipients() RawRecipientsMap {
Expand Down Expand Up @@ -90,6 +96,12 @@ func (c BaseConfig) GetPluginType() types.PluginType {
return c.PluginType
}

// GetTeleportUser returns the name of the teleport user that acts as the
// access request approver.
func (c BaseConfig) GetTeleportUser() string {
return c.TeleportUser
}

// GenericAPIConfig holds common configuration use by a messaging service.
// MessagingBots requiring more custom configuration (MSTeams for example) can
// implement their own APIConfig instead.
Expand Down
41 changes: 41 additions & 0 deletions integrations/access/datadog/bot.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"context"
"fmt"
"net/url"
"slices"
"strings"
"text/template"

Expand All @@ -32,6 +33,7 @@ import (
"github.com/gravitational/teleport/integrations/access/accessrequest"
"github.com/gravitational/teleport/integrations/access/common"
"github.com/gravitational/teleport/integrations/lib"
"github.com/gravitational/teleport/integrations/lib/logger"
pd "github.com/gravitational/teleport/integrations/lib/plugindata"
)

Expand Down Expand Up @@ -153,6 +155,45 @@ func (b Bot) FetchRecipient(ctx context.Context, name string) (*common.Recipient
}, nil
}

// FetchOncallUsers fetches on-call users filtered by the provided annotations.
func (b Bot) FetchOncallUsers(ctx context.Context, req types.AccessRequest) ([]string, error) {
log := logger.Get(ctx)

annotationKey := types.TeleportNamespace + types.ReqAnnotationApproveSchedulesLabel
teamNames, err := common.GetNamesFromAnnotations(req, annotationKey)
if err != nil {
log.Debug("Automatic approvals annotation is empty or unspecified.")
return nil, nil
}

// Fetch all on-call teams
oncallTeams, err := b.datadog.GetOncallTeams(ctx)
if err != nil {
return nil, trace.Wrap(err)
}

var oncallUserIDs []string
for _, oncallTeam := range oncallTeams.Data {
// Filter the list of teams to only the teams that match the datadog annotation.
if !slices.Contains(teamNames, oncallTeam.Attributes.Handle) &&
!slices.Contains(teamNames, oncallTeam.Attributes.Name) {
continue
}
// Collect users that are on-call for the specified team.
for _, oncallUser := range oncallTeam.Relationships.OncallUsers.Data {
oncallUserIDs = append(oncallUserIDs, oncallUser.ID)
}
}

var oncallUserEmails []string
for _, user := range oncallTeams.Included {
if slices.Contains(oncallUserIDs, user.ID) {
oncallUserEmails = append(oncallUserEmails, user.Attributes.Email)
}
}
return oncallUserEmails, nil
}

func buildIncidentSummary(clusterName, reqID string, reqData pd.AccessRequestData, webProxyURL *url.URL) (string, error) {
var requestLink string
if webProxyURL != nil {
Expand Down
Loading
Loading