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

Output filtering #808

Merged
merged 34 commits into from
Oct 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
d42ba54
Scaffold - added during rebase.
ezodude Oct 6, 2022
44a5f72
Wired the executor to filter results - rebase comment.
ezodude Oct 8, 2022
f0dbe33
Added actual text filter implementation with tests. - rebase comment.
ezodude Oct 10, 2022
b5d2eee
Tidy up.
ezodude Oct 10, 2022
458aa69
Fixed minor extract results filter regression and added tests. - reba…
ezodude Oct 10, 2022
163c71d
Ensured extract results filter works left/right single/double quotes.
ezodude Oct 10, 2022
8a6d925
Executor now corrects for left/right double/single quotes.
ezodude Oct 11, 2022
1c26b2c
Added --filter=xxx executor based e2e tests.
ezodude Oct 11, 2022
b8a3c86
Renamed to executor filter and added code documentation.
ezodude Oct 11, 2022
63f377d
extractExecutorFilter now returns whether a filter flag was detected …
ezodude Oct 11, 2022
197ec2a
Added filter used to analytics command reporting. - rebase comment
ezodude Oct 12, 2022
d6ec7f1
Revert back erroneously updated "filter". - rebase comment.
ezodude Oct 12, 2022
1587e85
declare filter flag regexp at top of the package.
ezodude Oct 12, 2022
89c5f1b
Add heredoc.Doc for better readability.
ezodude Oct 12, 2022
cddf0a1
Updated spacing of "expectedFilteredBody" in e2e tests for better rea…
ezodude Oct 12, 2022
8e1e8f8
Reworked ExecutorFilter to encapsulate previous command and withFilte…
ezodude Oct 13, 2022
96f5237
Extracting an ExecutorFilter can now return an error.
ezodude Oct 13, 2022
51ab35c
The filter flag is now parsed and validated similar in behaviour to s…
ezodude Oct 13, 2022
3fe0f37
Better error messaging.
ezodude Oct 13, 2022
2e2ce3a
Fix tests.
ezodude Oct 14, 2022
08de92c
Linter fixes.
ezodude Oct 14, 2022
a8f78ea
Ensure any regex characters are escaped when matching filters with ca…
ezodude Oct 14, 2022
d25452d
Merge branch 'main' into results-filter
ezodude Oct 14, 2022
856bbb2
Ensure kubectl commands run minus --filter flag after main rebase.
ezodude Oct 14, 2022
03d849b
Responses no longer rely on a closure while ensuring the interactive …
ezodude Oct 14, 2022
d4739b2
Ensure the kc-cmd-builder does not conflict with the executor filter.
ezodude Oct 14, 2022
08e49d6
Remove override cmd header candidates when they're not used.
ezodude Oct 14, 2022
ecfff0a
Extract args from the raw command.
ezodude Oct 14, 2022
1abde3f
Fixed double escape of regular expression metacharacters during rebase.
ezodude Oct 14, 2022
f7c90d3
Added unit tests to cover filtering using special characters.
ezodude Oct 14, 2022
b0b008b
Ensure we avoid panic when there are no matches to return the filtere…
ezodude Oct 14, 2022
35981c1
Merge branch 'main' into results-filter
ezodude Oct 14, 2022
45aeaa9
Merge fix!
ezodude Oct 14, 2022
b20d3b8
Removed missed use of execFilter in error to avoid panics.
ezodude Oct 17, 2022
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: 1 addition & 1 deletion internal/analytics/noop_reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (n NoopReporter) RegisterCurrentIdentity(_ context.Context, _ kubernetes.In
}

// ReportCommand reports a new executed command. The command should be anonymized before using this method.
func (n NoopReporter) ReportCommand(_ config.CommPlatformIntegration, _ string, _ command.Origin) error {
func (n NoopReporter) ReportCommand(_ config.CommPlatformIntegration, _ string, _ command.Origin, _ bool) error {
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion internal/analytics/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type Reporter interface {
RegisterCurrentIdentity(ctx context.Context, k8sCli kubernetes.Interface) error

// ReportCommand reports a new executed command. The command should be anonymized before using this method.
ReportCommand(platform config.CommPlatformIntegration, command string, origin command.Origin) error
ReportCommand(platform config.CommPlatformIntegration, command string, origin command.Origin, withFilter bool) error

// ReportBotEnabled reports an enabled bot.
ReportBotEnabled(platform config.CommPlatformIntegration) error
Expand Down
3 changes: 2 additions & 1 deletion internal/analytics/segment_reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,12 @@ func (r *SegmentReporter) RegisterCurrentIdentity(ctx context.Context, k8sCli ku

// ReportCommand reports a new executed command. The command should be anonymized before using this method.
// The RegisterCurrentIdentity needs to be called first.
func (r *SegmentReporter) ReportCommand(platform config.CommPlatformIntegration, command string, origin command.Origin) error {
func (r *SegmentReporter) ReportCommand(platform config.CommPlatformIntegration, command string, origin command.Origin, withFilter bool) error {
return r.reportEvent("Command executed", map[string]interface{}{
"platform": platform,
"command": command,
"origin": origin,
"filtered": withFilter,
})
}

Expand Down
6 changes: 3 additions & 3 deletions internal/analytics/segment_reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ func TestSegmentReporter_ReportCommand(t *testing.T) {
segmentReporter, segmentCli := fakeSegmentReporterWithIdentity(identity)

// when
err := segmentReporter.ReportCommand(config.DiscordCommPlatformIntegration, "notifier stop", command.TypedOrigin)
err := segmentReporter.ReportCommand(config.DiscordCommPlatformIntegration, "notifier stop", command.TypedOrigin, false)
require.NoError(t, err)

err = segmentReporter.ReportCommand(config.SlackCommPlatformIntegration, "get", command.ButtonClickOrigin)
err = segmentReporter.ReportCommand(config.SlackCommPlatformIntegration, "get", command.ButtonClickOrigin, false)
require.NoError(t, err)

err = segmentReporter.ReportCommand(config.TeamsCommPlatformIntegration, "notifier start", command.SelectValueChangeOrigin)
err = segmentReporter.ReportCommand(config.TeamsCommPlatformIntegration, "notifier start", command.SelectValueChangeOrigin, false)
require.NoError(t, err)

// then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"timestamp": "2009-11-17T20:34:58.651387237Z",
"properties": {
"command": "notifier stop",
"filtered": false,
"origin": "typed",
"platform": "discord"
}
Expand All @@ -19,6 +20,7 @@
"timestamp": "2009-11-17T20:34:58.651387237Z",
"properties": {
"command": "get",
"filtered": false,
"origin": "buttonClick",
"platform": "slack"
}
Expand All @@ -31,6 +33,7 @@
"timestamp": "2009-11-17T20:34:58.651387237Z",
"properties": {
"command": "notifier start",
"filtered": false,
"origin": "selectValueChange",
"platform": "teams"
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/bot/discord.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (b *Discord) Start(ctx context.Context) error {
msg := discordMessage{
Event: m,
}
if err := b.handleMessage(msg); err != nil {
if err := b.handleMessage(ctx, msg); err != nil {
b.log.Errorf("Message handling error: %s", err.Error())
}
})
Expand Down Expand Up @@ -227,7 +227,7 @@ func (b *Discord) SetNotificationsEnabled(channelID string, enabled bool) error
}

// HandleMessage handles the incoming messages.
func (b *Discord) handleMessage(dm discordMessage) error {
func (b *Discord) handleMessage(ctx context.Context, dm discordMessage) error {
// Handle message only if starts with mention
req, found := b.findAndTrimBotMention(dm.Event.Content)
if !found {
Expand All @@ -252,7 +252,7 @@ func (b *Discord) handleMessage(dm discordMessage) error {
User: fmt.Sprintf("<@%s>", dm.Event.Author.ID),
})

response := e.Execute()
response := e.Execute(ctx)
err := b.send(dm.Event, req, response)
if err != nil {
return fmt.Errorf("while sending message: %w", err)
Expand Down
6 changes: 3 additions & 3 deletions pkg/bot/mattermost.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func (b *Mattermost) SetNotificationsEnabled(channelID string, enabled bool) err
}

// Check incoming message and take action
func (mm *mattermostMessage) handleMessage(b *Mattermost) {
func (mm *mattermostMessage) handleMessage(ctx context.Context, b *Mattermost) {
post, err := postFromEvent(mm.Event)
if err != nil {
b.log.Error(err)
Expand Down Expand Up @@ -239,7 +239,7 @@ func (mm *mattermostMessage) handleMessage(b *Mattermost) {
},
Message: mm.Request,
})
response := e.Execute()
response := e.Execute(ctx)
mm.sendMessage(b, response)
}

Expand Down Expand Up @@ -364,7 +364,7 @@ func (b *Mattermost) listen(ctx context.Context) {
IsAuthChannel: false,
APIClient: b.apiClient,
}
mm.handleMessage(b)
mm.handleMessage(ctx, b)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/bot/slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func (b *Slack) Start(ctx context.Context) error {
ThreadTimeStamp: ev.ThreadTimestamp,
User: ev.User,
}
err := b.handleMessage(sm)
err := b.handleMessage(ctx, sm)
if err != nil {
wrappedErr := fmt.Errorf("while handling message: %w", err)
b.log.Errorf(wrappedErr.Error())
Expand Down Expand Up @@ -215,7 +215,7 @@ func (b *Slack) SetNotificationsEnabled(channelName string, enabled bool) error
return nil
}

func (b *Slack) handleMessage(msg slackMessage) error {
func (b *Slack) handleMessage(ctx context.Context, msg slackMessage) error {
// Handle message only if starts with mention
request, found := b.findAndTrimBotMention(msg.Text)
if !found {
Expand Down Expand Up @@ -248,7 +248,7 @@ func (b *Slack) handleMessage(msg slackMessage) error {
Message: request,
User: fmt.Sprintf("<@%s>", msg.User),
})
response := e.Execute()
response := e.Execute(ctx)
err = b.send(msg, request, response, response.OnlyVisibleForYou)
if err != nil {
return fmt.Errorf("while sending message: %w", err)
Expand Down
14 changes: 7 additions & 7 deletions pkg/bot/socketslack.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ type socketSlackMessage struct {
// socketSlackAnalyticsReporter defines a reporter that collects analytics data.
type socketSlackAnalyticsReporter interface {
FatalErrorAnalyticsReporter
ReportCommand(platform config.CommPlatformIntegration, command string, origin command.Origin) error
ReportCommand(platform config.CommPlatformIntegration, command string, origin command.Origin, withFilter bool) error
}

// NewSocketSlack creates a new SocketSlack instance.
Expand Down Expand Up @@ -158,7 +158,7 @@ func (b *SocketSlack) Start(ctx context.Context) error {
User: ev.User,
CommandOrigin: command.TypedOrigin,
}
if err := b.handleMessage(msg); err != nil {
if err := b.handleMessage(ctx, msg); err != nil {
b.log.Errorf("Message handling error: %s", err.Error())
}
}
Expand All @@ -183,7 +183,7 @@ func (b *SocketSlack) Start(ctx context.Context) error {

act := callback.ActionCallback.BlockActions[0]
if act == nil || strings.HasPrefix(act.ActionID, urlButtonActionIDPrefix) {
reportErr := b.reporter.ReportCommand(b.IntegrationName(), act.ActionID, command.ButtonClickOrigin)
reportErr := b.reporter.ReportCommand(b.IntegrationName(), act.ActionID, command.ButtonClickOrigin, false)
if reportErr != nil {
b.log.Errorf("while reporting URL command, error: %s", reportErr.Error())
}
Expand Down Expand Up @@ -217,7 +217,7 @@ func (b *SocketSlack) Start(ctx context.Context) error {
ResponseURL: callback.ResponseURL,
BlockID: act.BlockID,
}
if err := b.handleMessage(msg); err != nil {
if err := b.handleMessage(ctx, msg); err != nil {
b.log.Errorf("Message handling error: %s", err.Error())
}
case slack.InteractionTypeViewSubmission: // this event is received when modal is submitted
Expand All @@ -235,7 +235,7 @@ func (b *SocketSlack) Start(ctx context.Context) error {
CommandOrigin: cmdOrigin,
}

if err := b.handleMessage(msg); err != nil {
if err := b.handleMessage(ctx, msg); err != nil {
b.log.Errorf("Message handling error: %s", err.Error())
}
}
Expand Down Expand Up @@ -294,7 +294,7 @@ func (b *SocketSlack) SetNotificationsEnabled(channelName string, enabled bool)
return nil
}

func (b *SocketSlack) handleMessage(event socketSlackMessage) error {
func (b *SocketSlack) handleMessage(ctx context.Context, event socketSlackMessage) error {
// Handle message only if starts with mention
request, found := b.findAndTrimBotMention(event.Text)
if !found {
Expand Down Expand Up @@ -328,7 +328,7 @@ func (b *SocketSlack) handleMessage(event socketSlackMessage) error {
Message: request,
User: fmt.Sprintf("<@%s>", event.User),
})
response := e.Execute()
response := e.Execute(ctx)
err = b.send(event, request, response)
if err != nil {
return fmt.Errorf("while sending message: %w", err)
Expand Down
8 changes: 4 additions & 4 deletions pkg/bot/teams.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (b *Teams) processActivity(w http.ResponseWriter, req *http.Request) {

err = b.Adapter.ProcessActivity(ctx, activity, coreActivity.HandlerFuncs{
OnMessageFunc: func(turn *coreActivity.TurnContext) (schema.Activity, error) {
n, resp := b.processMessage(turn.Activity)
n, resp := b.processMessage(ctx, turn.Activity)
if n >= teamsMaxMessageSize {
if turn.Activity.Conversation.ConversationType == convTypePersonal {
// send file upload request
Expand Down Expand Up @@ -242,7 +242,7 @@ func (b *Teams) processActivity(w http.ResponseWriter, req *http.Request) {
}

activity.Text = consentCtx.Command
_, resp := b.processMessage(activity)
_, resp := b.processMessage(ctx, activity)

actJSON, err := json.MarshalIndent(turn.Activity, "", " ")
if err != nil {
Expand Down Expand Up @@ -276,7 +276,7 @@ func (b *Teams) processActivity(w http.ResponseWriter, req *http.Request) {
}
}

func (b *Teams) processMessage(activity schema.Activity) (int, string) {
func (b *Teams) processMessage(ctx context.Context, activity schema.Activity) (int, string) {
trimmedMsg := b.trimBotMention(activity.Text)

// Multicluster is not supported for Teams
Expand All @@ -300,7 +300,7 @@ func (b *Teams) processMessage(activity schema.Activity) (int, string) {
},
Message: trimmedMsg,
})
return b.convertInteractiveMessage(e.Execute(), false)
return b.convertInteractiveMessage(e.Execute(ctx), false)
}

func (b *Teams) convertInteractiveMessage(in interactive.Message, forceMarkdown bool) (int, string) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/execute/edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (e *EditExecutor) Do(args []string, commGroupName string, platform config.C

defer func() {
cmdToReport := fmt.Sprintf("%s %s", cmdName, cmdVerb)
err := e.analyticsReporter.ReportCommand(platform, cmdToReport, conversation.CommandOrigin)
err := e.analyticsReporter.ReportCommand(platform, cmdToReport, conversation.CommandOrigin, false)
if err != nil {
e.log.Errorf("while reporting edit command: %s", err.Error())
}
Expand Down
Loading