From 5479368f557d2b4f7d2886a92141492b38661b62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kosiec?= Date: Wed, 22 Mar 2023 11:04:47 +0100 Subject: [PATCH 1/5] Fix audit log reporting for source events and commands --- cmd/botkube/main.go | 6 +++ internal/analytics/segment_reporter.go | 8 ++++ internal/audit/gql_reporter.go | 7 ++- internal/audit/reporter.go | 9 +++- internal/config/remote/gql_client.go | 5 -- internal/remote/api.go | 33 +++++++------ internal/source/dispatcher.go | 39 +++++++++------- internal/source/scheduler.go | 8 +++- pkg/action/provider.go | 8 +++- pkg/action/provider_test.go | 14 ++++-- pkg/bot/bot.go | 1 + pkg/bot/discord.go | 22 +++++++-- pkg/bot/mattermost.go | 65 ++++++++++++++------------ pkg/bot/slack.go | 5 +- pkg/bot/socketslack.go | 27 ++++++++--- pkg/bot/teams.go | 4 ++ pkg/config/manager.go | 2 +- pkg/config/manager_remote.go | 28 +++++------ pkg/execute/executor.go | 41 +++++++++------- pkg/execute/factory.go | 9 +++- pkg/execute/mapping.go | 2 +- pkg/execute/sourcebinding.go | 14 ++++-- pkg/execute/sourcebinding_test.go | 16 +++++-- pkg/ptr/ptr.go | 5 ++ 24 files changed, 249 insertions(+), 129 deletions(-) diff --git a/cmd/botkube/main.go b/cmd/botkube/main.go index 269cf2712..a57eedd85 100644 --- a/cmd/botkube/main.go +++ b/cmd/botkube/main.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "log" "net/http" @@ -504,6 +505,11 @@ func getK8sClients(cfg *rest.Config) (discovery.DiscoveryInterface, error) { func reportFatalErrFn(logger logrus.FieldLogger, reporter analytics.Reporter, status status.StatusReporter) func(ctx string, err error) error { return func(ctx string, err error) error { + if errors.Is(err, context.Canceled) { + logger.Debugf("Context was cancelled. Skipping reporting error...") + return nil + } + // use separate ctx as parent ctx might be cancelled already ctxTimeout, cancel := context.WithTimeout(context.Background(), time.Second*10) defer cancel() diff --git a/internal/analytics/segment_reporter.go b/internal/analytics/segment_reporter.go index 8604db798..9734b391e 100644 --- a/internal/analytics/segment_reporter.go +++ b/internal/analytics/segment_reporter.go @@ -111,6 +111,10 @@ func (r *SegmentReporter) ReportHandledEventSuccess(event ReportEvent) error { // ReportHandledEventError reports a failure while handling event using a given communication platform. // The RegisterCurrentIdentity needs to be called first. func (r *SegmentReporter) ReportHandledEventError(event ReportEvent, err error) error { + if err == nil { + return nil + } + return r.reportEvent("Event handled", map[string]interface{}{ "platform": event.Platform, "type": event.IntegrationType, @@ -123,6 +127,10 @@ func (r *SegmentReporter) ReportHandledEventError(event ReportEvent, err error) // ReportFatalError reports a fatal app error. // It doesn't need a registered identity. func (r *SegmentReporter) ReportFatalError(err error) error { + if err == nil { + return nil + } + properties := map[string]interface{}{ "error": err.Error(), } diff --git a/internal/audit/gql_reporter.go b/internal/audit/gql_reporter.go index 2e59c9b67..94eceb624 100644 --- a/internal/audit/gql_reporter.go +++ b/internal/audit/gql_reporter.go @@ -71,8 +71,11 @@ func (r *GraphQLAuditReporter) ReportSourceAuditEvent(ctx context.Context, e Sou DeploymentID: r.gql.DeploymentID(), Type: remoteapi.AuditEventTypeSourceEventEmitted, SourceEventEmitted: &remoteapi.AuditEventSourceCreateInput{ - Event: e.Event, - Bindings: e.Bindings, + Event: e.Event, + Source: remoteapi.AuditEventSourceDetailsInput{ + Name: e.Source.Name, + DisplayName: e.Source.DisplayName, + }, }, }, } diff --git a/internal/audit/reporter.go b/internal/audit/reporter.go index 1737c66df..fc5293277 100644 --- a/internal/audit/reporter.go +++ b/internal/audit/reporter.go @@ -19,7 +19,7 @@ type ExecutorAuditEvent struct { CreatedAt string PluginName string PlatformUser string - BotPlatform remoteapi.BotPlatform + BotPlatform *remoteapi.BotPlatform Command string Channel string } @@ -29,7 +29,12 @@ type SourceAuditEvent struct { CreatedAt string PluginName string Event string - Bindings []string + Source SourceDetails +} + +type SourceDetails struct { + Name string + DisplayName string } // GetReporter creates new AuditReporter diff --git a/internal/config/remote/gql_client.go b/internal/config/remote/gql_client.go index 12bf24c0e..4eff64b72 100644 --- a/internal/config/remote/gql_client.go +++ b/internal/config/remote/gql_client.go @@ -52,11 +52,6 @@ func NewGqlClient(options ...Option) *Gql { opt(c) } - // skip client creation when not requested - if c.endpoint == "" { - return nil - } - httpCli := &http.Client{ Transport: newAPIKeySecuredTransport(c.apiKey), Timeout: defaultTimeout, diff --git a/internal/remote/api.go b/internal/remote/api.go index 032c6cbc7..7a9b736b9 100644 --- a/internal/remote/api.go +++ b/internal/remote/api.go @@ -1,7 +1,6 @@ package remote import ( - "fmt" "strings" ) @@ -17,16 +16,21 @@ type AuditEventCreateInput struct { // AuditEventCommandCreateInput contains create input specific to executor events type AuditEventCommandCreateInput struct { - PlatformUser string `json:"platformUser"` - Channel string `json:"channel"` - BotPlatform BotPlatform `json:"botPlatform"` - Command string `json:"command"` + PlatformUser string `json:"platformUser"` + Channel string `json:"channel"` + BotPlatform *BotPlatform `json:"botPlatform"` + Command string `json:"command"` } // AuditEventSourceCreateInput contains create input specific to source events type AuditEventSourceCreateInput struct { - Event string `json:"event"` - Bindings []string `json:"bindings"` + Event string `json:"event"` + Source AuditEventSourceDetailsInput `json:"source"` +} + +type AuditEventSourceDetailsInput struct { + Name string `json:"name"` + DisplayName string `json:"displayName"` } // BotPlatform are the supported bot platforms @@ -44,23 +48,26 @@ const ( ) // NewBotPlatform creates new BotPlatform from string -func NewBotPlatform(s string) (BotPlatform, error) { +func NewBotPlatform(s string) *BotPlatform { + var platform BotPlatform switch strings.ToUpper(s) { case "SLACK": fallthrough case "SOCKETSLACK": - return BotPlatformSlack, nil + platform = BotPlatformSlack case "DISCORD": - return BotPlatformDiscord, nil + platform = BotPlatformDiscord case "MATTERMOST": - return BotPlatformMattermost, nil + platform = BotPlatformMattermost case "TEAMS": fallthrough case "MS_TEAMS": - return BotPlatformMsTeams, nil + platform = BotPlatformMsTeams default: - return "", fmt.Errorf("given BotPlatform %s is not supported", s) + return nil } + + return &platform } // AuditEventType is the type of audit events diff --git a/internal/source/dispatcher.go b/internal/source/dispatcher.go index 7b01840bd..a9f896ee1 100644 --- a/internal/source/dispatcher.go +++ b/internal/source/dispatcher.go @@ -2,6 +2,7 @@ package source import ( "context" + "encoding/json" "fmt" "time" @@ -155,7 +156,7 @@ func (d *Dispatcher) dispatchMsg(ctx context.Context, event source.Event, dispat } err := n.SendMessage(ctx, msg, sources) if err != nil { - reportErr := d.reportError(ctx, err, n, pluginName, event, dispatch.sourceName) + reportErr := d.reportError(err, n, pluginName, event) if reportErr != nil { err = multierror.Append(err, fmt.Errorf("while reporting error: %w", reportErr)) } @@ -164,7 +165,7 @@ func (d *Dispatcher) dispatchMsg(ctx context.Context, event source.Event, dispat return } - reportErr := d.reportSuccess(ctx, n, pluginName, event, dispatch.sourceName) + reportErr := d.reportSuccess(n, pluginName, event) if reportErr != nil { d.log.Error(err) } @@ -176,7 +177,7 @@ func (d *Dispatcher) dispatchMsg(ctx context.Context, event source.Event, dispat defer analytics.ReportPanicIfOccurs(d.log, d.reporter) err := n.SendEvent(ctx, event.RawObject, sources) if err != nil { - reportErr := d.reportError(ctx, err, n, pluginName, event, dispatch.sourceName) + reportErr := d.reportError(err, n, pluginName, event) if reportErr != nil { err = multierror.Append(err, fmt.Errorf("while reporting error: %w", reportErr)) } @@ -185,13 +186,17 @@ func (d *Dispatcher) dispatchMsg(ctx context.Context, event source.Event, dispat return } - reportErr := d.reportSuccess(ctx, n, pluginName, event, dispatch.sourceName) + reportErr := d.reportSuccess(n, pluginName, event) if reportErr != nil { d.log.Error(err) } }(n) } + if err := d.reportAuditEvent(ctx, pluginName, event.RawObject, dispatch.sourceName, dispatch.sourceDisplayName); err != nil { + d.log.Errorf("while reporting audit event for source %q: %s", dispatch.sourceName, err.Error()) + } + // execute actions actions, err := d.actionProvider.RenderedActions(event.RawObject, sources) if err != nil { @@ -227,12 +232,20 @@ func (d *Dispatcher) dispatch(ctx context.Context, event []byte, dispatch Plugin }, dispatch) } -func (d *Dispatcher) reportAudit(ctx context.Context, pluginName, event, source string) error { +func (d *Dispatcher) reportAuditEvent(ctx context.Context, pluginName string, event any, sourceName, sourceDisplayName string) error { + eventBytes, err := json.Marshal(event) + if err != nil { + return fmt.Errorf("while marshaling audit event: %w", err) + } + e := audit.SourceAuditEvent{ CreatedAt: time.Now().Format(time.RFC3339), PluginName: pluginName, - Event: event, - Bindings: []string{source}, + Event: string(eventBytes), + Source: audit.SourceDetails{ + Name: sourceName, + DisplayName: sourceDisplayName, + }, } return d.auditReporter.ReportSourceAuditEvent(ctx, e) } @@ -242,7 +255,7 @@ type genericNotifier interface { Type() config.IntegrationType } -func (d *Dispatcher) reportSuccess(ctx context.Context, n genericNotifier, pluginName string, event source.Event, sourceName string) error { +func (d *Dispatcher) reportSuccess(n genericNotifier, pluginName string, event source.Event) error { errs := multierror.New() reportErr := d.reporter.ReportHandledEventSuccess(analytics.ReportEvent{ IntegrationType: n.Type(), @@ -253,14 +266,10 @@ func (d *Dispatcher) reportSuccess(ctx context.Context, n genericNotifier, plugi if reportErr != nil { errs = multierror.Append(errs, fmt.Errorf("while reporting %s analytics: %w", n.Type(), reportErr)) } - if err := d.reportAudit(ctx, pluginName, fmt.Sprintf("%v", event.RawObject), sourceName); err != nil { - errs = multierror.Append(errs, fmt.Errorf("while reporting %s audit event: %w", n.Type(), reportErr)) - } - return errs.ErrorOrNil() } -func (d *Dispatcher) reportError(ctx context.Context, err error, n genericNotifier, pluginName string, event source.Event, sourceName string) error { +func (d *Dispatcher) reportError(err error, n genericNotifier, pluginName string, event source.Event) error { errs := multierror.New() reportErr := d.reporter.ReportHandledEventError(analytics.ReportEvent{ IntegrationType: n.Type(), @@ -271,10 +280,6 @@ func (d *Dispatcher) reportError(ctx context.Context, err error, n genericNotifi if reportErr != nil { errs = multierror.Append(errs, fmt.Errorf("while reporting %s analytics: %w", n.Type(), reportErr)) } - // TODO: add additional metadata about event failed to send - if err := d.reportAudit(ctx, pluginName, fmt.Sprintf("%v", event.RawObject), sourceName); err != nil { - errs = multierror.Append(errs, fmt.Errorf("while reporting %s audit event: %w", n.Type(), reportErr)) - } return errs.ErrorOrNil() } diff --git a/internal/source/scheduler.go b/internal/source/scheduler.go index 7d2eabff6..b5e8f0e94 100644 --- a/internal/source/scheduler.go +++ b/internal/source/scheduler.go @@ -20,6 +20,7 @@ type PluginDispatch struct { pluginName string pluginConfigs []*source.Config sourceName string + sourceDisplayName string isInteractivitySupported bool cfg *config.Config pluginContext config.PluginContext @@ -145,7 +146,11 @@ func (d *Scheduler) schedulePlugin(ctx context.Context, isInteractivitySupported d.startProcesses[key] = struct{}{} sourcePluginConfigs := map[string][]*source.Config{} - plugins := d.cfg.Sources[sourceName].Plugins + srcConfig, exists := d.cfg.Sources[sourceName] + if !exists { + return fmt.Errorf("source %q not found", sourceName) + } + plugins := srcConfig.Plugins var pluginContext config.PluginContext for pluginName, pluginCfg := range plugins { if !pluginCfg.Enabled { @@ -170,6 +175,7 @@ func (d *Scheduler) schedulePlugin(ctx context.Context, isInteractivitySupported pluginConfigs: configs, isInteractivitySupported: isInteractivitySupported, sourceName: sourceName, + sourceDisplayName: srcConfig.DisplayName, cfg: d.cfg, pluginContext: pluginContext, }) diff --git a/pkg/action/provider.go b/pkg/action/provider.go index 594e3fa45..e7c8319ba 100644 --- a/pkg/action/provider.go +++ b/pkg/action/provider.go @@ -23,7 +23,7 @@ import ( const ( // unknownValue defines an unknown string value. - unknownValue = "unknown" + unknownValue = "n/a" ) // ExecutorFactory facilitates creation of execute.Executor instances. @@ -80,6 +80,7 @@ func (p *Provider) RenderedActions(e any, sourceBindings []string) ([]event.Acti // ExecuteAction executes action for given event. func (p *Provider) ExecuteAction(ctx context.Context, action event.Action) interactive.CoreMessage { + userName := fmt.Sprintf("Automation %q", action.DisplayName) e := p.executorFactory.NewDefault(execute.NewDefaultInput{ Conversation: execute.Conversation{ IsAuthenticated: true, @@ -92,7 +93,10 @@ func (p *Provider) ExecuteAction(ctx context.Context, action event.Action) inter Platform: unknownValue, NotifierHandler: &universalNotifierHandler{}, Message: strings.TrimSpace(strings.TrimPrefix(action.Command, api.MessageBotNamePlaceholder)), - User: fmt.Sprintf("Automation %q", action.DisplayName), + User: execute.UserInput{ + Mention: userName, + DisplayName: userName, + }, }) response := e.Execute(ctx) diff --git a/pkg/action/provider_test.go b/pkg/action/provider_test.go index 6c5e208a1..eda631674 100644 --- a/pkg/action/provider_test.go +++ b/pkg/action/provider_test.go @@ -89,6 +89,7 @@ func TestProvider_RenderedActionsForEvent(t *testing.T) { func TestProvider_ExecuteEventAction(t *testing.T) { // given botName := "my-bot" + userName := `Automation "Test"` executorBindings := []string{"executor-binding1", "executor-binding2"} eventAction := event.Action{ Command: "kubectl get po foo", @@ -96,18 +97,21 @@ func TestProvider_ExecuteEventAction(t *testing.T) { DisplayName: "Test", } expectedExecutorInput := execute.NewDefaultInput{ - CommGroupName: "unknown", - Platform: "unknown", + CommGroupName: "n/a", + Platform: "n/a", NotifierHandler: nil, // won't check it Conversation: execute.Conversation{ - Alias: "unknown", - ID: "unknown", + Alias: "n/a", + ID: "n/a", ExecutorBindings: executorBindings, IsAuthenticated: true, CommandOrigin: command.AutomationOrigin, }, Message: "kubectl get po foo", - User: `Automation "Test"`, + User: execute.UserInput{ + Mention: userName, + DisplayName: userName, + }, } execFactory := &fakeFactory{t: t, expectedInput: expectedExecutorInput} diff --git a/pkg/bot/bot.go b/pkg/bot/bot.go index d12ddf264..8bd70ac32 100644 --- a/pkg/bot/bot.go +++ b/pkg/bot/bot.go @@ -41,6 +41,7 @@ type channelConfigByID struct { alias string notify bool + name string } type channelConfigByName struct { diff --git a/pkg/bot/discord.go b/pkg/bot/discord.go index a95ffdef1..5a8fe6468 100644 --- a/pkg/bot/discord.go +++ b/pkg/bot/discord.go @@ -67,7 +67,10 @@ func NewDiscord(log logrus.FieldLogger, commGroupName string, cfg config.Discord return nil, fmt.Errorf("while creating Discord session: %w", err) } - channelsCfg := discordChannelsConfigFrom(cfg.Channels) + channelsCfg, err := discordChannelsConfigFrom(api, cfg.Channels) + if err != nil { + return nil, fmt.Errorf("while creating Discord channels config: %w", err) + } return &Discord{ log: log, @@ -218,13 +221,13 @@ func (b *Discord) handleMessage(ctx context.Context, dm discordMessage) error { b.log.Debugf("Discord incoming Request: %s", req) channel, isAuthChannel := b.getChannels()[dm.Event.ChannelID] - e := b.executorFactory.NewDefault(execute.NewDefaultInput{ CommGroupName: b.commGroupName, Platform: b.IntegrationName(), NotifierHandler: b, Conversation: execute.Conversation{ Alias: channel.alias, + DisplayName: channel.name, ID: channel.Identifier(), ExecutorBindings: channel.Bindings.Executors, SourceBindings: channel.Bindings.Sources, @@ -232,7 +235,10 @@ func (b *Discord) handleMessage(ctx context.Context, dm discordMessage) error { CommandOrigin: command.TypedOrigin, }, Message: req, - User: fmt.Sprintf("<@%s>", dm.Event.Author.ID), + User: execute.UserInput{ + Mention: fmt.Sprintf("<@%s>", dm.Event.Author.ID), + DisplayName: dm.Event.Author.String(), + }, }) response := e.Execute(ctx) @@ -329,17 +335,23 @@ func (b *Discord) formatMessage(msg interactive.CoreMessage) (*discordgo.Message }, nil } -func discordChannelsConfigFrom(channelsCfg config.IdentifiableMap[config.ChannelBindingsByID]) map[string]channelConfigByID { +func discordChannelsConfigFrom(api *discordgo.Session, channelsCfg config.IdentifiableMap[config.ChannelBindingsByID]) (map[string]channelConfigByID, error) { res := make(map[string]channelConfigByID) for channAlias, channCfg := range channelsCfg { + channelData, err := api.Channel(channCfg.Identifier()) + if err != nil { + return nil, fmt.Errorf("while getting channel name for ID %q: %w", channCfg.Identifier(), err) + } + res[channCfg.Identifier()] = channelConfigByID{ ChannelBindingsByID: channCfg, alias: channAlias, notify: !channCfg.Notification.Disabled, + name: channelData.Name, } } - return res + return res, nil } func discordBotMentionRegex(botID string) (*regexp.Regexp, error) { diff --git a/pkg/bot/mattermost.go b/pkg/bot/mattermost.go index a952197bd..5f71c2196 100644 --- a/pkg/bot/mattermost.go +++ b/pkg/bot/mattermost.go @@ -51,6 +51,7 @@ type Mattermost struct { reporter AnalyticsReporter serverURL string botName string + botUserID string teamName string webSocketURL string wsClient *model.WebSocketClient @@ -109,12 +110,18 @@ func NewMattermost(log logrus.FieldLogger, commGroupName string, cfg config.Matt return nil, fmt.Errorf("while producing channels configuration map by ID: %w", err) } + botUserID, err := getBotUserID(client, botTeam.Id, cfg.BotName) + if err != nil { + return nil, fmt.Errorf("while getting bot user ID: %w", err) + } + return &Mattermost{ log: log, executorFactory: executorFactory, reporter: reporter, serverURL: cfg.URL, botName: cfg.BotName, + botUserID: botUserID, teamName: cfg.Team, apiClient: client, webSocketURL: webSocketURL, @@ -206,6 +213,11 @@ func (b *Mattermost) handleMessage(ctx context.Context, mm *mattermostMessage) e return fmt.Errorf("while getting post from event: %w", err) } + // Skip if message posted by Botkube + if post.UserId == b.botUserID { + return nil + } + // Handle message only if starts with mention trimmedMsg, found := b.findAndTrimBotMention(post.Message) if !found { @@ -219,18 +231,28 @@ func (b *Mattermost) handleMessage(ctx context.Context, mm *mattermostMessage) e channel, exists := b.getChannels()[channelID] mm.IsAuthChannel = exists + user, _, err := b.apiClient.GetUser(post.UserId, "") + if err != nil { + b.log.Errorf("while getting user with ID %q: %s", post.UserId, err.Error()) + } + e := b.executorFactory.NewDefault(execute.NewDefaultInput{ CommGroupName: b.commGroupName, Platform: b.IntegrationName(), NotifierHandler: b, Conversation: execute.Conversation{ Alias: channel.alias, + DisplayName: channel.name, ID: channel.Identifier(), ExecutorBindings: channel.Bindings.Executors, SourceBindings: channel.Bindings.Sources, IsAuthenticated: mm.IsAuthChannel, CommandOrigin: command.TypedOrigin, }, + User: execute.UserInput{ + //Mention: "", // not used currently + DisplayName: user.Username, + }, Message: req, }) response := e.Execute(ctx) @@ -321,24 +343,6 @@ func (b *Mattermost) checkServerConnection() error { return nil } -// Check if team exists in Mattermost -func (b *Mattermost) getTeam() *model.Team { - botTeam, _, err := b.apiClient.GetTeamByName(b.teamName, "") - if err != nil { - b.log.Fatalf("There was a problem finding Mattermost team %s. %s", b.teamName, err) - } - return botTeam -} - -// Check if Botkube user exists in Mattermost -func (b *Mattermost) getUser() *model.User { - users, _, err := b.apiClient.AutocompleteUsersInTeam(b.getTeam().Id, b.botName, 1, "") - if err != nil { - b.log.Fatalf("There was a problem finding Mattermost user %s. %s", b.botName, err) - } - return users.Users[0] -} - func (b *Mattermost) listen(ctx context.Context) { b.wsClient.Listen() defer b.wsClient.Close() @@ -367,21 +371,11 @@ func (b *Mattermost) listen(ctx context.Context) { continue } - post, err := postFromEvent(event) - if err != nil { - continue - } - - // Skip if message posted by Botkube or doesn't start with mention - if post.UserId == b.getUser().Id { - continue - } - mm := &mattermostMessage{ Event: event, IsAuthChannel: false, } - err = b.handleMessage(ctx, mm) + err := b.handleMessage(ctx, mm) if err != nil { wrappedErr := fmt.Errorf("while handling message: %w", err) b.log.Errorf(wrappedErr.Error()) @@ -458,6 +452,18 @@ func (b *Mattermost) setChannels(channels map[string]channelConfigByID) { b.channels = channels } +func getBotUserID(client *model.Client4, teamID, botName string) (string, error) { + users, _, err := client.AutocompleteUsersInTeam(teamID, botName, 1, "") + if err != nil { + return "", fmt.Errorf("while getting user with name %q: %w", botName, err) + } + if users == nil || len(users.Users) == 0 || users.Users[0] == nil { + return "", fmt.Errorf("user with name %q not found", botName) + } + + return users.Users[0].Id, nil +} + func mattermostChannelsCfgFrom(client *model.Client4, teamID string, channelsCfg config.IdentifiableMap[config.ChannelBindingsByName]) (map[string]channelConfigByID, error) { res := make(map[string]channelConfigByID) for channAlias, channCfg := range channelsCfg { @@ -473,6 +479,7 @@ func mattermostChannelsCfgFrom(client *model.Client4, teamID string, channelsCfg }, alias: channAlias, notify: !channCfg.Notification.Disabled, + name: channCfg.Identifier(), } } diff --git a/pkg/bot/slack.go b/pkg/bot/slack.go index ac8caddf7..f778d454b 100644 --- a/pkg/bot/slack.go +++ b/pkg/bot/slack.go @@ -238,7 +238,10 @@ func (b *Slack) handleMessage(ctx context.Context, msg slackMessage) error { CommandOrigin: command.TypedOrigin, }, Message: request, - User: fmt.Sprintf("<@%s>", msg.User), + User: execute.UserInput{ + Mention: fmt.Sprintf("<@%s>", msg.User), + DisplayName: msg.User, // this integration is officially not supported, so no need to ensure it has a nice display name + }, }) response := e.Execute(ctx) err = b.send(ctx, msg, response, response.OnlyVisibleForYou) diff --git a/pkg/bot/socketslack.go b/pkg/bot/socketslack.go index f2e0fdd3a..4f5f1536a 100644 --- a/pkg/bot/socketslack.go +++ b/pkg/bot/socketslack.go @@ -51,7 +51,8 @@ type socketSlackMessage struct { Text string Channel string ThreadTimeStamp string - User string + UserID string + UserName string TriggerID string CommandOrigin command.Origin State *slack.BlockActionStates @@ -146,10 +147,19 @@ func (b *SocketSlack) Start(ctx context.Context) error { Text: ev.Text, Channel: ev.Channel, ThreadTimeStamp: ev.ThreadTimeStamp, - User: ev.User, + UserID: ev.User, + UserName: ev.User, // set to user ID if we can't get the real name CommandOrigin: command.TypedOrigin, } + user, err := b.client.GetUserInfoContext(ctx, ev.User) + if err != nil { + b.log.Errorf("While getting user info: %s", err.Error()) + } + if user != nil && user.RealName != "" { + msg.UserName = user.RealName + } + if err := b.handleMessage(ctx, msg); err != nil { b.log.Errorf("Message handling error: %s", err.Error()) } @@ -206,7 +216,8 @@ func (b *SocketSlack) Start(ctx context.Context) error { Channel: channelID, ThreadTimeStamp: threadTs, TriggerID: callback.TriggerID, - User: callback.User.ID, + UserID: callback.User.ID, + UserName: callback.User.RealName, CommandOrigin: cmdOrigin, State: state, ResponseURL: callback.ResponseURL, @@ -226,7 +237,8 @@ func (b *SocketSlack) Start(ctx context.Context) error { msg := socketSlackMessage{ Text: cmd, Channel: callback.View.PrivateMetadata, - User: callback.User.ID, + UserID: callback.User.ID, + UserName: callback.User.RealName, CommandOrigin: cmdOrigin, } @@ -347,7 +359,10 @@ func (b *SocketSlack) handleMessage(ctx context.Context, event socketSlackMessag SlackState: event.State, }, Message: request, - User: fmt.Sprintf("<@%s>", event.User), + User: execute.UserInput{ + Mention: fmt.Sprintf("<@%s>", event.UserID), + DisplayName: event.UserName, + }, }) response := e.Execute(ctx) err = b.send(ctx, event, response) @@ -407,7 +422,7 @@ func (b *SocketSlack) send(ctx context.Context, event socketSlackMessage, resp i } if resp.OnlyVisibleForYou { - if _, err := b.client.PostEphemeralContext(ctx, event.Channel, event.User, options...); err != nil { + if _, err := b.client.PostEphemeralContext(ctx, event.Channel, event.UserID, options...); err != nil { return fmt.Errorf("while posting Slack message visible only to user: %w", err) } } else { diff --git a/pkg/bot/teams.go b/pkg/bot/teams.go index a9dbab031..1aaa90f1f 100644 --- a/pkg/bot/teams.go +++ b/pkg/bot/teams.go @@ -291,6 +291,10 @@ func (b *Teams) processMessage(ctx context.Context, activity schema.Activity) (i SourceBindings: b.bindings.Sources, CommandOrigin: command.TypedOrigin, }, + User: execute.UserInput{ + //Mention: "", // not used currently + DisplayName: activity.From.Name, + }, Message: trimmedMsg, }) return b.convertInteractiveMessage(e.Execute(ctx), false) diff --git a/pkg/config/manager.go b/pkg/config/manager.go index 7a0b87d89..7e32e4e73 100644 --- a/pkg/config/manager.go +++ b/pkg/config/manager.go @@ -31,7 +31,7 @@ type GraphQLClient interface { DeploymentID() string } -// ConfigPersistenceManager manages persistence of the configuration. +// PersistenceManager manages persistence of the configuration. type PersistenceManager interface { PersistSourceBindings(ctx context.Context, commGroupName string, platform CommPlatformIntegration, channelAlias string, sourceBindings []string) error PersistNotificationsEnabled(ctx context.Context, commGroupName string, platform CommPlatformIntegration, channelAlias string, enabled bool) error diff --git a/pkg/config/manager_remote.go b/pkg/config/manager_remote.go index cc7387aac..a7187dd19 100644 --- a/pkg/config/manager_remote.go +++ b/pkg/config/manager_remote.go @@ -46,11 +46,12 @@ func (m *RemotePersistenceManager) PersistNotificationsEnabled(ctx context.Conte return ErrUnsupportedPlatform } - p, err := remoteapi.NewBotPlatform(platform.String()) - if err != nil { + p := remoteapi.NewBotPlatform(platform.String()) + if p == nil { return ErrUnsupportedPlatform } - err = m.withRetry(ctx, logger, func() error { + + err := m.withRetry(ctx, logger, func() error { var mutation struct { Success bool `graphql:"patchDeploymentConfig(id: $id, input: $input)"` } @@ -60,13 +61,13 @@ func (m *RemotePersistenceManager) PersistNotificationsEnabled(ctx context.Conte ResourceVersion: m.getResourceVersion(), Notification: &remoteapi.NotificationPatchDeploymentConfigInput{ CommunicationGroupName: commGroupName, - Platform: p, + Platform: *p, ChannelAlias: channelAlias, Disabled: !enabled, }, }, } - if err = m.gql.Client().Mutate(ctx, &mutation, variables); err != nil { + if err := m.gql.Client().Mutate(ctx, &mutation, variables); err != nil { return err } @@ -96,11 +97,11 @@ func (m *RemotePersistenceManager) PersistSourceBindings(ctx context.Context, co return ErrUnsupportedPlatform } - p, err := remoteapi.NewBotPlatform(string(platform)) - if err != nil { + p := remoteapi.NewBotPlatform(string(platform)) + if p == nil { return ErrUnsupportedPlatform } - err = m.withRetry(ctx, logger, func() error { + err := m.withRetry(ctx, logger, func() error { var mutation struct { Success bool `graphql:"patchDeploymentConfig(id: $id, input: $input)"` } @@ -110,14 +111,14 @@ func (m *RemotePersistenceManager) PersistSourceBindings(ctx context.Context, co ResourceVersion: m.getResourceVersion(), SourceBinding: &remoteapi.SourceBindingPatchDeploymentConfigInput{ CommunicationGroupName: commGroupName, - Platform: p, + Platform: *p, ChannelAlias: channelAlias, SourceBindings: sourceBindings, }, }, } - if err = m.gql.Client().Mutate(ctx, &mutation, variables); err != nil { + if err := m.gql.Client().Mutate(ctx, &mutation, variables); err != nil { return err } @@ -181,16 +182,16 @@ const ( delay = 200 * time.Millisecond ) -func (r *RemotePersistenceManager) withRetry(ctx context.Context, logger logrus.FieldLogger, fn func() error) error { +func (m *RemotePersistenceManager) withRetry(ctx context.Context, logger logrus.FieldLogger, fn func() error) error { err := retry.Do( fn, retry.OnRetry(func(n uint, err error) { logger.Debugf("Retrying (attempt no %d/%d): %s.\nFetching latest resource version...", n+1, retries, err) - resVer, err := r.resVerClient.GetResourceVersion(ctx) + resVer, err := m.resVerClient.GetResourceVersion(ctx) if err != nil { logger.Errorf("Error while fetching resource version: %s", err) } - r.SetResourceVersion(resVer) + m.SetResourceVersion(resVer) }), retry.Delay(delay), retry.Attempts(retries), @@ -206,6 +207,7 @@ func (r *RemotePersistenceManager) withRetry(ctx context.Context, logger logrus. func (m *RemotePersistenceManager) getResourceVersion() int { m.resVerMutex.RLock() + fmt.Println("m.resourceVersion", m.resourceVersion) defer m.resVerMutex.RUnlock() return m.resourceVersion } diff --git a/pkg/execute/executor.go b/pkg/execute/executor.go index 7b34c4648..6afe1b1e6 100644 --- a/pkg/execute/executor.go +++ b/pkg/execute/executor.go @@ -56,7 +56,7 @@ type DefaultExecutor struct { conversation Conversation merger *kubectl.Merger commGroupName string - user string + user UserInput kubectlCmdBuilder *KubectlCmdBuilder cmdsMapping *CommandMapping auditReporter audit.AuditReporter @@ -139,7 +139,7 @@ func (e *DefaultExecutor) Execute(ctx context.Context) interactive.CoreMessage { isPluginCmd := e.pluginExecutor.CanHandle(e.conversation.ExecutorBindings, cmdCtx.Args) if e.kubectlExecutor.CanHandle(cmdCtx.Args) && !isPluginCmd { - e.reportCommand(ctx, e.kubectlExecutor.GetCommandPrefix(cmdCtx.Args), cmdCtx.ExecutorFilter.IsActive(), cmdCtx) + e.reportCommand(ctx, "kubectl", e.kubectlExecutor.GetCommandPrefix(cmdCtx.Args), cmdCtx.ExecutorFilter.IsActive(), cmdCtx) out, err := e.kubectlExecutor.Execute(e.conversation.ExecutorBindings, cmdCtx.CleanCmd, e.conversation.IsAuthenticated, cmdCtx) switch { case err == nil: @@ -159,7 +159,7 @@ func (e *DefaultExecutor) Execute(ctx context.Context) interactive.CoreMessage { } if e.kubectlCmdBuilder.CanHandle(cmdCtx.Args) && !isPluginCmd { - e.reportCommand(ctx, e.kubectlCmdBuilder.GetCommandPrefix(cmdCtx.Args), false, cmdCtx) + e.reportCommand(ctx, "kubectl-builder", e.kubectlCmdBuilder.GetCommandPrefix(cmdCtx.Args), false, cmdCtx) out, err := e.kubectlCmdBuilder.Do(ctx, cmdCtx.Args, e.platform, e.conversation.ExecutorBindings, e.conversation.SlackState, header(cmdCtx), cmdCtx) if err != nil { // TODO: Return error when the DefaultExecutor is refactored as a part of https://github.com/kubeshop/botkube/issues/589 @@ -170,10 +170,13 @@ func (e *DefaultExecutor) Execute(ctx context.Context) interactive.CoreMessage { } if isPluginCmd { + _, fullPluginName := e.pluginExecutor.getEnabledPlugins(e.conversation.ExecutorBindings, cmdCtx.Args[0]) + e.reportCommand(ctx, fullPluginName, e.pluginExecutor.GetCommandPrefix(cmdCtx.Args), cmdCtx.ExecutorFilter.IsActive(), cmdCtx) + if isHelpCmd(cmdCtx.Args) { return e.ExecuteHelp(ctx, cmdCtx) } - e.reportCommand(ctx, e.pluginExecutor.GetCommandPrefix(cmdCtx.Args), cmdCtx.ExecutorFilter.IsActive(), cmdCtx) + out, err := e.pluginExecutor.Execute(ctx, e.conversation.ExecutorBindings, e.conversation.SlackState, cmdCtx) switch { case err == nil: @@ -195,7 +198,7 @@ func (e *DefaultExecutor) Execute(ctx context.Context) interactive.CoreMessage { fn, foundRes, foundFn := e.cmdsMapping.FindFn(cmdVerb, cmdRes) if !foundRes { - e.reportCommand(ctx, anonymizedInvalidVerb, false, cmdCtx) + e.reportCommand(ctx, "", anonymizedInvalidVerb, false, cmdCtx) e.log.Infof("received unsupported command: %q", cmdCtx.CleanCmd) return respond(unsupportedCmdMsg, cmdCtx) } @@ -206,7 +209,7 @@ func (e *DefaultExecutor) Execute(ctx context.Context) interactive.CoreMessage { e.log.Infof("received unsupported resource: %q", cmdCtx.CleanCmd) reportedCmd = fmt.Sprintf("%s {invalid feature}", reportedCmd) } - e.reportCommand(ctx, reportedCmd, false, cmdCtx) + e.reportCommand(ctx, "", reportedCmd, false, cmdCtx) msg := e.cmdsMapping.HelpMessageForVerb(cmdVerb) return respond(msg, cmdCtx) } else { @@ -214,7 +217,7 @@ func (e *DefaultExecutor) Execute(ctx context.Context) interactive.CoreMessage { if cmdRes != "" { cmdToReport = fmt.Sprintf("%s %s", cmdVerb, cmdRes) } - e.reportCommand(ctx, cmdToReport, false, cmdCtx) + e.reportCommand(ctx, "", cmdToReport, false, cmdCtx) } msg, err := fn(ctx, cmdCtx) @@ -236,7 +239,6 @@ func (e *DefaultExecutor) Execute(ctx context.Context) interactive.CoreMessage { } func (e *DefaultExecutor) ExecuteHelp(ctx context.Context, cmdCtx CommandContext) interactive.CoreMessage { - e.reportCommand(ctx, e.pluginExecutor.GetCommandPrefix(cmdCtx.Args), cmdCtx.ExecutorFilter.IsActive(), cmdCtx) msg, err := e.pluginExecutor.Help(ctx, e.conversation.ExecutorBindings, cmdCtx) if err != nil { e.log.Errorf("while executing help command %q: %s", cmdCtx.CleanCmd, err.Error()) @@ -280,32 +282,35 @@ func header(cmdCtx CommandContext) string { cmd = fmt.Sprintf("`%s`", cmd) out := fmt.Sprintf("%s on `%s`", cmd, cmdCtx.ClusterName) - return appendByUserOnlyIfNeeded(out, cmdCtx.User, cmdCtx.Conversation.CommandOrigin) + return appendByUserOnlyIfNeeded(out, cmdCtx.User.Mention, cmdCtx.Conversation.CommandOrigin) } func removeMultipleSpaces(s string) string { return strings.Join(strings.Fields(s), " ") } -func (e *DefaultExecutor) reportCommand(ctx context.Context, verb string, withFilter bool, cmdCtx CommandContext) { +func (e *DefaultExecutor) reportCommand(ctx context.Context, pluginName, verb string, withFilter bool, cmdCtx CommandContext) { if err := e.analyticsReporter.ReportCommand(e.platform, verb, e.conversation.CommandOrigin, withFilter); err != nil { e.log.Errorf("while reporting %s command: %s", verb, err.Error()) } - if err := e.reportAuditEvent(ctx, cmdCtx); err != nil { + if err := e.reportAuditEvent(ctx, pluginName, cmdCtx); err != nil { e.log.Errorf("while reporting executor audit event for %s: %s", verb, err.Error()) } } -func (e *DefaultExecutor) reportAuditEvent(ctx context.Context, cmdCtx CommandContext) error { - platform, err := remoteapi.NewBotPlatform(cmdCtx.Platform.String()) - if err != nil { - return err +func (e *DefaultExecutor) reportAuditEvent(ctx context.Context, pluginName string, cmdCtx CommandContext) error { + platform := remoteapi.NewBotPlatform(cmdCtx.Platform.String()) + + channelName := cmdCtx.Conversation.Alias + if cmdCtx.Conversation.DisplayName != "" { + channelName = cmdCtx.Conversation.DisplayName } + event := audit.ExecutorAuditEvent{ - PlatformUser: cmdCtx.User, + PlatformUser: cmdCtx.User.DisplayName, CreatedAt: time.Now().Format(time.RFC3339), - PluginName: cmdCtx.Args[0], - Channel: cmdCtx.CommGroupName, + PluginName: pluginName, + Channel: channelName, Command: cmdCtx.ExpandedRawCmd, BotPlatform: platform, } diff --git a/pkg/execute/factory.go b/pkg/execute/factory.go index 1a116a585..e291f80ee 100644 --- a/pkg/execute/factory.go +++ b/pkg/execute/factory.go @@ -183,6 +183,7 @@ func NewExecutorFactory(params DefaultExecutorFactoryParams) (*DefaultExecutorFa // Conversation contains details about the conversation. type Conversation struct { Alias string + DisplayName string ID string ExecutorBindings []string SourceBindings []string @@ -198,7 +199,13 @@ type NewDefaultInput struct { NotifierHandler NotifierHandler Conversation Conversation Message string - User string + User UserInput +} + +// UserInput contains details about the user. +type UserInput struct { + Mention string + DisplayName string } // NewDefault creates new Default Executor. diff --git a/pkg/execute/mapping.go b/pkg/execute/mapping.go index 75cc17885..a28bb205d 100644 --- a/pkg/execute/mapping.go +++ b/pkg/execute/mapping.go @@ -46,7 +46,7 @@ type CommandContext struct { CommGroupName string CleanCmd string ProvidedClusterName string - User string + User UserInput Conversation Conversation Platform config.CommPlatformIntegration ExecutorFilter executorFilter diff --git a/pkg/execute/sourcebinding.go b/pkg/execute/sourcebinding.go index f141519ef..fbfd336da 100644 --- a/pkg/execute/sourcebinding.go +++ b/pkg/execute/sourcebinding.go @@ -108,7 +108,7 @@ func (e *SourceBindingExecutor) Edit(ctx context.Context, cmdCtx CommandContext) return empty, errInvalidCommand } cmdArgs := cmdCtx.Args[2:] - msg, err := e.editSourceBindingHandler(ctx, cmdArgs, cmdCtx.CommGroupName, cmdCtx.Platform, cmdCtx.Conversation, cmdCtx.User) + msg, err := e.editSourceBindingHandler(ctx, cmdArgs, cmdCtx.CommGroupName, cmdCtx.Platform, cmdCtx.Conversation, cmdCtx.User.Mention) if err != nil { return empty, err } @@ -165,7 +165,11 @@ func (e *SourceBindingExecutor) editSourceBindingHandler(ctx context.Context, cm } return interactive.CoreMessage{ - Description: e.getEditedSourceBindingsMsg(userID, sourceList), + Message: api.Message{ + BaseBody: api.Body{ + Plaintext: e.getEditedSourceBindingsMsg(userID, sourceList), + }, + }, }, nil } @@ -181,7 +185,11 @@ func (e *SourceBindingExecutor) generateUnknownMessage(unknown []string) interac list := english.OxfordWordSeries(e.quoteEachItem(unknown), "and") word := english.PluralWord(len(unknown), "source was", "sources were") return interactive.CoreMessage{ - Description: fmt.Sprintf(unknownSourcesMsgFmt, list, word), + Message: api.Message{ + BaseBody: api.Body{ + Plaintext: fmt.Sprintf(unknownSourcesMsgFmt, list, word), + }, + }, } } diff --git a/pkg/execute/sourcebinding_test.go b/pkg/execute/sourcebinding_test.go index 92ba565a6..bdd8d23a0 100644 --- a/pkg/execute/sourcebinding_test.go +++ b/pkg/execute/sourcebinding_test.go @@ -133,7 +133,9 @@ func TestSourceBindingsHappyPath(t *testing.T) { CommGroupName: groupName, Platform: platform, Conversation: conversation, - User: userID, + User: UserInput{ + Mention: userID, + }, } expMessage := interactive.CoreMessage{ Description: tc.message, @@ -188,7 +190,9 @@ func TestSourceBindingsErrors(t *testing.T) { CommGroupName: groupName, Platform: platform, Conversation: conversation, - User: userID, + User: UserInput{ + Mention: userID, + }, } // when msg, err := executor.Edit(context.Background(), cmdCtx) @@ -265,7 +269,9 @@ func TestSourceBindingsMultiSelectMessage(t *testing.T) { CommGroupName: groupName, Platform: platform, Conversation: conversation, - User: userID, + User: UserInput{ + Mention: userID, + }, } // when gotMsg, err := executor.Edit(context.Background(), cmdCtx) @@ -329,7 +335,9 @@ func TestSourceBindingsMultiSelectMessageWithIncorrectBindingConfig(t *testing.T CommGroupName: groupName, Platform: platform, Conversation: conversation, - User: userID, + User: UserInput{ + Mention: userID, + }, } // when gotMsg, err := executor.Edit(context.Background(), cmdCtx) diff --git a/pkg/ptr/ptr.go b/pkg/ptr/ptr.go index d42a00feb..39e5662c5 100644 --- a/pkg/ptr/ptr.go +++ b/pkg/ptr/ptr.go @@ -13,6 +13,11 @@ func Bool(in bool) *bool { return &in } +// String returns pointer to a given input string value. +func String(in string) *string { + return &in +} + // IsTrue returns true if the given pointer is not nil and its value is true. func IsTrue(in *bool) bool { if in == nil { From 6afd6a09ceb104db7a13019c40f8d70b57f8e60e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kosiec?= Date: Fri, 24 Mar 2023 16:44:00 +0100 Subject: [PATCH 2/5] Cache MM usernames and fix another possible empty error log --- cmd/botkube/main.go | 10 +++++++--- pkg/bot/mattermost.go | 26 +++++++++++++++++++++++--- pkg/execute/sourcebinding_test.go | 18 +++++++++++++++--- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/cmd/botkube/main.go b/cmd/botkube/main.go index a57eedd85..a9e7086cd 100644 --- a/cmd/botkube/main.go +++ b/cmd/botkube/main.go @@ -403,9 +403,13 @@ func run(ctx context.Context) error { errGroup.Go(func() error { defer func() { - err := reportFatalError("while starting k8s collector", err) - if err != nil { - logger.Errorf("while reporting fatal error %w", err) + if err == nil { + return + } + + reportErr := reportFatalError("while starting k8s collector", err) + if reportErr != nil { + logger.Errorf("while reporting fatal error: %s", reportErr.Error()) } }() heartbeatReporter := heartbeat.GetReporter(remoteCfgEnabled, logger, gqlClient) diff --git a/pkg/bot/mattermost.go b/pkg/bot/mattermost.go index 5f71c2196..3a660f69b 100644 --- a/pkg/bot/mattermost.go +++ b/pkg/bot/mattermost.go @@ -62,6 +62,7 @@ type Mattermost struct { notifyMutex sync.Mutex botMentionRegex *regexp.Regexp renderer *MattermostRenderer + userNamesForID map[string]string } // mattermostMessage contains message details to execute command and send back the result @@ -129,6 +130,7 @@ func NewMattermost(log logrus.FieldLogger, commGroupName string, cfg config.Matt channels: channelsByIDCfg, botMentionRegex: botMentionRegex, renderer: NewMattermostRenderer(), + userNamesForID: map[string]string{}, }, nil } @@ -231,9 +233,12 @@ func (b *Mattermost) handleMessage(ctx context.Context, mm *mattermostMessage) e channel, exists := b.getChannels()[channelID] mm.IsAuthChannel = exists - user, _, err := b.apiClient.GetUser(post.UserId, "") + userName, err := b.getUserName(post.UserId) if err != nil { - b.log.Errorf("while getting user with ID %q: %s", post.UserId, err.Error()) + b.log.Errorf("while getting user name: %v", err) + } + if userName == "" { + userName = post.UserId } e := b.executorFactory.NewDefault(execute.NewDefaultInput{ @@ -251,7 +256,7 @@ func (b *Mattermost) handleMessage(ctx context.Context, mm *mattermostMessage) e }, User: execute.UserInput{ //Mention: "", // not used currently - DisplayName: user.Username, + DisplayName: userName, }, Message: req, }) @@ -452,6 +457,21 @@ func (b *Mattermost) setChannels(channels map[string]channelConfigByID) { b.channels = channels } +func (b *Mattermost) getUserName(userID string) (string, error) { + userName, exists := b.userNamesForID[userID] + if exists { + return userName, nil + } + + user, _, err := b.apiClient.GetUser(userID, "") + if err != nil { + return "", fmt.Errorf("while getting user with ID %q: %w", userID, err) + } + b.userNamesForID[userID] = user.Username + + return user.Username, nil +} + func getBotUserID(client *model.Client4, teamID, botName string) (string, error) { users, _, err := client.AutocompleteUsersInTeam(teamID, botName, 1, "") if err != nil { diff --git a/pkg/execute/sourcebinding_test.go b/pkg/execute/sourcebinding_test.go index bdd8d23a0..1f47c8667 100644 --- a/pkg/execute/sourcebinding_test.go +++ b/pkg/execute/sourcebinding_test.go @@ -138,7 +138,11 @@ func TestSourceBindingsHappyPath(t *testing.T) { }, } expMessage := interactive.CoreMessage{ - Description: tc.message, + Message: api.Message{ + BaseBody: api.Body{ + Plaintext: tc.message, + }, + }, } // when msg, err := executor.Edit(context.Background(), cmdCtx) @@ -167,7 +171,11 @@ func TestSourceBindingsErrors(t *testing.T) { expErr: nil, expMsg: interactive.CoreMessage{ - Description: ":exclamation: The `something-else` source was not found in configuration. To learn how to add custom source, visit https://docs.botkube.io/configuration/source.", + Message: api.Message{ + BaseBody: api.Body{ + Plaintext: ":exclamation: The `something-else` source was not found in configuration. To learn how to add custom source, visit https://docs.botkube.io/configuration/source.", + }, + }, }, }, { @@ -176,7 +184,11 @@ func TestSourceBindingsErrors(t *testing.T) { expErr: nil, expMsg: interactive.CoreMessage{ - Description: ":exclamation: The `something-else` and `other` sources were not found in configuration. To learn how to add custom source, visit https://docs.botkube.io/configuration/source.", + Message: api.Message{ + BaseBody: api.Body{ + Plaintext: ":exclamation: The `something-else` and `other` sources were not found in configuration. To learn how to add custom source, visit https://docs.botkube.io/configuration/source.", + }, + }, }, }, } From cf084dff4abf24345ff7b26f63880693d8f5595d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kosiec?= Date: Fri, 24 Mar 2023 19:48:00 +0100 Subject: [PATCH 3/5] Report not authenticated channels too --- pkg/bot/discord.go | 11 ++++++++++- pkg/bot/mattermost.go | 7 +++++++ pkg/bot/socketslack.go | 1 + pkg/execute/executor.go | 2 +- 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/bot/discord.go b/pkg/bot/discord.go index 5a8fe6468..05bdab327 100644 --- a/pkg/bot/discord.go +++ b/pkg/bot/discord.go @@ -220,7 +220,16 @@ func (b *Discord) handleMessage(ctx context.Context, dm discordMessage) error { b.log.Debugf("Discord incoming Request: %s", req) - channel, isAuthChannel := b.getChannels()[dm.Event.ChannelID] + channel, exists := b.getChannels()[dm.Event.ChannelID] + if !exists { + channel = channelConfigByID{ + ChannelBindingsByID: config.ChannelBindingsByID{ + ID: dm.Event.ChannelID, + }, + } + } + isAuthChannel := exists + e := b.executorFactory.NewDefault(execute.NewDefaultInput{ CommGroupName: b.commGroupName, Platform: b.IntegrationName(), diff --git a/pkg/bot/mattermost.go b/pkg/bot/mattermost.go index 3a660f69b..c878dd6d0 100644 --- a/pkg/bot/mattermost.go +++ b/pkg/bot/mattermost.go @@ -231,6 +231,13 @@ func (b *Mattermost) handleMessage(ctx context.Context, mm *mattermostMessage) e channelID := mm.Event.GetBroadcast().ChannelId channel, exists := b.getChannels()[channelID] + if !exists { + channel = channelConfigByID{ + ChannelBindingsByID: config.ChannelBindingsByID{ + ID: channelID, + }, + } + } mm.IsAuthChannel = exists userName, err := b.getUserName(post.UserId) diff --git a/pkg/bot/socketslack.go b/pkg/bot/socketslack.go index 4f5f1536a..f04711f75 100644 --- a/pkg/bot/socketslack.go +++ b/pkg/bot/socketslack.go @@ -352,6 +352,7 @@ func (b *SocketSlack) handleMessage(ctx context.Context, event socketSlackMessag Conversation: execute.Conversation{ Alias: channel.alias, ID: channel.Identifier(), + DisplayName: info.Name, ExecutorBindings: channel.Bindings.Executors, SourceBindings: channel.Bindings.Sources, IsAuthenticated: isAuthChannel, diff --git a/pkg/execute/executor.go b/pkg/execute/executor.go index 6afe1b1e6..da628c67f 100644 --- a/pkg/execute/executor.go +++ b/pkg/execute/executor.go @@ -301,7 +301,7 @@ func (e *DefaultExecutor) reportCommand(ctx context.Context, pluginName, verb st func (e *DefaultExecutor) reportAuditEvent(ctx context.Context, pluginName string, cmdCtx CommandContext) error { platform := remoteapi.NewBotPlatform(cmdCtx.Platform.String()) - channelName := cmdCtx.Conversation.Alias + channelName := cmdCtx.Conversation.ID if cmdCtx.Conversation.DisplayName != "" { channelName = cmdCtx.Conversation.DisplayName } From 46c64f098c6d6c82efbfeb75d54f79d461921939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kosiec?= Date: Mon, 27 Mar 2023 12:12:55 +0200 Subject: [PATCH 4/5] Fix invalid logging of deployment ID for heartbeat and status reporters --- internal/heartbeat/gql_reporter.go | 2 +- internal/status/gql_reporter.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/heartbeat/gql_reporter.go b/internal/heartbeat/gql_reporter.go index 40e52fe7c..bd8526fc9 100644 --- a/internal/heartbeat/gql_reporter.go +++ b/internal/heartbeat/gql_reporter.go @@ -31,7 +31,7 @@ func newGraphQLHeartbeatReporter(logger logrus.FieldLogger, client GraphQLClient func (r *GraphQLHeartbeatReporter) ReportHeartbeat(ctx context.Context, heartbeat DeploymentHeartbeatInput) error { logger := r.log.WithFields(logrus.Fields{ - "deploymentID": r.gql.DeploymentID, + "deploymentID": r.gql.DeploymentID(), "heartbeat": heartbeat, }) logger.Debug("Sending heartbeat...") diff --git a/internal/status/gql_reporter.go b/internal/status/gql_reporter.go index f08b2c31c..e7b634b51 100644 --- a/internal/status/gql_reporter.go +++ b/internal/status/gql_reporter.go @@ -45,7 +45,7 @@ func newGraphQLStatusReporter(logger logrus.FieldLogger, client GraphQLClient, r // ReportDeploymentStartup reports deployment startup to GraphQL server. func (r *GraphQLStatusReporter) ReportDeploymentStartup(ctx context.Context) error { logger := r.log.WithFields(logrus.Fields{ - "deploymentID": r.gql.DeploymentID, + "deploymentID": r.gql.DeploymentID(), "resourceVersion": r.getResourceVersion(), "type": "startup", }) @@ -73,7 +73,7 @@ func (r *GraphQLStatusReporter) ReportDeploymentStartup(ctx context.Context) err // ReportDeploymentShutdown reports deployment shutdown to GraphQL server. func (r *GraphQLStatusReporter) ReportDeploymentShutdown(ctx context.Context) error { logger := r.log.WithFields(logrus.Fields{ - "deploymentID": r.gql.DeploymentID, + "deploymentID": r.gql.DeploymentID(), "resourceVersion": r.getResourceVersion(), "type": "shutdown", }) @@ -100,7 +100,7 @@ func (r *GraphQLStatusReporter) ReportDeploymentShutdown(ctx context.Context) er // ReportDeploymentFailed reports deployment failure to GraphQL server. func (r *GraphQLStatusReporter) ReportDeploymentFailed(ctx context.Context) error { logger := r.log.WithFields(logrus.Fields{ - "deploymentID": r.gql.DeploymentID, + "deploymentID": r.gql.DeploymentID(), "resourceVersion": r.getResourceVersion(), "type": "failure", }) From d6d19f76c1c583a955489eb623d7d91a0ea09749 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Kosiec?= Date: Mon, 27 Mar 2023 14:24:24 +0200 Subject: [PATCH 5/5] Improve error logging --- pkg/bot/mattermost.go | 2 +- pkg/bot/socketslack.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/bot/mattermost.go b/pkg/bot/mattermost.go index c878dd6d0..44dbfedfb 100644 --- a/pkg/bot/mattermost.go +++ b/pkg/bot/mattermost.go @@ -242,7 +242,7 @@ func (b *Mattermost) handleMessage(ctx context.Context, mm *mattermostMessage) e userName, err := b.getUserName(post.UserId) if err != nil { - b.log.Errorf("while getting user name: %v", err) + b.log.Errorf("while getting user name: %s", err.Error()) } if userName == "" { userName = post.UserId diff --git a/pkg/bot/socketslack.go b/pkg/bot/socketslack.go index f04711f75..e7dbcde3c 100644 --- a/pkg/bot/socketslack.go +++ b/pkg/bot/socketslack.go @@ -154,14 +154,14 @@ func (b *SocketSlack) Start(ctx context.Context) error { user, err := b.client.GetUserInfoContext(ctx, ev.User) if err != nil { - b.log.Errorf("While getting user info: %s", err.Error()) + b.log.Errorf("while getting user info: %s", err.Error()) } if user != nil && user.RealName != "" { msg.UserName = user.RealName } if err := b.handleMessage(ctx, msg); err != nil { - b.log.Errorf("Message handling error: %s", err.Error()) + b.log.Errorf("while handling message: %s", err.Error()) } } }