Add comments, refactor command executor

cmd/executor/gh/main.go

@@ -162,7 +162,11 @@ func createGitHubIssue(cfg Config, title, mdBody string) (string, error) {
 		"GH_TOKEN": cfg.GitHub.Token,
 	}
 
-	return pluginx.ExecuteCommandWithEnvs(context.Background(), cmd, envs)
+	output, err := pluginx.ExecuteCommand(context.Background(), cmd, pluginx.ExecuteCommandEnvs(envs))
+	if err != nil {
+		return "", err
+	}
+	return output.Stdout, nil
 }
 
 // IssueDetails holds all available information about a given issue.
@@ -190,8 +194,8 @@ func getIssueDetails(ctx context.Context, namespace, name, kubeConfigPath string
 	return IssueDetails{
 		Type:      name,
 		Namespace: namespace,
-		Logs:      logs,
-		Version:   ver,
+		Logs:      logs.Stdout,
+		Version:   ver.Stdout,
 	}, nil
 }
 

cmd/executor/x/main.go

@@ -140,9 +140,9 @@ func (i *XExecutor) Execute(ctx context.Context, in executor.ExecuteInput) (exec
 			"downloadCmd": downloadCmd,
 		}).Info("Installing binary...")
 
-		if _, err := pluginx.ExecuteCommandWithEnvs(ctx, downloadCmd, map[string]string{
+		if _, err := pluginx.ExecuteCommand(ctx, downloadCmd, pluginx.ExecuteCommandEnvs(map[string]string{
 			"EGET_BIN": dir,
-		}); err != nil {
+		})); err != nil {
 			return executor.ExecuteOutput{}, err
 		}
 

internal/executor/helm/executor.go

@@ -44,15 +44,15 @@ var _ executor.Executor = &Executor{}
 
 // Executor provides functionality for running Helm CLI.
 type Executor struct {
-	pluginVersion          string
-	executeCommandWithEnvs func(ctx context.Context, rawCmd string, envs map[string]string) (string, error)
+	pluginVersion  string
+	executeCommand func(ctx context.Context, rawCmd string, mutators ...pluginx.ExecuteCommandMutation) (pluginx.ExecuteCommandOutput, error)
 }
 
 // NewExecutor returns a new Executor instance.
 func NewExecutor(ver string) *Executor {
 	return &Executor{
-		pluginVersion:          ver,
-		executeCommandWithEnvs: pluginx.ExecuteCommandWithEnvs,
+		pluginVersion:  ver,
+		executeCommand: pluginx.ExecuteCommand,
 	}
 }
 
@@ -187,13 +187,13 @@ func (e *Executor) handleHelmCommand(ctx context.Context, cmd command, cfg Confi
 		"KUBECONFIG":       kubeConfig,
 	}
 
-	out, err := e.executeCommandWithEnvs(ctx, rawCmd, envs)
+	out, err := e.executeCommand(ctx, rawCmd, pluginx.ExecuteCommandEnvs(envs))
 	if err != nil {
 		return executor.ExecuteOutput{}, fmt.Errorf("%s\n%s", out, err.Error())
 	}
 
 	return executor.ExecuteOutput{
-		Data: out,
+		Message: api.NewPlaintextMessage(out.Stdout, true),
 	}, nil
 }
 

internal/executor/helm/executor_test.go

@@ -10,7 +10,9 @@ import (
 	"gopkg.in/yaml.v3"
 	"gotest.tools/v3/golden"
 
+	"github.com/kubeshop/botkube/pkg/api"
 	"github.com/kubeshop/botkube/pkg/api/executor"
+	"github.com/kubeshop/botkube/pkg/pluginx"
 )
 
 const kc = "KUBECONFIG"
@@ -45,17 +47,24 @@ func TestExecutorHelmInstall(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			// given
-			const execOutput = "mocked"
+			execOutput := pluginx.ExecuteCommandOutput{
+				Stdout: "mocked",
+			}
 
 			hExec := NewExecutor("testing")
 
 			var (
 				gotCmd  string
 				gotEnvs map[string]string
 			)
-			hExec.executeCommandWithEnvs = func(_ context.Context, rawCmd string, envs map[string]string) (string, error) {
+			hExec.executeCommand = func(ctx context.Context, rawCmd string, mutators ...pluginx.ExecuteCommandMutation) (pluginx.ExecuteCommandOutput, error) {
 				gotCmd = rawCmd
-				gotEnvs = envs
+				var opts pluginx.ExecuteCommandOptions
+				for _, mutate := range mutators {
+					mutate(&opts)
+				}
+
+				gotEnvs = opts.Envs
 				return execOutput, nil
 			}
 
@@ -70,7 +79,7 @@ func TestExecutorHelmInstall(t *testing.T) {
 			// then
 			require.NoError(t, err)
 
-			assert.Equal(t, execOutput, out.Data)
+			assert.Equal(t, api.NewPlaintextMessage(execOutput.Stdout, true), out.Message)
 
 			assert.Equal(t, tc.expCommand, gotCmd)
 
@@ -112,7 +121,7 @@ func TestExecutorHelmInstallFlagsErrors(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			// given
 			hExec := NewExecutor("testing")
-			hExec.executeCommandWithEnvs = noopExecuteCommand
+			hExec.executeCommand = noopExecuteCommand
 
 			// when
 			out, err := hExec.Execute(context.Background(), executor.ExecuteInput{
@@ -124,6 +133,7 @@ func TestExecutorHelmInstallFlagsErrors(t *testing.T) {
 
 			// then
 			require.EqualError(t, err, tc.expErrMsg)
+			assert.Empty(t, out.Message)
 			assert.Empty(t, out.Data)
 		})
 	}
@@ -152,7 +162,7 @@ func TestExecutorHelmInstallHelp(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			// given
 			hExec := NewExecutor("testing")
-			hExec.executeCommandWithEnvs = noopExecuteCommand
+			hExec.executeCommand = noopExecuteCommand
 
 			// when
 			out, err := hExec.Execute(context.Background(), executor.ExecuteInput{
@@ -173,9 +183,14 @@ func TestExecutorConfigMerging(t *testing.T) {
 	// given
 	hExec := NewExecutor("testing")
 	var gotEnvs map[string]string
-	hExec.executeCommandWithEnvs = func(_ context.Context, _ string, envs map[string]string) (string, error) {
-		gotEnvs = envs
-		return "", nil
+	hExec.executeCommand = func(ctx context.Context, rawCmd string, mutators ...pluginx.ExecuteCommandMutation) (pluginx.ExecuteCommandOutput, error) {
+		var opts pluginx.ExecuteCommandOptions
+		for _, mutate := range mutators {
+			mutate(&opts)
+		}
+
+		gotEnvs = opts.Envs
+		return pluginx.ExecuteCommandOutput{}, nil
 	}
 
 	configA := Config{
@@ -217,7 +232,7 @@ func TestExecutorConfigMerging(t *testing.T) {
 func TestExecutorConfigMergingErrors(t *testing.T) {
 	// given
 	hExec := NewExecutor("testing")
-	hExec.executeCommandWithEnvs = noopExecuteCommand
+	hExec.executeCommand = noopExecuteCommand
 
 	configA := Config{
 		HelmDriver: "unknown-value",
@@ -247,6 +262,6 @@ func mustYAMLMarshal(t *testing.T, in any) []byte {
 	return out
 }
 
-func noopExecuteCommand(_ context.Context, _ string, _ map[string]string) (string, error) {
-	return "", nil
+func noopExecuteCommand(context.Context, string, ...pluginx.ExecuteCommandMutation) (pluginx.ExecuteCommandOutput, error) {
+	return pluginx.ExecuteCommandOutput{}, nil
 }

internal/executor/kubectl/executor_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/kubeshop/botkube/pkg/api/executor"
+	"github.com/kubeshop/botkube/pkg/pluginx"
 )
 
 func TestSetDefaultNamespace(t *testing.T) {
@@ -51,9 +52,11 @@ func TestSetDefaultNamespace(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			// given
 			var gotCmd string
-			mockFn := NewMockedBinaryRunner(func(_ context.Context, rawCmd string, _ map[string]string) (string, error) {
+			mockFn := NewMockedBinaryRunner(func(ctx context.Context, rawCmd string, mutators ...pluginx.ExecuteCommandMutation) (pluginx.ExecuteCommandOutput, error) {
 				gotCmd = rawCmd
-				return "mocked", nil
+				return pluginx.ExecuteCommandOutput{
+					Stdout: "mocked",
+				}, nil
 			})
 
 			exec := NewExecutor("dev", mockFn)
@@ -100,9 +103,11 @@ func TestSetOptionsCommand(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			// given
 			var wasKubectlCalled bool
-			mockFn := NewMockedBinaryRunner(func(_ context.Context, rawCmd string, _ map[string]string) (string, error) {
+			mockFn := NewMockedBinaryRunner(func(ctx context.Context, rawCmd string, mutators ...pluginx.ExecuteCommandMutation) (pluginx.ExecuteCommandOutput, error) {
 				wasKubectlCalled = true
-				return "mocked", nil
+				return pluginx.ExecuteCommandOutput{
+					Stdout: "mocked",
+				}, nil
 			})
 
 			exec := NewExecutor("dev", mockFn)
@@ -149,9 +154,11 @@ func TestNotSupportedCommandsAndFlags(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			// given
 			var wasKubectlCalled bool
-			mockFn := NewMockedBinaryRunner(func(_ context.Context, rawCmd string, _ map[string]string) (string, error) {
+			mockFn := NewMockedBinaryRunner(func(ctx context.Context, rawCmd string, mutators ...pluginx.ExecuteCommandMutation) (pluginx.ExecuteCommandOutput, error) {
 				wasKubectlCalled = true
-				return "mocked", nil
+				return pluginx.ExecuteCommandOutput{
+					Stdout: "mocked",
+				}, nil
 			})
 
 			exec := NewExecutor("dev", mockFn)

internal/executor/kubectl/kc_runner.go

@@ -16,13 +16,13 @@ const binaryName = "kubectl"
 
 // BinaryRunner runs a kubectl binary.
 type BinaryRunner struct {
-	executeCommandWithEnvs func(ctx context.Context, rawCmd string, envs map[string]string) (string, error)
+	executeCommand func(ctx context.Context, rawCmd string, mutators ...pluginx.ExecuteCommandMutation) (pluginx.ExecuteCommandOutput, error)
 }
 
 // NewBinaryRunner returns a new BinaryRunner instance.
 func NewBinaryRunner() *BinaryRunner {
 	return &BinaryRunner{
-		executeCommandWithEnvs: pluginx.ExecuteCommandWithEnvs,
+		executeCommand: pluginx.ExecuteCommand,
 	}
 }
 
@@ -55,12 +55,12 @@ func (e *BinaryRunner) RunKubectlCommand(ctx context.Context, kubeConfigPath, de
 	}
 
 	runCmd := fmt.Sprintf("%s %s", binaryName, cmd)
-	out, err := e.executeCommandWithEnvs(ctx, runCmd, envs)
+	out, err := e.executeCommand(ctx, runCmd, pluginx.ExecuteCommandEnvs(envs))
 	if err != nil {
 		return "", fmt.Errorf("%s\n%s", out, err.Error())
 	}
 
-	return color.ClearCode(out), nil
+	return color.ClearCode(out.Stdout), nil
 }
 
 // getAllNamespaceFlag returns the namespace value extracted from a given args.

internal/executor/kubectl/kc_runner_test.go

@@ -2,12 +2,14 @@ package kubectl
 
 import (
 	"context"
+
+	"github.com/kubeshop/botkube/pkg/pluginx"
 )
 
-type executeFn func(ctx context.Context, rawCmd string, envs map[string]string) (string, error)
+type executeFn func(ctx context.Context, rawCmd string, mutators ...pluginx.ExecuteCommandMutation) (pluginx.ExecuteCommandOutput, error)
 
 func NewMockedBinaryRunner(mock executeFn) *BinaryRunner {
 	return &BinaryRunner{
-		executeCommandWithEnvs: mock,
+		executeCommand: mock,
 	}
 }

internal/executor/x/cmd_parse.go

@@ -16,12 +16,14 @@ const (
 	PageIndexIndicator   = "@page:"
 )
 
+// Command holds command details.
 type Command struct {
 	ToExecute     string
 	IsRawRequired bool
 	PageIndex     int
 }
 
+// Parse returns parsed command string.
 func Parse(cmd string) Command {
 	out := Command{
 		ToExecute: cmd,

internal/executor/x/cmd_parse_test.go

@@ -35,7 +35,7 @@ func TestParse(t *testing.T) {
 		{
 			input: "x run kubectl get pods @idx:abc",
 			expected: Command{
-				ToExecute:     "x run kubectl get pods @idx:abc @page:1",
+				ToExecute:     "x run kubectl get pods @idx:abc",
 				IsRawRequired: false,
 				PageIndex:     1,
 			},

internal/executor/x/config.go

@@ -7,14 +7,14 @@ import (
 	"github.com/kubeshop/botkube/pkg/config"
 )
 
-type (
-	Config struct {
-		Templates []getter.Source `yaml:"templates"`
-		TmpDir    plugin.TmpDir   `yaml:"tmpDir"`
-		Logger    config.Logger
-	}
-)
+// Config holds x plugin configuration.
+type Config struct {
+	Templates []getter.Source `yaml:"templates"`
+	TmpDir    plugin.TmpDir   `yaml:"tmpDir"`
+	Logger    config.Logger
+}
 
+// GetPluginDependencies returns x plugin dependencies.
 func GetPluginDependencies() map[string]api.Dependency {
 	return map[string]api.Dependency{
 		"eget": {

internal/executor/x/getter/download.go

@@ -16,10 +16,13 @@ func sha(in string) string {
 	hasher.Write([]byte(in))
 	return base64.URLEncoding.EncodeToString(hasher.Sum(nil))
 }
+
+// EnsureDownloaded downloads given sources only if not yet downloaded.
+// It's a weak comparison based on the source path.
 func EnsureDownloaded(ctx context.Context, templateSources []Source, dir string) error {
 	for _, tpl := range templateSources {
 		dst := filepath.Join(dir, sha(tpl.Ref))
-		err := RunIfFileDoesNotExist(dst, func() error {
+		err := runIfFileDoesNotExist(dst, func() error {
 			return Download(ctx, tpl.Ref, dst)
 		})
 		if err != nil {
@@ -30,7 +33,7 @@ func EnsureDownloaded(ctx context.Context, templateSources []Source, dir string)
 	return nil
 }
 
-func RunIfFileDoesNotExist(path string, fn func() error) error {
+func runIfFileDoesNotExist(path string, fn func() error) error {
 	_, err := os.Stat(path)
 	switch {
 	case err == nil:

internal/executor/x/getter/load.go

@@ -9,10 +9,12 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
+// Source holds information about source location.
 type Source struct {
 	Ref string `yaml:"ref"`
 }
 
+// Load downloads defined sources and read them from the FS.
 func Load[T any](ctx context.Context, tmpDir string, templateSources []Source) ([]T, error) {
 	err := EnsureDownloaded(ctx, templateSources, tmpDir)
 	if err != nil {

internal/executor/x/mathx/int.go

@@ -1,5 +1,6 @@
 package mathx
 
+// IncreaseWithMax increase in by 1 but only up to max value.
 func IncreaseWithMax(in, max int) int {
 	in++
 	if in > max {
@@ -8,6 +9,7 @@ func IncreaseWithMax(in, max int) int {
 	return in
 }
 
+// DecreaseWithMin decreases in by 1 but only to min value.
 func DecreaseWithMin(in, min int) int {
 	in--
 	if in < min {