slackbot: api and config protos #1201
Conversation
scarlettperry
force-pushed
the
sperry-slackbot-api
branch
from
ff8446d
to
aafd9ee
Compare
|
|
||
| message Config { | ||
| // Bot tokens begin with "xoxb-", https://api.slack.com/authentication/token-types#bot | ||
| string bot_token = 1 [ (validate.rules).string = {min_bytes : 1} ]; |
There was a problem hiding this comment.
Is there a chance we would want to have potentially n bot tokens if somehow we wanted a single clutch instance to talk to multiple slack teams or potentially have many bots?
There was a problem hiding this comment.
yeah it crossed my mind that it's something we might do in the future. though if we were to support that, then I think the config structure would be very different. Perhaps it would be similar to Omnibot's https://lyft.github.io/omnibot/configuration.html#bots, where we'd want the slack workspace, bot app id, bot name, and bot token etc to know which messages to route to which bot.
I'll add a todo for now and come back to it when I open a PR for the module b/c may want to add more stuff
There was a problem hiding this comment.
👍 sounds good. My only concern would be that modifying the config down the road might be more difficult
api/bot/slackbot/v1/slackbot.proto
Outdated
| // image urls | ||
| map<string, string> icons = 6; |
There was a problem hiding this comment.
It's kind of unclear what icons is scoped to? Is it possible for a bot to have icons?
There was a problem hiding this comment.
I'll add a comment to clarify. the Slack API sends us something like the snippet below which refers to the display picture of the bot user. I'm guessing that Slack saves a given image in a few different sizes or saves a history of the different display images the bot has had
"icons": {
"image_36": "https://...",
"image_48": "https://...",
"image_72": "https://..."
}
api/bot/slackbot/v1/slackbot.proto
Outdated
| string bot_id = 3; | ||
| Bot bot_profile = 4; |
There was a problem hiding this comment.
yep! will add a comment
| // Respond back to the Slack Events API with the challenge or a 2xx, | ||
| // https://api.slack.com/apis/connections/events-api#the-events-api__responding-to-events | ||
| message EventResponse { | ||
| option (clutch.api.v1.redacted) = true; |
There was a problem hiding this comment.
add a todo with a link to #1227?
There was a problem hiding this comment.
in this case, the response just has the challenge field so i think it's fine to redact the entire response
api/bot/slackbot/v1/slackbot.proto
Outdated
| // an installation of the app, object type | ||
| google.protobuf.Value authorizations = 10; |
There was a problem hiding this comment.
It's a bit unclear what this is
There was a problem hiding this comment.
i'll add a comment, it indicates who the event is visible to
example of what it can look like
"authorizations": [
{
"enterprise_id": "E12345",
"team_id": "T12345",
"user_id": "U12345",
"is_bot": false
}
],
|
@dschaller, thank you for all the feedback! addressed, lmk if you have any Qs about the comments I left. |
Description
PR creates the protos for the API that will receive requests from the Slack Events API.
redactedoption to the request/response protos so that the field values don't get saved in the audit DB. Ideally, it'd be nice to specify which exact fields we want to redact, so I opened an issue. I can follow up on that later.PR also creates a new config for the user to specify the slack bot token and signing secret, which will be used to create a slack client instance via slack-go and confirm that each request came from Slack.
Testing Performed
Locally
GitHub Issue
#8