Merge branch 'main' into config-reload-improvements

pkg/execute/executor.go

@@ -166,11 +166,8 @@ func (e *DefaultExecutor) Execute() interactive.Message {
 	}
 
 	if e.kubectlExecutor.CanHandle(e.conversation.ExecutorBindings, 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.
-		verb := args[0]
-		err := e.analyticsReporter.ReportCommand(e.platform, verb)
+		cmdPrefix := e.kubectlExecutor.GetCommandPrefix(args)
+		err := e.analyticsReporter.ReportCommand(e.platform, cmdPrefix)
 		if err != nil {
 			e.log.Errorf("while reporting executed command: %s", err.Error())
 		}

pkg/execute/kubectl.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/pflag"
+	"k8s.io/utils/strings/slices"
 
 	"github.com/kubeshop/botkube/pkg/config"
 	"github.com/kubeshop/botkube/pkg/execute/kubectl"
@@ -23,6 +24,8 @@ const (
 	kubectlDefaultNamespace            = "default"
 )
 
+var kubectlAlias = []string{"kubectl", "kc", "k"}
+
 // resourcelessCommands holds all commands that don't specify resources directly. For example:
 // - kubectl logs foo
 // - kubectl cluster-info
@@ -47,6 +50,7 @@ type Kubectl struct {
 	kcChecker *kubectl.Checker
 	cmdRunner CommandCombinedOutputRunner
 	merger    *kubectl.Merger
+	alias     []string
 }
 
 // NewKubectl creates a new instance of Kubectl.
@@ -57,6 +61,7 @@ func NewKubectl(log logrus.FieldLogger, cfg config.Config, merger *kubectl.Merge
 		merger:    merger,
 		kcChecker: kcChecker,
 		cmdRunner: fn,
+		alias:     kubectlAlias,
 	}
 }
 
@@ -69,12 +74,49 @@ func (e *Kubectl) CanHandle(bindings []string, args []string) bool {
 		return false
 	}
 
+	// TODO Later first argument will be always kubectl alias so we can use it to check if this command is for k8s
+	// For now we support both approach @botkube get pods and @botkube kubectl|kc|k get pods
+	kcVerb := e.GetVerb(args)
+
 	// Check if such kubectl verb is enabled
-	if !e.kcChecker.IsKnownVerb(e.merger.MergeAllEnabledVerbs(bindings), args[0]) {
-		return false
+	return e.kcChecker.IsKnownVerb(e.merger.MergeAllEnabledVerbs(bindings), kcVerb)
+}
+
+// GetVerb gets verb command for k8s ignoring k8s alias prefix.
+func (e *Kubectl) GetVerb(args []string) string {
+	if len(args) == 0 {
+		return ""
+	}
+
+	if len(args) >= 2 && slices.Contains(e.alias, args[0]) {
+		return args[1]
+	}
+
+	return args[0]
+}
+
+// GetCommandPrefix gets verb command with k8s alias prefix.
+func (e *Kubectl) GetCommandPrefix(args []string) string {
+	if len(args) == 0 {
+		return ""
+	}
+
+	if len(args) >= 2 && slices.Contains(e.alias, args[0]) {
+		return fmt.Sprintf("%s %s", args[0], args[1])
+	}
+
+	return args[0]
+}
+
+// getArgsWithoutAlias gets command without k8s alias.
+func (e *Kubectl) getArgsWithoutAlias(msg string) []string {
+	msgParts := strings.Fields(strings.TrimSpace(msg))
+
+	if len(msgParts) >= 2 && slices.Contains(e.alias, msgParts[0]) {
+		return msgParts[1:]
 	}
 
-	return true
+	return msgParts
 }
 
 // Execute executes kubectl command based on a given args.
@@ -91,7 +133,7 @@ func (e *Kubectl) Execute(bindings []string, command string, isAuthChannel bool)
 	log.Debugf("Handling command...")
 
 	var (
-		args        = strings.Fields(strings.TrimSpace(command))
+		args        = e.getArgsWithoutAlias(command)
 		clusterName = e.cfg.Settings.ClusterName
 		verb        = args[0]
 		resource    = e.getResourceName(args)
@@ -110,7 +152,7 @@ func (e *Kubectl) Execute(bindings []string, command string, isAuthChannel bool)
 
 	if !isAuthChannel && kcConfig.RestrictAccess {
 		msg := fmt.Sprintf(kubectlNotAuthorizedMsgFmt, clusterName)
-		return e.omitIfIfWeAreNotExplicitlyTargetCluster(log, command, msg)
+		return e.omitIfWeAreNotExplicitlyTargetCluster(log, command, msg)
 	}
 
 	if !e.kcChecker.IsVerbAllowedInNs(kcConfig, verb) {
@@ -144,9 +186,9 @@ func (e *Kubectl) Execute(bindings []string, command string, isAuthChannel bool)
 	return out, nil
 }
 
-// omitIfIfWeAreNotExplicitlyTargetCluster returns verboseMsg if there is explicit '--cluster-name' flag that matches this cluster.
+// omitIfWeAreNotExplicitlyTargetCluster returns verboseMsg if there is explicit '--cluster-name' flag that matches this cluster.
 // It's useful if we want to be more verbose, but we also don't want to spam if we are not the target one.
-func (e *Kubectl) omitIfIfWeAreNotExplicitlyTargetCluster(log *logrus.Entry, cmd string, verboseMsg string) (string, error) {
+func (e *Kubectl) omitIfWeAreNotExplicitlyTargetCluster(log *logrus.Entry, cmd string, verboseMsg string) (string, error) {
 	if utils.GetClusterNameFromKubectlCmd(cmd) == e.cfg.Settings.ClusterName {
 		return verboseMsg, nil
 	}
@@ -159,8 +201,8 @@ func (e *Kubectl) omitIfIfWeAreNotExplicitlyTargetCluster(log *logrus.Entry, cmd
 //
 //	https://github.com/kubeshop/botkube/blob/0b99ac480c8e7e93ce721b345ffc54d89019a812/pkg/execute/executor.go#L242-L276
 //
-// Further refactoring in needed. For example, the cluster flag should be removed by an upper layer as it's strictly
-// as it's strictly BotKube related and not executor specific (e.g. kubectl, helm, istio etc.)
+// Further refactoring in needed. For example, the cluster flag should be removed by an upper layer
+// as it's strictly BotKube related and not executor specific (e.g. kubectl, helm, istio etc.).
 func (e *Kubectl) getFinalArgs(args []string) []string {
 	// Remove unnecessary flags
 	var finalArgs []string

pkg/execute/kubectl_test.go

@@ -408,6 +408,22 @@ func TestKubectlCanHandle(t *testing.T) {
 
 			expCanHandle: true,
 		},
+		{
+			name:    "Should allow for known verb with k8s prefix",
+			command: "kubectl get pod --cluster-name test",
+			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",
@@ -443,6 +459,167 @@ func TestKubectlCanHandle(t *testing.T) {
 	}
 }
 
+func TestKubectlGetVerb(t *testing.T) {
+	tests := []struct {
+		name         string
+		command      string
+		expectedVerb string
+	}{
+		{
+			name:         "Should get proper verb without k8s prefix",
+			command:      "get pods --cluster-name test",
+			expectedVerb: "get",
+		},
+		{
+			name:         "Should get proper verb with k8s prefix kubectl",
+			command:      "kubectl get pods --cluster-name test",
+			expectedVerb: "get",
+		},
+		{
+			name:         "Should get proper verb with k8s prefix kc",
+			command:      "kc get pods --cluster-name test",
+			expectedVerb: "get",
+		},
+		{
+			name:         "Should get proper verb with k8s prefix k",
+			command:      "k get pods --cluster-name test",
+			expectedVerb: "get",
+		},
+	}
+	logger, _ := logtest.NewNullLogger()
+	kubectlCfg := config.Kubectl{
+		Enabled: true,
+		Namespaces: config.Namespaces{
+			Include: []string{"team-a"},
+		},
+		Commands: config.Commands{
+			Verbs:     []string{"get"},
+			Resources: []string{"pod"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := fixCfgWithKubectlExecutor(t, kubectlCfg)
+			merger := kubectl.NewMerger(cfg.Executors)
+			kcChecker := kubectl.NewChecker(nil)
+			executor := NewKubectl(logger, config.Config{}, merger, kcChecker, nil)
+
+			args := strings.Fields(tc.command)
+			verb := executor.GetVerb(args)
+
+			assert.Equal(t, tc.expectedVerb, verb)
+		})
+	}
+}
+
+func TestKubectlGetCommandPrefix(t *testing.T) {
+	tests := []struct {
+		name     string
+		command  string
+		expected string
+	}{
+		{
+			name:     "Should get proper command without k8s prefix",
+			command:  "get pods --cluster-name test",
+			expected: "get",
+		},
+		{
+			name:     "Should get proper command with k8s prefix kubectl",
+			command:  "kubectl get pods --cluster-name test",
+			expected: "kubectl get",
+		},
+		{
+			name:     "Should get proper command with k8s prefix kc",
+			command:  "kc get pods --cluster-name test",
+			expected: "kc get",
+		},
+		{
+			name:     "Should get proper command with k8s prefix k",
+			command:  "k get pods --cluster-name test",
+			expected: "k get",
+		},
+	}
+	logger, _ := logtest.NewNullLogger()
+	kubectlCfg := config.Kubectl{
+		Enabled: true,
+		Namespaces: config.Namespaces{
+			Include: []string{"team-a"},
+		},
+		Commands: config.Commands{
+			Verbs:     []string{"get"},
+			Resources: []string{"pod"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := fixCfgWithKubectlExecutor(t, kubectlCfg)
+			merger := kubectl.NewMerger(cfg.Executors)
+			kcChecker := kubectl.NewChecker(nil)
+			executor := NewKubectl(logger, config.Config{}, merger, kcChecker, nil)
+
+			args := strings.Fields(tc.command)
+			verb := executor.GetCommandPrefix(args)
+
+			assert.Equal(t, tc.expected, verb)
+		})
+	}
+}
+
+func TestKubectlGetArgsWithoutAlias(t *testing.T) {
+	tests := []struct {
+		name     string
+		command  string
+		expected string
+	}{
+		{
+			name:     "Should get proper command without k8s prefix",
+			command:  "get pods --cluster-name test",
+			expected: "get pods --cluster-name test",
+		},
+		{
+			name:     "Should get proper command with k8s prefix kubectl",
+			command:  "kubectl get pods --cluster-name test",
+			expected: "get pods --cluster-name test",
+		},
+		{
+			name:     "Should get proper verb with k8s prefix kc",
+			command:  "kc get pods --cluster-name test",
+			expected: "get pods --cluster-name test",
+		},
+		{
+			name:     "Should get proper verb with k8s prefix k",
+			command:  "k get pods --cluster-name test",
+			expected: "get pods --cluster-name test",
+		},
+	}
+	logger, _ := logtest.NewNullLogger()
+	kubectlCfg := config.Kubectl{
+		Enabled: true,
+		Namespaces: config.Namespaces{
+			Include: []string{"team-a"},
+		},
+		Commands: config.Commands{
+			Verbs:     []string{"get"},
+			Resources: []string{"pod"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			cfg := fixCfgWithKubectlExecutor(t, kubectlCfg)
+			merger := kubectl.NewMerger(cfg.Executors)
+			kcChecker := kubectl.NewChecker(nil)
+			executor := NewKubectl(logger, config.Config{}, merger, kcChecker, nil)
+
+			verb := executor.getArgsWithoutAlias(tc.command)
+
+			assert.Equal(t, tc.expected, strings.Join(verb, " "))
+		})
+	}
+}
+
 var fixBindingsNames = []string{"default"}
 
 func fixCfgWithKubectlExecutor(t *testing.T, executor config.Kubectl) config.Config {

test/e2e/bots_test.go

@@ -416,6 +416,28 @@ func runBotTest(t *testing.T,
 				assert.NoError(t, err)
 			})
 		})
+
+		k8sPrefixTests := []string{"kubectl", "kc", "k"}
+		for _, prefix := range k8sPrefixTests {
+			t.Run(fmt.Sprintf("Get Pods with k8s prefix %s", prefix), func(t *testing.T) {
+				command := fmt.Sprintf("%s get pods --namespace %s", prefix, appCfg.Deployment.Namespace)
+				assertionFn := func(msg string) bool {
+					headerColumnNames := []string{"NAME", "READY", "STATUS", "RESTART", "AGE"}
+					containAllColumn := true
+					for _, cn := range headerColumnNames {
+						if !strings.Contains(msg, cn) {
+							containAllColumn = false
+						}
+					}
+					return strings.Contains(msg, heredoc.Doc(fmt.Sprintf("`%s` on `%s`", command, appCfg.ClusterName))) &&
+						containAllColumn
+				}
+
+				botDriver.PostMessageToBot(t, botDriver.Channel().Identifier(), command)
+				err = botDriver.WaitForMessagePosted(botDriver.BotUserID(), botDriver.Channel().ID(), 1, assertionFn)
+				assert.NoError(t, err)
+			})
+		}
 	})
 
 	t.Run("Multi-channel notifications", func(t *testing.T) {