From c455a5f7af8e532ff78db78e42480e756e20f83d Mon Sep 17 00:00:00 2001 From: Christoph Hartmann Date: Sat, 27 Jan 2024 18:37:08 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=9A=A4=20improve=20performance=20for=20la?= =?UTF-8?q?rge=20slack=20accounts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If you are a large organisation with 1000s of conversations, the previous implementation fetched information about the creator via the user rest call. The problem is that the slack api heavily rate limits the user rest call. This change makes the creator information asynchronous and therefore allows much speedier fetching of the conversations. This avoids reaching the rate limit. --- providers/slack/connection/connection_test.go | 38 ++++++++++++++++++ providers/slack/go.mod | 4 ++ providers/slack/resources/conversations.go | 39 ++++++++++++------- providers/slack/resources/slack.lr | 2 +- providers/slack/resources/slack.lr.go | 16 +++++++- 5 files changed, 83 insertions(+), 16 deletions(-) create mode 100644 providers/slack/connection/connection_test.go diff --git a/providers/slack/connection/connection_test.go b/providers/slack/connection/connection_test.go new file mode 100644 index 0000000000..e6c65e0c75 --- /dev/null +++ b/providers/slack/connection/connection_test.go @@ -0,0 +1,38 @@ +// Copyright (c) Mondoo, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +//go:build debugtest +// +build debugtest + +package connection + +import ( + "context" + "testing" + + "github.com/slack-go/slack" + "github.com/stretchr/testify/require" + "go.mondoo.com/cnquery/v10/providers-sdk/v1/inventory" + "go.mondoo.com/cnquery/v10/providers-sdk/v1/vault" +) + +func TestSlackProvider(t *testing.T) { + + cred := &vault.Credential{ + Type: vault.CredentialType_password, + Password: "", + } + cred.PreProcess() + + conn, err := NewSlackConnection(1, &inventory.Asset{}, &inventory.Config{ + Type: "slack", + Credentials: []*vault.Credential{cred}, + }) + require.NoError(t, err) + + client := conn.Client() + ctx := context.Background() + users, err := client.GetUsersContext(ctx, slack.GetUsersOptionLimit(999)) + require.NoError(t, err) + require.NotNil(t, users) +} diff --git a/providers/slack/go.mod b/providers/slack/go.mod index 4ae11913a2..2965595197 100644 --- a/providers/slack/go.mod +++ b/providers/slack/go.mod @@ -10,6 +10,7 @@ require ( github.com/hashicorp/go-retryablehttp v0.7.5 github.com/rs/zerolog v1.31.0 github.com/slack-go/slack v0.12.3 + github.com/stretchr/testify v1.8.4 go.mondoo.com/cnquery/v10 v10.0.3 ) @@ -47,6 +48,7 @@ require ( github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b // indirect github.com/cockroachdb/redact v1.1.5 // indirect github.com/danieljoos/wincred v1.2.1 // indirect + github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/dvsekhvalnov/jose2go v1.6.0 // indirect github.com/fatih/color v1.16.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect @@ -93,6 +95,7 @@ require ( github.com/muesli/termenv v0.15.2 // indirect github.com/oklog/run v1.1.0 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/rivo/uniseg v0.4.6 // indirect github.com/rogpeppe/go-internal v1.12.0 // indirect github.com/ryanuber/go-glob v1.0.0 // indirect @@ -124,6 +127,7 @@ require ( google.golang.org/genproto/googleapis/rpc v0.0.0-20240125205218-1f4bbc51befe // indirect google.golang.org/grpc v1.61.0 // indirect google.golang.org/protobuf v1.32.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect moul.io/http2curl v1.0.0 // indirect sigs.k8s.io/yaml v1.4.0 // indirect ) diff --git a/providers/slack/resources/conversations.go b/providers/slack/resources/conversations.go index 09c199205b..2fd3ee96db 100644 --- a/providers/slack/resources/conversations.go +++ b/providers/slack/resources/conversations.go @@ -118,21 +118,9 @@ func newMqlSlackConversation(runtime *plugin.Runtime, conversation slack.Channel created := conversation.Created.Time() - var creator plugin.Resource - - if conversation.Creator != "" { - creator, err = NewResource(runtime, "slack.user", map[string]*llx.RawData{ - "id": llx.StringData(conversation.Creator), - }) - if err != nil { - return nil, err - } - } - - return CreateResource(runtime, "slack.conversation", map[string]*llx.RawData{ + r, err := CreateResource(runtime, "slack.conversation", map[string]*llx.RawData{ "id": llx.StringData(conversation.ID), "name": llx.StringData(conversation.Name), - "creator": llx.ResourceData(creator, "slack.user"), "created": llx.TimeData(created), "locale": llx.StringData(conversation.Locale), "topic": llx.DictData(topic), @@ -150,12 +138,37 @@ func newMqlSlackConversation(runtime *plugin.Runtime, conversation slack.Channel "isOrgShared": llx.BoolData(conversation.IsOrgShared), "priority": llx.FloatData(conversation.Priority), }) + if err != nil { + return nil, err + } + mqlConversation := r.(*mqlSlackConversation) + mqlConversation.creatorId = conversation.Creator + return mqlConversation, nil +} + +type mqlSlackConversationInternal struct { + creatorId string } func (x *mqlSlackConversation) id() (string, error) { return "slack.conversation/" + x.Id.Data, nil } +func (s *mqlSlackConversation) creator() (*mqlSlackUser, error) { + if s.creatorId == "" { + s.Creator = plugin.TValue[*mqlSlackUser]{State: plugin.StateIsSet | plugin.StateIsNull} + return nil, nil + } + + r, err := NewResource(s.MqlRuntime, "slack.user", map[string]*llx.RawData{ + "id": llx.StringData(s.creatorId), + }) + if err != nil { + return nil, err + } + return r.(*mqlSlackUser), nil +} + func (s *mqlSlackConversation) members() ([]interface{}, error) { conn := s.MqlRuntime.Connection.(*connection.SlackConnection) client := conn.Client() diff --git a/providers/slack/resources/slack.lr b/providers/slack/resources/slack.lr index 7582b82a49..182957b152 100644 --- a/providers/slack/resources/slack.lr +++ b/providers/slack/resources/slack.lr @@ -179,7 +179,7 @@ slack.conversation @defaults("id name") { // Name of the conversation name string // User that created this conversation - creator slack.user + creator() slack.user // Timestamp of when the conversation was created created time // IETF language code that represents chosen language diff --git a/providers/slack/resources/slack.lr.go b/providers/slack/resources/slack.lr.go index c70e330a84..3319595f50 100644 --- a/providers/slack/resources/slack.lr.go +++ b/providers/slack/resources/slack.lr.go @@ -1662,7 +1662,7 @@ func (c *mqlSlackLogin) GetDateLast() *plugin.TValue[*time.Time] { type mqlSlackConversation struct { MqlRuntime *plugin.Runtime __id string - // optional: if you define mqlSlackConversationInternal it will be used here + mqlSlackConversationInternal Id plugin.TValue[string] Name plugin.TValue[string] Creator plugin.TValue[*mqlSlackUser] @@ -1731,7 +1731,19 @@ func (c *mqlSlackConversation) GetName() *plugin.TValue[string] { } func (c *mqlSlackConversation) GetCreator() *plugin.TValue[*mqlSlackUser] { - return &c.Creator + return plugin.GetOrCompute[*mqlSlackUser](&c.Creator, func() (*mqlSlackUser, error) { + if c.MqlRuntime.HasRecording { + d, err := c.MqlRuntime.FieldResourceFromRecording("slack.conversation", c.__id, "creator") + if err != nil { + return nil, err + } + if d != nil { + return d.Value.(*mqlSlackUser), nil + } + } + + return c.creator() + }) } func (c *mqlSlackConversation) GetCreated() *plugin.TValue[*time.Time] {