Skip to content

Commit

Permalink
Add logic to handle communicator bindings in kubectl, fix bugs with k…
Browse files Browse the repository at this point in the history
…ubectl commands (#672)
  • Loading branch information
mszostok committed Aug 5, 2022
1 parent e082c01 commit 0582539
Show file tree
Hide file tree
Showing 22 changed files with 1,276 additions and 472 deletions.
27 changes: 18 additions & 9 deletions cmd/botkube/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/kubeshop/botkube/pkg/config"
"github.com/kubeshop/botkube/pkg/controller"
"github.com/kubeshop/botkube/pkg/execute"
"github.com/kubeshop/botkube/pkg/execute/kubectl"
"github.com/kubeshop/botkube/pkg/filterengine"
"github.com/kubeshop/botkube/pkg/httpsrv"
"github.com/kubeshop/botkube/pkg/sink"
Expand Down Expand Up @@ -114,22 +115,30 @@ func run() error {
// Set up the filter engine
filterEngine := filterengine.WithAllFilters(logger, dynamicCli, mapper, conf)

// Create Executor Factory
resMapping, err := execute.LoadResourceMappingIfShould(
logger.WithField(componentLogFieldKey, "Resource Mapping Loader"),
conf,
discoveryCli,
)
if err != nil {
return reportFatalError("while loading resource mapping", err)
// Kubectl config merger
kcMerger := kubectl.NewMerger(conf.Executors)

// Load resource variants name if needed
var resourceNameNormalizerFunc kubectl.ResourceVariantsFunc
if kcMerger.IsAtLeastOneEnabled() {
resourceNameNormalizer, err := kubectl.NewResourceNormalizer(
logger.WithField(componentLogFieldKey, "Resource Name Normalizer"),
discoveryCli,
)
if err != nil {
return reportFatalError("while creating resource name normalizer", err)
}
resourceNameNormalizerFunc = resourceNameNormalizer.Normalize
}

// Create executor factor
executorFactory := execute.NewExecutorFactory(
logger.WithField(componentLogFieldKey, "Executor"),
execute.DefaultCommandRunnerFunc,
*conf,
filterEngine,
resMapping,
kubectl.NewChecker(resourceNameNormalizerFunc),
kcMerger,
reporter,
)

Expand Down
40 changes: 39 additions & 1 deletion helm/botkube/e2e-test-values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,16 @@ communications:
slack:
enabled: false # Tests will override this temporarily
token: "" # Provide a valid token for BotKube app
channel: "" # Tests will override this temporarily
channels:
'default':
name: "" # Test will override that
bindings:
executors:
- kubectl-read-only
- kubectl-wait-cmd
- kubectl-exec-cmd
- kubectl-allow-all

sources:
'k8s-events':
kubernetes:
Expand All @@ -30,6 +39,35 @@ executors:
include:
- botkube
- default
'kubectl-wait-cmd':
kubectl:
enabled: true
namespaces:
include:
- botkube
- default
commands:
verbs: [ "wait" ]
restrictAccess: false
'kubectl-exec-cmd':
kubectl:
enabled: false
namespaces:
include:
- botkube
- default
commands:
verbs: [ "exec" ]
restrictAccess: false
'kubectl-allow-all':
kubectl:
enabled: true
namespaces:
include:
- all
commands:
verbs: [ "get" ]
resources: [ "deployments" ]

settings:
clusterName: sample
Expand Down
12 changes: 3 additions & 9 deletions pkg/bot/discord.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,9 @@ type Discord struct {

Notification config.Notification
Token string
AllowKubectl bool
RestrictAccess bool
ClusterName string
ChannelID string
BotID string
DefaultNamespace string
ExecutorBindings []string
}

// discordMessage contains message details to execute command and send back the result.
Expand Down Expand Up @@ -82,11 +79,8 @@ func NewDiscord(log logrus.FieldLogger, c *config.Config, executorFactory Execut

Token: discord.Token,
BotID: discord.BotID,
AllowKubectl: c.Executors.GetFirst().Kubectl.Enabled,
RestrictAccess: c.Executors.GetFirst().Kubectl.RestrictAccess,
ClusterName: c.Settings.ClusterName,
ChannelID: discord.Channels.GetFirst().ID,
DefaultNamespace: c.Executors.GetFirst().Kubectl.DefaultNamespace,
ExecutorBindings: discord.Channels.GetFirst().Bindings.Executors,
Notification: discord.Notification,
}, nil
}
Expand Down Expand Up @@ -213,7 +207,7 @@ func (dm *discordMessage) HandleMessage(b *Discord) {

e := dm.executorFactory.NewDefault(b.IntegrationName(), b, dm.IsAuthChannel, dm.Request)

dm.Response = e.Execute()
dm.Response = e.Execute(b.ExecutorBindings)
dm.Send()
}

Expand Down
12 changes: 3 additions & 9 deletions pkg/bot/mattermost.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,11 @@ type Mattermost struct {
BotName string
TeamName string
ChannelID string
ClusterName string
AllowKubectl bool
RestrictAccess bool
ServerURL string
WebSocketURL string
WSClient *model.WebSocketClient
APIClient *model.Client4
DefaultNamespace string
ExecutorBindings []string
}

// mattermostMessage contains message details to execute command and send back the result
Expand Down Expand Up @@ -120,10 +117,7 @@ func NewMattermost(log logrus.FieldLogger, c *config.Config, executorFactory Exe
Token: mattermost.Token,
TeamName: mattermost.Team,
ChannelID: channel.Id,
ClusterName: c.Settings.ClusterName,
AllowKubectl: c.Executors.GetFirst().Kubectl.Enabled,
RestrictAccess: c.Executors.GetFirst().Kubectl.RestrictAccess,
DefaultNamespace: c.Executors.GetFirst().Kubectl.DefaultNamespace,
ExecutorBindings: mattermost.Channels.GetFirst().Bindings.Executors,
APIClient: client,
WebSocketURL: webSocketURL,
}, nil
Expand Down Expand Up @@ -212,7 +206,7 @@ func (mm *mattermostMessage) handleMessage(b *Mattermost) {
mm.Request = r.ReplaceAllString(post.Message, ``)

e := mm.executorFactory.NewDefault(b.IntegrationName(), b, mm.IsAuthChannel, mm.Request)
mm.Response = e.Execute()
mm.Response = e.Execute(b.ExecutorBindings)
mm.sendMessage()
}

Expand Down
12 changes: 3 additions & 9 deletions pkg/bot/slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,8 @@ type Slack struct {

Client *slack.Client
Notification config.Notification
AllowKubectl bool
RestrictAccess bool
ClusterName string
ChannelName string
DefaultNamespace string
ExecutorBindings []string
}

// slackMessage contains message details to execute command and send back the result
Expand Down Expand Up @@ -87,11 +84,8 @@ func NewSlack(log logrus.FieldLogger, c *config.Config, executorFactory Executor
botID: botID,
Client: client,
Notification: slackCfg.Notification,
AllowKubectl: c.Executors.GetFirst().Kubectl.Enabled,
RestrictAccess: c.Executors.GetFirst().Kubectl.RestrictAccess,
ClusterName: c.Settings.ClusterName,
ChannelName: slackCfg.Channels.GetFirst().Name,
DefaultNamespace: c.Executors.GetFirst().Kubectl.DefaultNamespace,
ExecutorBindings: slackCfg.Channels.GetFirst().Bindings.Executors,
}, nil
}

Expand Down Expand Up @@ -219,7 +213,7 @@ func (sm *slackMessage) HandleMessage(b *Slack) error {
sm.Request = strings.TrimPrefix(sm.Event.Text, "<@"+sm.BotID+">")

e := sm.executorFactory.NewDefault(b.IntegrationName(), b, sm.IsAuthChannel, sm.Request)
sm.Response = e.Execute()
sm.Response = e.Execute(b.ExecutorBindings)
err = sm.Send()
if err != nil {
return fmt.Errorf("while sending message: %w", err)
Expand Down
50 changes: 23 additions & 27 deletions pkg/bot/teams.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,18 @@ type Teams struct {
notifyMutex sync.RWMutex
notify bool

BotName string
AppID string
AppPassword string
MessagePath string
Port string
AllowKubectl bool
RestrictAccess bool
ClusterName string
Notification config.Notification
Adapter core.Adapter
DefaultNamespace string
BotName string
AppID string
AppPassword string
MessagePath string
Port string
ClusterName string
Notification config.Notification
Adapter core.Adapter

conversationRefMutex sync.RWMutex
conversationRef *schema.ConversationReference
ExecutorsBindings []string
}

type consentContext struct {
Expand All @@ -87,20 +85,18 @@ func NewTeams(log logrus.FieldLogger, c *config.Config, executorFactory Executor
msgPath = "/"
}
return &Teams{
log: log,
executorFactory: executorFactory,
reporter: reporter,
notify: false, // disabled by default
BotName: teams.BotName,
AppID: teams.AppID,
AppPassword: teams.AppPassword,
Notification: teams.Notification,
MessagePath: msgPath,
Port: port,
AllowKubectl: c.Executors.GetFirst().Kubectl.Enabled,
RestrictAccess: c.Executors.GetFirst().Kubectl.RestrictAccess,
DefaultNamespace: c.Executors.GetFirst().Kubectl.DefaultNamespace,
ClusterName: c.Settings.ClusterName,
log: log,
executorFactory: executorFactory,
reporter: reporter,
notify: false, // disabled by default
BotName: teams.BotName,
AppID: teams.AppID,
AppPassword: teams.AppPassword,
Notification: teams.Notification,
MessagePath: msgPath,
Port: port,
ExecutorsBindings: teams.Channels.GetFirst().Bindings.Executors,
ClusterName: c.Settings.ClusterName,
}
}

Expand Down Expand Up @@ -220,7 +216,7 @@ func (b *Teams) processActivity(w http.ResponseWriter, req *http.Request) {
msgWithoutPrefix := strings.TrimPrefix(consentCtx.Command, msgPrefix)
msg := strings.TrimSpace(msgWithoutPrefix)
e := b.executorFactory.NewDefault(b.IntegrationName(), newTeamsNotifMgrForActivity(b, activity), true, msg)
out := e.Execute()
out := e.Execute(b.ExecutorsBindings)

actJSON, _ := json.MarshalIndent(turn.Activity, "", " ")
b.log.Debugf("Incoming MSTeams Activity: %s", actJSON)
Expand Down Expand Up @@ -259,7 +255,7 @@ func (b *Teams) processMessage(activity schema.Activity) string {
// Multicluster is not supported for Teams

e := b.executorFactory.NewDefault(b.IntegrationName(), newTeamsNotifMgrForActivity(b, activity), true, msg)
return format.CodeBlock(e.Execute())
return format.CodeBlock(e.Execute(b.ExecutorsBindings))
}

func (b *Teams) putRequest(u string, data []byte) (err error) {
Expand Down
10 changes: 5 additions & 5 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ const (
// Config structure of configuration yaml file
type Config struct {
Sources IndexableMap[Sources] `yaml:"sources"`
Executors IndexableMap[Executors] `yaml:"executors" validate:"required,eq=1"`
Executors IndexableMap[Executors] `yaml:"executors" validate:"required,min=1"`
Communications IndexableMap[Communications] `yaml:"communications" validate:"required,eq=1"`

Analytics Analytics `yaml:"analytics"`
Expand Down Expand Up @@ -332,11 +332,11 @@ type Webhook struct {

// Kubectl configuration for executing commands inside cluster
type Kubectl struct {
Namespaces Namespaces `yaml:"namespaces"`
Namespaces Namespaces `yaml:"namespaces,omitempty"`
Enabled bool `yaml:"enabled"`
Commands Commands `yaml:"commands"`
DefaultNamespace string `yaml:"defaultNamespace"`
RestrictAccess bool `yaml:"restrictAccess"`
Commands Commands `yaml:"commands,omitempty"`
DefaultNamespace string `yaml:"defaultNamespace,omitempty"`
RestrictAccess *bool `yaml:"restrictAccess,omitempty"`
}

// Commands allowed in bot
Expand Down
13 changes: 1 addition & 12 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,23 +162,12 @@ func TestLoadedConfigValidation(t *testing.T) {
name: "empty executors and communications settings",
expErrMsg: heredoc.Doc(`
while validating loaded configuration: 2 errors occurred:
* Key: 'Config.Executors' Error:Field validation for 'Executors' failed on the 'eq' tag
* Key: 'Config.Executors' Error:Field validation for 'Executors' failed on the 'min' tag
* Key: 'Config.Communications' Error:Field validation for 'Communications' failed on the 'eq' tag`),
configFiles: []string{
testdataFile(t, "empty-executors-communications.yaml"),
},
},
{
// TODO(remove): https://github.com/kubeshop/botkube/issues/596
name: "multiple executors and communications settings",
expErrMsg: heredoc.Doc(`
while validating loaded configuration: 2 errors occurred:
* Key: 'Config.Executors' Error:Field validation for 'Executors' failed on the 'eq' tag
* Key: 'Config.Communications' Error:Field validation for 'Communications' failed on the 'eq' tag`),
configFiles: []string{
testdataFile(t, "multiple-executors-communications.yaml"),
},
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand Down
Loading

0 comments on commit 0582539

Please sign in to comment.