Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/commands/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,13 @@ func (cb *CommandBuilder) executeCommand(ctx context.Context, command string, ar
executor := cmd.GetShellExecutor(ctx)
output, err := executor.Exec(ctx, command, args...)
if err != nil {
// Keep what the command printed, such as kubectl's "Error from server
// (Forbidden): ...". The exit status alone does not say why the command
// failed, and the error creators below pick their suggestions from it.
if commandOutput := strings.TrimSpace(string(output)); commandOutput != "" {
err = fmt.Errorf("%w: %s", err, commandOutput)
}

// Create appropriate error based on command type
var toolError *errors.ToolError
switch command {
Expand Down
63 changes: 63 additions & 0 deletions internal/commands/builder_errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package commands

import (
"context"
"fmt"
"testing"

"github.com/kagent-dev/tools/internal/cmd"
"github.com/kagent-dev/tools/internal/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// TestExecuteKeepsCommandOutputOnFailure checks that a failed command's own
// message reaches the caller, not only its exit status.
func TestExecuteKeepsCommandOutputOnFailure(t *testing.T) {
exitStatus := fmt.Errorf("exit status 1")

testCases := []struct {
name string
output string
wantInError string
wantErrorCode string
}{
{
name: "admission webhook denial",
output: "Error from server (Forbidden): admission webhook \"policy.example.com\" denied the request: scaling prod needs approval\n",
wantInError: `admission webhook "policy.example.com" denied the request: scaling prod needs approval`,
},
{
name: "RBAC denial gets the permission suggestions",
output: "Error from server (Forbidden): deployments.apps \"checkout\" is forbidden: User \"system:serviceaccount:kagent:kagent-tools\" cannot patch resource \"deployments\" in API group \"apps\" in the namespace \"prod\"\n",
wantInError: `cannot patch resource "deployments"`,
wantErrorCode: "K8S_PERMISSION_ERROR",
},
{
name: "no output keeps the error as it was",
output: "",
wantInError: "failed: exit status 1",
},
}

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
mockExecutor := cmd.NewMockShellExecutor()
mockExecutor.AddPartialMatcherString("kubectl", []string{"patch"}, testCase.output, exitStatus)
ctx := cmd.WithShellExecutor(context.Background(), mockExecutor)

_, err := NewCommandBuilder("kubectl").
WithArgs("patch", "deployment", "checkout", "--type=merge", "-p", `{"spec":{"replicas":6}}`).
WithNamespace("prod").
Execute(ctx)

var toolError *errors.ToolError
require.ErrorAs(t, err, &toolError)
assert.Contains(t, err.Error(), testCase.wantInError)
assert.ErrorIs(t, toolError.Cause, exitStatus, "the original error stays wrapped")
if testCase.wantErrorCode != "" {
assert.Equal(t, testCase.wantErrorCode, toolError.ErrorCode)
}
})
}
}