From 05825396a3aa01e68fdbec71e22fe1c2d8a3f06b Mon Sep 17 00:00:00 2001 From: Mateusz Szostok Date: Fri, 5 Aug 2022 09:18:10 +0200 Subject: [PATCH] Add logic to handle communicator bindings in kubectl, fix bugs with kubectl commands (#672) --- cmd/botkube/main.go | 27 +- helm/botkube/e2e-test-values.yaml | 40 ++- pkg/bot/discord.go | 12 +- pkg/bot/mattermost.go | 12 +- pkg/bot/slack.go | 12 +- pkg/bot/teams.go | 50 ++-- pkg/config/config.go | 10 +- pkg/config/config_test.go | 13 +- pkg/execute/executor.go | 45 ++- pkg/execute/executor_test.go | 207 ++++++++------ pkg/execute/factory.go | 18 +- pkg/execute/kubectl.go | 177 +++++++----- pkg/execute/kubectl/checker.go | 51 ++++ pkg/execute/kubectl/checker_test.go | 140 +++++++++ pkg/execute/kubectl/helpers_test.go | 67 +++++ pkg/execute/kubectl/merger.go | 138 +++++++++ pkg/execute/kubectl/merger_test.go | 156 ++++++++++ pkg/execute/kubectl/resource-normalizer.go | 55 ++++ pkg/execute/kubectl_test.go | 315 ++++++++++++++------- pkg/execute/resource.go | 61 ---- pkg/ptr/ptr.go | 14 + test/e2e/slack_test.go | 128 +++++++-- 22 files changed, 1276 insertions(+), 472 deletions(-) create mode 100644 pkg/execute/kubectl/checker.go create mode 100644 pkg/execute/kubectl/checker_test.go create mode 100644 pkg/execute/kubectl/helpers_test.go create mode 100644 pkg/execute/kubectl/merger.go create mode 100644 pkg/execute/kubectl/merger_test.go create mode 100644 pkg/execute/kubectl/resource-normalizer.go delete mode 100644 pkg/execute/resource.go create mode 100644 pkg/ptr/ptr.go diff --git a/cmd/botkube/main.go b/cmd/botkube/main.go index 235bfe581..3c99a2438 100644 --- a/cmd/botkube/main.go +++ b/cmd/botkube/main.go @@ -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" @@ -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, ) diff --git a/helm/botkube/e2e-test-values.yaml b/helm/botkube/e2e-test-values.yaml index 7b5d0a479..c6ff20ea6 100644 --- a/helm/botkube/e2e-test-values.yaml +++ b/helm/botkube/e2e-test-values.yaml @@ -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: @@ -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 diff --git a/pkg/bot/discord.go b/pkg/bot/discord.go index 9b1193219..41d189f9d 100644 --- a/pkg/bot/discord.go +++ b/pkg/bot/discord.go @@ -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. @@ -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 } @@ -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() } diff --git a/pkg/bot/mattermost.go b/pkg/bot/mattermost.go index dc4268360..f278609d0 100644 --- a/pkg/bot/mattermost.go +++ b/pkg/bot/mattermost.go @@ -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 @@ -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 @@ -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() } diff --git a/pkg/bot/slack.go b/pkg/bot/slack.go index dc4f9caea..ba07045bd 100644 --- a/pkg/bot/slack.go +++ b/pkg/bot/slack.go @@ -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 @@ -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 } @@ -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) diff --git a/pkg/bot/teams.go b/pkg/bot/teams.go index ddb0071ca..69056dcb8 100644 --- a/pkg/bot/teams.go +++ b/pkg/bot/teams.go @@ -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 { @@ -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, } } @@ -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) @@ -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) { diff --git a/pkg/config/config.go b/pkg/config/config.go index 1ebd166af..acf86f2b2 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -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"` @@ -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 diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 183b9beb8..0b6370ac1 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -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) { diff --git a/pkg/execute/executor.go b/pkg/execute/executor.go index 25921ba0e..4f427b263 100644 --- a/pkg/execute/executor.go +++ b/pkg/execute/executor.go @@ -3,7 +3,6 @@ package execute import ( "bytes" "fmt" - "sort" "strings" "text/tabwriter" @@ -71,7 +70,6 @@ type DefaultExecutor struct { filterEngine filterengine.FilterEngine log logrus.FieldLogger runCmdFn CommandRunnerFunc - resMapping ResourceMapping notifierExecutor *NotifierExecutor notifierHandler NotifierHandler @@ -140,7 +138,7 @@ func (action FiltersAction) String() string { } // Execute executes commands and returns output -func (e *DefaultExecutor) Execute() string { +func (e *DefaultExecutor) Execute(channelBindings []string) string { // Remove hyperlink if it got added automatically command := utils.RemoveHyperlink(e.Message) @@ -164,7 +162,7 @@ func (e *DefaultExecutor) Execute() string { return "" // user specified different target cluster } - if e.kubectlExecutor.CanHandle(args) { + if e.kubectlExecutor.CanHandle(channelBindings, args) { // Currently the verb is always at the first place of `args`, and, in a result, `finalArgs`. // The length of the slice was already checked before // See the DefaultExecutor.Execute() logic. @@ -174,10 +172,11 @@ func (e *DefaultExecutor) Execute() string { // TODO: Return error when the DefaultExecutor is refactored as a part of https://github.com/kubeshop/botkube/issues/589 e.log.Errorf("while reporting executed command: %s", err.Error()) } - out, err := e.kubectlExecutor.Execute(e.Message, e.IsAuthChannel) + out, err := e.kubectlExecutor.Execute(channelBindings, e.Message, e.IsAuthChannel) if err != nil { // TODO: Return error when the DefaultExecutor is refactored as a part of https://github.com/kubeshop/botkube/issues/589 e.log.Errorf("while executing kubectl: %s", err.Error()) + return "" } return out } @@ -307,15 +306,13 @@ func (e *DefaultExecutor) runInfoCommand(args []string, isAuthChannel bool) stri return fmt.Sprintf(WrongClusterCmdMsg, args[3]) } - allowedVerbs := e.getSortedEnabledCommands("allowed verbs", e.resMapping.AllowedKubectlVerbMap) - allowedResources := e.getSortedEnabledCommands("allowed resources", e.resMapping.AllowedKubectlResourceMap) - allowedNamespaces, err := e.getNamespaceConfig() + enabledKubectls, err := e.getEnabledKubectlConfigs() if err != nil { // TODO: Return error when the DefaultExecutor is refactored as a part of https://github.com/kubeshop/botkube/issues/589 e.log.Errorf("while rendering namespace config: %s", err.Error()) } - return fmt.Sprintf("%s%s%s", allowedVerbs, allowedResources, allowedNamespaces) + return enabledKubectls } // Use tabwriter to display string in tabular form @@ -377,32 +374,24 @@ func (e *DefaultExecutor) runVersionCommand(args []string, clusterName string) s return e.findBotKubeVersion() } -func (e *DefaultExecutor) getSortedEnabledCommands(header string, commands map[string]bool) string { - var keys []string - for key := range commands { - if !commands[key] { +func (e *DefaultExecutor) getEnabledKubectlConfigs() (string, error) { + type kubectlCollection map[string]config.Kubectl + + enabledKubectls := kubectlCollection{} + for name, item := range e.cfg.Executors { + if !item.Kubectl.Enabled { continue } - keys = append(keys, key) + enabledKubectls[name] = item.Kubectl } - sort.Strings(keys) - if len(keys) == 0 { - return fmt.Sprintf("%s: []", header) + out := map[string]map[string]kubectlCollection{ + "Enabled executors": { + "kubectl": enabledKubectls, + }, } - const itemSeparator = "\n - " - items := strings.Join(keys, itemSeparator) - return fmt.Sprintf("%s:%s%s\n", header, itemSeparator, items) -} - -func (e *DefaultExecutor) getNamespaceConfig() (string, error) { - ns := e.cfg.Executors.GetFirst().Kubectl.Namespaces - - out := map[string]config.Namespaces{ - "allowed namespaces": ns, - } var buff strings.Builder encode := yaml.NewEncoder(&buff) encode.SetIndent(2) diff --git a/pkg/execute/executor_test.go b/pkg/execute/executor_test.go index dd4699523..c4ec5cb96 100644 --- a/pkg/execute/executor_test.go +++ b/pkg/execute/executor_test.go @@ -6,117 +6,134 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" "github.com/kubeshop/botkube/pkg/config" ) -func TestDefaultExecutor_getSortedEnabledCommands(t *testing.T) { - testCases := []struct { - Name string - InputHeader string - InputMap map[string]bool - ExpectedOutput string - }{ - { - Name: "All commands disabled", - InputHeader: "test", - InputMap: map[string]bool{ - "cmd1": false, - "cmd2": false, - }, - ExpectedOutput: "test: []", - }, - { - Name: "All commands enabled", - InputHeader: "foo", - InputMap: map[string]bool{ - "b1": true, - "a2": false, - "a3": true, - "a4": true, - "a5": true, - "a6": false, - }, - ExpectedOutput: heredoc.Doc(` - foo: - - a3 - - a4 - - a5 - - b1 - `), - }, - } - for _, testCase := range testCases { - t.Run(testCase.Name, func(t *testing.T) { - e := &DefaultExecutor{} - res := e.getSortedEnabledCommands(testCase.InputHeader, testCase.InputMap) +var rawEnabledExecutorsConfig = ` +executors: + 'kubectl-team-a': + kubectl: + enabled: true + namespaces: + include: [ "team-a" ] + commands: + verbs: [ "get" ] + resources: [ "deployments" ] + defaultNamespace: "team-a" + 'kubectl-team-b': + kubectl: + enabled: true + namespaces: + include: [ "team-b" ] + commands: + verbs: [ "get", "describe" ] + resources: [ "deployments", "pods" ] + 'kubectl-global': + kubectl: + enabled: true + namespaces: + include: [ "all" ] + commands: + verbs: [ "logs", "top" ] + resources: [ ] + 'kubectl-exec': + kubectl: + enabled: false + namespaces: + include: [ "all" ] + commands: + verbs: [ "exec" ] + resources: [ ]` - assert.Equal(t, testCase.ExpectedOutput, res) - }) - } -} +var rawDisabledExecutorsConfig = ` +executors: + 'kubectl-team-a': + kubectl: + enabled: false + namespaces: + include: [ "team-a" ] + commands: + verbs: [ "get" ] + resources: [ "deployments" ] + defaultNamespace: "team-a" + 'kubectl-team-b': + kubectl: + enabled: false + namespaces: + include: [ "team-b" ] + commands: + verbs: [ "get", "describe" ] + resources: [ "deployments", "pods" ]` -func TestDefaultExecutor_getSortedNamespaceConfig(t *testing.T) { +func TestDefaultExecutor_getEnabledKubectlConfigs(t *testing.T) { testCases := []struct { - name string - nsConfig config.Namespaces - expOutput string + name string + executorsConfig string + expOutput string }{ { - name: "All namespaces enabled", - nsConfig: config.Namespaces{ - Include: []string{config.AllNamespaceIndicator}, - }, - expOutput: heredoc.Doc(` - allowed namespaces: - include: - - all - `), - }, - { - name: "All namespace enabled and a few ignored", - nsConfig: config.Namespaces{ - Include: []string{config.AllNamespaceIndicator}, - Ignore: []string{"demo", "abc", "ns-*-test"}, - }, + name: "All namespaces enabled", + executorsConfig: rawEnabledExecutorsConfig, expOutput: heredoc.Doc(` - allowed namespaces: - include: - - all - ignore: - - demo - - abc - - ns-*-test - `), + Enabled executors: + kubectl: + kubectl-global: + namespaces: + include: + - all + enabled: true + commands: + verbs: + - logs + - top + resources: [] + kubectl-team-a: + namespaces: + include: + - team-a + enabled: true + commands: + verbs: + - get + resources: + - deployments + defaultNamespace: team-a + kubectl-team-b: + namespaces: + include: + - team-b + enabled: true + commands: + verbs: + - get + - describe + resources: + - deployments + - pods + `), }, { - name: "Only some namespace enabled", - nsConfig: config.Namespaces{ - Include: []string{"demo", "abc"}, - }, + name: "All namespace enabled and a few ignored", + executorsConfig: rawDisabledExecutorsConfig, expOutput: heredoc.Doc(` - allowed namespaces: - include: - - demo - - abc - `), + Enabled executors: + kubectl: {} + `), }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { // given - e := &DefaultExecutor{cfg: config.Config{ - Executors: config.IndexableMap[config.Executors]{ - "default": config.Executors{ - Kubectl: config.Kubectl{ - Namespaces: tc.nsConfig, - }, - }, + executor := &DefaultExecutor{ + cfg: config.Config{ + Executors: fixExecutorsConfig(t, tc.executorsConfig), }, - }} + } // when - res, err := e.getNamespaceConfig() + res, err := executor.getEnabledKubectlConfigs() // then require.NoError(t, err) @@ -124,3 +141,13 @@ func TestDefaultExecutor_getSortedNamespaceConfig(t *testing.T) { }) } } + +func fixExecutorsConfig(t *testing.T, raw string) config.IndexableMap[config.Executors] { + t.Helper() + + var givenCfg config.Config + err := yaml.Unmarshal([]byte(raw), &givenCfg) + require.NoError(t, err) + + return givenCfg.Executors +} diff --git a/pkg/execute/factory.go b/pkg/execute/factory.go index 752352775..9e7d79cd7 100644 --- a/pkg/execute/factory.go +++ b/pkg/execute/factory.go @@ -4,6 +4,7 @@ import ( "github.com/sirupsen/logrus" "github.com/kubeshop/botkube/pkg/config" + "github.com/kubeshop/botkube/pkg/execute/kubectl" "github.com/kubeshop/botkube/pkg/filterengine" ) @@ -13,7 +14,6 @@ type DefaultExecutorFactory struct { runCmdFn CommandRunnerFunc cfg config.Config filterEngine filterengine.FilterEngine - resMapping ResourceMapping analyticsReporter AnalyticsReporter notifierExecutor *NotifierExecutor kubectlExecutor *Kubectl @@ -21,7 +21,7 @@ type DefaultExecutorFactory struct { // Executor is an interface for processes to execute commands type Executor interface { - Execute() string + Execute(bindings []string) string } // AnalyticsReporter defines a reporter that collects analytics data. @@ -31,20 +31,12 @@ type AnalyticsReporter interface { } // NewExecutorFactory creates new DefaultExecutorFactory. -func NewExecutorFactory( - log logrus.FieldLogger, - runCmdFn CommandRunnerFunc, - cfg config.Config, - filterEngine filterengine.FilterEngine, - resMapping ResourceMapping, - analyticsReporter AnalyticsReporter, -) *DefaultExecutorFactory { +func NewExecutorFactory(log logrus.FieldLogger, runCmdFn CommandRunnerFunc, cfg config.Config, filterEngine filterengine.FilterEngine, kcChecker *kubectl.Checker, merger *kubectl.Merger, analyticsReporter AnalyticsReporter) *DefaultExecutorFactory { return &DefaultExecutorFactory{ log: log, runCmdFn: runCmdFn, cfg: cfg, filterEngine: filterEngine, - resMapping: resMapping, analyticsReporter: analyticsReporter, notifierExecutor: NewNotifierExecutor( log.WithField("component", "Notifier Executor"), @@ -54,7 +46,8 @@ func NewExecutorFactory( kubectlExecutor: NewKubectl( log.WithField("component", "Kubectl Executor"), cfg, - resMapping, + merger, + kcChecker, runCmdFn, ), } @@ -66,7 +59,6 @@ func (f *DefaultExecutorFactory) NewDefault(platform config.CommPlatformIntegrat log: f.log, runCmdFn: f.runCmdFn, cfg: f.cfg, - resMapping: f.resMapping, analyticsReporter: f.analyticsReporter, kubectlExecutor: f.kubectlExecutor, notifierExecutor: f.notifierExecutor, diff --git a/pkg/execute/kubectl.go b/pkg/execute/kubectl.go index 42aacee63..6a3f00de4 100644 --- a/pkg/execute/kubectl.go +++ b/pkg/execute/kubectl.go @@ -3,37 +3,44 @@ package execute import ( "fmt" "strings" + "unicode" "github.com/sirupsen/logrus" "github.com/spf13/pflag" "github.com/kubeshop/botkube/pkg/config" + "github.com/kubeshop/botkube/pkg/execute/kubectl" "github.com/kubeshop/botkube/pkg/utils" ) const ( - kubectlDisabledMsgFmt = "Sorry, the admin hasn't given me the permission to execute kubectl command on cluster '%s'." - kubectlNotAuthorizedMsgFmt = "Sorry, this channel is not authorized to execute kubectl command on cluster '%s'." - kubectlNotAllowedNamespaceMsgFmt = "Sorry, the kubectl command cannot be executed in the '%s' Namespace on cluster '%s'. Use 'commands list' to see all allowed namespaces." - kubectlNotAllowedKindMsgFmt = "Sorry, the kubectl command is not authorized to work with '%s' resources on cluster '%s'. Use 'commands list' to see all allowed resources." - kubectlDefaultNamespace = "default" + kubectlNotAuthorizedMsgFmt = "Sorry, this channel is not authorized to execute kubectl command on cluster '%s'." + kubectlNotAllowedVerbMsgFmt = "Sorry, the kubectl '%s' command cannot be executed in the '%s' Namespace on cluster '%s'. Use 'commands list' to see allowed commands." + kubectlNotAllowedVerbInAllNsMsgFmt = "Sorry, the kubectl '%s' command cannot be executed for all Namespaces on cluster '%s'. Use 'commands list' to see allowed commands." + kubectlNotAllowedKindMsgFmt = "Sorry, the kubectl command is not authorized to work with '%s' resources in the '%s' Namespace on cluster '%s'. Use 'commands list' to see allowed commands." + kubectlNotAllowedKinInAllNsMsgFmt = "Sorry, the kubectl command is not authorized to work with '%s' resources for all Namespaces on cluster '%s'. Use 'commands list' to see allowed commands." + kubectlFlagAfterVerbMsg = "Please specify the resource name after the verb, and all flags after the resource name. Format [flags]" + kubectlDefaultNamespace = "default" ) // Kubectl executes kubectl commands using local binary. type Kubectl struct { - log logrus.FieldLogger - cfg config.Config - resMapping ResourceMapping - runCmdFn CommandRunnerFunc + log logrus.FieldLogger + cfg config.Config + + kcChecker *kubectl.Checker + runCmdFn CommandRunnerFunc + merger *kubectl.Merger } // NewKubectl creates a new instance of Kubectl. -func NewKubectl(log logrus.FieldLogger, cfg config.Config, mapping ResourceMapping, fn CommandRunnerFunc) *Kubectl { +func NewKubectl(log logrus.FieldLogger, cfg config.Config, merger *kubectl.Merger, kcChecker *kubectl.Checker, fn CommandRunnerFunc) *Kubectl { return &Kubectl{ - log: log, - cfg: cfg, - resMapping: mapping, - runCmdFn: fn, + log: log, + cfg: cfg, + merger: merger, + kcChecker: kcChecker, + runCmdFn: fn, } } @@ -41,13 +48,13 @@ func NewKubectl(log logrus.FieldLogger, cfg config.Config, mapping ResourceMappi // // TODO: we should just introduce a command name explicitly. In this case `@BotKube kubectl get po` instead of `@BotKube get po` // As a result, we are able to detect kubectl command but say that you're simply not authorized to use it instead of "Command not supported. (..)" -func (e *Kubectl) CanHandle(args []string) bool { +func (e *Kubectl) CanHandle(bindings []string, args []string) bool { if len(args) == 0 { return false } // Check if such kubectl verb is enabled - if !e.resMapping.AllowedKubectlVerbMap[args[0]] { + if !e.kcChecker.IsKnownVerb(e.merger.MergeAllEnabledVerbs(bindings), args[0]) { return false } @@ -59,7 +66,7 @@ func (e *Kubectl) CanHandle(args []string) bool { // This method should be called ONLY if: // - we are a target cluster, // - and Kubectl.CanHandle returned true. -func (e *Kubectl) Execute(command string, isAuthChannel bool) (string, error) { +func (e *Kubectl) Execute(bindings []string, command string, isAuthChannel bool) (string, error) { log := e.log.WithFields(logrus.Fields{ "isAuthChannel": isAuthChannel, "command": command, @@ -68,49 +75,55 @@ func (e *Kubectl) Execute(command string, isAuthChannel bool) (string, error) { log.Debugf("Handling command...") var ( - // TODO: https://github.com/kubeshop/botkube/issues/596 - // use a related config from communicator bindings. - kubectlCfg = e.cfg.Executors.GetFirst().Kubectl - - args = strings.Fields(strings.TrimSpace(command)) - clusterName = e.cfg.Settings.ClusterName - defaultNamespace = kubectlCfg.DefaultNamespace + args = strings.Fields(strings.TrimSpace(command)) + clusterName = e.cfg.Settings.ClusterName + verb = args[0] + resource = e.getResourceName(args) ) - if !isAuthChannel && kubectlCfg.RestrictAccess { + executionNs, err := e.getCommandNamespace(args) + if err != nil { + return "", fmt.Errorf("while extracting Namespace from command: %w", err) + } + if executionNs == "" { // namespace not found in command, so find default and add `-n` flag to args + executionNs = e.findDefaultNamespace(bindings) + args = e.addNamespaceFlag(args, executionNs) + } + + kcConfig := e.merger.MergeForNamespace(bindings, executionNs) + + if !isAuthChannel && kcConfig.RestrictAccess { msg := fmt.Sprintf(kubectlNotAuthorizedMsgFmt, clusterName) return e.omitIfIfWeAreNotExplicitlyTargetCluster(log, command, msg) } - if !kubectlCfg.Enabled { - msg := fmt.Sprintf(kubectlDisabledMsgFmt, clusterName) - return e.omitIfIfWeAreNotExplicitlyTargetCluster(log, command, msg) + if !e.kcChecker.IsVerbAllowedInNs(kcConfig, verb) { + if executionNs == config.AllNamespaceIndicator { + return fmt.Sprintf(kubectlNotAllowedVerbInAllNsMsgFmt, verb, clusterName), nil + } + return fmt.Sprintf(kubectlNotAllowedVerbMsgFmt, verb, executionNs, clusterName), nil } // Some commands don't have resources specified directly in command. For example: // - kubectl logs foo - if !validDebugCommands[args[0]] { + if !validDebugCommands[verb] && resource != "" { + if !e.validResourceName(resource) { + return kubectlFlagAfterVerbMsg, nil + } // Check if user has access to a given Kubernetes resource // TODO: instead of using config with allowed verbs and commands we simply should use related SA. - if len(args) > 1 && !e.matchesAllowedResources(args[1]) { - return fmt.Sprintf(kubectlNotAllowedKindMsgFmt, args[1], clusterName), nil + if !e.kcChecker.IsResourceAllowedInNs(kcConfig, resource) { + if executionNs == config.AllNamespaceIndicator { + return fmt.Sprintf(kubectlNotAllowedKinInAllNsMsgFmt, resource, clusterName), nil + } + return fmt.Sprintf(kubectlNotAllowedKindMsgFmt, resource, executionNs, clusterName), nil } } - args, executionNs, err := e.ensureNamespaceFlag(args, defaultNamespace) - if err != nil { - return "", fmt.Errorf("while ensuring Namespace for command execution: %w", err) - } - - if !kubectlCfg.Namespaces.IsAllowed(executionNs) { - return fmt.Sprintf(kubectlNotAllowedNamespaceMsgFmt, executionNs, clusterName), nil - } - finalArgs := e.getFinalArgs(args) out, err := e.runCmdFn(kubectlBinary, finalArgs) if err != nil { - errCtx := fmt.Errorf("while executing kubectl command: %w", err) - return fmt.Sprintf("Cluster: %s\n%s%s", clusterName, out, err.Error()), errCtx + return fmt.Sprintf("Cluster: %s\n%s%s", clusterName, out, err.Error()), nil } return fmt.Sprintf("Cluster: %s\n%s", clusterName, out), nil @@ -161,7 +174,7 @@ func (e *Kubectl) getFinalArgs(args []string) []string { } // getNamespaceFlag returns the namespace value extracted from a given args. -// If `--namespace/-n` was not found, returns 'default'. +// If `--namespace/-n` was not found, returns empty string. func (e *Kubectl) getNamespaceFlag(args []string) (string, error) { f := pflag.NewFlagSet("extract-ns", pflag.ContinueOnError) // ignore unknown flags errors, e.g. `--cluster-name` etc. @@ -175,40 +188,70 @@ func (e *Kubectl) getNamespaceFlag(args []string) (string, error) { return out, nil } -// ensureNamespaceFlag ensures that a Namespace flag is available in args. If necessary, add it to returned args list. -func (e *Kubectl) ensureNamespaceFlag(args []string, defaultNamespace string) ([]string, string, error) { - executionNs, err := e.getNamespaceFlag(args) +// getAllNamespaceFlag returns the namespace value extracted from a given args. +// If `--A, --all-namespaces` was not found, returns empty string. +func (e *Kubectl) getAllNamespaceFlag(args []string) (bool, error) { + f := pflag.NewFlagSet("extract-ns", pflag.ContinueOnError) + // ignore unknown flags errors, e.g. `--cluster-name` etc. + f.ParseErrorsWhitelist.UnknownFlags = true + + var out bool + f.BoolVarP(&out, "all-namespaces", "A", false, "Kubernetes All Namespaces") + if err := f.Parse(args); err != nil { + return false, err + } + return out, nil +} + +func (e *Kubectl) getCommandNamespace(args []string) (string, error) { + // 1. Check for `-A, --all-namespaces` in args. Based on the kubectl manual: + // "Namespace in current context is ignored even if specified with --namespace." + inAllNs, err := e.getAllNamespaceFlag(args) if err != nil { - return nil, "", fmt.Errorf("while getting Namespace for command execution: %w", err) + return "", err } - if executionNs != "" { // was specified in a received command - return args, executionNs, nil + if inAllNs { + return config.AllNamespaceIndicator, nil // TODO: find all namespaces } - if defaultNamespace == "" { - defaultNamespace = kubectlDefaultNamespace + // 2. Check for `-n/--namespace` in args + executionNs, err := e.getNamespaceFlag(args) + if err != nil { + return "", err + } + if executionNs != "" { + return executionNs, nil } - args = append([]string{"-n", defaultNamespace}, utils.DeleteDoubleWhiteSpace(args)...) - - return args, defaultNamespace, nil + return "", nil } -func (e *Kubectl) matchesAllowedResources(name string) bool { - variants := []string{ - // received name - name, - // normalized short name - e.resMapping.ShortnameResourceMap[strings.ToLower(name)], - // normalized kind name - e.resMapping.KindResourceMap[strings.ToLower(name)], +func (e *Kubectl) findDefaultNamespace(bindings []string) string { + // 1. Merge all enabled kubectls, to find the defaultNamespace settings + cfg := e.merger.MergeAllEnabled(bindings) + if cfg.DefaultNamespace != "" { + // 2. Use user defined default + return cfg.DefaultNamespace } - for _, name := range variants { - if e.resMapping.AllowedKubectlResourceMap[name] { - return true - } + // 3. If not found, explicitly use `default` namespace. + return kubectlDefaultNamespace +} + +// addNamespaceFlag add namespace to returned args list. +func (e *Kubectl) addNamespaceFlag(args []string, defaultNamespace string) []string { + return append([]string{"-n", defaultNamespace}, utils.DeleteDoubleWhiteSpace(args)...) +} + +func (e *Kubectl) getResourceName(args []string) string { + if len(args) < 2 { + return "" } + resource, _, _ := strings.Cut(args[1], "/") + return resource +} - return false +func (e *Kubectl) validResourceName(resource string) bool { + // ensures that resource name starts with letter + return unicode.IsLetter(rune(resource[0])) } diff --git a/pkg/execute/kubectl/checker.go b/pkg/execute/kubectl/checker.go new file mode 100644 index 000000000..c118d5d5d --- /dev/null +++ b/pkg/execute/kubectl/checker.go @@ -0,0 +1,51 @@ +package kubectl + +// ResourceVariantsFunc returns list of alternative namings for a given resource. +type ResourceVariantsFunc func(resource string) []string + +// Checker provides helper functionality to check whether a given kubectl verb and resource are allowed. +type Checker struct { + resourceVariants ResourceVariantsFunc +} + +// NewChecker returns a new Checker instance. +func NewChecker(resourceVariants ResourceVariantsFunc) *Checker { + return &Checker{resourceVariants: resourceVariants} +} + +// IsResourceAllowedInNs returns true if resource was found in a given config. +func (c *Checker) IsResourceAllowedInNs(config EnabledKubectl, resource string) bool { + if len(config.AllowedKubectlResource) == 0 { + return false + } + + // try a given name + if _, found := config.AllowedKubectlResource[resource]; found { + return true + } + + if c.resourceVariants == nil { + return false + } + + // try other variants + for _, name := range c.resourceVariants(resource) { + if _, found := config.AllowedKubectlResource[name]; found { + return true + } + } + + return false +} + +// IsVerbAllowedInNs returns true if verb was found in a given config. +func (c *Checker) IsVerbAllowedInNs(config EnabledKubectl, verb string) bool { + _, found := config.AllowedKubectlVerb[verb] + return found +} + +// IsKnownVerb returns true if verb was found in a given map. +func (c *Checker) IsKnownVerb(allVerbs map[string]struct{}, verb string) bool { + _, found := allVerbs[verb] + return found +} diff --git a/pkg/execute/kubectl/checker_test.go b/pkg/execute/kubectl/checker_test.go new file mode 100644 index 000000000..224c86e00 --- /dev/null +++ b/pkg/execute/kubectl/checker_test.go @@ -0,0 +1,140 @@ +package kubectl_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/kubeshop/botkube/pkg/execute/kubectl" +) + +func TestKubectlCheckerIsResourceAllowedInNs(t *testing.T) { + tests := []struct { + name string + + namespace string + bindings []string + resource string + variantFn kubectl.ResourceVariantsFunc + + expIsAllowed bool + }{ + { + name: "Should allow deployments", + namespace: "team-a", + resource: "deployments", + bindings: []string{ + "kubectl-team-a", + "kubectl-global", + }, + + expIsAllowed: true, + }, + { + name: "Should allow deploy variant", + namespace: "team-a", + resource: "deploy", + bindings: []string{ + "kubectl-team-a", + "kubectl-global", + }, + variantFn: func(resource string) []string { + if resource == "deploy" { + return []string{"deployments"} + } + return nil + }, + + expIsAllowed: true, + }, + { + name: "Should not allow pods", + namespace: "team-a", + resource: "pods", + bindings: []string{ + "kubectl-team-a", + "kubectl-global", + }, + variantFn: func(resource string) []string { + if resource == "deploy" { + return []string{"deployments"} + } + return nil + }, + + expIsAllowed: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // given + config := kubectl.NewMerger(fixExecutorsConfig(t)).MergeForNamespace(tc.bindings, tc.namespace) + checker := kubectl.NewChecker(tc.variantFn) + + // when + gotIsAllowed := checker.IsResourceAllowedInNs(config, tc.resource) + + // then + assert.Equal(t, tc.expIsAllowed, gotIsAllowed) + }) + } +} + +func TestKubectlCheckerIsVerbAllowedInNs(t *testing.T) { + tests := []struct { + name string + + namespace string + bindings []string + verb string + + expIsAllowed bool + }{ + { + name: "Should allow get from team-a settings", + namespace: "team-a", + verb: "get", + bindings: []string{ + "kubectl-team-a", + "kubectl-global", + }, + + expIsAllowed: true, + }, + { + name: "Should allow logs taken from global settings", + namespace: "team-a", + verb: "logs", + bindings: []string{ + "kubectl-team-a", + "kubectl-global", + }, + + expIsAllowed: true, + }, + { + name: "Should not allow pods", + namespace: "team-a", + verb: "exec", + bindings: []string{ + "kubectl-team-a", + "kubectl-global", + }, + + expIsAllowed: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // given + config := kubectl.NewMerger(fixExecutorsConfig(t)).MergeForNamespace(tc.bindings, tc.namespace) + checker := kubectl.NewChecker(nil) + + // when + gotIsAllowed := checker.IsVerbAllowedInNs(config, tc.verb) + + // then + assert.Equal(t, tc.expIsAllowed, gotIsAllowed) + }) + } +} diff --git a/pkg/execute/kubectl/helpers_test.go b/pkg/execute/kubectl/helpers_test.go new file mode 100644 index 000000000..a55aad817 --- /dev/null +++ b/pkg/execute/kubectl/helpers_test.go @@ -0,0 +1,67 @@ +package kubectl_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" + + "github.com/kubeshop/botkube/pkg/config" +) + +var RawExecutorsConfig = ` +executors: + 'kubectl-team-a': + kubectl: + enabled: true + namespaces: + include: [ "team-a" ] + commands: + verbs: [ "get" ] + resources: [ "deployments" ] + defaultNamespace: "team-a" + restrictAccess: false + 'kubectl-team-b': + kubectl: + enabled: true + namespaces: + include: [ "team-b" ] + commands: + verbs: [ "get", "describe" ] + resources: [ "deployments", "pods" ] + 'kubectl-global': + kubectl: + enabled: true + namespaces: + include: [ "all" ] + commands: + verbs: [ "logs", "top" ] + resources: [ ] + restrictAccess: true + 'kubectl-all': + kubectl: + enabled: true + namespaces: + include: [ "all" ] + commands: + verbs: [ "cluster-info" ] + resources: [ ] + defaultNamespace: "foo" + 'kubectl-exec': + kubectl: + enabled: false + namespaces: + include: [ "all" ] + commands: + verbs: [ "exec" ] + resources: [ ]` + +func fixExecutorsConfig(t *testing.T) config.IndexableMap[config.Executors] { + t.Helper() + + var givenCfg config.Config + err := yaml.Unmarshal([]byte(RawExecutorsConfig), &givenCfg) + require.NoError(t, err) + + return givenCfg.Executors +} diff --git a/pkg/execute/kubectl/merger.go b/pkg/execute/kubectl/merger.go new file mode 100644 index 000000000..da3c2d023 --- /dev/null +++ b/pkg/execute/kubectl/merger.go @@ -0,0 +1,138 @@ +package kubectl + +import ( + "github.com/kubeshop/botkube/pkg/config" +) + +// EnabledKubectl configuration for executing commands inside cluster +type EnabledKubectl struct { + AllowedKubectlVerb map[string]struct{} + AllowedKubectlResource map[string]struct{} + + DefaultNamespace string + RestrictAccess bool +} + +// Merger provides functionality to merge multiple bindings +// associated with the kubectl executor. +type Merger struct { + executors config.IndexableMap[config.Executors] +} + +// NewMerger returns a new Merger instance. +func NewMerger(executors config.IndexableMap[config.Executors]) *Merger { + return &Merger{ + executors: executors, + } +} + +// MergeForNamespace returns kubectl configuration for a given set of bindings. +// +// It merges entries only if a given Namespace is matched. +// - kubectl.commands.verbs - strategy append +// - kubectl.commands.resources - strategy append +// - kubectl.defaultNamespace - strategy override (if not empty) +// - kubectl.restrictAccess - strategy override (if not empty) +// +// The order of merging is the same as the order of items specified in the includeBindings list. +func (kc *Merger) MergeForNamespace(includeBindings []string, forNamespace string) EnabledKubectl { + enabledInNs := func(executor config.Kubectl) bool { + return executor.Enabled && executor.Namespaces.IsAllowed(forNamespace) + } + return kc.merge(kc.collect(includeBindings, enabledInNs)) +} + +// MergeAllEnabled returns kubectl configuration for all kubectl configs. +func (kc *Merger) MergeAllEnabled(includeBindings []string) EnabledKubectl { + onlyEnabled := func(executor config.Kubectl) bool { + return executor.Enabled + } + return kc.merge(kc.collect(includeBindings, onlyEnabled)) +} + +// IsAtLeastOneEnabled returns true if at least one kubectl executor is enabled. +func (kc *Merger) IsAtLeastOneEnabled() bool { + for _, executor := range kc.executors { + if executor.Kubectl.Enabled { + return true + } + } + return false +} + +// MergeAllEnabledVerbs returns verbs collected from all enabled kubectl executors. +func (kc *Merger) MergeAllEnabledVerbs(bindings []string) map[string]struct{} { + verbs := map[string]struct{}{} + + for _, name := range bindings { + executor, found := kc.executors[name] + if !found { + continue + } + + if !executor.Kubectl.Enabled { + continue + } + + for _, name := range executor.Kubectl.Commands.Verbs { + verbs[name] = struct{}{} + } + } + return verbs +} + +func (kc *Merger) merge(collectedKubectls []config.Kubectl) EnabledKubectl { + if len(collectedKubectls) == 0 { + return EnabledKubectl{} + } + + var ( + defaultNs string + restrictAccess bool + + allowedResources = map[string]struct{}{} + allowedVerbs = map[string]struct{}{} + ) + for _, item := range collectedKubectls { + for _, resourceName := range item.Commands.Resources { + allowedResources[resourceName] = struct{}{} + } + for _, verbName := range item.Commands.Verbs { + allowedVerbs[verbName] = struct{}{} + } + if item.DefaultNamespace != "" { + defaultNs = item.DefaultNamespace + } + + if item.RestrictAccess != nil { + restrictAccess = *item.RestrictAccess + } + } + + return EnabledKubectl{ + AllowedKubectlResource: allowedResources, + AllowedKubectlVerb: allowedVerbs, + DefaultNamespace: defaultNs, + RestrictAccess: restrictAccess, + } +} + +type collectPredicateFunc func(executor config.Kubectl) bool + +func (kc *Merger) collect(includeBindings []string, predicate collectPredicateFunc) []config.Kubectl { + var out []config.Kubectl + for _, name := range includeBindings { + executor, found := kc.executors[name] + if !found { + continue + } + + if !predicate(executor.Kubectl) { + continue + } + + out = append(out, executor.Kubectl) + } + + return out +} diff --git a/pkg/execute/kubectl/merger_test.go b/pkg/execute/kubectl/merger_test.go new file mode 100644 index 000000000..d81367105 --- /dev/null +++ b/pkg/execute/kubectl/merger_test.go @@ -0,0 +1,156 @@ +package kubectl_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/kubeshop/botkube/pkg/config" + "github.com/kubeshop/botkube/pkg/execute/kubectl" +) + +func TestKubectlMerger(t *testing.T) { + // given + tests := []struct { + name string + + givenBindings []string + expectKubectlConfig kubectl.EnabledKubectl + givenNamespace string + }{ + { + name: "Should collect settings with ignored settings for team-b", + givenBindings: []string{ + "kubectl-team-a", + "kubectl-team-b", + "kubectl-global", + "kubectl-exec", + }, + givenNamespace: "team-a", + expectKubectlConfig: kubectl.EnabledKubectl{ + AllowedKubectlVerb: map[string]struct{}{ + "get": {}, + "logs": {}, + "top": {}, + }, + AllowedKubectlResource: map[string]struct{}{ + "deployments": {}, + }, + DefaultNamespace: "team-a", + RestrictAccess: true, + }, + }, + { + name: "Should collect settings only for 'all' namespace", + givenBindings: []string{ + "kubectl-team-a", + "kubectl-team-b", + "kubectl-global", + "kubectl-exec", + "kubectl-all", + }, + givenNamespace: config.AllNamespaceIndicator, + expectKubectlConfig: kubectl.EnabledKubectl{ + AllowedKubectlVerb: map[string]struct{}{ + "logs": {}, + "top": {}, + "cluster-info": {}, + }, + AllowedKubectlResource: map[string]struct{}{}, + DefaultNamespace: "foo", + RestrictAccess: true, + }, + }, + { + name: "Should collect only team-a settings", + givenBindings: []string{ + "kubectl-team-a", + "kubectl-team-b", + }, + givenNamespace: "team-a", + expectKubectlConfig: kubectl.EnabledKubectl{ + AllowedKubectlVerb: map[string]struct{}{ + "get": {}, + }, + AllowedKubectlResource: map[string]struct{}{ + "deployments": {}, + }, + DefaultNamespace: "team-a", + RestrictAccess: false, + }, + }, + { + name: "Should enable restrict access based on the bindings order", + givenBindings: []string{ + "kubectl-team-a", // disables restrict + "kubectl-global", // enables restrict + }, + givenNamespace: "team-a", + expectKubectlConfig: kubectl.EnabledKubectl{ + AllowedKubectlVerb: map[string]struct{}{ + "get": {}, + "logs": {}, + "top": {}, + }, + AllowedKubectlResource: map[string]struct{}{ + "deployments": {}, + }, + DefaultNamespace: "team-a", + RestrictAccess: true, + }, + }, + { + name: "Should disable restrict access based on the bindings order", + givenBindings: []string{ + "kubectl-global", // enables restrict + "kubectl-team-a", // disables restrict + }, + givenNamespace: "team-a", + expectKubectlConfig: kubectl.EnabledKubectl{ + AllowedKubectlVerb: map[string]struct{}{ + "get": {}, + "logs": {}, + "top": {}, + }, + AllowedKubectlResource: map[string]struct{}{ + "deployments": {}, + }, + DefaultNamespace: "team-a", + RestrictAccess: false, + }, + }, + { + name: "Should enable restrict access based on the bindings order", + givenBindings: []string{ + "kubectl-global", // enables restrict + "kubectl-team-b", // doesn't specify restrict + }, + givenNamespace: "team-b", + expectKubectlConfig: kubectl.EnabledKubectl{ + AllowedKubectlVerb: map[string]struct{}{ + "get": {}, + "describe": {}, + "logs": {}, + "top": {}, + }, + AllowedKubectlResource: map[string]struct{}{ + "deployments": {}, + "pods": {}, + }, + RestrictAccess: true, + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // given + kubectlMerger := kubectl.NewMerger(fixExecutorsConfig(t)) + + // when + gotKubectlConfig := kubectlMerger.MergeForNamespace(tc.givenBindings, tc.givenNamespace) + + // then + assert.Equal(t, tc.expectKubectlConfig, gotKubectlConfig) + }) + } +} diff --git a/pkg/execute/kubectl/resource-normalizer.go b/pkg/execute/kubectl/resource-normalizer.go new file mode 100644 index 000000000..7dc6b580a --- /dev/null +++ b/pkg/execute/kubectl/resource-normalizer.go @@ -0,0 +1,55 @@ +package kubectl + +import ( + "fmt" + "strings" + + "github.com/sirupsen/logrus" + "k8s.io/client-go/discovery" +) + +// ResourceNormalizer contains helper maps to normalize the resource name specified in the kubectl command. +type ResourceNormalizer struct { + kindResourceMap map[string]string + shortnameResourceMap map[string]string +} + +// NewResourceNormalizer returns new ResourceNormalizer instance. +func NewResourceNormalizer(log logrus.FieldLogger, discoveryCli discovery.DiscoveryInterface) (ResourceNormalizer, error) { + resMapping := ResourceNormalizer{ + kindResourceMap: make(map[string]string), + shortnameResourceMap: make(map[string]string), + } + + _, resourceList, err := discoveryCli.ServerGroupsAndResources() + if err != nil { + return ResourceNormalizer{}, fmt.Errorf("while getting resource list from K8s cluster: %w", err) + } + for _, resource := range resourceList { + for _, r := range resource.APIResources { + // Ignore subresources + if strings.Contains(r.Name, "/") { + continue + } + resMapping.kindResourceMap[strings.ToLower(r.Kind)] = r.Name + for _, sn := range r.ShortNames { + resMapping.shortnameResourceMap[sn] = r.Name + } + } + } + log.Infof("Loaded resource mapping: %+v", resMapping) + return resMapping, nil +} + +// Normalize returns list with alternative names for a given input resource. +func (r ResourceNormalizer) Normalize(in string) []string { + variants := []string{ + // normalized received name + strings.ToLower(in), + // normalized short name + r.shortnameResourceMap[strings.ToLower(in)], + // normalized kind name + r.kindResourceMap[strings.ToLower(in)], + } + return variants +} diff --git a/pkg/execute/kubectl_test.go b/pkg/execute/kubectl_test.go index 7c1ff9555..d0fe693a6 100644 --- a/pkg/execute/kubectl_test.go +++ b/pkg/execute/kubectl_test.go @@ -9,6 +9,8 @@ import ( "github.com/stretchr/testify/require" "github.com/kubeshop/botkube/pkg/config" + "github.com/kubeshop/botkube/pkg/execute/kubectl" + "github.com/kubeshop/botkube/pkg/ptr" ) func TestKubectlExecute(t *testing.T) { @@ -19,7 +21,6 @@ func TestKubectlExecute(t *testing.T) { command string channelNotAuthorized bool - resMapping ResourceMapping kubectlCfg config.Kubectl expKubectlExecuted bool expOutMsg string @@ -30,12 +31,13 @@ func TestKubectlExecute(t *testing.T) { command: "get pod --cluster-name test", channelNotAuthorized: true, kubectlCfg: config.Kubectl{ - Enabled: true, - RestrictAccess: true, - }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"default"}, + }, + RestrictAccess: ptr.Bool(true), + Commands: config.Commands{ + Verbs: []string{"get"}, }, }, @@ -48,12 +50,13 @@ func TestKubectlExecute(t *testing.T) { command: "get pod --cluster-name other-cluster", channelNotAuthorized: true, kubectlCfg: config.Kubectl{ - Enabled: true, - RestrictAccess: true, - }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"default"}, + }, + RestrictAccess: ptr.Bool(true), + Commands: config.Commands{ + Verbs: []string{"get"}, }, }, @@ -66,12 +69,13 @@ func TestKubectlExecute(t *testing.T) { command: "get pod", channelNotAuthorized: true, kubectlCfg: config.Kubectl{ - Enabled: true, - RestrictAccess: true, - }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"default"}, + }, + RestrictAccess: ptr.Bool(true), + Commands: config.Commands{ + Verbs: []string{"get"}, }, }, @@ -79,49 +83,56 @@ func TestKubectlExecute(t *testing.T) { expOutMsg: "", }, { - name: "Should forbid execution if kubectl is disabled in config", + name: "Should forbid execution if resource is not allowed in config", - command: "get pod --cluster-name test", - kubectlCfg: config.Kubectl{Enabled: false}, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + command: "get pod -n foo", + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"foo"}, + }, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: nil, }, }, - expKubectlExecuted: false, - expOutMsg: "Sorry, the admin hasn't given me the permission to execute kubectl command on cluster 'test'.", + expOutMsg: "Sorry, the kubectl command is not authorized to work with 'pod' resources in the 'foo' Namespace on cluster 'test'. Use 'commands list' to see allowed commands.", }, { - name: "Should forbid execution if resource is not allowed in config", + name: "Should all execution if resource is missing, so kubectl can validate it further", - command: "get pod", - kubectlCfg: config.Kubectl{Enabled: true}, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + command: "get", + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"default"}, + }, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: nil, }, }, - - expKubectlExecuted: false, - expOutMsg: "Sorry, the kubectl command is not authorized to work with 'pod' resources on cluster 'test'. Use 'commands list' to see all allowed resources.", + expKubectlExecuted: true, + expOutMsg: "Cluster: test\nkubectl executed", }, { name: "Should forbid execution if namespace is not allowed in config", - command: "get pod", - kubectlCfg: config.Kubectl{Enabled: true}, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + command: "get pod", + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: nil, // no namespace allowed. }, - AllowedKubectlResourceMap: map[string]bool{ - "pod": true, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, }, }, expKubectlExecuted: false, - expOutMsg: "Sorry, the kubectl command cannot be executed in the 'default' Namespace on cluster 'test'. Use 'commands list' to see all allowed namespaces.", + expOutMsg: "Sorry, the kubectl 'get' command cannot be executed in the 'default' Namespace on cluster 'test'. Use 'commands list' to see allowed commands.", }, { name: "Should use default Namespace from config if not specified in command", @@ -130,18 +141,17 @@ func TestKubectlExecute(t *testing.T) { kubectlCfg: config.Kubectl{ Enabled: true, DefaultNamespace: "from-config", - }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + Namespaces: config.Namespaces{ + Include: nil, // forbid `from-config` to get a suitable error message. }, - AllowedKubectlResourceMap: map[string]bool{ - "pod": true, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, }, }, expKubectlExecuted: false, - expOutMsg: "Sorry, the kubectl command cannot be executed in the 'from-config' Namespace on cluster 'test'. Use 'commands list' to see all allowed namespaces.", + expOutMsg: "Sorry, the kubectl 'get' command cannot be executed in the 'from-config' Namespace on cluster 'test'. Use 'commands list' to see allowed commands.", }, { name: "Should explicitly use 'default' Namespace if not specified both in command and config", @@ -149,18 +159,17 @@ func TestKubectlExecute(t *testing.T) { command: "get pod", kubectlCfg: config.Kubectl{ Enabled: true, - }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + Namespaces: config.Namespaces{ + Include: nil, // forbid `default` to get a suitable error message. }, - AllowedKubectlResourceMap: map[string]bool{ - "pod": true, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, }, }, expKubectlExecuted: false, - expOutMsg: "Sorry, the kubectl command cannot be executed in the 'default' Namespace on cluster 'test'. Use 'commands list' to see all allowed namespaces.", + expOutMsg: "Sorry, the kubectl 'get' command cannot be executed in the 'default' Namespace on cluster 'test'. Use 'commands list' to see allowed commands.", }, { name: "Should forbid execution in not allowed namespace", @@ -171,18 +180,14 @@ func TestKubectlExecute(t *testing.T) { Namespaces: config.Namespaces{ Include: []string{"team-a"}, }, - }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, - }, - AllowedKubectlResourceMap: map[string]bool{ - "pod": true, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, }, }, expKubectlExecuted: false, - expOutMsg: "Sorry, the kubectl command cannot be executed in the 'team-b' Namespace on cluster 'test'. Use 'commands list' to see all allowed namespaces.", + expOutMsg: "Sorry, the kubectl 'get' command cannot be executed in the 'team-b' Namespace on cluster 'test'. Use 'commands list' to see allowed commands.", }, { name: "Should forbid execution if all namespace are allowed but command namespace is explicitly ignored in config", @@ -194,18 +199,14 @@ func TestKubectlExecute(t *testing.T) { Include: []string{config.AllNamespaceIndicator}, Ignore: []string{"team-b"}, }, - }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, - }, - AllowedKubectlResourceMap: map[string]bool{ - "pod": true, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, }, }, expKubectlExecuted: false, - expOutMsg: "Sorry, the kubectl command cannot be executed in the 'team-b' Namespace on cluster 'test'. Use 'commands list' to see all allowed namespaces.", + expOutMsg: "Sorry, the kubectl 'get' command cannot be executed in the 'team-b' Namespace on cluster 'test'. Use 'commands list' to see allowed commands.", }, { name: "Should allow execution if verb, resource, and all namespaces are allowed", @@ -216,13 +217,9 @@ func TestKubectlExecute(t *testing.T) { Namespaces: config.Namespaces{ Include: []string{config.AllNamespaceIndicator}, }, - }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, - }, - AllowedKubectlResourceMap: map[string]bool{ - "pod": true, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, }, }, @@ -238,13 +235,28 @@ func TestKubectlExecute(t *testing.T) { Namespaces: config.Namespaces{ Include: []string{"team-a"}, }, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, + }, }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + + expKubectlExecuted: true, + expOutMsg: "Cluster: test\nkubectl executed", + }, + { + name: "Should allow execution from not authorized channel if restrictions are disabled", + + command: "get pod -n team-a", + channelNotAuthorized: true, + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"team-a"}, }, - AllowedKubectlResourceMap: map[string]bool{ - "pod": true, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, }, }, @@ -252,46 +264,117 @@ func TestKubectlExecute(t *testing.T) { expOutMsg: "Cluster: test\nkubectl executed", }, { - name: "Should allow execution from not authorized channel if restrictions are not enabled", + name: "Should allow execution from not authorized channel if restrictions are disabled", - command: "get pod -n team-a", + command: "get pod/name-foo-42 -n team-a", channelNotAuthorized: true, kubectlCfg: config.Kubectl{ Enabled: true, Namespaces: config.Namespaces{ Include: []string{"team-a"}, }, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, + }, }, - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + + expKubectlExecuted: true, + expOutMsg: "Cluster: test\nkubectl executed", + }, + { + name: "Should allow execution for all Namespaces", + + command: "get pod/name-foo-42 -n team-a", + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"team-a"}, + }, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, + }, + }, + + expKubectlExecuted: true, + expOutMsg: "Cluster: test\nkubectl executed", + }, + { + name: "Should forbid execution for all Namespaces", + + command: "get pod -A", + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"team-a"}, + }, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, + }, + }, + + expKubectlExecuted: false, + expOutMsg: "Sorry, the kubectl 'get' command cannot be executed for all Namespaces on cluster 'test'. Use 'commands list' to see allowed commands.", + }, + { + name: "Should all execution for all Namespaces", + + command: "get pod -A", + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"all"}, }, - AllowedKubectlResourceMap: map[string]bool{ - "pod": true, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, }, }, expKubectlExecuted: true, expOutMsg: "Cluster: test\nkubectl executed", }, + { + name: "Known limitation (since v0.12.4): Return error if flag is added before resource name", + + command: "get -n team-a pod", + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"team-a"}, + }, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, + }, + }, + + expKubectlExecuted: false, + expOutMsg: "Please specify the resource name after the verb, and all flags after the resource name. Format [flags]", + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { // given cfg := fixCfgWithKubectlExecutor(t, tc.kubectlCfg) + merger := kubectl.NewMerger(cfg.Executors) + kcChecker := kubectl.NewChecker(nil) wasKubectlExecuted := false - executor := NewKubectl(logger, cfg, tc.resMapping, func(command string, args []string) (string, error) { + + executor := NewKubectl(logger, cfg, merger, kcChecker, func(command string, args []string) (string, error) { wasKubectlExecuted = true return "kubectl executed", nil }) // when - canHandle := executor.CanHandle(strings.Fields(strings.TrimSpace(tc.command))) - gotOutMsg, err := executor.Execute(tc.command, !tc.channelNotAuthorized) + canHandle := executor.CanHandle(fixBindingsNames, strings.Fields(strings.TrimSpace(tc.command))) + gotOutMsg, err := executor.Execute(fixBindingsNames, tc.command, !tc.channelNotAuthorized) // then - assert.True(t, canHandle) + assert.True(t, canHandle, "it should be able to handle the execution") require.NoError(t, err) assert.Equal(t, tc.expKubectlExecuted, wasKubectlExecuted) assert.Equal(t, tc.expOutMsg, gotOutMsg) @@ -307,23 +390,37 @@ func TestKubectlCanHandle(t *testing.T) { command string expCanHandle bool - resMapping ResourceMapping + kubectlCfg config.Kubectl }{ { name: "Should allow for known verb", command: "get pod --cluster-name test", - resMapping: ResourceMapping{ - AllowedKubectlVerbMap: map[string]bool{ - "get": true, + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"team-a"}, + }, + Commands: config.Commands{ + Verbs: []string{"get"}, + Resources: []string{"pod"}, }, }, expCanHandle: true, }, { - name: "Should forbid if verbs is unknown", - command: "get pod --cluster-name test", - resMapping: ResourceMapping{}, + name: "Should forbid if verbs is unknown", + command: "get pod --cluster-name test", + kubectlCfg: config.Kubectl{ + Enabled: true, + Namespaces: config.Namespaces{ + Include: []string{"team-a"}, + }, + Commands: config.Commands{ + Verbs: []string{"describe"}, + Resources: []string{"pod"}, + }, + }, expCanHandle: false, }, @@ -331,10 +428,14 @@ func TestKubectlCanHandle(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { // given - executor := NewKubectl(logger, config.Config{}, tc.resMapping, nil) + cfg := fixCfgWithKubectlExecutor(t, tc.kubectlCfg) + merger := kubectl.NewMerger(cfg.Executors) + kcChecker := kubectl.NewChecker(nil) + + executor := NewKubectl(logger, config.Config{}, merger, kcChecker, nil) // when - canHandle := executor.CanHandle(strings.Fields(strings.TrimSpace(tc.command))) + canHandle := executor.CanHandle(fixBindingsNames, strings.Fields(strings.TrimSpace(tc.command))) // then assert.Equal(t, tc.expCanHandle, canHandle) @@ -342,6 +443,8 @@ func TestKubectlCanHandle(t *testing.T) { } } +var fixBindingsNames = []string{"default"} + func fixCfgWithKubectlExecutor(t *testing.T, executor config.Kubectl) config.Config { t.Helper() diff --git a/pkg/execute/resource.go b/pkg/execute/resource.go deleted file mode 100644 index 26b15d73a..000000000 --- a/pkg/execute/resource.go +++ /dev/null @@ -1,61 +0,0 @@ -package execute - -import ( - "fmt" - "strings" - - "github.com/sirupsen/logrus" - "k8s.io/client-go/discovery" - - "github.com/kubeshop/botkube/pkg/config" -) - -// ResourceMapping contains helper maps for kubectl execution. -type ResourceMapping struct { - KindResourceMap map[string]string - ShortnameResourceMap map[string]string - AllowedKubectlResourceMap map[string]bool - AllowedKubectlVerbMap map[string]bool -} - -// LoadResourceMappingIfShould initializes helper maps to allow kubectl execution for required resources. -// If Kubectl support is disabled, it returns empty ResourceMapping without an error. -func LoadResourceMappingIfShould(log logrus.FieldLogger, conf *config.Config, discoveryCli discovery.DiscoveryInterface) (ResourceMapping, error) { - if !conf.Executors.GetFirst().Kubectl.Enabled { - log.Infof("Kubectl disabled. Finishing...") - return ResourceMapping{}, nil - } - - resMapping := ResourceMapping{ - KindResourceMap: make(map[string]string), - ShortnameResourceMap: make(map[string]string), - AllowedKubectlResourceMap: make(map[string]bool), - AllowedKubectlVerbMap: make(map[string]bool), - } - - for _, r := range conf.Executors.GetFirst().Kubectl.Commands.Resources { - resMapping.AllowedKubectlResourceMap[r] = true - } - for _, r := range conf.Executors.GetFirst().Kubectl.Commands.Verbs { - resMapping.AllowedKubectlVerbMap[r] = true - } - - _, resourceList, err := discoveryCli.ServerGroupsAndResources() - if err != nil { - return ResourceMapping{}, fmt.Errorf("while getting resource list from K8s cluster: %w", err) - } - for _, resource := range resourceList { - for _, r := range resource.APIResources { - // Ignore subresources - if strings.Contains(r.Name, "/") { - continue - } - resMapping.KindResourceMap[strings.ToLower(r.Kind)] = r.Name - for _, sn := range r.ShortNames { - resMapping.ShortnameResourceMap[sn] = r.Name - } - } - } - log.Infof("Loaded resource mapping: %+v", resMapping) - return resMapping, nil -} diff --git a/pkg/ptr/ptr.go b/pkg/ptr/ptr.go new file mode 100644 index 000000000..4db1a457e --- /dev/null +++ b/pkg/ptr/ptr.go @@ -0,0 +1,14 @@ +package ptr + +// ToBool returns bool value for a given pointer. +func ToBool(in *bool) bool { + if in == nil { + return false + } + return *in +} + +// Bool returns pointer to a given input bool value. +func Bool(in bool) *bool { + return &in +} diff --git a/test/e2e/slack_test.go b/test/e2e/slack_test.go index b01e99fc1..d1dcfdcf1 100644 --- a/test/e2e/slack_test.go +++ b/test/e2e/slack_test.go @@ -121,30 +121,58 @@ func TestSlack(t *testing.T) { t.Run("Commands list", func(t *testing.T) { command := "commands list" expectedMessage := codeBlock(heredoc.Doc(` - allowed verbs: - - api-resources - - api-versions - - auth - - cluster-info - - describe - - diff - - explain - - get - - logs - - top - allowed resources: - - configmaps - - daemonsets - - deployments - - namespaces - - nodes - - pods - - statefulsets - - storageclasses - allowed namespaces: - include: - - botkube - - default`)) + Enabled executors: + kubectl: + kubectl-allow-all: + namespaces: + include: + - all + enabled: true + commands: + verbs: + - get + resources: + - deployments + kubectl-read-only: + namespaces: + include: + - botkube + - default + enabled: true + commands: + verbs: + - api-resources + - api-versions + - cluster-info + - describe + - diff + - explain + - get + - logs + - top + - auth + resources: + - deployments + - pods + - namespaces + - daemonsets + - statefulsets + - storageclasses + - nodes + - configmaps + defaultNamespace: default + restrictAccess: false + kubectl-wait-cmd: + namespaces: + include: + - botkube + - default + enabled: true + commands: + verbs: + - wait + resources: [] + restrictAccess: false`)) t.Run("With default cluster", func(t *testing.T) { slackTester.PostMessageToBot(t, channel.Name, command) @@ -200,7 +228,7 @@ func TestSlack(t *testing.T) { t.Run("Get forbidden resource", func(t *testing.T) { command := "get ingress" - expectedMessage := codeBlock(fmt.Sprintf("Sorry, the kubectl command is not authorized to work with 'ingress' resources on cluster '%s'. Use 'commands list' to see all allowed resources.", appCfg.ClusterName)) + expectedMessage := codeBlock(fmt.Sprintf("Sorry, the kubectl command is not authorized to work with 'ingress' resources in the 'default' Namespace on cluster '%s'. Use 'commands list' to see allowed commands.", appCfg.ClusterName)) slackTester.PostMessageToBot(t, channel.Name, command) err = slackTester.WaitForLastMessageEqual(botUserID, channel.ID, expectedMessage) @@ -233,12 +261,60 @@ func TestSlack(t *testing.T) { t.Run("Specify forbidden namespace", func(t *testing.T) { command := "get po --namespace team-b" - expectedMessage := codeBlock(fmt.Sprintf("Sorry, the kubectl command cannot be executed in the 'team-b' Namespace on cluster '%s'. Use 'commands list' to see all allowed namespaces.", appCfg.ClusterName)) + expectedMessage := codeBlock(fmt.Sprintf("Sorry, the kubectl command is not authorized to work with 'po' resources in the 'team-b' Namespace on cluster '%s'. Use 'commands list' to see allowed commands.", appCfg.ClusterName)) slackTester.PostMessageToBot(t, channel.Name, command) err = slackTester.WaitForLastMessageEqual(botUserID, channel.ID, expectedMessage) assert.NoError(t, err) }) + + t.Run("Based on other bindings", func(t *testing.T) { + t.Run("Wait for Deployment (the 2st binding)", func(t *testing.T) { + command := fmt.Sprintf("wait deployment -n %s %s --for condition=Available=True", appCfg.Deployment.Namespace, appCfg.Deployment.Name) + assertionFn := func(msg slack.Message) bool { + return strings.Contains(msg.Text, heredoc.Doc(fmt.Sprintf("Cluster: %s", appCfg.ClusterName))) && + strings.Contains(msg.Text, "deployment.apps/botkube condition met") + } + + slackTester.PostMessageToBot(t, channel.Name, command) + err = slackTester.WaitForMessagePosted(botUserID, channel.ID, 1, assertionFn) + assert.NoError(t, err) + }) + + t.Run("Exec (the 3rd binding which is disabled)", func(t *testing.T) { + command := "exec" + expectedMessage := codeBlock("Command not supported. Please run /botkubehelp to see supported commands.") + + slackTester.PostMessageToBot(t, channel.Name, command) + err = slackTester.WaitForLastMessageEqual(botUserID, channel.ID, expectedMessage) + assert.NoError(t, err) + }) + + t.Run("Get all Pods (the 4th binding)", func(t *testing.T) { + command := "get pods -A" + expectedMessage := codeBlock(fmt.Sprintf("Sorry, the kubectl command is not authorized to work with 'pods' resources for all Namespaces on cluster '%s'. Use 'commands list' to see allowed commands.", appCfg.ClusterName)) + + slackTester.PostMessageToBot(t, channel.Name, command) + err = slackTester.WaitForLastMessageEqual(botUserID, channel.ID, expectedMessage) + assert.NoError(t, err) + }) + + t.Run("Get all Deployments (the 4th binding)", func(t *testing.T) { + command := "get deploy -A" + assertionFn := func(msg slack.Message) bool { + return strings.Contains(msg.Text, heredoc.Doc(fmt.Sprintf("Cluster: %s", appCfg.ClusterName))) && + strings.Contains(msg.Text, "local-path-provisioner") && + strings.Contains(msg.Text, "coredns") && + strings.Contains(msg.Text, "traefik") && + strings.Contains(msg.Text, "metrics-server") && + strings.Contains(msg.Text, "botkube") + } + + slackTester.PostMessageToBot(t, channel.Name, command) + err = slackTester.WaitForMessagePosted(botUserID, channel.ID, 1, assertionFn) + assert.NoError(t, err) + }) + }) }) t.Run("Notifications", func(t *testing.T) {