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

slackbot: module #1250

Merged
merged 22 commits into from
Apr 19, 2021
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
2 changes: 2 additions & 0 deletions backend/gateway/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
authnmod "github.com/lyft/clutch/backend/module/authn"
authzmod "github.com/lyft/clutch/backend/module/authz"
awsmod "github.com/lyft/clutch/backend/module/aws"
slackbotmod "github.com/lyft/clutch/backend/module/bot/slackbot"
experimentationapi "github.com/lyft/clutch/backend/module/chaos/experimentation/api"
"github.com/lyft/clutch/backend/module/chaos/redisexperimentation"
"github.com/lyft/clutch/backend/module/chaos/serverexperimentation"
Expand Down Expand Up @@ -68,6 +69,7 @@ var Modules = module.Factory{
xdsmod.Name: xdsmod.New,
serverexperimentation.Name: serverexperimentation.New,
redisexperimentation.Name: redisexperimentation.New,
slackbotmod.Name: slackbotmod.New,
sourcecontrol.Name: sourcecontrol.New,
topologymod.Name: topologymod.New,
}
Expand Down
17 changes: 17 additions & 0 deletions backend/mock/service/botmock/botmock.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package botmock

import (
"context"

"github.com/lyft/clutch/backend/service/bot"
)

type svc struct{}

func (s svc) MatchCommand(ctx context.Context, command string) (reply string) {
return ""
}

func New() bot.Service {
return &svc{}
}
182 changes: 182 additions & 0 deletions backend/module/bot/slackbot/slackbot.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
package slackbot

danielhochman marked this conversation as resolved.
Show resolved Hide resolved
// <!-- START clutchdoc -->
// description: Receives bot events from the Slack Events API
// <!-- END clutchdoc -->

import (
"context"
"errors"
"fmt"

"github.com/slack-go/slack"
"github.com/slack-go/slack/slackevents"
"github.com/uber-go/tally"
"go.uber.org/zap"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/anypb"

slackbotv1 "github.com/lyft/clutch/backend/api/bot/slackbot/v1"
slackbotconfigv1 "github.com/lyft/clutch/backend/api/config/module/bot/slackbot/v1"
"github.com/lyft/clutch/backend/gateway/log"
"github.com/lyft/clutch/backend/module"
"github.com/lyft/clutch/backend/service"
"github.com/lyft/clutch/backend/service/bot"
)

const (
Name = "clutch.module.bot.slackbot"
dmMessage = "im"
)

func New(cfg *anypb.Any, logger *zap.Logger, scope tally.Scope) (module.Module, error) {
config := &slackbotconfigv1.Config{}

if err := cfg.UnmarshalTo(config); err != nil {
return nil, err
}

svc, ok := service.Registry["clutch.service.bot"]
if !ok {
return nil, errors.New("could not find service")
}

bot, ok := svc.(bot.Service)
if !ok {
return nil, errors.New("service was not the correct type")
}

m := &mod{
slack: slack.New(config.BotToken),
verificationToken: config.VerificationToken,
bot: bot,
logger: logger,
scope: scope,
}

return m, nil
}

type mod struct {
slack *slack.Client
verificationToken string
bot bot.Service
logger *zap.Logger
scope tally.Scope
}

func (m *mod) Register(r module.Registrar) error {
slackbotv1.RegisterSlackBotAPIServer(r.GRPCServer(), m)
return r.RegisterJSONGateway(slackbotv1.RegisterSlackBotAPIHandler)
}

// One request, one event. https://api.slack.com/apis/connections/events-api#the-events-api__receiving-events
// TODO: (sperry) capture success/failure metrics when handling the event
func (m *mod) Event(ctx context.Context, req *slackbotv1.EventRequest) (*slackbotv1.EventResponse, error) {
err := verifySlackRequest(m.verificationToken, req.Token)
if err != nil {
m.logger.Error("verify slack request error", log.ErrorField(err))
return nil, status.Error(codes.Unauthenticated, err.Error())
}

if req.Type == slackevents.URLVerification {
m.logger.Info("recieved url verification event")
return m.handleURLVerificationEvent(req.Challenge)
}

// at this point in the flow, we can ack the Slack API with a 2xx response and process the event seperately
// TODO: (sperry) use a worker goroutine / save event to DB
go m.handleEvent(context.Background(), req)

return &slackbotv1.EventResponse{}, nil
}

// TODO: (sperry) Slack plans to deprecate this form of verification in the future. we'll refactor to check the
// signing secret instead (need to change API design)
func verifySlackRequest(token, requestToken string) error {
t := slackevents.TokenComparator{VerificationToken: token}
ok := t.Verify(requestToken)
if !ok {
return errors.New("invalid verification token")
}
return nil
}

// event type recieved the first time when configuring Slack API to send requests to our endpoint, https://api.slack.com/events/url_verification
func (m *mod) handleURLVerificationEvent(challenge string) (*slackbotv1.EventResponse, error) {
// this should never happen
if challenge == "" {
msg := "slack API provided an empty challenge string"
m.logger.Error(msg)
return nil, status.Error(codes.InvalidArgument, msg)
}
return &slackbotv1.EventResponse{Challenge: challenge}, nil
}

// this outer event is 1 of 3 types: url_verification, event_callback, and app_rate_limited
// this should be called via `go` in order to avoid blocking main execution
func (m *mod) handleEvent(ctx context.Context, req *slackbotv1.EventRequest) {
switch req.Type {
case slackevents.CallbackEvent:
err := m.handleCallBackEvent(ctx, req.Event)
if err != nil {
m.logger.Error("handle callback event error", log.ErrorField(err))
}
case slackevents.AppRateLimited:
// received if endpoint got more than 30,000 request events in 60 minutes, https://api.slack.com/apis/connections/events-api#the-events-api__responding-to-events__rate-limiting
m.logger.Error("app's event subscriptions are being rate limited")
default:
m.logger.Error("received unexpected event type", zap.String("event type", req.Type))
}
}

// for the inner event, we currently support 2 types: app_mention (messages that mention the bot directly) and message (specifically DMs with the bot)
// full list of Slack event types: https://api.slack.com/events
func (m *mod) handleCallBackEvent(ctx context.Context, event *slackbotv1.Event) error {
switch event.Type {
case slackevents.AppMention:
return m.handleAppMentionEvent(ctx, event)
case slackevents.Message:
return m.handleMessageEvent(ctx, event)
default:
return fmt.Errorf("received unsuported event type: %s", event.Type)
}
}

// event type for messages that mention the bot directly
func (m *mod) handleAppMentionEvent(ctx context.Context, event *slackbotv1.Event) error {
m.logger.Info(
"recieved app_mention event",
zap.String("text", event.Text),
// TODO: (sperry) request just provides the id of the channel and user. we'd have to make seperate slack API calls using the ids to get the names
zap.String("channel id", event.Channel),
zap.String("user id", event.User),
)
Copy link
Contributor Author

@scarlettperry scarlettperry Apr 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a todo on getting the channel/user name b/c the ids are not really helpful. we can use these ids to make separate slack API calls to get the channel/user name. though, then we'd want to cache the results to avoid having to make these calls each time there's an event and to avoid getting rate limited by those APIs


match := m.bot.MatchCommand(ctx, event.Text)
return sendBotReply(ctx, m.slack, event.Channel, match)
}

// event type for DMs with the bot
func (m *mod) handleMessageEvent(ctx context.Context, event *slackbotv1.Event) error {
if event.ChannelType != dmMessage {
return fmt.Errorf("unexpected channel type: %s", event.ChannelType)
}
// it's standard behavior of the Slack Events API that the bot will receive all message events, including from it's own posts
// so we need to filter out these events by checking the BotId field; otherwise by reacting to them we will create an infinte loop
if event.BotId != "" {
// it's the bot's own post, ignore it
return nil
}

m.logger.Info(
"recieved DM event",
zap.String("text", event.Text),
// TODO: (sperry) request just provides the id of the user. we'd have to make seperate slack API calls using the id to get the name
zap.String("user id", event.User),
)

match := m.bot.MatchCommand(ctx, event.Text)
return sendBotReply(ctx, m.slack, event.Channel, match)
}
15 changes: 15 additions & 0 deletions backend/module/bot/slackbot/slackbot_helper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package slackbot

import (
"context"

"github.com/slack-go/slack"
)

func sendBotReply(ctx context.Context, client *slack.Client, channel, message string) error {
_, _, err := client.PostMessageContext(ctx, channel, slack.MsgOptionText(message, false))
if err != nil {
return err
}
return nil
}
143 changes: 143 additions & 0 deletions backend/module/bot/slackbot/slackbot_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
package slackbot

import (
"context"
"testing"

"github.com/slack-go/slack"
"github.com/slack-go/slack/slacktest"
"github.com/stretchr/testify/assert"
"github.com/uber-go/tally"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest"
"go.uber.org/zap/zaptest/observer"
"google.golang.org/protobuf/types/known/anypb"

slackbotv1 "github.com/lyft/clutch/backend/api/bot/slackbot/v1"
slackbotconfigv1 "github.com/lyft/clutch/backend/api/config/module/bot/slackbot/v1"
"github.com/lyft/clutch/backend/mock/service/botmock"
"github.com/lyft/clutch/backend/module/moduletest"
"github.com/lyft/clutch/backend/service"
)

func TestModule(t *testing.T) {
service.Registry["clutch.service.bot"] = botmock.New()

log := zaptest.NewLogger(t)
scope := tally.NewTestScope("", nil)

cfg, _ := anypb.New(&slackbotconfigv1.Config{BotToken: "foo", VerificationToken: "bar"})

m, err := New(cfg, log, scope)
assert.NoError(t, err)

r := moduletest.NewRegisterChecker()
assert.NoError(t, m.Register(r))
assert.NoError(t, r.HasAPI("clutch.bot.slackbot.v1.SlackBotAPI"))
assert.True(t, r.JSONRegistered())
}

func modMock(t *testing.T) *mod {
log := zaptest.NewLogger(t)
svc := botmock.New()
return &mod{logger: log, bot: svc}
}

func TestVerifySlackRequest(t *testing.T) {
token := "foo"

err := verifySlackRequest(token, "bar")
assert.Error(t, err)

err = verifySlackRequest(token, "foo")
assert.NoError(t, err)
}

func TestHandleURLVerificationEvent(t *testing.T) {
challenge := "foo"
m := modMock(t)

resp, err := m.handleURLVerificationEvent(challenge)
assert.NoError(t, err)
assert.Equal(t, resp.Challenge, challenge)

resp, err = m.handleURLVerificationEvent("")
assert.Error(t, err)
assert.Nil(t, resp)
}

func TestHandleEvent(t *testing.T) {
// testing log count
testCases := []struct {
req *slackbotv1.EventRequest
count int
}{
{
req: &slackbotv1.EventRequest{
Type: "event_callback",
Event: &slackbotv1.Event{Type: "foo"},
},
count: 1,
},
{req: &slackbotv1.EventRequest{Type: "app_rate_limited"}, count: 1},
{req: &slackbotv1.EventRequest{Type: "foo"}, count: 1},
}

for _, test := range testCases {
core, recorded := observer.New(zapcore.DebugLevel)
log := zap.New(core)
m := &mod{logger: log}

m.handleEvent(context.Background(), test.req)
assert.Equal(t, test.count, recorded.Len())
}
}

func TestHandleCallBackEvent(t *testing.T) {
m := modMock(t)

// invalid event type
event := &slackbotv1.Event{Type: "foo"}
err := m.handleCallBackEvent(context.Background(), event)
assert.Error(t, err)
}

func TestAppMentionEvent(t *testing.T) {
m := modMock(t)

// setting up slack mocks
slacktest.NewTestServer()
s := slacktest.NewTestServer()
go s.Start()
danielhochman marked this conversation as resolved.
Show resolved Hide resolved
defer s.Stop()

client := slack.New("ABCDEFG", slack.OptionAPIURL(s.GetAPIURL()))
m.slack = client

event := &slackbotv1.Event{Type: "app_mention"}
err := m.handleAppMentionEvent(context.Background(), event)
assert.NoError(t, err)
}

func TestHandleMessageEvent(t *testing.T) {
m := modMock(t)

// invalid event type
event := &slackbotv1.Event{Type: "foo"}
err := m.handleMessageEvent(context.Background(), event)
assert.Error(t, err)

// setting up slack mocks
slacktest.NewTestServer()
s := slacktest.NewTestServer()
go s.Start()
defer s.Stop()

client := slack.New("ABCDEFG", slack.OptionAPIURL(s.GetAPIURL()))
m.slack = client

event = &slackbotv1.Event{Type: "message", ChannelType: "im"}
err = m.handleMessageEvent(context.Background(), event)
assert.NoError(t, err)
}
Loading